| Message ID | 20210117160555.78376-4-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,01/12] mfd: arizona: Drop arizona-extcon cells | expand |
On Sun, Jan 17, 2021 at 05:05:46PM +0100, Hans de Goede wrote: > When the jack is partially inserted and then removed again it may be > removed while the hpdet code is running. In this case the following > may happen: Fixes like this should be sent at the start of the series (prior to things like the move) so they can go to stable as bug fixes.
Hi, On 1/18/21 1:47 PM, Mark Brown wrote: > On Sun, Jan 17, 2021 at 05:05:46PM +0100, Hans de Goede wrote: >> When the jack is partially inserted and then removed again it may be >> removed while the hpdet code is running. In this case the following >> may happen: > > Fixes like this should be sent at the start of the series (prior to > things like the move) so they can go to stable as bug fixes. Ok, I'll reshuffle the series for v3 to put these first and have them apply to drivers/extcon/extcon-arizona.c before it is moved to sound/soc/codecs/arizona-jack.c Regards, Hans
On Sun, Jan 17, 2021 at 05:05:46PM +0100, Hans de Goede wrote: > When the jack is partially inserted and then removed again it may be > removed while the hpdet code is running. In this case the following > may happen: > > 1. The "JACKDET rise" or ""JACKDET fall" IRQ triggers > 2. arizona_jackdet runs and takes info->lock > 3. The "HPDET" IRQ triggers > 4. arizona_hpdet_irq runs, blocks on info->lock > 5. arizona_jackdet calls arizona_stop_mic() and clears info->hpdet_done > 6. arizona_jackdet releases info->lock > 7. arizona_hpdet_irq now can continue running and: > 7.1 Calls arizona_start_mic() (if a mic was detected) > 7.2 sets info->hpdet_done > > Step 7 is undesirable / a bug: > 7.1 causes the device to stay in a high power-state (with MICVDD enabled) > 7.2 causes hpdet to not run on the next jack insertion, which in turn > causes the EXTCON_JACK_HEADPHONE state to never get set > > This fixes both issues by skipping these 2 steps when arizona_hpdet_irq > runs after the jack has been unplugged. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com> Thanks, Charles
diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c index aae82db542a5..f7ef247de46a 100644 --- a/sound/soc/codecs/arizona-jack.c +++ b/sound/soc/codecs/arizona-jack.c @@ -601,7 +601,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) struct arizona *arizona = info->arizona; int id_gpio = arizona->pdata.hpdet_id_gpio; unsigned int report = EXTCON_JACK_HEADPHONE; - int ret, reading; + int ret, reading, state; bool mic = false; mutex_lock(&info->lock); @@ -614,12 +614,11 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) } /* If the cable was removed while measuring ignore the result */ - ret = extcon_get_state(info->edev, EXTCON_MECHANICAL); - if (ret < 0) { - dev_err(arizona->dev, "Failed to check cable state: %d\n", - ret); + state = extcon_get_state(info->edev, EXTCON_MECHANICAL); + if (state < 0) { + dev_err(arizona->dev, "Failed to check cable state: %d\n", state); goto out; - } else if (!ret) { + } else if (!state) { dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n"); goto done; } @@ -667,7 +666,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) gpio_set_value_cansleep(id_gpio, 0); /* If we have a mic then reenable MICDET */ - if (mic || info->mic) + if (state && (mic || info->mic)) arizona_start_mic(info); if (info->hpdet_active) { @@ -675,7 +674,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data) info->hpdet_active = false; } - info->hpdet_done = true; + /* Do not set hp_det done when the cable has been unplugged */ + if (state) + info->hpdet_done = true; out: mutex_unlock(&info->lock);
When the jack is partially inserted and then removed again it may be removed while the hpdet code is running. In this case the following may happen: 1. The "JACKDET rise" or ""JACKDET fall" IRQ triggers 2. arizona_jackdet runs and takes info->lock 3. The "HPDET" IRQ triggers 4. arizona_hpdet_irq runs, blocks on info->lock 5. arizona_jackdet calls arizona_stop_mic() and clears info->hpdet_done 6. arizona_jackdet releases info->lock 7. arizona_hpdet_irq now can continue running and: 7.1 Calls arizona_start_mic() (if a mic was detected) 7.2 sets info->hpdet_done Step 7 is undesirable / a bug: 7.1 causes the device to stay in a high power-state (with MICVDD enabled) 7.2 causes hpdet to not run on the next jack insertion, which in turn causes the EXTCON_JACK_HEADPHONE state to never get set This fixes both issues by skipping these 2 steps when arizona_hpdet_irq runs after the jack has been unplugged. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- sound/soc/codecs/arizona-jack.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)