Thoughts on ASOC v2 driver architecture

Thoughts on ASOC v2 driver architecture

[Jon Smirl]

I'm trying to sort out an architecture that will let all of the code
used in my ASOC drivers to be automatically loaded by the kernel.  So
if I'm confused and have some of this wrong please explain where I've
gone astray.

First, what are the physical hardware devices present? From the device
tree it says I have two i2s controllers each with an i2c controlled
codec. It also says I have a system wide DMA controller. And finally
there is the "fabric" which represent the specific pcb (populated
jacks, etc) used by the system (the root of the device tree).

Upon booting the kernel it will find the five open firmware devices
(two i2s, two codecs, one DMA) and load the appropriate of drivers for
them. Note that this is the system wide DMA driver, not the ASOC one.
Three drivers will be loaded and five devices will be created.

The codec devices will each register one codec and multiple codec
DAIs. Each i2c channel will register a platform_dai.

The next part, the platform device,  is where I'm having trouble. Why
is the DMA subsystem in it's own ASOC device? I already have a
separate system wide device for my DMA hardware.  Looking at the
implementations of a couple of the ASOC DMA drivers, there is nothing
global about the driver, everything is done with per channel
instances.

Should the DMA code instead simply be a library that is linked to by
the i2s drivers? And then move the entry points registered by the DMA
driver up into the platform_dai? It seems to me like implementation of
the DMA code should be a private matter for the platform_dai/i2s
driver. Note that I'm not saying that there shouldn't be modularized
ASOC DMA code, instead, why should ASOC be aware of this shared code?
To see if this model would work I tried to visualize a system where
one i2s controller is using DMA and the second is doing programmed IO.

So if the DMA API is pushed into the platform_dai that would free up
the platform device. It could then be used to support the fabric code
(fabric is the motherboard specific bits of code). It should probably
be renamed to the machine device instead of platform device. To handle
the case of HDA not needing a fabric driver,  ASOC would have a
default internal machine driver that a fabric driver could override.
This also implies that the fabric driver can't be the last driver
registered with ASOC.

There would be one global fabric driver, but it would get called on a
per instance basis. You could implement (made up scenarios)  two amps
on the motherboard and only enough power to turn one of them on. Or
maybe a front panel switch that switches which channel the front panel
controls act on.

I'd end up with this object model

generic machine (optional machine specific fabric override)
    platform i2s (platform_dai, soc_snd_card)
        generic codec (choice of codec dai)
    platform i2s (platform_dai, soc_snd_card)
        generic codec (choice of codec dai)

The platform specific i2s drivers are linked to a shared library of
platform DMA code.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Liam Girdwood]

On Sun, 2008-06-15 at 14:10 -0400, Jon Smirl wrote:
> I'm trying to sort out an architecture that will let all of the code
> used in my ASOC drivers to be automatically loaded by the kernel.  So
> if I'm confused and have some of this wrong please explain where I've
> gone astray.
> 
> First, what are the physical hardware devices present? From the device
> tree it says I have two i2s controllers each with an i2c controlled
> codec. It also says I have a system wide DMA controller. And finally
> there is the "fabric" which represent the specific pcb (populated
> jacks, etc) used by the system (the root of the device tree).
> 
> Upon booting the kernel it will find the five open firmware devices
> (two i2s, two codecs, one DMA) and load the appropriate of drivers for
> them. Note that this is the system wide DMA driver, not the ASOC one.
> Three drivers will be loaded and five devices will be created.
> 

I assume your system is PPC.

> The codec devices will each register one codec and multiple codec
> DAIs. Each i2c channel will register a platform_dai.
> 

I also assume you mean I2S instead of I2C here ;)

A codec will only register a DAI (Digital Audio Interface - I2S, PCM,
AC97) for each physical or logical DAI it has. i.e. the WM8750 on the
Sharp Tosa only has 1 physical DAI and registers only 1 DAI with the
core. However, the WM8753 used on the OpenMoko Neo1973 has 2 physical
DAIs and can also reconfigure them at runtime and hence registers 4 DAIs
with the core.  

A platform (i.e. SoC CPU platform) will register a DAI for every DAI
present at DAI module load time. i.e. on pxa2xx we can register a DAI
for AC97, I2S and PCM (SSP). 

> The next part, the platform device,  is where I'm having trouble. Why
> is the DMA subsystem in it's own ASOC device? I already have a
> separate system wide device for my DMA hardware.  Looking at the
> implementations of a couple of the ASOC DMA drivers, there is nothing
> global about the driver, everything is done with per channel
> instances.
> 
> Should the DMA code instead simply be a library that is linked to by
> the i2s drivers? And then move the entry points registered by the DMA
> driver up into the platform_dai? It seems to me like implementation of
> the DMA code should be a private matter for the platform_dai/i2s
> driver. Note that I'm not saying that there shouldn't be modularized
> ASOC DMA code, instead, why should ASOC be aware of this shared code?
> To see if this model would work I tried to visualize a system where
> one i2s controller is using DMA and the second is doing programmed IO.
> 
> So if the DMA API is pushed into the platform_dai that would free up
> the platform device. It could then be used to support the fabric code
> (fabric is the motherboard specific bits of code). It should probably
> be renamed to the machine device instead of platform device. To handle
> the case of HDA not needing a fabric driver,  ASOC would have a
> default internal machine driver that a fabric driver could override.
> This also implies that the fabric driver can't be the last driver
> registered with ASOC.

HDA has it's own controller and codec spec and hence won't require ASoC.
ASoC is only for codecs and CPUs with simple DAIs i.e. I2S, PCM and
AC97. 

> 
> There would be one global fabric driver, but it would get called on a
> per instance basis. You could implement (made up scenarios)  two amps
> on the motherboard and only enough power to turn one of them on. Or
> maybe a front panel switch that switches which channel the front panel
> controls act on.
> 
> I'd end up with this object model
> 
> generic machine (optional machine specific fabric override)
>     platform i2s (platform_dai, soc_snd_card)
>         generic codec (choice of codec dai)
>     platform i2s (platform_dai, soc_snd_card)
>         generic codec (choice of codec dai)
> 
> The platform specific i2s drivers are linked to a shared library of
> platform DMA code.
> 

Ok, I think I have a good idea of what you are proposing here. 

The ASoC DMA is currently very much like a library in that it exports
it's ALSA PCM functions to the core (by registering itself) and whilst
the DMA functions are not directly called by client CPU DAI drivers they
are indirectly called on the client DAIs behalf by the ASoC core (in
response to ALSA core). This means we don't have to burden our DAI
drivers with CPU DMA specific calls.

Fwiw, I do think platform driver could really be better named to be more
function specific e.g. dma driver. I'm also considering a rename of
machine to fabric.

We also have CPU DAIs that are shared between different CPU
architectures (with different DMA). e.g. i.MX31 and FSL PPC share the
same SSI architecture - breaking the tightly coupled library model.

Liam

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Liam Girdwood <lg@opensource.wolfsonmicro.com> wrote:
> On Sun, 2008-06-15 at 14:10 -0400, Jon Smirl wrote:
>  > I'm trying to sort out an architecture that will let all of the code
>  > used in my ASOC drivers to be automatically loaded by the kernel.  So
>  > if I'm confused and have some of this wrong please explain where I've
>  > gone astray.
>  >
>  > First, what are the physical hardware devices present? From the device
>  > tree it says I have two i2s controllers each with an i2c controlled
>  > codec. It also says I have a system wide DMA controller. And finally
>  > there is the "fabric" which represent the specific pcb (populated
>  > jacks, etc) used by the system (the root of the device tree).
>  >
>  > Upon booting the kernel it will find the five open firmware devices
>  > (two i2s, two codecs, one DMA) and load the appropriate of drivers for
>  > them. Note that this is the system wide DMA driver, not the ASOC one.
>  > Three drivers will be loaded and five devices will be created.
>  >
> I assume your system is PPC.

Yes, similar patterns to this happen on the other platforms. They use
code in the arch directories to load their drivers instead of a device
tree.

>
>
>  > The codec devices will each register one codec and multiple codec
>  > DAIs. Each i2c channel will register a platform_dai.
>  >
>
> I also assume you mean I2S instead of I2C here ;)

yes

>
>  A codec will only register a DAI (Digital Audio Interface - I2S, PCM,
>  AC97) for each physical or logical DAI it has. i.e. the WM8750 on the
>  Sharp Tosa only has 1 physical DAI and registers only 1 DAI with the
>  core. However, the WM8753 used on the OpenMoko Neo1973 has 2 physical
>  DAIs and can also reconfigure them at runtime and hence registers 4 DAIs
>  with the core.

yes, the ones I am writing have done this

>  A platform (i.e. SoC CPU platform) will register a DAI for every DAI
>  present at DAI module load time. i.e. on pxa2xx we can register a DAI
>  for AC97, I2S and PCM (SSP).

i agree

>  > The next part, the platform device,  is where I'm having trouble. Why
>  > is the DMA subsystem in it's own ASOC device? I already have a
>  > separate system wide device for my DMA hardware.  Looking at the
>  > implementations of a couple of the ASOC DMA drivers, there is nothing
>  > global about the driver, everything is done with per channel
>  > instances.
>  >
>  > Should the DMA code instead simply be a library that is linked to by
>  > the i2s drivers? And then move the entry points registered by the DMA
>  > driver up into the platform_dai? It seems to me like implementation of
>  > the DMA code should be a private matter for the platform_dai/i2s
>  > driver. Note that I'm not saying that there shouldn't be modularized
>  > ASOC DMA code, instead, why should ASOC be aware of this shared code?
>  > To see if this model would work I tried to visualize a system where
>  > one i2s controller is using DMA and the second is doing programmed IO.
>  >
>  > So if the DMA API is pushed into the platform_dai that would free up
>  > the platform device. It could then be used to support the fabric code
>  > (fabric is the motherboard specific bits of code). It should probably
>  > be renamed to the machine device instead of platform device. To handle
>  > the case of HDA not needing a fabric driver,  ASOC would have a
>  > default internal machine driver that a fabric driver could override.
>  > This also implies that the fabric driver can't be the last driver
>  > registered with ASOC.
>
> HDA has it's own controller and codec spec and hence won't require ASoC.
>  ASoC is only for codecs and CPUs with simple DAIs i.e. I2S, PCM and
>  AC97.

I haven't worked with HDA, only i2s and ac97. So far none of the SOC
CPUs have HDA.

>  > There would be one global fabric driver, but it would get called on a
>  > per instance basis. You could implement (made up scenarios)  two amps
>  > on the motherboard and only enough power to turn one of them on. Or
>  > maybe a front panel switch that switches which channel the front panel
>  > controls act on.
>  >
>  > I'd end up with this object model
>  >
>  > generic machine (optional machine specific fabric override)
>  >     platform i2s (platform_dai, soc_snd_card)
>  >         generic codec (choice of codec dai)
>  >     platform i2s (platform_dai, soc_snd_card)
>  >         generic codec (choice of codec dai)
>  >
>  > The platform specific i2s drivers are linked to a shared library of
>  > platform DMA code.
>  >
>
>
> Ok, I think I have a good idea of what you are proposing here.
>
>  The ASoC DMA is currently very much like a library in that it exports
>  it's ALSA PCM functions to the core (by registering itself) and whilst
>  the DMA functions are not directly called by client CPU DAI drivers they
>  are indirectly called on the client DAIs behalf by the ASoC core (in
>  response to ALSA core). This means we don't have to burden our DAI
>  drivers with CPU DMA specific calls.

You're missing the part I'm have trouble with. My company has four
board models that all use the same PCB but are populated differently .
The device tree for each different population model has a different
name so the kernel can tell exactly which hardware it is running on.

I'd like to have these drivers:
codec - generic driver for my codec chip
platform_dai - in my case an i2s driver, links to a private DMA
library, this code is generic to my SOC CPU
machine(fabric) - a driver that specifically handle this board's configuration.

I'd like for the fabric driver to be dynamically loaded by the device tree.

Given the way I'm looking at the problem, the current design is
inverted. With current code the machine driver is loaded, it then in
turn registers the cpu_dai and codec. This causes all of the common
registration code to end up in the machine driver. These drivers then
get copy/pasted for all of the different machine variants.

The root of my problem is that I want to handle my four board variants
with a single kernel. So when the kernel loads, I need to figure out
which board specific code to load. In the current ASOC design it is
assumed that this code is permanently linked in as a platform driver.

I need a slightly different boot sequence.

kernel boots
of_platform drivers load - this loads the i2s driver and the codec driver
in i2s probe it looks at the device tree and loads the right
machine(fabric) driver.
asoc_register hooks everything up

The current model works like this:
kernel boots
linked in platform drivers initialize - machine(fabric) driver
of_platform drivers load - this loads the i2s driver and the codec driver
i2s probe occurs
asoc_register hooks everything up
--- the machine(fabric) driver was determined at kernel compile time.

I can't make the machine(fabric) driver in to an of_platform driver
because there is no real hardware behind it. In older threads we
discussed creating a virtual OF device for it but
nobody liked the idea.

If I keep thinking about this sooner or later I'll come up with a boot
sequence that works.


>  Fwiw, I do think platform driver could really be better named to be more
>  function specific e.g. dma driver. I'm also considering a rename of
>  machine to fabric.
>
>  We also have CPU DAIs that are shared between different CPU
>  architectures (with different DMA). e.g. i.MX31 and FSL PPC share the
>  same SSI architecture - breaking the tightly coupled library model.

Nothing I propose stops this, there just isn't any requirement that
ASOC be aware that it is happing. struct snd_pcm_ops could hang off
from the cpu_dai.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Liam Girdwood <lg@opensource.wolfsonmicro.com> wrote:
> Ok, I think I have a good idea of what you are proposing here.
>
>  The ASoC DMA is currently very much like a library in that it exports
>  it's ALSA PCM functions to the core (by registering itself) and whilst
>  the DMA functions are not directly called by client CPU DAI drivers they
>  are indirectly called on the client DAIs behalf by the ASoC core (in
>  response to ALSA core). This means we don't have to burden our DAI
>  drivers with CPU DMA specific calls.

Doesn't the current model of a global ASOC DMA driver require that all
cpu_dai channels use the same transport? Should the model allow
different transports on different cpu_dai channels (maybe some
programmed IO, and some DMA)?

I don't need this for my hardware, I'm just wondering if it should be allowed.

>  Fwiw, I do think platform driver could really be better named to be more
>  function specific e.g. dma driver. I'm also considering a rename of
>  machine to fabric.
>
>  We also have CPU DAIs that are shared between different CPU
>  architectures (with different DMA). e.g. i.MX31 and FSL PPC share the
>  same SSI architecture - breaking the tightly coupled library model.
>
>
>  Liam
>
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Jon Smirl wrote:

> I'd like for the fabric driver to be dynamically loaded by the device tree.

You have a PPC problem, not an ASoC problem.  You're trying to use the 
device tree to load a device without specifying a specific node.  This 
is particularly difficult with device trees because once a driver has 
claimed a node via a probe, that node isn't probed again.  So you can't 
use any I2S, DMA, or codec nodes.

> The root of my problem is that I want to handle my four board variants
> with a single kernel. So when the kernel loads, I need to figure out
> which board specific code to load. In the current ASOC design it is
> assumed that this code is permanently linked in as a platform driver.

Not with ASoC V2.  I load my fabric driver as a regular module.  It also 
doesn't matter what order I load the drivers in.  Once the fourth driver 
is loaded, ASoC does its magic and creates a sound card (or two) for me.

So do what I did: deal with the limitations of ASoC V1 as best you can, 
and port your driver to ASoC V2.

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
>
> > I'd like for the fabric driver to be dynamically loaded by the device
> tree.
> >
>
>  You have a PPC problem, not an ASoC problem.  You're trying to use the
> device tree to load a device without specifying a specific node.  This is
> particularly difficult with device trees because once a driver has claimed a
> node via a probe, that node isn't probed again.  So you can't use any I2S,
> DMA, or codec nodes.
>
>
> > The root of my problem is that I want to handle my four board variants
> > with a single kernel. So when the kernel loads, I need to figure out
> > which board specific code to load. In the current ASOC design it is
> > assumed that this code is permanently linked in as a platform driver.
> >
>
>  Not with ASoC V2.  I load my fabric driver as a regular module.  It also
> doesn't matter what order I load the drivers in.  Once the fourth driver is
> loaded, ASoC does its magic and creates a sound card (or two) for me.
>
>  So do what I did: deal with the limitations of ASoC V1 as best you can, and
> port your driver to ASoC V2.

I'm already using ASOC V2.

I think I've come up with a solution. Push all of that generic setup
code in mpc8610_hpcd into fsl_ssi. Leave only the platform specific
support in mpc8610_hpcd. Now turn mpc8610_hpcd into a loadable module
but leave it as a platform_driver. During the fsl_ssi probe function
extract the platform name from the device tree and use it to
dynamically load the mpc8610_hpcd driver.


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Mark Brown]

On Mon, Jun 16, 2008 at 10:32:37AM -0400, Jon Smirl wrote:
> On 6/16/08, Timur Tabi <timur@freescale.com> wrote:
> > Jon Smirl wrote:

> > > I'd like for the fabric driver to be dynamically loaded by the device
> > tree.

> >  You have a PPC problem, not an ASoC problem.  You're trying to use the
> > device tree to load a device without specifying a specific node.  This is

...

> I think I've come up with a solution. Push all of that generic setup
> code in mpc8610_hpcd into fsl_ssi. Leave only the platform specific
> support in mpc8610_hpcd. Now turn mpc8610_hpcd into a loadable module
> but leave it as a platform_driver. During the fsl_ssi probe function
> extract the platform name from the device tree and use it to
> dynamically load the mpc8610_hpcd driver.

That should work from an ASoC point of view.

As Timur says, this is a PowerPC-specific problem - ASoC v2 itself
doesn't care how the various devices get instantiated.  The fabric,
codec and SoC drivers can all come up in any order and the core will
sort things out.

OOI, I guess that if there were some visible control on the board (eg, a
few controls via GPIO) then this would be less of an issue since there
would be real hardware for the machine/fabric driver to control?

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Jon Smirl wrote:

> I think I've come up with a solution. Push all of that generic setup
> code in mpc8610_hpcd into fsl_ssi.

I would NACK a patch that did that, if you were to submit it here.

> Leave only the platform specific
> support in mpc8610_hpcd. Now turn mpc8610_hpcd into a loadable module
> but leave it as a platform_driver. During the fsl_ssi probe function
> extract the platform name from the device tree and use it to
> dynamically load the mpc8610_hpcd driver.

If you want to augment the drivers to get more information from the device tree,
like names, I would be okay with that.  But I don't like moving any fabric
functionality from the fabric driver into another driver.  The SSI driver cannot
ever be aware of more than one SSI at a time.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Mark Brown wrote:

> OOI, I guess that if there were some visible control on the board (eg, a
> few controls via GPIO) then this would be less of an issue since there
> would be real hardware for the machine/fabric driver to control?

Probably not, because the GPIO node in the device tree would be owned by the
GPIO driver.  I don't think there's a way to have the fabric driver probed from
the device tree.  It could be probed manually from the arch/powerpc platform driver.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Mark Brown]

On Mon, Jun 16, 2008 at 10:36:19AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> > OOI, I guess that if there were some visible control on the board (eg, a
> > few controls via GPIO) then this would be less of an issue since there
> > would be real hardware for the machine/fabric driver to control?

> Probably not, because the GPIO node in the device tree would be owned by the
> GPIO driver.  I don't think there's a way to have the fabric driver probed from
> the device tree.  It could be probed manually from the arch/powerpc platform driver.

But wouldn't it now be legal to represent the machine driver as a device
in its own right, even if it is connected via GPIOs?

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Mark Brown wrote:

> But wouldn't it now be legal to represent the machine driver as a device
> in its own right, even if it is connected via GPIOs?

I'm not sure I understand that, so let me say this:

When a driver wants to be probed, it creates a list that describes the kind of
nodes it wants to be probed on.  Typically, the list includes the contents of
the "compatible" property.  The kernel then scans the device tree, and calls the
driver for each matching node.

In the driver's probe function, the driver can either return success or failure.
If it returns success, the driver "owns" the node.  No other driver will ever
get probed for that node again.  This prevents more than one driver from talking
to a particular hardware device.

So if the fabric driver were to list the GPIO node in its probe request, then a
*real* GPIO driver would never get probed (or the other way around).

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Jun 16, 2008 at 10:32:37AM -0400, Jon Smirl wrote:
>  > On 6/16/08, Timur Tabi <timur@freescale.com> wrote:
>  > > Jon Smirl wrote:
>
>  > > > I'd like for the fabric driver to be dynamically loaded by the device
>  > > tree.
>
>  > >  You have a PPC problem, not an ASoC problem.  You're trying to use the
>  > > device tree to load a device without specifying a specific node.  This is
>
>
> ...
>
>
>  > I think I've come up with a solution. Push all of that generic setup
>  > code in mpc8610_hpcd into fsl_ssi. Leave only the platform specific
>  > support in mpc8610_hpcd. Now turn mpc8610_hpcd into a loadable module
>  > but leave it as a platform_driver. During the fsl_ssi probe function
>  > extract the platform name from the device tree and use it to
>  > dynamically load the mpc8610_hpcd driver.
>
>
> That should work from an ASoC point of view.
>
>  As Timur says, this is a PowerPC-specific problem - ASoC v2 itself
>  doesn't care how the various devices get instantiated.  The fabric,
>  codec and SoC drivers can all come up in any order and the core will
>  sort things out.
>
>  OOI, I guess that if there were some visible control on the board (eg, a
>  few controls via GPIO) then this would be less of an issue since there
>  would be real hardware for the machine/fabric driver to control?

This seems to be a more global problem. I'm working on audio devices
so I was putting it into the ASOC bucket.

Some more pieces of the puzzle are starting to come together for me.
It's this type of code is causing the problem. Timur's version of this
is more complicated.

static struct platform_device codec = {
	.name		= "wm9713-codec",
	.id		= -1,
};

static struct platform_device platform = {
	.name		= "Mainstone-WM9713",
	.id		= -1,
};

static struct platform_device *devices[] = {
	&codec,
	&platform,
};

static int __init mainstone_asoc_init(void)
{
	platform_add_devices(&devices[0], ARRAY_SIZE(devices));
	return platform_driver_register(&mainstone_wm9713_driver);
}

The drivers are creating their own platform devices. Constructing them
in the driver builds in the assumption that if the driver is loaded,
then the devices are present.  But that's not right in my case, I have
four fabric drivers built in and I only want one of them active.

The devices are being created in the wrong place. So instead of my
proposal of reading the platform name out of the device tree and
loading the driver from fsl_ssi,  in Timur's case the fabric device
should be created in arch/powerpc/platforms/86xx/mpc8610_hpcd.c.
PowerPC is already capable of making the codec device from the device
tree.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Jon Smirl wrote:

> The drivers are creating their own platform devices. Constructing them
> in the driver builds in the assumption that if the driver is loaded,
> then the devices are present.  But that's not right in my case, I have
> four fabric drivers built in and I only want one of them active.

You have two choices:

1) Load all drivers.  In each driver's __init section, let it determine whether
it should load or not.  If not, it can fail silently.  Off the top of my head, I
don't remember how to do that.

2) Use the Kernel Module Loader to dynamically load the module you want.  I
don't know how this works either.

> The devices are being created in the wrong place. So instead of my
> proposal of reading the platform name out of the device tree and
> loading the driver from fsl_ssi,  in Timur's case the fabric device
> should be created in arch/powerpc/platforms/86xx/mpc8610_hpcd.c.
> PowerPC is already capable of making the codec device from the device
> tree.

True, but I chose to keep it in sound/soc/fsl so that I wouldn't have to push 3
drivers to alsa-devel and one driver to linuxppc-dev.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

A more familiar example of this problem is unified graphics drivers.
NVidia probably has 100 variations on their GPUs. They used to have
dozens of device drivers for these chips. Figuring out exactly which
GPU you have and loading the right driver for it is too hard for the
customer to do. So nvidia has built a unified driver for all their
hardware. When it initializes it detects the exact GPU type and
discards all the unneeded code.

I have the same problem. We have four devices that are almost
identical but not quite. Each of these devices has a different product
name in their device tree. I want to be able to send out a single
kernel image that can support all four devices.

To do this I'll put all of the device drivers on an initrd. Then as
the kernel processes the device tree the right drivers will get
loaded. After the boot process is finished the initrd and unused
drivers disappear. Moving the fabric device creation in the my
equivalent of arch/powerpc/platforms/86xx/mpc8610_hpcd.c will let me
do this.

BTW, if you read in /Documentation about platform drivers they say not
to make the devices in the module init function because it causes
problems like the one I'm having.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
>  > The drivers are creating their own platform devices. Constructing them
>  > in the driver builds in the assumption that if the driver is loaded,
>  > then the devices are present.  But that's not right in my case, I have
>  > four fabric drivers built in and I only want one of them active.
>
>
> You have two choices:
>
>  1) Load all drivers.  In each driver's __init section, let it determine whether
>  it should load or not.  If not, it can fail silently.  Off the top of my head, I
>  don't remember how to do that.
>
>  2) Use the Kernel Module Loader to dynamically load the module you want.  I
>  don't know how this works either.
>
>
>  > The devices are being created in the wrong place. So instead of my
>  > proposal of reading the platform name out of the device tree and
>  > loading the driver from fsl_ssi,  in Timur's case the fabric device
>  > should be created in arch/powerpc/platforms/86xx/mpc8610_hpcd.c.
>  > PowerPC is already capable of making the codec device from the device
>  > tree.
>
>
> True, but I chose to keep it in sound/soc/fsl so that I wouldn't have to push 3
>  drivers to alsa-devel and one driver to linuxppc-dev.

The driver stays in sound/soc/fsl. It's only the creation of the
device that moves. You just need to add this to
arch/powerpc/platforms/86xx/mpc8610_hpcd.c and remove it from
mpc8610_hpcd.c. Adding this to
arch/powerpc/platforms/86xx/mpc8610_hpcd.c will have no effect if the
mpc8610_hpcd.c driver is not to be found.

You've complicated it by creating two fabric devices but I believe
your code could be changed to only make one.

static struct platform_device alsa_fabric = {
	.name		= "MPC8610HPCD",
	.id		= -1,
};

static struct platform_device *devices[] = {
	&alsa_fabric,
};

platform_add_devices(&devices[0], ARRAY_SIZE(devices));


I'd rename the driver to something less generic.
	.name		= "MPC8610HPCD-fabric",



>
>
>  --
>  Timur Tabi
>  Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Jon Smirl wrote:

> The driver stays in sound/soc/fsl. It's only the creation of the
> device that moves. You just need to add this to
> arch/powerpc/platforms/86xx/mpc8610_hpcd.c and remove it from
> mpc8610_hpcd.c. Adding this to
> arch/powerpc/platforms/86xx/mpc8610_hpcd.c will have no effect if the
> mpc8610_hpcd.c driver is not to be found.

Ok, I'll do that.

> You've complicated it by creating two fabric devices but I believe
> your code could be changed to only make one.
> 
> static struct platform_device alsa_fabric = {
> 	.name		= "MPC8610HPCD",
> 	.id		= -1,
> };
> 
> static struct platform_device *devices[] = {
> 	&alsa_fabric,
> };
> 
> platform_add_devices(&devices[0], ARRAY_SIZE(devices));
> 
> 
> I'd rename the driver to something less generic.
> 	.name		= "MPC8610HPCD-fabric",

These are all good ideas.  Thanks.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Mark Brown]

On Mon, Jun 16, 2008 at 11:53:27AM -0400, Jon Smirl wrote:

> static struct platform_device codec = {
> 	.name		= "wm9713-codec",
> 	.id		= -1,
> };

> static struct platform_device platform = {
> 	.name		= "Mainstone-WM9713",
> 	.id		= -1,
> };

> The drivers are creating their own platform devices. Constructing them
> in the driver builds in the assumption that if the driver is loaded,
> then the devices are present.  But that's not right in my case, I have
> four fabric drivers built in and I only want one of them active.

The above is a bodge on two fronts:

 - Currently ASoC doesn't autoprobe AC97 devices (in part because the
   general kernel infrastructure doesn't handle AC97 devices too well,
   in part because V1 didn't).  The platform device is a workaround for
   this.  Hopefully once V2 is out someone will look into this - it'll
   need some hoops jumping through to allow the system to handle clock
   configuration issues at startup.

 - The hardware that's being controlled here is not a standard part of
   the Mainstone hardware, it's a plugin module for them and there's no
   programmatic way to identify exactly what's on one of these boards.
   The devices are registered at module load since the only way to tell
   what's plugged in is via visual inspection of the hardware.

The ideal thing for ARM platforms like this would be that the machine
init code would register appropriate platform devices, allowing the
platform device autoload support to do its thing.

Re: Thoughts on ASOC v2 driver architecture

[Mark Brown]

On Mon, Jun 16, 2008 at 12:11:31PM -0400, Jon Smirl wrote:

> A more familiar example of this problem is unified graphics drivers.
> NVidia probably has 100 variations on their GPUs. They used to have

Yes, nothing you're doing should be this difficult - it's a very
standard problem.  AFAICT the main issue you're running into here is
that the way the PowerPC device trees are done it's difficult for you to
idomatically do things based on the board type you're running on.

I'm wondering if it might be worth adding some sort of infrastructure
which would allow drivers to load based on the machine type - not by
defining a device type, just by saying "load me on board X"?  This would
be very similar to how DMI information is used on x86 systems to handle
similar situations.

> To do this I'll put all of the device drivers on an initrd. Then as
> the kernel processes the device tree the right drivers will get
> loaded. After the boot process is finished the initrd and unused
> drivers disappear. Moving the fabric device creation in the my
> equivalent of arch/powerpc/platforms/86xx/mpc8610_hpcd.c will let me
> do this.

That's pretty much how most platforms using platform devices would
handle this.

> BTW, if you read in /Documentation about platform drivers they say not
> to make the devices in the module init function because it causes
> problems like the one I'm having.

Right, and this is not expected to happen in production ASoC v2 drivers
- for ARM boards the idomatic thing would be for the setup to happen in
board specific initialisation code under arch/arm.

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Mark Brown wrote:

> I'm wondering if it might be worth adding some sort of infrastructure
> which would allow drivers to load based on the machine type - not by
> defining a device type, just by saying "load me on board X"?  This would
> be very similar to how DMI information is used on x86 systems to handle
> similar situations.

This would be a question worth asking on linuxppc-dev.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Mark Brown]

On Mon, Jun 16, 2008 at 10:49:27AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> > But wouldn't it now be legal to represent the machine driver as a device
> > in its own right, even if it is connected via GPIOs?

> I'm not sure I understand that, so let me say this:

> When a driver wants to be probed, it creates a list that describes the kind of
> nodes it wants to be probed on.  Typically, the list includes the contents of
> the "compatible" property.  The kernel then scans the device tree, and calls the
> driver for each matching node.

Right, but you could not then idiomatically have a device tree entry
saying something to the effect of "This board has a Frobnitz 2000 with
control line 1 connected to GPIO4 and control line 2 connected to GPIO5"
which would register the presence of this other device (in the same way
as you have an entry for an I2C device)?

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Mark Brown wrote:

> Right, but you could not then idiomatically have a device tree entry
> saying something to the effect of "This board has a Frobnitz 2000 with
> control line 1 connected to GPIO4 and control line 2 connected to GPIO5"

Yes.

> which would register the presence of this other device (in the same way
> as you have an entry for an I2C device)?

I'm still not sure I understand.  I2C devices are represented by complete nodes
under the I2C adapter node.  The file fsl_soc.c scans each I2C adapter node, and
enumerates every child node under the adapter node.  It creates I2C platform
devices this way.

I'm not sure how we could use this model for ASoC.  There is no "ASoC adapter
node" in the device tree, because there is no ASoC adapter.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Thoughts on ASOC v2 driver architecture

[Jon Smirl]

On 6/16/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
>  > The driver stays in sound/soc/fsl. It's only the creation of the
>  > device that moves. You just need to add this to
>  > arch/powerpc/platforms/86xx/mpc8610_hpcd.c and remove it from
>  > mpc8610_hpcd.c. Adding this to
>  > arch/powerpc/platforms/86xx/mpc8610_hpcd.c will have no effect if the
>  > mpc8610_hpcd.c driver is not to be found.
>
>
> Ok, I'll do that.

The architecture still doesn't appear right to me. fsl-dma.c is
another driver that is self-creating its own device. We can certainly
add code to arch/powerpc/platforms/86xx/mpc8610_hpcd.c to create a
"fsl-elo" device, but it's another fake device. Plus there's already a
driver for the real "fsl,eloplus-dma" hardware in the system. Needing
to create fake devices is a sign that the design is not correct. dma
support should probably be a library that is linked into the ssi
driver and not be a standalone device driver.

I'm going to be gone for a week with no email. When I get back I'll
work on where to do snd_soc_card_create("MPC8610"). If you do it in
the fabric driver, then the fabric driver can never be optional.  But
if you don't do it in the fabric driver, how does the fabric driver
find the soc_card? There's no call to retrieve the soc_card for
"MPC8610".

>
>
>  > You've complicated it by creating two fabric devices but I believe
>  > your code could be changed to only make one.
>  >
>  > static struct platform_device alsa_fabric = {
>  >       .name           = "MPC8610HPCD",
>  >       .id             = -1,
>  > };
>  >
>  > static struct platform_device *devices[] = {
>  >       &alsa_fabric,
>  > };
>  >
>  > platform_add_devices(&devices[0], ARRAY_SIZE(devices));
>  >
>  >
>  > I'd rename the driver to something less generic.
>  >       .name           = "MPC8610HPCD-fabric",
>
>
> These are all good ideas.  Thanks.
>
>
>  --
>
> Timur Tabi
>  Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Thoughts on ASOC v2 driver architecture

[Timur Tabi]

Jon Smirl wrote:

> The architecture still doesn't appear right to me. fsl-dma.c is
> another driver that is self-creating its own device. 

Well, giving it its own device makes it easier to create sysfs entries.

> We can certainly
> add code to arch/powerpc/platforms/86xx/mpc8610_hpcd.c to create a
> "fsl-elo" device, but it's another fake device. 

I don't understand your penchant for putting code in the wrong drivers.  First
it's fabric code in the SSI driver.  And now you want DMA code in the platform
driver.

> Plus there's already a
> driver for the real "fsl,eloplus-dma" hardware in the system.

The DMA nodes that the DMA driver is supposed to use are marked with a different
compatible property (I haven't figured out exactly what).  This prevents the
standard DMA driver from touching them.

> Needing
> to create fake devices is a sign that the design is not correct. 

That sounds like an overgeneralization to me.

> dma
> support should probably be a library that is linked into the ssi
> driver and not be a standalone device driver.

Each SSI needs two specific DMA channels.  I suppose we could augment ASoC to
figure out which channels map to which SSI, but that's a lot of extra work.  I
don't know if it's possible to support all architectures with a model like that.

> If you do it in
> the fabric driver, then the fabric driver can never be optional.

I don't have any problem with that at all.

-- 
Timur Tabi
Linux kernel developer at Freescale