Re: [PATCH 3/5] ASoC: fsl: mpc5200-soc-audio driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Tue, Sep 11, 2012 at 10:14:47PM -0400, Eric Millbrandt wrote:
> Add a generic mpc5200 driver that allows asoc cards to be defined in the
> device tree.

ASoC - you've misspelt this throughout.

This changelog should discuss the subset of devices supported by your
binding, it's only possible to define bindings like this for a subset of
possible cards.  Looking at the code it seems like this driver can only
work for CODECs which require no runtime configuration (which is a very
small subset of CODEC drivers).

> --- a/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt

Audio bindings generally go under sound.

> +Multiple DAI nodes may be attached to a sound node
> +- stream-name         - The asoc name of the platform DAI stream
> +- codec-name          - The name of codec to bind to
> +- codec-dai-name      - The codec DAI to bind to
> +- cpu-dai-name        - The cpu DAI to bind to

What are the allowable values for these strings?

> +config SND_MPC52xx_SOC_AUDIO
> +	tristate "SoC Generic Audio support for MPC5200"
> +	depends on (SND_SOC_MPC5200_AC97 || SND_SOC_MPC5200_I2S)

It seems wrong that this depends on both AC'97 and I2S - users should be
able to use one or the other.  Given how AC'97 works we really should be
defining it as a bus in the device tree anyway, and ideally trying to
enumerate the CODECs at runtime, so it should probably be a separate
binding.

> +#define DEBUG

Remove this for mainline.

> +	card = kzalloc(sizeof(struct snd_soc_card), GFP_KERNEL);

devm_kzalloc()