Re: [alsa-devel] ASoC and a codec that can't be controlled
Liam Girdwood wrote:
I guess I just don't understand why the codec driver is acting like the "master" driver of ASOC. IMHO, the codec driver should be doing two things:
I guess this is due to an ordering issue we had during early development. We had to make sure that the I2C probe of the codec succeeded before registering the card, pcms, etc. Fwiw, I do intend to address this in future versions (I should probably add a road map to the wiki now).
In that case, I think we can solve this problem as well as the PowerPC device tree problem in one shot, because they're basically the same problem.
Anyway, let me see if I get this right:
1. The first function called is eti_b1_init(). 2. eti_b1_init() calls platform_device_add() to add an soc-audio class device and register the eti_b1_snd_devdata structure. 3. The registration causes a number of things to happen, one of which is a call to soc_probe(). 4. soc_probe() sees that the snd_soc_machine_eti_b1.probe is NULL, so that part is skipped. 3. soc_probe() sees that snd_soc_machine_eti_b1.dai_link->cpu_dai->probe is also NULL, so that part is skipped. (eti_b1_snd_devdata.machine == snd_soc_machine_eti_b1) 4. soc_probe() sees that soc_codec_dev_wm8731.probe is not NULL, so it calls wm8731_probe(). 5. wm8731_probe() registers an I2C driver. 6. The I2C class driver calls wm8731_i2c_driver.attach_adapter, which is wm8731_i2c_attach(). 7. wm8731_i2c_attach() calls i2c_probe(), which in turn calls wm8731_codec_probe() 8. wm8731_codec_probe() allocates a snd_soc_codec structure. 8. wm8731_codec_probe() calls wm8731_init().
This means that if there is no I2C support, wm8731_init() will never be called.
9. wm8731_init() initializes the snd_soc_codec structure.
Question: why doesn't wm8731_probe() initialize the non-I2C parts of the snd_soc_codec structure? For example, snd_soc_codec.dai, snd_soc_codec.name, and snd_soc_codec.owner? That way, the snd_soc_codec structure will be properly initialized even when there's no I2C.
10. wm8731_init() calls snd_soc_new_pcms. 11. wm8731_init() exits, wm8731_codec_probe() exits, wm8731_i2c_attach() exits, and then wm8731_probe() exits. Control returns to soc_probe(). 12. soc_probe() notices that eti_b1_snd_devdata.platform.probe is NULL, so it skips that.
Question: why don't you define a function for at91_soc_platform.probe, and in that function you can call snd_soc_new_pcms()? That way, you guarantee the codec driver's I2C interface is initialized before snd_soc_new_pcms() is called, and you avoid have PCM calls in the codec driver.
ASOC and the machine driver should then work in tandem to decide which driver will do what and which capabilities are *actually* supported. *Something* needs to look at the entire system and say to each device, "Well, yes, I know about this little feature of yours, but we're just not going to support that today."
The machine driver pretty much already does this. It can override valid hardware configurations and return -EINVAl if required.
Do you have an example of that? Would that be eti_b1_hw_params()?
On Wed, 2007-05-30 at 13:10 -0500, Timur Tabi wrote:
Liam Girdwood wrote:
I guess I just don't understand why the codec driver is acting like the "master" driver of ASOC. IMHO, the codec driver should be doing two things:
I guess this is due to an ordering issue we had during early development. We had to make sure that the I2C probe of the codec succeeded before registering the card, pcms, etc. Fwiw, I do intend to address this in future versions (I should probably add a road map to the wiki now).
Wiki roadmap now added.
https://bugtrack.alsa-project.org/wiki/wikka.php?wakka=ASoCRoadMap
In that case, I think we can solve this problem as well as the PowerPC device tree problem in one shot, because they're basically the same problem.
Anyway, let me see if I get this right:
- The first function called is eti_b1_init().
- eti_b1_init() calls platform_device_add() to add an soc-audio class device and register
the eti_b1_snd_devdata structure. 3. The registration causes a number of things to happen, one of which is a call to soc_probe(). 4. soc_probe() sees that the snd_soc_machine_eti_b1.probe is NULL, so that part is skipped. 3. soc_probe() sees that snd_soc_machine_eti_b1.dai_link->cpu_dai->probe is also NULL, so that part is skipped. (eti_b1_snd_devdata.machine == snd_soc_machine_eti_b1) 4. soc_probe() sees that soc_codec_dev_wm8731.probe is not NULL, so it calls wm8731_probe(). 5. wm8731_probe() registers an I2C driver. 6. The I2C class driver calls wm8731_i2c_driver.attach_adapter, which is wm8731_i2c_attach(). 7. wm8731_i2c_attach() calls i2c_probe(), which in turn calls wm8731_codec_probe() 8. wm8731_codec_probe() allocates a snd_soc_codec structure. 8. wm8731_codec_probe() calls wm8731_init().
This means that if there is no I2C support, wm8731_init() will never be called.
- wm8731_init() initializes the snd_soc_codec structure.
Question: why doesn't wm8731_probe() initialize the non-I2C parts of the snd_soc_codec structure? For example, snd_soc_codec.dai, snd_soc_codec.name, and snd_soc_codec.owner? That way, the snd_soc_codec structure will be properly initialized even when there's no I2C.
This codec only supports I2C, so it shouldn't matter too much. But it would be better for codecs using other IO mechanisms.
- wm8731_init() calls snd_soc_new_pcms.
- wm8731_init() exits, wm8731_codec_probe() exits, wm8731_i2c_attach() exits, and then
wm8731_probe() exits. Control returns to soc_probe(). 12. soc_probe() notices that eti_b1_snd_devdata.platform.probe is NULL, so it skips that.
Question: why don't you define a function for at91_soc_platform.probe, and in that function you can call snd_soc_new_pcms()? That way, you guarantee the codec driver's I2C interface is initialized before snd_soc_new_pcms() is called, and you avoid have PCM calls in the codec driver.
I think the best place to call snd_soc_new_pcms is in the machine driver. This means we don't have to add any pcms that are not used.
ASOC and the machine driver should then work in tandem to decide which driver will do what and which capabilities are *actually* supported. *Something* needs to look at the entire system and say to each device, "Well, yes, I know about this little feature of yours, but we're just not going to support that today."
The machine driver pretty much already does this. It can override valid hardware configurations and return -EINVAl if required.
Do you have an example of that? Would that be eti_b1_hw_params()?
Yes, the hw_params could return error on anything the machine didn't like. e.g. this could be used to workaround quirks.
Liam
Liam Girdwood wrote:
I think the best place to call snd_soc_new_pcms is in the machine driver. This means we don't have to add any pcms that are not used.
True, but the machine driver is probed before the codec driver is probed. So if you really need the codec driver to initialize the I2C bus first, then you've got a problem. Re-arranging the order of probes in soc_probe() is not really a solution.
So on your baord, why does the I2C interface need to be running before anything else can be probed?
Yes, the hw_params could return error on anything the machine didn't like. e.g. this could be used to workaround quirks.
Would ALSA know to try something else, if a particular combination was rejected?
On Thu, 2007-05-31 at 14:49 -0500, Timur Tabi wrote:
Liam Girdwood wrote:
I think the best place to call snd_soc_new_pcms is in the machine driver. This means we don't have to add any pcms that are not used.
True, but the machine driver is probed before the codec driver is probed. So if you really need the codec driver to initialize the I2C bus first, then you've got a problem. Re-arranging the order of probes in soc_probe() is not really a solution.
So on your baord, why does the I2C interface need to be running before anything else can be probed?
Imho, there is little point in probing or allocating any more resources until we know the I2C codec probe succeeds. We would just have to free any resources allocated up to that point.
Yes, the hw_params could return error on anything the machine didn't like. e.g. this could be used to workaround quirks.
Would ALSA know to try something else, if a particular combination was rejected?
This would be up to your user space application. In general all common sample rates and formats are supported by most drivers e.g. 44.1k stereo. If some combinations are rejected, it's most likely due to hardware limitations.
Liam
Liam Girdwood wrote:
Imho, there is little point in probing or allocating any more resources until we know the I2C codec probe succeeds. We would just have to free any resources allocated up to that point.
True, but if the I2C probe fails, then I would think that the whole system is shot, so this sounds like a very unlikely situation. If you changed the code to clean up after an I2C failure, you could make the whole thing more modular, including eliminating the call to snd_soc_new() in the codec driver.
Since ASoC divides everything into four more-or-less independent drivers, it makes sense that it should be able to handle a situation where one of those drivers just fails to load.
Liam Girdwood wrote:
I think the best place to call snd_soc_new_pcms is in the machine driver. This means we don't have to add any pcms that are not used.
Are you talking about the snd_soc_dai_link.init() function? If so, then where do you call snd_soc_free_pcms()? The snd_soc_dai_link structure has no "exit" function.
participants (2)
-
Liam Girdwood -
Timur Tabi