leandrof/linux
1
0
Fork 0
forked from Minki/linux
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ASoC: max98090: fix possible race conditions

Browse Source 
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>
This commit is contained in:
Tzung-Bi Shih 2019-11-22 15:31:14 +08:00 committed by Mark Brown
parent 6f49919d11
commit 45dfbf5697
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0
2 changed files with 2 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
sound/soc/codecs/max98090.c
View File

@ -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;
unsigned int pll;
int i;
@ -2275,7 +2273,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) {
@ -2438,7 +2436,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,
@ -2491,7 +2488,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;
}

1
sound/soc/codecs/max98090.h
View File

@ -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;