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 0x1 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.
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.
Signed-off-by: Tzung-Bi Shih tzungbi@google.com --- sound/soc/codecs/max98090.c | 8 ++------ sound/soc/codecs/max98090.h | 1 - 2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c index 2ccdfb2383b7..75abe98dfc44 100644 --- a/sound/soc/codecs/max98090.c +++ b/sound/soc/codecs/max98090.c @@ -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; @@ -2268,7 +2266,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) { @@ -2431,7 +2429,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, @@ -2484,7 +2481,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; }
diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h index 57965cd678b4..a197114b0dad 100644 --- a/sound/soc/codecs/max98090.h +++ b/sound/soc/codecs/max98090.h @@ -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;