lkml.org 
[lkml]   [2021]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v4 03/13] extcon: arizona: Fix various races on driver unbind
    Date
    We must free/disable all interrupts and cancel all pending works
    before doing further cleanup.

    Before this commit arizona_extcon_remove() was doing several
    register writes to shut things down before disabling the IRQs
    and it was cancelling only 1 of the 3 different works used.

    Move all the register-writes shutting things down to after
    the disabling of the IRQs and add the 2 missing
    cancel_delayed_work_sync() calls.

    This fixes various possible races on driver unbind. One of which
    would always trigger on devices using the mic-clamp feature for
    jack detection. The ARIZONA_MICD_CLAMP_MODE_MASK update was
    done before disabling the IRQs, causing:
    1. arizona_jackdet() to run
    2. detect a jack being inserted (clamp disabled means jack inserted)
    3. call arizona_start_mic() which:
    3.1 Enables the MICVDD regulator
    3.2 takes a pm_runtime_reference

    And this was all happening after the ARIZONA_MICD_ENA bit clearing,
    which would undo 3.1 and 3.2 because the ARIZONA_MICD_CLAMP_MODE_MASK
    update was being done after the ARIZONA_MICD_ENA bit clearing.

    So this means that arizona_extcon_remove() would exit with
    1. MICVDD enabled and 2. The pm_runtime_reference being unbalanced.

    MICVDD still being enabled caused the following oops when the
    regulator is released by the devm framework:

    [ 2850.745757] ------------[ cut here ]------------
    [ 2850.745827] WARNING: CPU: 2 PID: 2098 at drivers/regulator/core.c:2123 _regulator_put.part.0+0x19f/0x1b0
    [ 2850.745835] Modules linked in: extcon_arizona ...
    ...
    [ 2850.746909] Call Trace:
    [ 2850.746932] regulator_put+0x2d/0x40
    [ 2850.746946] release_nodes+0x22a/0x260
    [ 2850.746984] __device_release_driver+0x190/0x240
    [ 2850.747002] driver_detach+0xd4/0x120
    ...
    [ 2850.747337] ---[ end trace f455dfd7abd9781f ]---

    Note this oops is just one of various theoretically possible races caused
    by the wrong ordering inside arizona_extcon_remove(), this fixes the
    ordering fixing all possible races, including the reported oops.

    Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
    Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
    Signed-off-by: Hans de Goede <hdegoede@redhat.com>
    ---
    drivers/extcon/extcon-arizona.c | 40 +++++++++++++++++----------------
    1 file changed, 21 insertions(+), 19 deletions(-)

    diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
    index f7ef247de46a..76aacbac5869 100644
    --- a/drivers/extcon/extcon-arizona.c
    +++ b/drivers/extcon/extcon-arizona.c
    @@ -1760,25 +1760,6 @@ static int arizona_extcon_remove(struct platform_device *pdev)
    bool change;
    int ret;

    - ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1,
    - ARIZONA_MICD_ENA, 0,
    - &change);
    - if (ret < 0) {
    - dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
    - ret);
    - } else if (change) {
    - regulator_disable(info->micvdd);
    - pm_runtime_put(info->dev);
    - }
    -
    - gpiod_put(info->micd_pol_gpio);
    -
    - pm_runtime_disable(&pdev->dev);
    -
    - regmap_update_bits(arizona->regmap,
    - ARIZONA_MICD_CLAMP_CONTROL,
    - ARIZONA_MICD_CLAMP_MODE_MASK, 0);
    -
    if (info->micd_clamp) {
    jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
    jack_irq_fall = ARIZONA_IRQ_MICD_CLAMP_FALL;
    @@ -1794,10 +1775,31 @@ static int arizona_extcon_remove(struct platform_device *pdev)
    arizona_free_irq(arizona, jack_irq_rise, info);
    arizona_free_irq(arizona, jack_irq_fall, info);
    cancel_delayed_work_sync(&info->hpdet_work);
    + cancel_delayed_work_sync(&info->micd_detect_work);
    + cancel_delayed_work_sync(&info->micd_timeout_work);
    +
    + ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1,
    + ARIZONA_MICD_ENA, 0,
    + &change);
    + if (ret < 0) {
    + dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
    + ret);
    + } else if (change) {
    + regulator_disable(info->micvdd);
    + pm_runtime_put(info->dev);
    + }
    +
    + regmap_update_bits(arizona->regmap,
    + ARIZONA_MICD_CLAMP_CONTROL,
    + ARIZONA_MICD_CLAMP_MODE_MASK, 0);
    regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
    ARIZONA_JD1_ENA, 0);
    arizona_clk32k_disable(arizona);

    + gpiod_put(info->micd_pol_gpio);
    +
    + pm_runtime_disable(&pdev->dev);
    +
    return 0;
    }

    --
    2.29.2
    \
     
     \ /
      Last update: 2021-01-23 13:16    [W:3.802 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site