diff mbox series

[1/3] ASoC: max98090: remove msleep in PLL unlocked workaround

Message ID 20191120060256.212818-2-tzungbi@google.com (mailing list archive)
State New, archived
Headers show
Series ASoC: max98090: fix PLL unlocked workaround-related | expand

Commit Message

Tzung-Bi Shih Nov. 20, 2019, 6:02 a.m. UTC
It was observed Baytrail-based chromebooks could cause continuous PLL
unlocked when using playback stream and capture stream simultaneously.
Specifically, starting a capture stream after started a playback stream.
As a result, the audio data could corrupt or turn completely silent.

As the datasheet suggested, the maximum PLL lock time should be 7 msec.
The workaround resets the codec softly by toggling SHDN off and on if
PLL failed to lock for 10 msec.  Notably, there is no suggested hold
time for SHDN off.

On Baytrail-based chromebooks, it would easily happen continuous PLL
unlocked if there is a 10 msec delay between SHDN off and on.  Removes
the msleep().

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 sound/soc/codecs/max98090.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Pierre-Louis Bossart Nov. 20, 2019, 2:33 p.m. UTC | #1
On 11/20/19 12:02 AM, Tzung-Bi Shih wrote:
> It was observed Baytrail-based chromebooks could cause continuous PLL
> unlocked when using playback stream and capture stream simultaneously.
> Specifically, starting a capture stream after started a playback stream.
> As a result, the audio data could corrupt or turn completely silent.
> 
> As the datasheet suggested, the maximum PLL lock time should be 7 msec.
> The workaround resets the codec softly by toggling SHDN off and on if
> PLL failed to lock for 10 msec.  Notably, there is no suggested hold
> time for SHDN off.
> 
> On Baytrail-based chromebooks, it would easily happen continuous PLL
> unlocked if there is a 10 msec delay between SHDN off and on.  Removes
> the msleep().
> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
>   sound/soc/codecs/max98090.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
> index f6bf4cfbea23..8382a77586ee 100644
> --- a/sound/soc/codecs/max98090.c
> +++ b/sound/soc/codecs/max98090.c
> @@ -2117,7 +2117,6 @@ static void max98090_pll_work(struct work_struct *work)
>   	/* Toggle shutdown OFF then ON */
>   	snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
>   			    M98090_SHDNN_MASK, 0);
> -	msleep(10);

maybe add a comment here that the off/on sequence is done on purpose (as 
stated in the commit message)?

>   	snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
>   			    M98090_SHDNN_MASK, M98090_SHDNN_MASK);
>   
>
Tzung-Bi Shih Nov. 21, 2019, 2:37 a.m. UTC | #2
On Thu, Nov 21, 2019 at 12:10 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> maybe add a comment here that the off/on sequence is done on purpose (as
> stated in the commit message)?

Will do, thanks.
diff mbox series

Patch

diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index f6bf4cfbea23..8382a77586ee 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -2117,7 +2117,6 @@  static void max98090_pll_work(struct work_struct *work)
 	/* Toggle shutdown OFF then ON */
 	snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
 			    M98090_SHDNN_MASK, 0);
-	msleep(10);
 	snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN,
 			    M98090_SHDNN_MASK, M98090_SHDNN_MASK);