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. > -- Péter _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel