[alsa-devel] Dynamically registering a codec
On PowerPC the device tree is used to dynamically load the correct codec module. How am I supposed to get this dynamically loaded module registered? The WM8580 codec registers itself.
Modifying the WM9712 codec to register itself on device load works for me. If I don't do this how am I supposed to dynamically find the DAI pointer so that I can call snd_soc_register_dais()?
diff --git a/sound/soc/codecs/wm9712.c b/sound/soc/codecs/wm9712.c index 550c903..cb0e679 100644 --- a/sound/soc/codecs/wm9712.c +++ b/sound/soc/codecs/wm9712.c @@ -20,6 +20,7 @@ #include <sound/initval.h> #include <sound/soc.h> #include <sound/soc-dapm.h> +#include <sound/soc-of-simple.h> #include "wm9712.h"
#define WM9712_VERSION "0.4" @@ -740,6 +741,39 @@ struct snd_soc_codec_device soc_codec_dev_wm9712 = { }; EXPORT_SYMBOL_GPL(soc_codec_dev_wm9712);
+static int __init wm9712_probe(struct platform_device *pdev) +{ + snd_soc_register_dais(wm9712_dai, ARRAY_SIZE(wm9712_dai)); +#if defined(CONFIG_SND_SOC_OF_SIMPLE) + /* Tell the of_soc helper about this codec */ + of_snd_soc_register_codec(&soc_codec_dev_wm9712, pdev->dev.archdata.of_node, + wm9712_dai, ARRAY_SIZE(wm9712_dai), + pdev->dev.archdata.of_node); +#endif + return 0; +} + +static struct platform_driver wm9712_driver = +{ + .probe = wm9712_probe, + .driver = { + .name = "wm9712", + }, +}; + +static __init int wm9712_driver_init(void) +{ + return platform_driver_register(&wm9712_driver); +} + +static __exit void wm9712_driver_exit(void) +{ +} + +module_init(wm9712_driver_init); +module_exit(wm9712_driver_exit); + + MODULE_DESCRIPTION("ASoC WM9711/WM9712 driver"); MODULE_AUTHOR("Liam Girdwood"); MODULE_LICENSE("GPL");
On Wed, May 13, 2009 at 03:18:36PM -0400, Jon Smirl wrote:
On PowerPC the device tree is used to dynamically load the correct codec module. How am I supposed to get this dynamically loaded module
For an AC97 CODEC in the standard configuration there should be no need for the CODEC to appear in the device tree for it to be discovered. However...
registered? The WM8580 codec registers itself.
The CODEC driver should use the normal kernel APIs to get itself probed.
In the specific case of AC97 what ought to be happening is that the CODEC driver is dynamically probed based on the ID registers (as the non-ASoC AC97 core code does). There's quite a bit of infrastructure work that needs to be done on the AC97 bus to make that viable, though - at the minute it doesn't support dynamic module loading and is only used for secondary CODEC functions like touchscreens. There doesn't seem to be much prospect of anyone doing anything with this in the immediate future, most of the PC side effort is on HDA these days and in the embedded space the situation is similar and there's more urgent things with AC97 like platform data that are likely to get dealt with first.
What's happening at the minute for ASoC is that AC97 CODECs are not required to be registered before the card instantiates. A better solution would involve having the AC97 controllers probed for CODECs when they register but that would require some cooperation from the machine driver since random actions may be required to ensure that the CODECs are operational (see Zylonite with CLK_POUT in use for a simple example). There would also be some fun handling the ac97.c adaptor but we can cross that bridge when we come to it.
Modifying the WM9712 codec to register itself on device load works for me. If I don't do this how am I supposed to dynamically find the DAI pointer so that I can call snd_soc_register_dais()?
I'm unenthusiastic about including any more of this soc-of-simple stuff in CODEC drivers, if it's going to be more generally used it ought to be pushed into the core. soc-of-simple predates the core registration support and since registration is now required for everything there doesn't seem to be any sense in going through all the existing CODEC drivers adding specific bodges for this - we should just move the soc-of-simple stuff into the core registration functions so we don't need to modify the individual drivers.
That doesn't solve your OpenFirmware problems; the easiest thing there is probably to do a specific soc-of-simple variant or option that knows how to find the various AC97 driver options there are. That's not ideal but it's tractable since there aren't that many AC97 CODECs. I don't think I'd object to core code to read the ID registers either so long as it doesn't impact boards that need to jump through hoops to get the CODEC running.
I definitely don't want to be adding random platform devices for AC97 CODECs if at all possible since it feels like it complicates the AC97 support more than it already is.
On Wed, May 13, 2009 at 6:44 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, May 13, 2009 at 03:18:36PM -0400, Jon Smirl wrote:
On PowerPC the device tree is used to dynamically load the correct codec module. How am I supposed to get this dynamically loaded module
For an AC97 CODEC in the standard configuration there should be no need for the CODEC to appear in the device tree for it to be discovered. However...
registered? The WM8580 codec registers itself.
The CODEC driver should use the normal kernel APIs to get itself probed.
In the specific case of AC97 what ought to be happening is that the CODEC driver is dynamically probed based on the ID registers (as the non-ASoC AC97 core code does). There's quite a bit of infrastructure work that needs to be done on the AC97 bus to make that viable, though - at the minute it doesn't support dynamic module loading and is only used for secondary CODEC functions like touchscreens. There doesn't seem to be much prospect of anyone doing anything with this in the immediate future, most of the PC side effort is on HDA these days and in the embedded space the situation is similar and there's more urgent things with AC97 like platform data that are likely to get dealt with first.
Can we stick with the modifications to of_simple I just posted for 2.6.31 and then try to get the core fixed in 2.6.32 so that of_simple can be removed? I have no love for of_simple, I will immediately remove it in favor of core support. Removing it has no impact on user space.
of_simple handles both i2s and ac97. There's no probing mechanism for i2s so we will have to keep device tree support that attaches i2s codecs. There's also the problem of platform data for the ac97 codec contained in the of node.
I would switch to HDA immediately but none of the embedded CPUs have HDA controllers.
What's happening at the minute for ASoC is that AC97 CODECs are not required to be registered before the card instantiates. A better solution would involve having the AC97 controllers probed for CODECs when they register but that would require some cooperation from the machine driver since random actions may be required to ensure that the CODECs are operational (see Zylonite with CLK_POUT in use for a simple example). There would also be some fun handling the ac97.c adaptor but we can cross that bridge when we come to it.
Modifying the WM9712 codec to register itself on device load works for me. If I don't do this how am I supposed to dynamically find the DAI pointer so that I can call snd_soc_register_dais()?
I'm unenthusiastic about including any more of this soc-of-simple stuff in CODEC drivers, if it's going to be more generally used it ought to be pushed into the core. soc-of-simple predates the core registration support and since registration is now required for everything there doesn't seem to be any sense in going through all the existing CODEC drivers adding specific bodges for this - we should just move the soc-of-simple stuff into the core registration functions so we don't need to modify the individual drivers.
That doesn't solve your OpenFirmware problems; the easiest thing there is probably to do a specific soc-of-simple variant or option that knows how to find the various AC97 driver options there are. That's not ideal but it's tractable since there aren't that many AC97 CODECs. I don't think I'd object to core code to read the ID registers either so long as it doesn't impact boards that need to jump through hoops to get the CODEC running.
I definitely don't want to be adding random platform devices for AC97 CODECs if at all possible since it feels like it complicates the AC97 support more than it already is.
On Thu, 2009-05-14 at 11:44 -0400, Jon Smirl wrote:
Can we stick with the modifications to of_simple I just posted for 2.6.31 and then try to get the core fixed in 2.6.32 so that of_simple can be removed? I have no love for of_simple, I will immediately remove it in favor of core support. Removing it has no impact on user space.
My main objection is to your abuse of platform devices here - there is no need for this on non-PowerPC platforms and a platform device affects everyone. So long as you come up with a solution that does not impact other platforms I'm less worried.
of_simple handles both i2s and ac97. There's no probing mechanism for i2s so we will have to keep device tree support that attaches i2s codecs. There's also the problem of platform data for the ac97 codec contained in the of node.
Like I said, Linux does not support platform data for AC97 devices.
On Thu, May 14, 2009 at 12:15 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, 2009-05-14 at 11:44 -0400, Jon Smirl wrote:
Can we stick with the modifications to of_simple I just posted for 2.6.31 and then try to get the core fixed in 2.6.32 so that of_simple can be removed? I have no love for of_simple, I will immediately remove it in favor of core support. Removing it has no impact on user space.
My main objection is to your abuse of platform devices here - there is no need for this on non-PowerPC platforms and a platform device affects everyone. So long as you come up with a solution that does not impact other platforms I'm less worried.
This is the part you don't like? +static void __init efika_declare_platform_devices(void) +{ + mpc52xx_declare_of_platform_devices(); + platform_device_register_simple("efika-audio-fabric", 0, NULL, 0); +} +
I can remove that by making the Efika fabric driver always load and then check if it is on an Efika in it's probe function. If it is not on an Efika it can exit. There were multiple discussions about this on the ppc list. The conclusion was that the mess of wires on the mainboard implementing audio really was a platform device.
There is a general kernel design problem in that there is no automated way to load something like an audio fabric driver. The Efika fabric driver is just example code, my Digispeaker hardware requires a board specific fabric driver since it has an external audio clock chip. Digispeaker drivers will come later after we decide on the final hardware design.
The problem with fabric drivers is that they are optional and ALSA drivers can load in any order. That requires a mechanism to say registration time is now closed, proceed without waiting for a fabric driver. The core could be changed to remove this requirement by imposing ordering constraints on driver registration.
That's what this snippet is doing: static int __init alsa_sound_last_init(void) { int idx, ok = 0; - + +#if defined(CONFIG_SND_SOC_OF_SIMPLE) + of_snd_soc_register_default_fabric(); +#endif
of_simple handles both i2s and ac97. There's no probing mechanism for i2s so we will have to keep device tree support that attaches i2s codecs. There's also the problem of platform data for the ac97 codec contained in the of node.
Like I said, Linux does not support platform data for AC97 devices.
On Thu, 2009-05-14 at 12:43 -0400, Jon Smirl wrote:
My main objection is to your abuse of platform devices here - there is no need for this on non-PowerPC platforms and a platform device affects everyone. So long as you come up with a solution that does not impact other platforms I'm less worried.
This is the part you don't like? +static void __init efika_declare_platform_devices(void) +{
That bit's OK - it's the device you've added for the CODEC which is concerning me.
on an Efika it can exit. There were multiple discussions about this on the ppc list. The conclusion was that the mess of wires on the mainboard implementing audio really was a platform device.
I agree, this is the most sensible way to handle machine drivers.
There is a general kernel design problem in that there is no automated way to load something like an audio fabric driver. The Efika fabric driver is just example code, my Digispeaker hardware requires a board specific fabric driver since it has an external audio clock chip. Digispeaker drivers will come later after we decide on the final hardware design.
It's only really a problem on platforms using the device tree, everything else can happily define random Linux-specific platform devices in the board initialisation code and everything is happy.
The problem with fabric drivers is that they are optional and ALSA drivers can load in any order. That requires a mechanism to say registration time is now closed, proceed without waiting for a fabric driver. The core could be changed to remove this requirement by imposing ordering constraints on driver registration.
Indeed - this is only a problem due to the fact that you're trying to provide a generic machine driver.
On Thu, May 14, 2009 at 1:40 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, 2009-05-14 at 12:43 -0400, Jon Smirl wrote:
My main objection is to your abuse of platform devices here - there is no need for this on non-PowerPC platforms and a platform device affects everyone. So long as you come up with a solution that does not impact other platforms I'm less worried.
This is the part you don't like? +static void __init efika_declare_platform_devices(void) +{
That bit's OK - it's the device you've added for the CODEC which is concerning me.
Now I see what you mean. I need to make the codec into a device so that I can dynamically load and find it. On PowerPC the codecs don't have to be compiled in, they can be on an initrd. The code in of_simple will find the right driver and get it into memory. The platform device is what triggers the kernel to go searching for the module on initrd.
The .name field is what lets the kernel find the module on initrd.... static struct platform_driver wm9712_driver = { .probe = wm9712_probe, .driver = { .name = "wm9712", }, };
The i2c core just went through this same change. Previously i2c drivers had to be compiled in. Now they can just be on initrd and loaded on demand by the device tree. The equivalent to of_simple for i2c is in drivers/of/of_i2c.c. It is much smaller since more of the support has been pushed into the i2c core.
This isn't platform specific, other platforms could implement this but they don't have anything like a device tree telling them which modules to load. A device tree is a probing mechanism for devices that can't be probed.
On Thu, May 14, 2009 at 1:57 PM, Jon Smirl jonsmirl@gmail.com wrote:
On Thu, May 14, 2009 at 1:40 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, 2009-05-14 at 12:43 -0400, Jon Smirl wrote:
My main objection is to your abuse of platform devices here - there is no need for this on non-PowerPC platforms and a platform device affects everyone. So long as you come up with a solution that does not impact other platforms I'm less worried.
This is the part you don't like? +static void __init efika_declare_platform_devices(void) +{
That bit's OK - it's the device you've added for the CODEC which is concerning me.
Now I see what you mean. I need to make the codec into a device so that I can dynamically load and find it. On PowerPC the codecs don't have to be compiled in, they can be on an initrd. The code in of_simple will find the right driver and get it into memory. The platform device is what triggers the kernel to go searching for the module on initrd.
I'm juggling too many balls. ASoC needs some more rework to make the modules load dynamically. The codecs need to implement .id_table.
.id_table is pretty trivial
static const struct i2c_device_id ds1682_id[] = { { "ds1682", 0 }, { } }; MODULE_DEVICE_TABLE(i2c, ds1682_id);
static struct i2c_driver ds1682_driver = { .driver = { .name = "ds1682", }, .probe = ds1682_probe, .remove = ds1682_remove, .id_table = ds1682_id, };
The .name field is what lets the kernel find the module on initrd.... static struct platform_driver wm9712_driver = { .probe = wm9712_probe, .driver = { .name = "wm9712", }, };
The i2c core just went through this same change. Previously i2c drivers had to be compiled in. Now they can just be on initrd and loaded on demand by the device tree. The equivalent to of_simple for i2c is in drivers/of/of_i2c.c. It is much smaller since more of the support has been pushed into the i2c core.
This isn't platform specific, other platforms could implement this but they don't have anything like a device tree telling them which modules to load. A device tree is a probing mechanism for devices that can't be probed.
-- Jon Smirl jonsmirl@gmail.com
On Thu, May 14, 2009 at 02:15:17PM -0400, Jon Smirl wrote:
I'm juggling too many balls. ASoC needs some more rework to make the modules load dynamically. The codecs need to implement .id_table.
ASoC is fine here, please take a look at drivers like wm8903 - the DAI registration functions are all about letting things take advantage of this. I'll be insisting that all CODEC drivers do this at some point but it's not yet blocking anything else. Like I say, AC97 is a bit special due to general lack of infrastructure for it.
On Thu, May 14, 2009 at 01:57:37PM -0400, Jon Smirl wrote:
Now I see what you mean. I need to make the codec into a device so that I can dynamically load and find it. On PowerPC the codecs don't have to be compiled in, they can be on an initrd.
This is the case for all platforms, it's not in any way PowerPC specific.
The code inof_simple will find the right driver and get it into memory. The platform device is what triggers the kernel to go searching for the module on initrd.
On every other platform the codec driver is linked to directly by the machine driver so loading the machine driver will also cause the codec driver to be loaded - at this stage of_simple isn't involved at all except in that it has removed the usual way of getting the codec driver loaded for AC97 devices.
This isn't platform specific, other platforms could implement this but they don't have anything like a device tree telling them which modules to load. A device tree is a probing mechanism for devices that can't be probed.
Other platforms use platform devices in just the same way, they just happen to set them up in a C file rather than a .dts file. The mechanisms for loading things after that are identical. What's PowerPC specific here is that you are trying to use this of-simple mechainsm and that's defeating the way this is handled for AC97 codec drivers.
Please re-read my comments about the specific problems with AC97 - this is key here.
On Thu, May 14, 2009 at 2:38 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, May 14, 2009 at 01:57:37PM -0400, Jon Smirl wrote:
Now I see what you mean. I need to make the codec into a device so that I can dynamically load and find it. On PowerPC the codecs don't have to be compiled in, they can be on an initrd.
This is the case for all platforms, it's not in any way PowerPC specific.
The code in of_simple will find the right driver and get it into memory. The platform device is what triggers the kernel to go searching for the module on initrd.
On every other platform the codec driver is linked to directly by the machine driver so loading the machine driver will also cause the codec driver to be loaded - at this stage of_simple isn't involved at all except in that it has removed the usual way of getting the codec driver loaded for AC97 devices.
This isn't platform specific, other platforms could implement this but they don't have anything like a device tree telling them which modules to load. A device tree is a probing mechanism for devices that can't be probed.
Other platforms use platform devices in just the same way, they just happen to set them up in a C file rather than a .dts file. The mechanisms for loading things after that are identical. What's PowerPC specific here is that you are trying to use this of-simple mechainsm and that's defeating the way this is handled for AC97 codec drivers.
Please re-read my comments about the specific problems with AC97 - this is key here.
So I need to make fabric drivers for Efika and Phytec that statically link to the respective AC97 codecs? And the core is going to get updated so that I can remove these static links?
On Thu, May 14, 2009 at 02:56:32PM -0400, Jon Smirl wrote:
On Thu, May 14, 2009 at 2:38 PM, Mark Brown
Please re-read my comments about the specific problems with AC97 - this is key here.
So I need to make fabric drivers for Efika and Phytec that statically link to the respective AC97 codecs? And the core is going to get
They just need to reference a symbol that's exported by the codec driver such as the DAI.
updated so that I can remove these static links?
As I said in my original mail there is currently little work on AC97 in general so unless someone steps forward to enhance the AC97 infrastructure something like that is going to have to remain for the time being with AC97 CODECs.
participants (2)
-
Jon Smirl -
Mark Brown