• Tzung-Bi Shih's avatar
    ASoC: max98090: fix possible race conditions · 45dfbf56
    Tzung-Bi Shih authored
    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.
    
    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 0x01
                            assert ULK == 0 (2).
    
    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.
    Note: the case (2) race is introduced by the previous commit ("ASoC:
    max98090: exit workaround earlier if PLL is locked") to check the status
    and exit the loop earlier in max98090_pll_work.
    
    There are 2 possible solution 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.
    
    Option A cannot fix the case (2) race because it still has 2 threads
    access the same clear-on-read register simultaneously.  Although we
    could suppose the register is volatile and read the status via I2C could
    be much slower than the hardware raises the bits.
    
    Option B introduces a maximum 10~12 msec penalty delay in the interrupt
    handler.  However, it could only punish the jack detection by extra
    10~12 msec.
    
    Adopts option B which is the better solution overall.
    Signed-off-by: default avatarTzung-Bi Shih <tzungbi@google.com>
    Link: https://lore.kernel.org/r/20191122073114.219945-4-tzungbi@google.comReviewed-by: default avatarPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
    Signed-off-by: default avatarMark Brown <broonie@kernel.org>
    45dfbf56
max98090.c 84.6 KB