On Tuesday 20 October 2009 13:25:40 ext Mark Brown wrote:
On Mon, Oct 19, 2009 at 03:42:20PM +0300, Peter Ujfalusi wrote:
TWL4030 codec is now using the device registration via tlw4030_codec MFD device.
This looks pretty good but obviously depends on the MFD changes.
Thanks, yes it all depends on the MFD changes.
The major thing that jumps out at me is the removal of the register definitions from the ASoC headers - it might be nice to have that done as part of the MFD patch, or as a separate patch.
The reason why I have done it like this is that with one patch I only touch one subsystem at the time: 1. MFD changes only 2. OMAP related changes 3. soc codec driver change
In patch 1, the register definitions had to be added, so that the twl4030_codec driver knows the registers (and there could be the vibra driver placed separately from the soc codec driver). In patch 3, where I modify the soc codec driver to use the new method, than I remove the definitions and use the existing header file, introduced by the first patch. All in all, after each patch the kernel can be builds, boots and works as before.
You've also got the bias being brought up when the ASoC system comes up rather than when the driver comes up. To be honest it doesn't really make any difference either way, it's just slightly different to other drivers.
I was thinking that if you built the kernel with SND_SOC_ALL_CODECS on OMAP platform for some reason and you don't actually use the twl4030 as audio device -> no machine driver, which would use it, than the codec part would be off. But yes, probably I can move the povering up to the probe function.
What is useful with things like twl4030 which take a little while to come up is if you can do the bias bringup out of line from device probe, avoiding blocking system startup on CODEC bringup. That's definitely a separate patch, I'm just mentioning it for interest here.
I'm sure there will be another round for this series, so I can make this change at the same time within the patch, since this anyway changes the way how the driver is loaded/probed.
There's also a couple of debug prints (like in the remove function) and
I'll get rid of them. But at least this time I did not had those unneeded casts, which I usually have ;)
+MODULE_ALIAS("platform:twl4030_codec:audio");
Is that second colon right given...
I'm not sure about it at all either. I did not found any other 'nested MFD' drivers around, so this is just a guess Should it be:
+MODULE_ALIAS("platform:twl4030_codec_audio");
- .driver = {
.name = "twl4030_codec_audio",.owner = THIS_MODULE,this.