lkml.org 
[lkml]   [2009]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 4/4] ASoC: TWL4030: Driver registration via twl4030_codec MFD
Date
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-10-20 13:33    [from the cache]
©2003-2011 Jasper Spaans