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

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

[Timur Tabi]

An upcoming change in the architecture of ALSA SoC (ASoC) will require the
MPC8610 HPCD's ASoC fabric driver to register as a standard platform driver.
Therefore, we need to call platform_device_register_simple() from the board's
platform code.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

Kumar, the ASoC mainters are willing to pick up this patch, but they want an
ACK from you first.  Or, you could pick it up, since by itself it's harmless.

 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 5abe137..66afff3 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -98,6 +98,9 @@ static int __init mpc8610_declare_of_platform_devices(void)
 	/* Without this call, the SSI device driver won't get probed. */
 	of_platform_bus_probe(NULL, mpc8610_ids, NULL);
 
+	/* Register the platform device for the ASoC fabric driver */
+	platform_device_register_simple("snd-soc-mpc8610-hpcd", 0, NULL, 0);
+
 	return 0;
 }
 machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);
-- 
1.6.5

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

[Benjamin Herrenschmidt]

On Mon, 2010-04-26 at 15:49 -0500, Timur Tabi wrote:
> An upcoming change in the architecture of ALSA SoC (ASoC) will require the
> MPC8610 HPCD's ASoC fabric driver to register as a standard platform driver.
> Therefore, we need to call platform_device_register_simple() from the board's
> platform code.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---

Gross. Loses the linkage to device-tree etc... which is useful for audio
especially. You should at least make sure the device node follows for
the target driver to be able to use it :-) I'd like you to sync with
Grant work on that matter if you are going to convert of_devices into
platform_devices.

Cheers,
Ben.

> Kumar, the ASoC mainters are willing to pick up this patch, but they want an
> ACK from you first.  Or, you could pick it up, since by itself it's harmless.
> 
>  arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> index 5abe137..66afff3 100644
> --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> @@ -98,6 +98,9 @@ static int __init mpc8610_declare_of_platform_devices(void)
>  	/* Without this call, the SSI device driver won't get probed. */
>  	of_platform_bus_probe(NULL, mpc8610_ids, NULL);
>  
> +	/* Register the platform device for the ASoC fabric driver */
> +	platform_device_register_simple("snd-soc-mpc8610-hpcd", 0, NULL, 0);
> +
>  	return 0;
>  }
>  machine_device_initcall(mpc86xx_hpcd, mpc8610_declare_of_platform_devices);

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

[Liam Girdwood]

On Tue, 2010-04-27 at 16:36 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2010-04-26 at 15:49 -0500, Timur Tabi wrote:
> > An upcoming change in the architecture of ALSA SoC (ASoC) will require the
> > MPC8610 HPCD's ASoC fabric driver to register as a standard platform driver.
> > Therefore, we need to call platform_device_register_simple() from the board's
> > platform code.
> > 
> > Signed-off-by: Timur Tabi <timur@freescale.com>
> > ---
> 
> Gross. Loses the linkage to device-tree etc... which is useful for audio
> especially. You should at least make sure the device node follows for
> the target driver to be able to use it :-) I'd like you to sync with
> Grant work on that matter if you are going to convert of_devices into
> platform_devices.

Timur, please correct my device tree understanding form our IRC
conversation if I'm wrong here.

I think one of the difficulties encountered here is that the device tree
root for sound in this case is the SSI (Digital Audio Interface)
controller and not the sound card as in ASoC. So maybe the problem is
how do we represent an ASoC sound card in the device tree ?

The sound card within ASoC is a compound device that is made up of the
SSI, Codec and audio DMA engine components. The SSI/Codec/DMA components
do not have to be the sound cards child devices wrt the driver model but
do register with the ASoC core and are 'joined' with each other and the
sound card driver to form a working audio device.

Now I don't know the mechanics of adding an ASoC sound card to the
device tree, but the device tree should be able to define an ASoC sound
card and also represent the relationships between the sound card and the
SSI/Codec/DMA components. The components should also be represented in
the device tree. Parsing the device tree should probe() all the ASoC
components and sound card. The ASoC core would then instantiated them as
a sound device.  

Thanks

Liam 

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

[Timur Tabi]

Liam Girdwood wrote:

> I think one of the difficulties encountered here is that the device tree
> root for sound in this case is the SSI (Digital Audio Interface)
> controller and not the sound card as in ASoC. So maybe the problem is
> how do we represent an ASoC sound card in the device tree ?

Yes, that is a topic of concern.

If you look at the 8610 DTS, you'll see that the SSI (i2s) is effectively the master.  In reality, I just modeled the board in the DTS.  The SSI node, for example, lists the two DMA channels it needs.  It also points to the codec that it's connected to.  And the clock-frequency property is in the codec node because the oscillator is connected to the codec on the board.

In theory, would could probably get rid of the DMA node pointers if I can update the standard DMA driver to allow me to reserve two DMA channels.  That's not an easy fix, though.

> The sound card within ASoC is a compound device that is made up of the
> SSI, Codec and audio DMA engine components. The SSI/Codec/DMA components
> do not have to be the sound cards child devices wrt the driver model but
> do register with the ASoC core and are 'joined' with each other and the
> sound card driver to form a working audio device.

Yes, today I just walk the device tree looking for these connections implicitly. The code may be ugly, but the device tree is more streamlined.

> Now I don't know the mechanics of adding an ASoC sound card to the
> device tree, but the device tree should be able to define an ASoC sound
> card and also represent the relationships between the sound card and the
> SSI/Codec/DMA components. The components should also be represented in
> the device tree. Parsing the device tree should probe() all the ASoC
> components and sound card. The ASoC core would then instantiated them as
> a sound device.  

Another problem is that ASoC won't let me probe the DMA channels independently.  That is, I cannot tell ASoC that I have a playback DMA and a capture DMA.  ASoC does not recognize two DMA devices for a single SSI.  If you can fix that, then I can turn the DMA driver into an OF driver.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Liam Girdwood]

On Tue, 2010-04-27 at 09:52 -0500, Timur Tabi wrote:
> Liam Girdwood wrote:
> 

> > Now I don't know the mechanics of adding an ASoC sound card to the
> > device tree, but the device tree should be able to define an ASoC sound
> > card and also represent the relationships between the sound card and the
> > SSI/Codec/DMA components. The components should also be represented in
> > the device tree. Parsing the device tree should probe() all the ASoC
> > components and sound card. The ASoC core would then instantiated them as
> > a sound device.  
> 
> Another problem is that ASoC won't let me probe the DMA channels
> independently.  That is, I cannot tell ASoC that I have a playback DMA
> and a capture DMA.  ASoC does not recognize two DMA devices for a
> single SSI.  If you can fix that, then I can turn the DMA driver into
> an OF driver.
> 

Iirc, the SSI and DMA controllers on your SoC mean that each DMA device
can only do one direction (either Playback or Capture). So I'm thinking
we create two DAI link entries for your sound card (one for playback and
the other for capture) and they both use the same SSI device but each
would have it's own DMA device.

This would result in two separate pcm devices being exported to
userspace i.e one for playback only and the other for capture only. I
think this is also a more accurate representation of your hardware too
(since we have different DMA devices for each pcm stream direction).

Liam 

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

[Timur Tabi]

Liam Girdwood wrote:

> Iirc, the SSI and DMA controllers on your SoC mean that each DMA device
> can only do one direction (either Playback or Capture). So I'm thinking
> we create two DAI link entries for your sound card (one for playback and
> the other for capture) and they both use the same SSI device but each
> would have it's own DMA device.

Do you mean here:

	machine_data->card.probe = mpc8610_hpcd_machine_probe;
	machine_data->card.remove = mpc8610_hpcd_machine_remove;
	machine_data->card.name = "MPC8610 HPCD";
	machine_data->card.num_links = 1;
	machine_data->card.dai_link = &machine_data->dai;

So that num_links would be 2 instead of 1?

I would need some way for fsl_dma_open() to get a pointer to private, DMA-specific data.  I'm not sure how I can do that.

> This would result in two separate pcm devices being exported to
> userspace i.e one for playback only and the other for capture only. I
> think this is also a more accurate representation of your hardware too
> (since we have different DMA devices for each pcm stream direction).

I can say for certain whether that will actually work.  My gut tells me that I might run into problems trying to implement that.  The only way to know for sure is to start hacking.  Unfortunately, my window of opportunity to work on this just closed, and it may not open for a while.  I know my current patch is less-than-ideal, but it does restore functionality to the 8610, and without needing any U-Boot or device-tree changes.  So unless there are any real objections, I'd like it to be merged.  Otherwise, 8610 audio will be broken in the next release.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Timur Tabi]

On Tue, Apr 27, 2010 at 10:28 AM, Timur Tabi <timur@freescale.com> wrote:
> I can say for certain whether that will actually work.

Ugh, I meant to write, "I can't say for certain".

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Liam Girdwood]

On Tue, 2010-04-27 at 10:28 -0500, Timur Tabi wrote:
> Liam Girdwood wrote:
> 
> > Iirc, the SSI and DMA controllers on your SoC mean that each DMA device
> > can only do one direction (either Playback or Capture). So I'm thinking
> > we create two DAI link entries for your sound card (one for playback and
> > the other for capture) and they both use the same SSI device but each
> > would have it's own DMA device.
> 
> Do you mean here:
> 
> 	machine_data->card.probe = mpc8610_hpcd_machine_probe;
> 	machine_data->card.remove = mpc8610_hpcd_machine_remove;
> 	machine_data->card.name = "MPC8610 HPCD";
> 	machine_data->card.num_links = 1;
> 	machine_data->card.dai_link = &machine_data->dai;
> 
> So that num_links would be 2 instead of 1?
> 

Yes.

> I would need some way for fsl_dma_open() to get a pointer to private,
> DMA-specific data.  I'm not sure how I can do that.
> 

In multi-component we now have platform_data and device private data
(from the regular driver model).

We also have stream snd_soc_dai_set_dma_data() for runtime DMA config.

So it depends on who is setting your DMA data. If it's DTS then it would
be the of_ platform equivalent, if it's your DMA probe() then dev data
otherwise you can use the snd_soc_dai_set_dma_data().

> > This would result in two separate pcm devices being exported to
> > userspace i.e one for playback only and the other for capture only. I
> > think this is also a more accurate representation of your hardware too
> > (since we have different DMA devices for each pcm stream direction).
> 
> I can say for certain whether that will actually work.  My gut tells
> me that I might run into problems trying to implement that.  The only
> way to know for sure is to start hacking.  Unfortunately, my window of
> opportunity to work on this just closed, and it may not open for a
> while.  I know my current patch is less-than-ideal, but it does
> restore functionality to the 8610, and without needing any U-Boot or
> device-tree changes.  So unless there are any real objections, I'd
> like it to be merged.  Otherwise, 8610 audio will be broken in the
> next release.

Ok, I can live with this providing we can mark it as a TODO: and have a
PPC Ack. It may be easier to fix in the future too as the ASoC card
registration clean-up should be complete/in-progress (i.e. card
platform_data and private_data will be available for passing in anything
you like).

Thanks

Liam 

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

[Timur Tabi]

Liam Girdwood wrote:

>> I would need some way for fsl_dma_open() to get a pointer to private,
>> DMA-specific data.  I'm not sure how I can do that.
> 
> In multi-component we now have platform_data and device private data
> (from the regular driver model).

In that case, I still have the same problem with how to generate an 'id' based on a device tree node.  We have the idea of a 'phandle', which is a unique integer for a node, but there's no way to create phandles from within Linux.  They have to exist in the DTS first, and I'm loathe to modify the DTS.


-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Grant Likely]

On Tue, Apr 27, 2010 at 12:32 PM, Timur Tabi <timur@freescale.com> wrote:
> Liam Girdwood wrote:
>
>>> I would need some way for fsl_dma_open() to get a pointer to private,
>>> DMA-specific data. =A0I'm not sure how I can do that.
>>
>> In multi-component we now have platform_data and device private data
>> (from the regular driver model).
>
> In that case, I still have the same problem with how to generate an 'id' =
based on a device tree node. =A0We have the idea of a 'phandle', which is a=
 unique integer for a node, but there's no way to create phandles from with=
in Linux. =A0They have to exist in the DTS first, and I'm loathe to modify =
the DTS.

Can you not dynamically assign an id?  If alsa soc needs a unique id
number, then just create a lookup function.  Something like
of_asoc_phandle_to_codec_id() that will either return a previously
assigned id, or will assign a new id.  You shouldn't ever need to add
data to the tree at runtime.

g.

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

[Timur Tabi]

Grant Likely wrote:

> Can you not dynamically assign an id?  If alsa soc needs a unique id
> number, then just create a lookup function. 

What I need is something like a hashing function that can convert a "struct device_node *" into an "int".  I'm going to have two functions that independently parse the device tree and locate a specific node.  Both functions will "register the node" with asoc, but they'll use an integer ID to uniquely identify the node.

At least, that's the way ASoC likes to operate.  AsoC takes a fixed string plus a unique integer.  I could technically create a unique string for each DMA device, and have the integer always be 0.

> Something like
> of_asoc_phandle_to_codec_id() that will either return a previously
> assigned id, or will assign a new id.  You shouldn't ever need to add
> data to the tree at runtime.

I know.  It just would have been nice if my nodes already had phandles in them.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Tue, Apr 27, 2010 at 03:04:33PM -0500, Timur Tabi wrote:

[Reflowed into 80 columns]

> At least, that's the way ASoC likes to operate.  AsoC takes a fixed
> string plus a unique integer.  I could technically create a unique
> string for each DMA device, and have the integer always be 0.

This seems like the most direct approach to your problem - as with other
subsystems the .id is there mostly to provide deduping when the string
is always the same, if you can easily come up with sensible names you
can just use those.

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

[Benjamin Herrenschmidt]

On Tue, 2010-04-27 at 15:04 -0500, Timur Tabi wrote:
> What I need is something like a hashing function that can convert a
> "struct device_node *" into an "int".  I'm going to have two functions
> that independently parse the device tree and locate a specific node.
> Both functions will "register the node" with asoc, but they'll use an
> integer ID to uniquely identify the node.
> 
> At least, that's the way ASoC likes to operate.  AsoC takes a fixed
> string plus a unique integer.  I could technically create a unique
> string for each DMA device, and have the integer always be 0. 

That's just plain gross and horrible. You could use phandles you
know :-)

Or you could use path in your strings, or something like that.

Note that any time you have a struct device, you have a free device_node
pointer as well.

Cheers,
Ben.

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

[Benjamin Herrenschmidt]

On Tue, 2010-04-27 at 13:15 -0600, Grant Likely wrote:
> 
> Can you not dynamically assign an id?  If alsa soc needs a unique id
> number, then just create a lookup function.  Something like
> of_asoc_phandle_to_codec_id() that will either return a previously
> assigned id, or will assign a new id.  You shouldn't ever need to add
> data to the tree at runtime. 

Numerical magic IDs are evil... why not a name ? Or it's existing alsa
breakage ? :-)

Cheers,
Ben.

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

[Timur Tabi]

On Tue, Apr 27, 2010 at 10:20 AM, Liam Girdwood <lrg@slimlogic.co.uk> wrote=
:

>> Another problem is that ASoC won't let me probe the DMA channels
>> independently. =A0That is, I cannot tell ASoC that I have a playback DMA
>> and a capture DMA. =A0ASoC does not recognize two DMA devices for a
>> single SSI. =A0If you can fix that, then I can turn the DMA driver into
>> an OF driver.
>>
>
> Iirc, the SSI and DMA controllers on your SoC mean that each DMA device
> can only do one direction (either Playback or Capture). So I'm thinking
> we create two DAI link entries for your sound card (one for playback and
> the other for capture) and they both use the same SSI device but each
> would have it's own DMA device.
>
> This would result in two separate pcm devices being exported to
> userspace i.e one for playback only and the other for capture only. I
> think this is also a more accurate representation of your hardware too
> (since we have different DMA devices for each pcm stream direction).

Ok, I'm trying to do this now, and I'm running into problems.

So here's the device list:

One machine
One SSI
Two DMA channels
One codec

So I create two dai_links in the machine driver.  Each dai_link has
two DAIs in it.  The DAIs are identical, except for the platform_drv
field.  The platform_drv in the first DAI points to the first DMA
channel, and the platform_drv of the second DAI points to the second
DMA channel.

When I boot Linux, I get this:

asoc: cs4270 <-> /soc@e0000000/ssi@16000 mapping ok
sysfs: cannot create duplicate filename '/devices/platform/soc-audio/cs4270=
'

so it looks like when asoc is processing the dai_link, it tries to
create a sysfs device for the codec twice.

How do I avoid this?

--=20
Timur Tabi
Linux kernel developer at Freescale

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

[Timur Tabi]

On Fri, Apr 30, 2010 at 4:46 PM, Timur Tabi <timur@freescale.com> wrote:

> When I boot Linux, I get this:
>
> asoc: cs4270 <-> /soc@e0000000/ssi@16000 mapping ok
> sysfs: cannot create duplicate filename '/devices/platform/soc-audio/cs4270'

*sigh* never mind.  The problem was that the stream_name was the same
for both DAIs.

What exactly should the stream names be set to, anyway?  I did this:

	machine_data->dai[0].stream_name = "playback";
	machine_data->dai[1].stream_name = "capture";

but I have a suspicion that this is too simplistic.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Grant Likely]

On Tue, Apr 27, 2010 at 2:07 AM, Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> On Tue, 2010-04-27 at 16:36 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2010-04-26 at 15:49 -0500, Timur Tabi wrote:
>> > An upcoming change in the architecture of ALSA SoC (ASoC) will require the
>> > MPC8610 HPCD's ASoC fabric driver to register as a standard platform driver.
>> > Therefore, we need to call platform_device_register_simple() from the board's
>> > platform code.
>> >
>> > Signed-off-by: Timur Tabi <timur@freescale.com>
>> > ---
>>
>> Gross. Loses the linkage to device-tree etc... which is useful for audio
>> especially. You should at least make sure the device node follows for
>> the target driver to be able to use it :-) I'd like you to sync with
>> Grant work on that matter if you are going to convert of_devices into
>> platform_devices.
>
> Timur, please correct my device tree understanding form our IRC
> conversation if I'm wrong here.
>
> I think one of the difficulties encountered here is that the device tree
> root for sound in this case is the SSI (Digital Audio Interface)
> controller and not the sound card as in ASoC. So maybe the problem is
> how do we represent an ASoC sound card in the device tree ?

Most likely this is currently a problem, yes.  *however* the device
tree is just data.  It is convenient and useful to have a common
representation between similar kinds of devices, but it isn't
mandatory.  The device tree structure does *not* need to have a 1:1
correspondence to the ASoC device registrations.

So, the current 86xx device tree binding assumes a simple layout with
a node describing an DAI controller, and another node describing the
codec with a single phandle (pointer) from the DAI to the codec.  In
this configuration, it is completely reasonable for the DAI node to
trigger both the instantiation of the ASoC DAI controller device and
the sound card device.  Linux can treat them as separate even though
the current device tree has a simplistic representation.

> The sound card within ASoC is a compound device that is made up of the
> SSI, Codec and audio DMA engine components. The SSI/Codec/DMA components
> do not have to be the sound cards child devices wrt the driver model but
> do register with the ASoC core and are 'joined' with each other and the
> sound card driver to form a working audio device.

Right.  This makes sense to me.

> Now I don't know the mechanics of adding an ASoC sound card to the
> device tree, but the device tree should be able to define an ASoC sound
> card and also represent the relationships between the sound card and the
> SSI/Codec/DMA components. The components should also be represented in
> the device tree. Parsing the device tree should probe() all the ASoC
> components and sound card. The ASoC core would then instantiated them as
> a sound device.

I've tried very hard to maintain a distinction between device tree
binding (representation) and Linux kernel internal implementation
details.  The real question is whether or not the binding provides
sufficient detail for the operating system to figure out what to do.
In the extreme minimalist case, the audio driver could decide how to
configure itself solely on the board name property of the root node.
There is nothing wrong with that, but it also means that no data is
available to dynamically select common modules or modify connections;
it all has to be hard coded.

The 8610 device tree looks something like this right now:

	[...]
		cs4270:codec@4f {
			compatible = "cirrus,cs4270";
			reg = <0x4f>;
			/* MCLK source is a stand-alone oscillator */
			clock-frequency = <12288000>;
		};
	[...]
	ssi@16000 {
		compatible = "fsl,mpc8610-ssi";
		[...]
		fsl,mode = "i2s-slave";
		codec-handle = <&cs4270>;
		fsl,playback-dma = <&dma00>;
		fsl,capture-dma = <&dma01>;
		fsl,fifo-depth = <8>;
		sleep = <&pmc 0 0x08000000>;
	};
	[...]

(I've omitted the DMA nodes and some irrelevant details)  This is
enough information for a simplistic driver registration that probably
makes a lot of assumptions.  Such as the ssi represents a single
logical sound device.  It won't handle complex representations, but in
a lot of cases that may be just fine.

However, as you and Mark rightly point out, it completely fails to
represent complex sound devices with weird clocking and strange
routes.  Something like this is probably more appropriate:

	[...]
		codec1 :codec@4f {
			compatible = "cirrus,cs4270";
			reg = <0x4f>;
			/* MCLK source is a stand-alone oscillator */
			clock-frequency = <12288000>;
		};
	[...]
	ssi1: ssi@16000 {
		compatible = "fsl,mpc8610-ssi";
		[...]
		fsl,mode = "i2s-slave";
		fsl,playback-dma = <&dma00>;
		fsl,capture-dma = <&dma01>;
		fsl,fifo-depth = <8>;
		sleep = <&pmc 0 0x08000000>;
	};
	[...]
	sound {
		compatible = "fsl,mpc8610-hpcd-sound";
		/* maybe something like (totally off the top of my head) */
		dai-links = <&ssi1 0 &codec 0
		             &ssi1 1 &codec 1>;
		[...]
	};

Where the 'sound' node is now the starting point for representing a
logical sound device instead of the ssi node.  This binding probably
makes more sense (but I'm not committing to anything like this until I
see a real proposal for a real device).

The question for the drivers is more around how to deal with the data
provided.  I've got zero issues with platform specific code to handle
things that aren't easily described in a device tree.  The point is to
make the common cases data-driven so that common code will handle
them.  The complex corner cases are still complex corner cases that
need platform specific code.

Or, in other words, the device tree should *not* be used to describe
every possible detail and permutation.  It is best used to describe
the permutations that are common so that they don't need to be hard
coded for each and every board.

I would solve the problem this way: In the ssi driver, if the
codec-node property is present, then call a function to instantiate a
simple or platform specific sound card instance that makes the
assumptions listed above.  If not, then just register the ssi and
exit, which leaves the ssi available for a separate driver to pick it
up.  I wouldn't do this for new platforms, but it gracefully makes use
of the data provided in the current 8610 device tree.

BTW Timur, there is nothing wrong with registering multiple devices
that all have the of_node pointer set to the same node.

g.

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

[Timur Tabi]

Grant Likely wrote:

> So, the current 86xx device tree binding assumes a simple layout with
> a node describing an DAI controller, and another node describing the
> codec with a single phandle (pointer) from the DAI to the codec.  In
> this configuration, it is completely reasonable for the DAI node to
> trigger both the instantiation of the ASoC DAI controller device and
> the sound card device.  Linux can treat them as separate even though
> the current device tree has a simplistic representation.

The only problem with this is that there is board-specific programming that needs to be done (look at mpc8610_hpcd_machine_probe), so we still need to have a fabric driver that is independent of the SSI, codec, or DMA drivers.  The P1022 also has an SSI, and I'm hoping that all I need to do is create a new fabric driver, not hack up the SSI driver to support board programming. 

So if the fabric driver still needs to exist, then it still needs a struct device, and it still needs to register with asoc.  I don't see how I can register the sound card itself in the SSI driver, because it won't know anything about the board-specific code in the fabric driver.

> I've tried very hard to maintain a distinction between device tree
> binding (representation) and Linux kernel internal implementation
> details.  The real question is whether or not the binding provides
> sufficient detail for the operating system to figure out what to do.

I think it does, because it's working today.

> In the extreme minimalist case, the audio driver could decide how to
> configure itself solely on the board name property of the root node.
> There is nothing wrong with that, but it also means that no data is
> available to dynamically select common modules or modify connections;
> it all has to be hard coded.

Well, asoc already has several hard-coded requirements:

	machine_data->dai.cpu_dai_drv = &fsl_ssi_dai;
	machine_data->dai.codec_dai_drv = &cs4270_dai;
	machine_data->dai.codec_drv = &soc_codec_device_cs4270;
	machine_data->dai.ops = &mpc8610_hpcd_ops;
	machine_data->dai.platform_drv = &fsl_soc_platform;

So even though I probe for each device separately and register them separately, the fabric driver still needs to have hard-coded addresses 

Maybe the asoc guys can tell me why I need to register the cpu_dai_drv structure via platform_device_add(), when it's already being registered via snd_soc_register_dai().

> (I've omitted the DMA nodes and some irrelevant details)  This is
> enough information for a simplistic driver registration that probably
> makes a lot of assumptions.  Such as the ssi represents a single
> logical sound device.  It won't handle complex representations, but in
> a lot of cases that may be just fine.

Why would I ever represent the SSI as anything but a single logical sound device?  Let ALSA handle synchronizing multiple streams together if it wants to.

> 	sound {
> 		compatible = "fsl,mpc8610-hpcd-sound";
> 		/* maybe something like (totally off the top of my head) */
> 		dai-links = <&ssi1 0 &codec 0
> 		             &ssi1 1 &codec 1>;
> 		[...]
> 	};

I don't know when I would ever do this.  The two SSI devices are completely independent.  Why would I bind them together into one "device"?

> Where the 'sound' node is now the starting point for representing a
> logical sound device instead of the ssi node.  This binding probably
> makes more sense (but I'm not committing to anything like this until I
> see a real proposal for a real device).

The only issue I have with this is all it does is turn the fabric driver from a platform driver that scans the OF tree, into an OF driver that still needs to query the device tree to get all the data it needs.  For example, the fabric driver still needs to know the clock frequency and direction of the codec node, so that it can call snd_soc_dai_set_fmt() and snd_soc_dai_set_sysclk() properly.  To eliminate that, we could have the fabric driver never call these functions, and expect the SSI and codec drivers to gather this information itself.  But the codec driver is not an OF driver, so it has no expectation of being able to query the codec node.

> I would solve the problem this way: In the ssi driver, if the
> codec-node property is present, then call a function to instantiate a
> simple or platform specific sound card instance that makes the
> assumptions listed above.  If not, then just register the ssi and
> exit, which leaves the ssi available for a separate driver to pick it
> up.  I wouldn't do this for new platforms, but it gracefully makes use
> of the data provided in the current 8610 device tree.

Eh, I'll have to think about that.  The absence of a codec pointer in the SSI node means that the SSI is not connected to a codec, so it should just be ignored altogether.  An SSI is useless if it's not connected to a codec.

> BTW Timur, there is nothing wrong with registering multiple devices
> that all have the of_node pointer set to the same node.

Sorry, I don't understand what you're getting at.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Tue, Apr 27, 2010 at 03:46:04PM -0500, Timur Tabi wrote:

[Reflowed into 80 columns; please fix your mail client.]

> > (I've omitted the DMA nodes and some irrelevant details)  This is
> > enough information for a simplistic driver registration that probably
> > makes a lot of assumptions.  Such as the ssi represents a single
> > logical sound device.  It won't handle complex representations, but in

> Why would I ever represent the SSI as anything but a single logical
> sound device?  Let ALSA handle synchronizing multiple streams together
> if it wants to.

...

> > 		dai-links = <&ssi1 0 &codec 0
> >                            &ssi1 1 &codec 1>;
> > 		[...]

> I don't know when I would ever do this.  The two SSI devices are
> completely independent.  Why would I bind them together into one
> "device"?

It's entirely possible that if the board designer intended the verious
SSIs to be used in concert they've done something like cross wire the
clocks which creates a board-specific interrelationship that needs to be
dealt with.

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

[Timur Tabi]

Mark Brown wrote:
> It's entirely possible that if the board designer intended the verious
> SSIs to be used in concert they've done something like cross wire the
> clocks which creates a board-specific interrelationship that needs to be
> dealt with.

Fine, but I don't see how that can be handled with the current code.  Each SSI is independent, and audio is streamed to it via DMA.  The current SSI driver would need to be completely rewritten in order to initiate both DMA operations simultaneously.  The clocking is the least of my problems.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Tue, Apr 27, 2010 at 04:03:27PM -0500, Timur Tabi wrote:

[Reflowing the text into 80 columns again]

> Mark Brown wrote:
> > It's entirely possible that if the board designer intended the verious
> > SSIs to be used in concert they've done something like cross wire the
> > clocks which creates a board-specific interrelationship that needs to be
> > dealt with.

> Fine, but I don't see how that can be handled with the current code.
> Each SSI is independent, and audio is streamed to it via DMA.  The
> current SSI driver would need to be completely rewritten in order to
> initiate both DMA operations simultaneously.  The clocking is the least
> of my problems.

I believe the usual technique is to start the DMA then clock the bus -
data doesn't flow over the bus until the clock appears and that appears
everywhere simultaneously.  Obviously some hardware really doesn't like
having the DMA blocked like that, but not all.

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

[Benjamin Herrenschmidt]

On Tue, 2010-04-27 at 21:59 +0100, Mark Brown wrote:
> It's entirely possible that if the board designer intended the verious
> SSIs to be used in concert they've done something like cross wire the
> clocks which creates a board-specific interrelationship that needs to
> be dealt with. 

In which case, have that in some board specific code :-) Really, as I
said earlier, I think there's no point to aim toward a
uber-representation that can describe everything along with code that
can cope with anything the tree can describe :-) That's just insane.

I'd stay stick to the basics and move incrementally up until it stops
making sense:

 - First, the basics: nodes for actual physical devices. i2c codecs on
their i2c busses, DMA controllers, etc...

 - From there, see what simple properties are better off being put in
those respective nodes because they represent common enough variants of
functionality or simply because they are specific attributes of a given
device that aren't too concerned by the way things are interconnected.

 - Provide at least one identifier (either in a "sound" virtual device,
or just use the board's own "compatible" property) to have a .c file to
put everything together.

>From there, you may decide that 90% of the simple cases can be very
easily represented by a couple of properties in the said "sound" node,
and henceforth, create a simple.c "board" file that matches against a
list of boards known to fit within that simple model, and that pretty
much exclusively use the device-tree to put things together.

But you don't have to.

The whole thing is a matter of common sense and a bit of taste :-)

Cheers,
Ben.

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

[Mark Brown]

On Wed, Apr 28, 2010 at 02:25:29PM +1000, Benjamin Herrenschmidt wrote:

> I'd stay stick to the basics and move incrementally up until it stops
> making sense:

>  - First, the basics: nodes for actual physical devices. i2c codecs on
> their i2c busses, DMA controllers, etc...

This is already supported by the ASoC core and has been for about a year
or so, it just needs the device drivers to make use of it.  IIRC the
drivers relevant to PowerPC are pretty much doing so already, but I
didn't actually check.

>  - From there, see what simple properties are better off being put in
> those respective nodes because they represent common enough variants of
> functionality or simply because they are specific attributes of a given
> device that aren't too concerned by the way things are interconnected.

This one is just a case of writing boiler plate for the existing
platform data devices have to convert that from device tree into the
existing formats.  It's not a monumentally edifying task, but I don't
think we really need to worry about it here.

>  - Provide at least one identifier (either in a "sound" virtual device,
> or just use the board's own "compatible" property) to have a .c file to
> put everything together.

This has been the big sticking point so far.  It has been difficult to
get people to accept this idea at all, the idea that there may be more
to the machine specific setup than can be readily expressed in static
data and might even vary dynamically at runtime is what seems to upset
people.  I've been told on a number of occasions that instantiating 
machine specific code that's not part of a specific chip is very
difficult and would cause the device tree to be horribly buggy.

What you're suggesting here is exactly what is supposed to happen from
an ASoC point of view.

> The whole thing is a matter of common sense and a bit of taste :-)

The impression that has been created in the past is that there are
inflexible device tree rules which can't be varied.

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

[Benjamin Herrenschmidt]

On Wed, 2010-04-28 at 14:00 +0100, Mark Brown wrote:
> > The whole thing is a matter of common sense and a bit of taste :-)
> 
> The impression that has been created in the past is that there are
> inflexible device tree rules which can't be varied.

I'm a bit sad this is how things have been perceived since that's
clearly not the policy I've applied to the powerpc architecture.

Or rather, there are -some- inflexible rules yes, which are to:

 - Have a device-tree :-)

 - Have a /compatible property at the toplevel to identify your board

 - Have the /cpus nodes for representing the CPUs.

That's pretty much the only absolute requirements from a code
perspective.

Now I -do- require people to also have nodes for things like PCI host
bridge, since that allows using a ton of existing code for handling most
aspects of PCI, and I -do- complain if people just hard wire platform
devices everywhere or interrupt numbers without even trying to consider
using the device-tree appropriately.

However, I've always been against the one-bsp-fits-all approach, and
it's always been my clear policy that there should be a per-machine .c
file. I did bend when folks pushed the "simple" platform but with the
understanding that it must contain an -explicit- list of boards it
supports.

You'll also notice that all of my virtual interrupt handling stuff is
such that you -can- use it without device-tree nodes, the DT just makes
it easier. Same goes with PCI devices (only the PHB requires a DT node
at this stage) etc... 

Cheers,
Ben.

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

[Grant Likely]

On Tue, Apr 27, 2010 at 2:46 PM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> So, the current 86xx device tree binding assumes a simple layout with
>> a node describing an DAI controller, and another node describing the
>> codec with a single phandle (pointer) from the DAI to the codec. =A0In
>> this configuration, it is completely reasonable for the DAI node to
>> trigger both the instantiation of the ASoC DAI controller device and
>> the sound card device. =A0Linux can treat them as separate even though
>> the current device tree has a simplistic representation.
>
> The only problem with this is that there is board-specific programming th=
at needs to be done (look at mpc8610_hpcd_machine_probe), so we still need =
to have a fabric driver that is independent of the SSI, codec, or DMA drive=
rs. =A0The P1022 also has an SSI, and I'm hoping that all I need to do is c=
reate a new fabric driver, not hack up the SSI driver to support board prog=
ramming.
>
> So if the fabric driver still needs to exist, then it still needs a struc=
t device, and it still needs to register with asoc. =A0I don't see how I ca=
n register the sound card itself in the SSI driver, because it won't know a=
nything about the board-specific code in the fabric driver.

Why not?  Just have the ssi driver probe routine register the fabric
device based on the existence of the codec-handle property.  It is the
best way to go about things with the data that you've got available,
and it is no big deal.  The relevant fabric driver can then bind
against that.  You should probably also stuff the ssi device node
pointer into the fabric device of_node pointer.

>> I've tried very hard to maintain a distinction between device tree
>> binding (representation) and Linux kernel internal implementation
>> details. =A0The real question is whether or not the binding provides
>> sufficient detail for the operating system to figure out what to do.
>
> I think it does, because it's working today.
>
>> In the extreme minimalist case, the audio driver could decide how to
>> configure itself solely on the board name property of the root node.
>> There is nothing wrong with that, but it also means that no data is
>> available to dynamically select common modules or modify connections;
>> it all has to be hard coded.
>
> Well, asoc already has several hard-coded requirements:
>
> =A0 =A0 =A0 =A0machine_data->dai.cpu_dai_drv =3D &fsl_ssi_dai;
> =A0 =A0 =A0 =A0machine_data->dai.codec_dai_drv =3D &cs4270_dai;
> =A0 =A0 =A0 =A0machine_data->dai.codec_drv =3D &soc_codec_device_cs4270;
> =A0 =A0 =A0 =A0machine_data->dai.ops =3D &mpc8610_hpcd_ops;
> =A0 =A0 =A0 =A0machine_data->dai.platform_drv =3D &fsl_soc_platform;
>
> So even though I probe for each device separately and register them separ=
ately, the fabric driver still needs to have hard-coded addresses
>
> Maybe the asoc guys can tell me why I need to register the cpu_dai_drv st=
ructure via platform_device_add(), when it's already being registered via s=
nd_soc_register_dai().
>
>> (I've omitted the DMA nodes and some irrelevant details) =A0This is
>> enough information for a simplistic driver registration that probably
>> makes a lot of assumptions. =A0Such as the ssi represents a single
>> logical sound device. =A0It won't handle complex representations, but in
>> a lot of cases that may be just fine.
>
> Why would I ever represent the SSI as anything but a single logical sound=
 device? =A0Let ALSA handle synchronizing multiple streams together if it w=
ants to.
>
>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 };
>
> I don't know when I would ever do this. =A0The two SSI devices are comple=
tely independent. =A0Why would I bind them together into one "device"?

If there is no use case for binding them together, then you're right.
The current binding is probably just fine.  I cannot comment on
whether or not it will be used that way by platform designers.

>> Where the 'sound' node is now the starting point for representing a
>> logical sound device instead of the ssi node. =A0This binding probably
>> makes more sense (but I'm not committing to anything like this until I
>> see a real proposal for a real device).
>
> The only issue I have with this is all it does is turn the fabric driver =
from a platform driver that scans the OF tree, into an OF driver that still=
 needs to query the device tree to get all the data it needs. =A0For exampl=
e, the fabric driver still needs to know the clock frequency and direction =
of the codec node, so that it can call snd_soc_dai_set_fmt() and snd_soc_da=
i_set_sysclk() properly. =A0To eliminate that, we could have the fabric dri=
ver never call these functions, and expect the SSI and codec drivers to gat=
her this information itself. =A0But the codec driver is not an OF driver, s=
o it has no expectation of being able to query the codec node.
>
>> I would solve the problem this way: In the ssi driver, if the
>> codec-node property is present, then call a function to instantiate a
>> simple or platform specific sound card instance that makes the
>> assumptions listed above. =A0If not, then just register the ssi and
>> exit, which leaves the ssi available for a separate driver to pick it
>> up. =A0I wouldn't do this for new platforms, but it gracefully makes use
>> of the data provided in the current 8610 device tree.
>
> Eh, I'll have to think about that. =A0The absence of a codec pointer in t=
he SSI node means that the SSI is not connected to a codec, so it should ju=
st be ignored altogether. =A0An SSI is useless if it's not connected to a c=
odec.

I'm suggesting the case where a third node describes the mappings
between codecs and SSIs.

>> BTW Timur, there is nothing wrong with registering multiple devices
>> that all have the of_node pointer set to the same node.
>
> Sorry, I don't understand what you're getting at.

Linux struct device registrations are cheap, and every struct device
has a device_node pointer available.  It is totally fine to have both
the ssi device and the fabric device point to the same device node if
that helps solve your problem of finding references to the right
things in each driver.  (Just as long as only one of them is an
of_platform driver).

g.

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

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

[Timur Tabi]

On Wed, Apr 28, 2010 at 12:37 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:

> Why not? =A0Just have the ssi driver probe routine register the fabric
> device based on the existence of the codec-handle property. =A0It is the
> best way to go about things with the data that you've got available,
> and it is no big deal. =A0The relevant fabric driver can then bind
> against that. =A0You should probably also stuff the ssi device node
> pointer into the fabric device of_node pointer.

And then where do I put the board-specific initialization code that's
currently in the fabric driver?  The programming information for that
initialization is not in the device tree.

It sounds to me like you're saying I should take all the code from the
fabric driver and shove it into the SSI driver, just so that I can
avoid instantiating a platform driver.

Keep in mind that asoc likes to have a different struct device for the
fabric driver and the SSI nodes, so I would need to manually create a
struct device for the fabric device anyway.

>> I don't know when I would ever do this. =A0The two SSI devices are compl=
etely independent. =A0Why would I bind them together into one "device"?
>
> If there is no use case for binding them together, then you're right.
> The current binding is probably just fine. =A0I cannot comment on
> whether or not it will be used that way by platform designers.

Although the 8610 has two SSI devices, the reference board we ship
only has one wired up.  I have a prototype board in the lab that has
both wired up, but even then, I don't think I can get the two SSIs
synchronized in any meaningful fashion.  Mark suggested a technique,
but even if I could get that to work, the driver code would need to be
rewritten to handle two SSI devices in concert.  In short, it might be
theoretically possible, but I'm not going to try to make it work.

> Linux struct device registrations are cheap, and every struct device
> has a device_node pointer available. =A0It is totally fine to have both
> the ssi device and the fabric device point to the same device node if
> that helps solve your problem of finding references to the right
> things in each driver. =A0(Just as long as only one of them is an
> of_platform driver).

But I already have it set up like that.  The SSI driver is an OF
driver, and the fabric driver is a platform driver.  I might be able
to move some code from the fabric driver into the SSI driver to make
it the fabric driver less obnoxious about scanning the device tree.

--=20
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 7:35 AM, Timur Tabi <timur@freescale.com> wrote:
> On Wed, Apr 28, 2010 at 12:37 AM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
>
>> Why not? =A0Just have the ssi driver probe routine register the fabric
>> device based on the existence of the codec-handle property. =A0It is the
>> best way to go about things with the data that you've got available,
>> and it is no big deal. =A0The relevant fabric driver can then bind
>> against that. =A0You should probably also stuff the ssi device node
>> pointer into the fabric device of_node pointer.
>
> And then where do I put the board-specific initialization code that's
> currently in the fabric driver? =A0The programming information for that
> initialization is not in the device tree.

In the fabric driver; where it is right now.  I'm saying *instantiate*
the device when the ssi driver gets probed.  Use the top level board
name when assigning the name so that the correct asoc machine driver
gets bound to it.

> It sounds to me like you're saying I should take all the code from the
> fabric driver and shove it into the SSI driver, just so that I can
> avoid instantiating a platform driver.

Nope.

> Keep in mind that asoc likes to have a different struct device for the
> fabric driver and the SSI nodes, so I would need to manually create a
> struct device for the fabric device anyway.

You can do it this way too, but this is not what I'm saying.

>> Linux struct device registrations are cheap, and every struct device
>> has a device_node pointer available. =A0It is totally fine to have both
>> the ssi device and the fabric device point to the same device node if
>> that helps solve your problem of finding references to the right
>> things in each driver. =A0(Just as long as only one of them is an
>> of_platform driver).
>
> But I already have it set up like that. =A0The SSI driver is an OF
> driver, and the fabric driver is a platform driver. =A0I might be able
> to move some code from the fabric driver into the SSI driver to make
> it the fabric driver less obnoxious about scanning the device tree.

I'm just saying move the registration of the machine device out of
arch/powerpc platform code and into the ssi driver.  Then you've got a
reasonable place to pass shared data (either the ssi device node or
device instance or name.  Whatever you need) to the machine driver.

g.

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

[Timur Tabi]

On Wed, Apr 28, 2010 at 8:57 AM, Grant Likely <grant.likely@secretlab.ca> w=
rote:

> I'm just saying move the registration of the machine device out of
> arch/powerpc platform code and into the ssi driver.

But the SSI driver is an OF driver, and it gets probed for every SSI
node in the device tree.  On the 8610, that means being probed twice.
But I should only call platform_device_register_simple() once.

Are you saying that I should call platform_device_register_simple()
from the SSI's driver initialization function, fsl_ssi_init()?

> Then you've got a
> reasonable place to pass shared data (either the ssi device node or
> device instance or name. =A0Whatever you need) to the machine driver.

The problem is that the fabric driver needs much more information from
the device tree than the SSI driver needs.  So if the SSI driver is
going to pass that information to the fabric driver via the platform
data, it's going to have to know what information the fabric driver
needs.  Then the SSI driver is not board-independent.

--=20
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 10:20 AM, Timur Tabi <timur@freescale.com> wrote:
> On Wed, Apr 28, 2010 at 8:57 AM, Grant Likely <grant.likely@secretlab.ca>=
 wrote:
>
>> I'm just saying move the registration of the machine device out of
>> arch/powerpc platform code and into the ssi driver.
>
> But the SSI driver is an OF driver, and it gets probed for every SSI
> node in the device tree. =A0On the 8610, that means being probed twice.
> But I should only call platform_device_register_simple() once.

Didn't you just finish saying that you cannot see any situation where
you would want the SSI devices linked into a single audio device?  So
then if both SSIs are being used for audio, then do you not need a
machine driver for each ssi?

> Are you saying that I should call platform_device_register_simple()
> from the SSI's driver initialization function, fsl_ssi_init()?

No, I'm saying call it from the probe hook.

However, you should probably do a two stage platform_device_alloc() /
platform_device_add() so you can add data (node pointer) to the
platform device before it gets probed by the machine driver.

>> Then you've got a
>> reasonable place to pass shared data (either the ssi device node or
>> device instance or name. =A0Whatever you need) to the machine driver.
>
> The problem is that the fabric driver needs much more information from
> the device tree than the SSI driver needs. =A0So if the SSI driver is
> going to pass that information to the fabric driver via the platform
> data, it's going to have to know what information the fabric driver
> needs. =A0Then the SSI driver is not board-independent.

I'm not talking about platform_data or about the ssi driver decoding
things that the machine driver needs.  I'm talking about giving the
machine driver a pointer to the ssi device tree node so you don't need
to go through weird gymnastics to find the correct node again.

g.

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

[Timur Tabi]

Grant Likely wrote:

> Didn't you just finish saying that you cannot see any situation where
> you would want the SSI devices linked into a single audio device?  So
> then if both SSIs are being used for audio, then do you not need a
> machine driver for each ssi?

That's right.  But in order for my fabric/machine driver to be called at all, I thought I needed to make it a platform driver and use platform_device_register_simple() and platform_driver_register().

>> Are you saying that I should call platform_device_register_simple()
>> from the SSI's driver initialization function, fsl_ssi_init()?
> 
> No, I'm saying call it from the probe hook.

But then platform_device_register_simple() will be called twice, and it should be called only once. 

I must be missing something here.

> However, you should probably do a two stage platform_device_alloc() /
> platform_device_add() so you can add data (node pointer) to the
> platform device before it gets probed by the machine driver.

Don't you mean platform_device_add_resources() instead of platform_device_add()?  That is, use platform_device_add_resources() too add information about each SSI that gets probed, and then after all of the SSIs are probed, then call platform_device_add().

If that's what you mean, then where do I call platform_device_add()?

> I'm not talking about platform_data or about the ssi driver decoding
> things that the machine driver needs.  I'm talking about giving the
> machine driver a pointer to the ssi device tree node so you don't need
> to go through weird gymnastics to find the correct node again.

The only gymnastics I need to go through is this:

while ((np = of_find_compatible_node(np, NULL, "fsl,mpc8610-ssi"))) {

This finds every SSI node.  I then extract the information from the SSI nodes and all the other nodes, and then register it with ASoC. 

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Tue, Apr 27, 2010 at 02:24:34PM -0600, Grant Likely wrote:

> However, as you and Mark rightly point out, it completely fails to
> represent complex sound devices with weird clocking and strange
> routes.  Something like this is probably more appropriate:

> 
> 	[...]
> 		codec1 :codec@4f {
> 			compatible = "cirrus,cs4270";
> 			reg = <0x4f>;
> 			/* MCLK source is a stand-alone oscillator */
> 			clock-frequency = <12288000>;
> 		};

You also want to be representing unused pins here.

> 	[...]
> 	ssi1: ssi@16000 {
> 		compatible = "fsl,mpc8610-ssi";
> 		[...]
> 		fsl,mode = "i2s-slave";

I'd not include the master/slave decision; it's either implied by the
fact that the CODEC has a standalone clock, a property of the link/card,
or a policy decision that the running software can change on a whim.

> 	sound {
> 		compatible = "fsl,mpc8610-hpcd-sound";
> 		/* maybe something like (totally off the top of my head) */
> 		dai-links = <&ssi1 0 &codec 0
> 		             &ssi1 1 &codec 1>;

I'm having a hard time loving this.  I'd be looking for a lot more
semantics on the links (things like information about separate clocks
for the two directions, for example) which makes me think that that
simple list format is far too simple and you want a list of more complex
objects.

There's also analogue interconnects to deal with, and jacks (including
detection and accessories).  Jacks can be particularly entertaining
here.

> Or, in other words, the device tree should *not* be used to describe
> every possible detail and permutation.  It is best used to describe
> the permutations that are common so that they don't need to be hard
> coded for each and every board.

I think the ideal is something that's purely descriptive of the hardware
and doesn't include any policy decisions.  Policy decisions covers an
awful lot of the interesting issues, though - but they're the sort of
things that can easily be changed with a new software load and therefore
don't belong in the device tree.

On the other hand from a pragmatic point of view it's just much less
hassle to just only provide the mechanism for instantiating a machine
with custom code and use that for everything.

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

[Grant Likely]

On Tue, Apr 27, 2010 at 4:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Apr 27, 2010 at 02:24:34PM -0600, Grant Likely wrote:
>
>> However, as you and Mark rightly point out, it completely fails to
>> represent complex sound devices with weird clocking and strange
>> routes. =A0Something like this is probably more appropriate:
>
>>
>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 codec1 :codec@4f {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "cirrus,cs427=
0";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x4f>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* MCLK source is a stand-al=
one oscillator */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <1228800=
0>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
>
> You also want to be representing unused pins here.
>
>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 ssi1: ssi@16000 {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-ssi";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,mode =3D "i2s-slave";
>
> I'd not include the master/slave decision; it's either implied by the
> fact that the CODEC has a standalone clock, a property of the link/card,
> or a policy decision that the running software can change on a whim.
>
>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>
> I'm having a hard time loving this. =A0I'd be looking for a lot more
> semantics on the links (things like information about separate clocks
> for the two directions, for example) which makes me think that that
> simple list format is far too simple and you want a list of more complex
> objects.

Oh, absolutely.  This example is no where near complete.  Mostly I
just wanted to give a concrete example of a 'virtual' device like Ben
was talking about.  I'm not going to even attempt to draft a complete
binding until I've got time to properly go over the issues involved
and discuss them with you and Liam.

> There's also analogue interconnects to deal with, and jacks (including
> detection and accessories). =A0Jacks can be particularly entertaining
> here.
>
>> Or, in other words, the device tree should *not* be used to describe
>> every possible detail and permutation. =A0It is best used to describe
>> the permutations that are common so that they don't need to be hard
>> coded for each and every board.
>
> I think the ideal is something that's purely descriptive of the hardware
> and doesn't include any policy decisions. =A0Policy decisions covers an
> awful lot of the interesting issues, though - but they're the sort of
> things that can easily be changed with a new software load and therefore
> don't belong in the device tree.

Agreed.

> On the other hand from a pragmatic point of view it's just much less
> hassle to just only provide the mechanism for instantiating a machine
> with custom code and use that for everything.

Also true, but this approach carries with it an incremental cost that
distributions feel the pain of.  Ultimately I think we'll find a sweet
spot somewhere in between.

Cheers,
g.

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

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

[Mark Brown]

On Tue, Apr 27, 2010 at 08:31:18PM -0600, Grant Likely wrote:
> On Tue, Apr 27, 2010 at 4:29 PM, Mark Brown

> > On the other hand from a pragmatic point of view it's just much less
> > hassle to just only provide the mechanism for instantiating a machine
> > with custom code and use that for everything.

> Also true, but this approach carries with it an incremental cost that
> distributions feel the pain of.  Ultimately I think we'll find a sweet
> spot somewhere in between.

Meh, it's not really much hassle for the distributions - it's all
handled by the kernel, they don't need to explicitly do anything.

None of the machine-specific stuff has ever been a hassle getting stuff
merged, problems have always been in the drivers for the devices which
device tree isn't going to make a blind bit of difference to (other than
the usual discussions about what exactly the device tree should look
like).

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

[Benjamin Herrenschmidt]

On Tue, 2010-04-27 at 23:29 +0100, Mark Brown wrote:
> 
> On the other hand from a pragmatic point of view it's just much less
> hassle to just only provide the mechanism for instantiating a machine
> with custom code and use that for everything.

Right, that's the balance to find. A too descriptive device-tree becomes
a mess, and attempting to deal with any kind of representation in SW is
horrible.

There is a fine balance to be found between how much goes into the
device-tree and how much is eventually just a plain C file that puts
things together.

For example, one could imagine a /sound node with simply a "compatible"
property that matches what we call in AOA terminology a "fabric" driver.
IE. The one thing specific to the board that puts bits and pieces
together.

Now, the device-tree is still obviously useful to provide things like
the actual i2c IDs of codecs, GPIOs used for various actions, link to
from various components to their clock source devices, etc.. All these
things simplify the code, avoids horrid board specific code in the
actual drivers (codecs, busses, etc...) and overall help keeping the
code more maintainable. 

This is not an issue specific to audio. The same problem to some extent
shows up at the arch level, which is why I was never too much in favor
of doing a "generic" platform on powerpc, but still want to have a per
board (or at least board family) platform .c file which has the upper
hand, even if it ends up mostly using device-tree based "helpers" to put
things together.

The device-tree helps keep the platform .c file simple and devoid of too
horrible hacks, it allows to easily pass various configuration data to
leaf drivers such as i2c thingies, PHY devices etc... without gross
hooks between these and the platform, but the platform code still has
the upper hand for doing ad-hoc bits and pieces (or overwriting the
device-tree based behaviour) if necessary.

Cheers,
Ben.

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

[Mark Brown]

On Wed, Apr 28, 2010 at 02:10:11PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2010-04-27 at 23:29 +0100, Mark Brown wrote:

> > On the other hand from a pragmatic point of view it's just much less
> > hassle to just only provide the mechanism for instantiating a machine
> > with custom code and use that for everything.

> Right, that's the balance to find. A too descriptive device-tree becomes
> a mess, and attempting to deal with any kind of representation in SW is
> horrible.

Sadly this often seems to be what the device tree folks are pushing
for.  I really want whatever you guys come up with to explicitly say
that it's OK to just write standard code like we have at the minute and
not faff around defining device tree representations for everything so
that we can just point people at that when the discussion gets too
bogged down.

> For example, one could imagine a /sound node with simply a "compatible"
> property that matches what we call in AOA terminology a "fabric" driver.
> IE. The one thing specific to the board that puts bits and pieces
> together.

This is the current ASoC design (someone probably ought to look at
merging AOA into ASoC, it's approximately the same hardware and at least
the CPU driver ought to be useful for other systems).

> Now, the device-tree is still obviously useful to provide things like
> the actual i2c IDs of codecs, GPIOs used for various actions, link to
> from various components to their clock source devices, etc.. All these
> things simplify the code, avoids horrid board specific code in the
> actual drivers (codecs, busses, etc...) and overall help keeping the
> code more maintainable. 

You're preaching to the choir here.  With ASoC all this system specific
code ends up in the machine driver (which you guys are calling the
fabric driver).  All the design stuff you're talking about here is
already dealt with as-is, it's just that the parameterisation is done
with C code and data structures in the kernel (which can deal with
multiple boards if it chooses to) rather than with device tree stuff.
The chip specific drivers do not have any board specific code so there
is no meaningful change that's being proposed to them.

The problems with the device tree have been that people have been
hostile to the idea of there being any board specific code at all in the
kernel (or a board specific device tree node for the audio), and that
people keep wanting to define some OF stuff that is supposed to cover a
wide range of boards but makes unrealistic simplifying assumptions about
what general hardware looks like.

> The device-tree helps keep the platform .c file simple and devoid of too
> horrible hacks, it allows to easily pass various configuration data to
> leaf drivers such as i2c thingies, PHY devices etc... without gross
> hooks between these and the platform, but the platform code still has
> the upper hand for doing ad-hoc bits and pieces (or overwriting the
> device-tree based behaviour) if necessary.

Once again, if you can get the device tree guys to buy into this and
stick with it that sounds good but my experience has been that this
isn't where any of these discussions end up.

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

[Benjamin Herrenschmidt]

On Wed, 2010-04-28 at 13:07 +0100, Mark Brown wrote:
> > The device-tree helps keep the platform .c file simple and devoid of too
> > horrible hacks, it allows to easily pass various configuration data to
> > leaf drivers such as i2c thingies, PHY devices etc... without gross
> > hooks between these and the platform, but the platform code still has
> > the upper hand for doing ad-hoc bits and pieces (or overwriting the
> > device-tree based behaviour) if necessary.
> 
> Once again, if you can get the device tree guys to buy into this and
> stick with it that sounds good but my experience has been that this
> isn't where any of these discussions end up. 

Well, as the person who came up with the flattened device-tree format in
the first place I suppose I qualify as a "device-tree" guy here :-)

At the moment, I'd say Grant (and to some extent Jeremy Kerr) are the
guys in charge though, but yes, I agree with you, there's a tendency to
be too over-exited and to want to do "too much" with the DT and that is
counter productive. It's a good tool but it's not going to solve world
hunger and in some places an ad-hoc bit of C code is a better option :)

Now, I don't think Grant is totally off the tracks here but I must admit
I haven't taken the time to ensure I understand perfectly everybody's
position in that debate. At least I made mine clear, hope this helps :-)

Cheers,
Ben.

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

[Grant Likely]

On Wed, Apr 28, 2010 at 6:36 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2010-04-28 at 13:07 +0100, Mark Brown wrote:
>> > The device-tree helps keep the platform .c file simple and devoid of too
>> > horrible hacks, it allows to easily pass various configuration data to
>> > leaf drivers such as i2c thingies, PHY devices etc... without gross
>> > hooks between these and the platform, but the platform code still has
>> > the upper hand for doing ad-hoc bits and pieces (or overwriting the
>> > device-tree based behaviour) if necessary.
>>
>> Once again, if you can get the device tree guys to buy into this and
>> stick with it that sounds good but my experience has been that this
>> isn't where any of these discussions end up.
>
> Well, as the person who came up with the flattened device-tree format in
> the first place I suppose I qualify as a "device-tree" guy here :-)
>
> At the moment, I'd say Grant (and to some extent Jeremy Kerr) are the
> guys in charge though, but yes, I agree with you, there's a tendency to
> be too over-exited and to want to do "too much" with the DT and that is
> counter productive. It's a good tool but it's not going to solve world
> hunger and in some places an ad-hoc bit of C code is a better option :)
>
> Now, I don't think Grant is totally off the tracks here but I must admit
> I haven't taken the time to ensure I understand perfectly everybody's
> position in that debate. At least I made mine clear, hope this helps :-)

After an IRC conversation with Timur, I think we've pretty much sorted
out the best way to handle the mpc8610 use case that allows the
ssi/dma/codec drivers to remain blissfully ignorant and bind in the
appropriate ASoC machine driver for the board.

g.

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

[Timur Tabi]

On Tue, Apr 27, 2010 at 5:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 codec1 :codec@4f {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "cirrus,cs427=
0";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x4f>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* MCLK source is a stand-al=
one oscillator */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <1228800=
0>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
>
> You also want to be representing unused pins here.

If they're unused, how do I represent them?  Can you give me an example?

>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 ssi1: ssi@16000 {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-ssi";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,mode =3D "i2s-slave";
>
> I'd not include the master/slave decision; it's either implied by the
> fact that the CODEC has a standalone clock, a property of the link/card,
> or a policy decision that the running software can change on a whim.

I know it's redundant, but at the time, it seemed a lot simpler than
walking the device tree.

Frankly, I'd rather not consider minor device tree changes at this
point.  I'm hoping I don't need to change the device tree at all.

>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>
> I'm having a hard time loving this. =A0I'd be looking for a lot more
> semantics on the links (things like information about separate clocks
> for the two directions, for example) which makes me think that that
> simple list format is far too simple and you want a list of more complex
> objects.

Yeah, I don't like it either.  It seems arbitrary.

> I think the ideal is something that's purely descriptive of the hardware
> and doesn't include any policy decisions.

I like to think that this is what we have today.

--=20
Timur Tabi
Linux kernel developer at Freescale

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

[Mark Brown]

On Wed, Apr 28, 2010 at 08:19:00AM -0500, Timur Tabi wrote:
> On Tue, Apr 27, 2010 at 5:29 PM, Mark Brown

> > You also want to be representing unused pins here.

> If they're unused, how do I represent them?  Can you give me an example?

You should arrange for something to call snd_soc_dapm_nc_pin() on any
unconnected pins.

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

[Mark Brown]

On Tue, Apr 27, 2010 at 04:36:08PM +1000, Benjamin Herrenschmidt wrote:

> Gross. Loses the linkage to device-tree etc... which is useful for audio
> especially. You should at least make sure the device node follows for
> the target driver to be able to use it :-) I'd like you to sync with
> Grant work on that matter if you are going to convert of_devices into
> platform_devices.

So, Liam already sent a message explaining the technical side of this -
the audio subsystem needs to glue together two or more components that
don't immediately have anything to do with each other from a device tree
point of view and may have rather complex interrelationships.

I'd just like to add that I *really* want to see you guys come to some
sort of firm and documented conclusion about the way to handle
situations like this.  Some variant of this seems to come up every
single time anyone tries to do anything to do with audio on a system
using the device tree and it's getting really repetitive.  What would be
really useful for audio at this point would be if we could get some sort
of decision about how to represent this stuff which we can point people
at so that work on systems using the device tree can be done without
having to deal with the device tree layout discussions that frequently
seem to be involved.

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

[Benjamin Herrenschmidt]

On Tue, 2010-04-27 at 10:54 +0100, Mark Brown wrote:
> I'd just like to add that I *really* want to see you guys come to some
> sort of firm and documented conclusion about the way to handle
> situations like this.  Some variant of this seems to come up every
> single time anyone tries to do anything to do with audio on a system
> using the device tree and it's getting really repetitive.  What would be
> really useful for audio at this point would be if we could get some sort
> of decision about how to represent this stuff which we can point people
> at so that work on systems using the device tree can be done without
> having to deal with the device tree layout discussions that frequently
> seem to be involved. 

Agreed. Just seeing how Apple fucked it up so many times, it's not a
simple problem :-)

The device-tree allows to express all of these relationship but we
should be able to come up with a reasonably "standard" way to do so to
avoid every SoC or platform doing it it's "own" way.

I think the main deal is to decide who gets to be the "master" node
which contains the various properties doing the linkage. My gut feeling
is that it could be the main transport, ie, the i2s or ac97, but people
with more experience dealing with that stuff might have other ideas.

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.

I don't have bandwidth to contribute much in this discussion right now,
at least not to lead it, so I'm happy to let others do so, but I'm happy
to provide feedback from my own experience as proposals are made.

Cheers,
Ben.

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

[Mark Brown]

On Tue, Apr 27, 2010 at 08:09:15PM +1000, Benjamin Herrenschmidt wrote:

> I think the main deal is to decide who gets to be the "master" node
> which contains the various properties doing the linkage. My gut feeling
> is that it could be the main transport, ie, the i2s or ac97, but people
> with more experience dealing with that stuff might have other ideas.

You're not going to find a single master transport node on more complex
systems - some systems have multiple audio interfaces to the CPU (to
allow use of hardware mixing, for example), and in systems with things
like offboard DSPs or basebands it's not always clear that the CPU Linux
is running on is in charge of anything.

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

In terms of where to shove any data this is sort of the solution I
favour, it's pretty much exactly what's implemented on other platforms
at the moment and it seems to adequately represent the physical board
(it's not a million miles away from what Timur has here).

The main problem is that trying to define a language which can represent
the needs of modern mobile audio subsystems just doesn't seem worth the
effort.  The clocking arrangements for modern mobile audio subsystems
aren't trivial, are normally highly device and system specific, and when
you add on more exotic things like sharing the bit and frame clocks
between multiple audio interfaces it gets even more involved.

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

[Grant Likely]

On Tue, Apr 27, 2010 at 4:09 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2010-04-27 at 10:54 +0100, Mark Brown wrote:
>> I'd just like to add that I *really* want to see you guys come to some
>> sort of firm and documented conclusion about the way to handle
>> situations like this. =A0Some variant of this seems to come up every
>> single time anyone tries to do anything to do with audio on a system
>> using the device tree and it's getting really repetitive. =A0What would =
be
>> really useful for audio at this point would be if we could get some sort
>> of decision about how to represent this stuff which we can point people
>> at so that work on systems using the device tree can be done without
>> having to deal with the device tree layout discussions that frequently
>> seem to be involved.

Yes, you're right.  I completely agree.

[...]
> 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.
>
> I don't have bandwidth to contribute much in this discussion right now,
> at least not to lead it, so I'm happy to let others do so, but I'm happy
> to provide feedback from my own experience as proposals are made.

Unfortunately, I'm in the same boat.  :-(  However, I'll be at UDS in
2 weeks time, and I know audio is a big concern for the Ubuntu folks.
A bunch of the ARM vendors will be there too.  I'll schedule a session
to talk about audio bindings and hopefully that way make some headway
on defining a binding that makes sense and is actually useful.

g.

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

[Mark Brown]

On Tue, Apr 27, 2010 at 02:27:42PM -0600, Grant Likely wrote:
> On Tue, Apr 27, 2010 at 4:09 AM, Benjamin Herrenschmidt

> > I don't have bandwidth to contribute much in this discussion right now,
> > at least not to lead it, so I'm happy to let others do so, but I'm happy
> > to provide feedback from my own experience as proposals are made.

> Unfortunately, I'm in the same boat.  :-(  However, I'll be at UDS in
> 2 weeks time, and I know audio is a big concern for the Ubuntu folks.
> A bunch of the ARM vendors will be there too.  I'll schedule a session
> to talk about audio bindings and hopefully that way make some headway
> on defining a binding that makes sense and is actually useful.

Hrm, the only issue that's been raised upstream is multi-CODEC (there
are one or two other things that boil down to multi-CODEC, but nothing
else I'm aware of).  If you schedule something please announce it here,
I believe that UDS generally has arrangements for remote participation.

Note that most of the complexity explosion you'll see here is visible in
things like phones more than the sort of devices you'd run Ubuntu on
currently and much of it isn't really visible to a lot of the CPU
vendors at all since it's generally all handled off the CPU.

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

[Timur Tabi]

Mark Brown wrote:
> Hrm, the only issue that's been raised upstream is multi-CODEC (there
> are one or two other things that boil down to multi-CODEC, but nothing
> else I'm aware of).  If you schedule something please announce it here,
> I believe that UDS generally has arrangements for remote participation.

Keep in mind that the phandle-type properties can accept multiple phandles.  So for instance, if I had two codecs attached to an SSI, the SSI node would look like this:

	ssi@16000 {
		compatible = "fsl,mpc8610-ssi";
		...
		codec-handle = <&cs4270_1 &cs4270_2>;
	};

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Liam Girdwood]

On Tue, 2010-04-27 at 14:27 -0600, Grant Likely wrote:

> 
> Unfortunately, I'm in the same boat.  :-(  However, I'll be at UDS in
> 2 weeks time, and I know audio is a big concern for the Ubuntu folks.
> A bunch of the ARM vendors will be there too.  I'll schedule a session
> to talk about audio bindings and hopefully that way make some headway
> on defining a binding that makes sense and is actually useful.

It's not clear if I'm going to UDS atm, but I'll join via IRC. Graeme is
going to UDS so he will probably pop in to the discussion if he's free.

Liam 

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

[Timur Tabi]

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

-- 
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 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. =A0I'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? =A0It would
> be a root-level node.
>
> =A0 =A0 =A0 =A0sound-devices {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sound0 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ssi =3D &ssi0;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0playback-dma =3D &dma00;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0capture-dma =3D &dma01;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0codec =3D &cs4270;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0};

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 =3D "system-devices";
        sound0 {
                compatible =3D "fsl,mpc8610hpcd-sound";
                fsl,ssi =3D &ssi0;
                fsl,playback-dma =3D &dma00;
                fsl,capture-dma =3D &dma01;
                fsl,codec =3D &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: [alsa-devel] [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> w=
rote:

> 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. =A0It 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. =A0Other 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. =A0It would be nice to have a single place for collecting
> nodes for stuff like this. =A0Perhaps this:
>
> system {
> =A0 =A0 =A0 =A0compatible =3D "system-devices";
> =A0 =A0 =A0 =A0sound0 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "fsl,mpc8610hpcd-sound";
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsl,ssi =3D &ssi0;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsl,playback-dma =3D &dma00;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsl,capture-dma =3D &dma01;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsl,codec =3D &cs4270;
> =A0 =A0 =A0 =A0};
> };

I like that.

--=20
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@gmail.com> 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.

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. =A0It 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. =A0Other 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. =A0I 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.

--=20
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@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.
> 

Re: [alsa-devel] [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.

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

[Benjamin Herrenschmidt]

On Wed, 2010-04-28 at 15:35 -0500, Timur Tabi wrote:
> 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;
>                 }
>         }; 

Make sure you also have a "compatible" property to uniquely identify the
design. You could use the toplevel board one but I'd rather keep a
separate one here. I've seen case where the exact same base board may
have different sound components (because they are dautherboards for
example, but there's a few other cases). 

Cheers,
Ben.

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

[Grant Likely]

On Mon, Apr 26, 2010 at 2:49 PM, Timur Tabi <timur@freescale.com> wrote:
> An upcoming change in the architecture of ALSA SoC (ASoC) will require th=
e
> MPC8610 HPCD's ASoC fabric driver to register as a standard platform driv=
er.
> Therefore, we need to call platform_device_register_simple() from the boa=
rd's
> platform code.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> Kumar, the ASoC mainters are willing to pick up this patch, but they want=
 an
> ACK from you first. =A0Or, you could pick it up, since by itself it's har=
mless.
>
> =A0arch/powerpc/platforms/86xx/mpc8610_hpcd.c | =A0 =A03 +++
> =A01 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/pl=
atforms/86xx/mpc8610_hpcd.c
> index 5abe137..66afff3 100644
> --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> @@ -98,6 +98,9 @@ static int __init mpc8610_declare_of_platform_devices(v=
oid)
> =A0 =A0 =A0 =A0/* Without this call, the SSI device driver won't get prob=
ed. */
> =A0 =A0 =A0 =A0of_platform_bus_probe(NULL, mpc8610_ids, NULL);
>
> + =A0 =A0 =A0 /* Register the platform device for the ASoC fabric driver =
*/
> + =A0 =A0 =A0 platform_device_register_simple("snd-soc-mpc8610-hpcd", 0, =
NULL, 0);
> +

Since this is a temporary measure, this device registration is
probably best put into the .probe() hook of the i2s driver.  That will
keep everything contained in the same place until we can hammer out a
reasonable binding.

g.

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

[Timur Tabi]

Grant Likely wrote:
>> +       /* Register the platform device for the ASoC fabric driver */
>> > +       platform_device_register_simple("snd-soc-mpc8610-hpcd", 0, NULL, 0);
>> > +
> Since this is a temporary measure, this device registration is
> probably best put into the .probe() hook of the i2s driver.  That will
> keep everything contained in the same place until we can hammer out a
> reasonable binding.

This part is not temporary, I think.  The fabric driver will always be a standard platform driver, not an OF driver, because there are no DT nodes that it can probe against.  BenH is suggestion that maybe we can create one, but I'm not sure that's really the best approach.

-- 
Timur Tabi
Linux kernel developer at Freescale