[alsa-devel] [PATCH 3/3] ASoC: max98090: fix possible race conditions

[Pierre-Louis Bossart]

On 11/20/19 12:02 AM, Tzung-Bi Shih wrote:
> max98090_interrupt() and max98090_pll_work() run in 2 different threads.
> There are 2 possible races:
> 
> Note: M98090_REG_DEVICE_STATUS = 0x01.
> Note: ULK == 0, PLL is locked; ULK == 1, PLL is unlocked.
> 
> max98090_interrupt      max98090_pll_work
> ----------------------------------------------
> schedule max98090_pll_work
>                          restart max98090 codec
> receive ULK INT
>                          assert ULK == 0
> schedule max98090_pll_work (1).
> 
> In the case (1), the PLL is locked but max98090_interrupt unnecessarily
> schedules another max98090_pll_work.

if you re-test that the PLL is already running, then you can exit the 
work function immediately without redoing the sequence?

maybe also play with the masks so that the PLL unlock is masked in the 
interrupt and unmasked after the PLL locks?

> 
> max98090_interrupt      max98090_pll_work      max98090 codec
> ----------------------------------------------------------------------
>                                                 ULK = 1
> receive ULK INT
> read 0x01
>                                                 ULK = 0 (clear on read)
> schedule max98090_pll_work
>                          restart max98090 codec
>                                                 ULK = 1
> receive ULK INT
> read 0x01
>                                                 ULK = 0 (clear on read)
>                          read 0x1
>                          assert ULK == 0 (2).

what are those 0x01 and 0x1? is the second a typo possibly?

> 
> In the case (2), both max98090_interrupt and max98090_pll_work read
> the same clear-on-read register.  max98090_pll_work would falsely
> thought PLL is locked.
> 
> There are 2 possible options:
> A. turn off ULK interrupt before scheduling max98090_pll_work; and turn
> on again before exiting max98090_pll_work.
> B. remove the second thread of execution.
> 
> Adopts option B which is more straightforward.

but has the side effect of possibly adding a 10ms delay in the interrupt 
thread?