[alsa-devel] Questions about max98090 codec driver

[Pierre-Louis Bossart]

On 10/24/19 1:23 PM, Tzung-Bi Shih wrote:
> Hi,
> 
> I am studying an odd issue of max98090 codec on Baytrail-based
> chromebook.  The issue is: when user playback and capture
> simultaneously, it seems the PLL never get locked if msleep(10)
> between the SHDN off and on
> (https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L2120).
> The playback stream becomes silent and the console keeps printing "PLL
> unlocked".  But, if comment out the msleep(10) between the SHDN off
> and on, the issue fixed.  I am trying to find the reason but facing
> further more questions and may need your inputs.

IIRC this PPL unlocked problem also affects Cherrytrail/Braswell 
Chromebooks with the same codec, but strangely with a lower probability 
of occurrence (maybe due to clocking differences?). My contribution was 
to make the dmesg log ratelimited, but I could never figure out either 
what the root cause was.

> 
> 1. The commit b8a3ee820f7b ("ASoC: max98090: Add recovery for PLL lock
> failure") enables ULK interrupts
> (https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L2088)
> when PCM stream starting.  If max98090 claims its PLL is unlocked,
> max98090_pll_work() get scheduled to workaround it by SHDN off and on
> (https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L2106).
> 
> I feel it is weird to sleep in max98090_pll_work().  Especially, at
> this line https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L2125
> (it makes less sense to "wait" in another thread).  Note that, the
> threaded IRQF_ONESHOT handler and max98090_pll_work() are in 2
> different threads.
> 
> I guess the original intention is:
> - disable ULK interrupt in IRQ handler
> (https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L2260)
> - schedule max98090_pll_work() to workaround it
> - wait 10ms to give PLL chance to lock
> - enable ULK interrupt again
> If max98090 claims its PLL is unlocked again, repeat the above by
> receiving another ULK interrupt.
> 
> Unfortunately, the odd issue seems not be fixed by my rough
> implementation of these.
> 
> 2. According to the datasheet page 164 table 90
> (https://datasheets.maximintegrated.com/en/ds/MAX98090.pdf), there are
> some registers should only be adjusted when SHDN==0.  But I fail to
> find max98090.c tries to set SHDN to 0 and restore it afterwards when
> writing to these registers.  For example,
> https://elixir.bootlin.com/linux/v5.4-rc2/source/sound/soc/codecs/max98090.c#L1897.
> I am wondering if it would bring any side effects because the
> datasheet states "Changing these settings during normal operation
> (SHDN=1) can compromise device stability and performance
> specifications."
> 
> 3. By searching some history data, I found a previous version did not
> have the msleep(10) between the SHDN off and on
> (https://crrev.com/c/191740, click the file name in the middle of the
> window to see the diff.  Pardon me, I do not find another public
> repository for this).  I am curious if anyone of you still remember
> why the upstream version contains the msleep(10).  I am also curious
> if anyone of your environment works well with the upstream version
> max98090.c.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>