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.

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.

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.

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

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.

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.

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.

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.

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.

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.

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