Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec

[Mark Brown <broonie@sirena.org.uk>]

On Thu, Sep 04, 2008 at 07:32:28AM -0700, Steve Sakoman wrote:
> On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula <jarkko.nikula@nokia.com> wrote:

> >> +static void twl4030_dump_registers(void)
> >> +{

> > This is not needed since there is already nice function
> > for it: sound/soc/soc-core.c: codec_reg_show.

> I could probably get rid of this function.  It was quite useful during
> debugging and I was not aware of codec_reg_show.  IIRC, most of the
> other codec drivers also have the equivalent of this function, so we
> might want to clean them up too if there is a standard function to
> replace them.

Any references?  None of the in-tree drivers have them...

> >> +static void twl4030_power_down(struct snd_soc_codec *codec)
> >> +{
> > ...
> >> +     udelay(10);
> >> +}
> > REVISIT comment for these kind of magic delays if doesn't work without.

> It *seems* to work without them, but every historic TI driver seemed
> to have them.  I figured that they might know something not reflected
> in the documentation.  I will add a REVIST comment.

This sort of stuff is very common in codec drivers - normally the delays
are there to allow the analogue side of the system time to settle down
(waiting for capacitors to charge/discharge or reference voltages to
stabalise, for example).  Ideally they have comments saying what's going
on, of course.  Missing these delays often won't actually stop things
working completely but will instead do things like reduce performance or
generate audio artefacts - and sometimes it's application dependant if
these are important.