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

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

[Grant Likely]

On Wed, Apr 28, 2010 at 2:35 PM, Timur Tabi <timur.tabi@gmail.com> wrote:
> On Tue, Apr 27, 2010 at 5:09 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>
>> Keep in mind that it's perfectly kosher to create nodes for "virtual"
>> devices. IE. We could imagine a node for the "sound subsystem" that
>> doesn't actually correspond to any physical device but contain the
>> necessary properties that binds everything together. You could even have
>> multiple of these if you have separate set of sound HW that aren't
>> directly dependant.
>
> First, I want to officially retract this patch.  I've talked with
> Grant, and we've come up with a different approach to this problem.
>
> Second, how about this binding for the virtual sound node?  It would
> be a root-level node.
>
>        sound-devices {
>                sound0 {
>                        ssi = &ssi0;
>                        playback-dma = &dma00;
>                        capture-dma = &dma01;
>                        codec = &cs4270;
>                }
>        };

The sound0 node needs a compatible value, the sound-device node should
probably have one too.

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).

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;
        };
};

But I really don't have any knowledge of what has been done previously
in this regard or if any conventions have been established.  Ben, any
thoughts?

g.

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

[Timur Tabi]

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.

> 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.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Grant Likely]

On Wed, Apr 28, 2010 at 4:13 PM, Timur Tabi <timur.tabi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Apr 28, 2010 at 4:58 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> 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.

Well, there are typically three ways to find a node; by name, by
device_type and by compatible.  device_type is meaningless for the
flattened tree, so that's out.  Matching by name could potentially
have namespace collisions, but I'm not sure.  I'll defer to Ben &
Mitch's judgment here.

The difference with aliases, cpus and memory nodes is that the
conventions around them were defined and agreed on a very long time
ago.  We could get consensus to do the same here, but I cannot make
that call.

>> 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.

aliases is not a vendor-specific or limited scope convention.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

[Benjamin Herrenschmidt]

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-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> 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.
> 

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

[Grant Likely]

On Wed, Apr 28, 2010 at 6:52 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> 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 ?

Mostly I'm concerned about 'polluting' the root node in a way that
we'd regret later; but perhaps I'm being overly conservative.  The
sound node will still be uniquely identified by it's compatible
property, so perhaps I'm fretting over nothing.

> In any case, it's no big deal and shouldn't have much impact on
> the design.

Right, the point has been reached of quibbling over trivialities.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.