Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2010-04-28 at 17:13 -0500, Timur Tabi wrote:
> On Wed, Apr 28, 2010 at 4:58 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > The sound0 node needs a compatible value,
> 
> I knew I was forgetting something
> 
> > the sound-device node should
> > probably have one too.
> 
> The aliases, cpus, and memory node don't have a compatible property,
> and I was modeling the design after the aliases node.

aliases is a bad choice, it's very very special and is neither a device
nor a virtual device, like chosen.

cpus is more of a match in your case.

In any case, I agree, you may not really need a compatible prop for the
virtual device. In fact, Grant, do we really need an enclosing node like
that ? In any case, it's no big deal and shouldn't have much impact on
the design.

Cheers,
Ben.

> > The sound0 node should have something board specific like
> > "fsl,mpc8610hpcd-sound" to make it clear that the binding really only
> > applies to this particular board.  It would also be a good idea to
> > prefix all of the property names with 'fsl,' to avoid conflicting with
> > any future common bindings or conventions.  Other boards can use the
> > same binding, but they would get a different compatible value (the
> > driver could bind on both).
> 
> The aliases node doesn't have an fsl, prefix.  I understand the need
> for the prefix, but I wonder why we don't do that for the aliases
> node.
> 
> > I'm not a huge fan of the name "sound-devices" for the parent node.
> > There are other sorts of things that we need 'virtual' device nodes to
> > describe.  It would be nice to have a single place for collecting
> > nodes for stuff like this.  Perhaps this:
> >
> > system {
> >        compatible = "system-devices";
> >        sound0 {
> >                compatible = "fsl,mpc8610hpcd-sound";
> >                fsl,ssi = &ssi0;
> >                fsl,playback-dma = &dma00;
> >                fsl,capture-dma = &dma01;
> >                fsl,codec = &cs4270;
> >        };
> > };
> 
> I like that.
>