Re: [RFC][PATCH 2/2 v2] ASoC: simple-card: add Device Tree support

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 01/14/2013 07:40 PM, Kuninori Morimoto wrote:
> Support for loading the simple-card module via devicetree.
> It requests cpu/codec/platform information for probing.

> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt

> +Required properties:
> +
> +- compatible					: "simple-audio"
> +- simple-audio,card-name			: simple-audio card name
> +
> +- simple-audio,platform,controller		: phandle for platform

Rename that simple-audio,dma-controller perhaps? "platform" is a word
specific to ASoC, and the bindings really should be generic across OSs.

But I wonder why you'd even need the ASoC platform to be specified in
DT; instead, the following seem better:

a) Have the CPU DAI's driver register the platform itself. Tegra does this.

b) Assume the ASoC "platform" device simply does DMA via a standard
dmaengine driber, and instead refer to the DMA controller using DMA
engine DT bindings.

> +- simple-audio,platform,name			: simple-audio platform name

Can you explain why you'd need the platform name in the DT? Doesn't the
phandle always uniquely identify it? The example doesn't use this property.

> +- simple-audio,cpu,controller			: phandle for CPU DAI
> +- simple-audio,cpu,dai,name			: simple-audio CPU DAI name

It'd be a bit more typical of device-tree to have a single property that
defines both the controller and any properties of the controller at
once, e.g. something like:

simple-audio,cpu-interface = <&codec_phandle AK4648_I2S_ITF_A>;

where we assume something like:

#define AK4648_I2S_ITF_A 0 // Interface A's ID

That would remove the need to put string names into the DT.

> +simple-audio,xxx,dai,clock-gating
> +	"continuous"
> +	"gated"

Don't you need to use the common clock bindings to define which clock to
gate? Or, is the I2S/... node's binding supposed to provide that
information?