[alsa-devel] [PATCH 5/5] ASoC: simple-card: add Device Tree support

[Kuninori Morimoto]

Hi Stephen, Mark

> > Support for loading the simple-card module via devicetree.
> > It requests platform/codec driver's DT blob for probing.
> 
> This binding proposal should be sent to
> devicetree-discuss at lists.ozlabs.org; that's where all DT bindings are
> dicussed.

will do

> > +Required properties:
> > +for simple-card
> > +- compatible			: "asoc,simple-card"
> 
> I don't think "asoc," is a good vendor prefix; DT should strive to
> describe the hardware, not a particular OS's driver structure.
> 
> Perhaps "simple-audio" would be better.

Thanks.

> > +- simple,asoc,cpu		: phandle for platform
> 
> "cpu" isn't a very good name; it couuld mean anything. "platform" is an
> ASoC-specific term. I think this would be better written as:
> 
> simple-audio,cpu-audio-controller : phandle of the CPU's audio controller
> 
> or perhaps:
> 
> simple-audio,cpu-port : phandle of the CPU's audio port

Thank you.
I would like to use "cpu-port" and "codec-port"

> > +for platform
> > +- simple,asoc,dai		: dai name
> > +
> > +for codec
> > +- simple,asoc,dai		: dai name
> > +- simple,asoc,name		: codec name
> 
> Why are names needed? The existing audio-related DT bindings don't put
> any names into the device tree; everything binds with phandles.
(snip)
> Oh, I see. Nothing in the text above implies that the DAI format
> properties would exist in the CODEC and I2S controller nodes. That's
> rather an imposition on the DT bindings for the CODEC and I2S
> controller; the individual bindings for those devices should define all
> the properties that go into those nodes, and those devices won't always
> be used with a "simple" sound card. It feels to me like the DAI
> configuration properties should be part of the sound node, not the
> I2S/CODEC nodes.

Thank you for your advice.
I modify this design.

> > +- simple,asoc,sysclk			: system clock rate
> 
> "clock-frequency" rather than "sysclk" would be more consistent with
> other bindings.
(snip)
> I think specifying the particular clock being referenced is useful here
> - we may also end up wanting to specify the BCLK or sample rate due to
> some hardware limitation on some system both of which are clocks too.

I will use "system-clock-frequency" here