ASoC: max98090: fix possible race conditions
[ Upstream commit 45dfbf5697
]
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: Tzung-Bi Shih <tzungbi@google.com>
Link: https://lore.kernel.org/r/20191122073114.219945-4-tzungbi@google.com
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
8d333a5d26
commit
efd10b1171
|
@ -2103,10 +2103,8 @@ static void max98090_pll_det_disable_work(struct work_struct *work)
|
|||
M98090_IULK_MASK, 0);
|
||||
}
|
||||
|
||||
static void max98090_pll_work(struct work_struct *work)
|
||||
static void max98090_pll_work(struct max98090_priv *max98090)
|
||||
{
|
||||
struct max98090_priv *max98090 =
|
||||
container_of(work, struct max98090_priv, pll_work);
|
||||
struct snd_soc_component *component = max98090->component;
|
||||
|
||||
if (!snd_soc_component_is_active(component))
|
||||
|
@ -2259,7 +2257,7 @@ static irqreturn_t max98090_interrupt(int irq, void *data)
|
|||
|
||||
if (active & M98090_ULK_MASK) {
|
||||
dev_dbg(component->dev, "M98090_ULK_MASK\n");
|
||||
schedule_work(&max98090->pll_work);
|
||||
max98090_pll_work(max98090);
|
||||
}
|
||||
|
||||
if (active & M98090_JDET_MASK) {
|
||||
|
@ -2422,7 +2420,6 @@ static int max98090_probe(struct snd_soc_component *component)
|
|||
max98090_pll_det_enable_work);
|
||||
INIT_WORK(&max98090->pll_det_disable_work,
|
||||
max98090_pll_det_disable_work);
|
||||
INIT_WORK(&max98090->pll_work, max98090_pll_work);
|
||||
|
||||
/* Enable jack detection */
|
||||
snd_soc_component_write(component, M98090_REG_JACK_DETECT,
|
||||
|
@ -2475,7 +2472,6 @@ static void max98090_remove(struct snd_soc_component *component)
|
|||
cancel_delayed_work_sync(&max98090->jack_work);
|
||||
cancel_delayed_work_sync(&max98090->pll_det_enable_work);
|
||||
cancel_work_sync(&max98090->pll_det_disable_work);
|
||||
cancel_work_sync(&max98090->pll_work);
|
||||
max98090->component = NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -1530,7 +1530,6 @@ struct max98090_priv {
|
|||
struct delayed_work jack_work;
|
||||
struct delayed_work pll_det_enable_work;
|
||||
struct work_struct pll_det_disable_work;
|
||||
struct work_struct pll_work;
|
||||
struct snd_soc_jack *jack;
|
||||
unsigned int dai_fmt;
|
||||
int tdm_slots;
|
||||
|
|
Loading…
Reference in New Issue