From: Hans de Goede <hdegoede@redhat.com> To: Lee Jones <lee.jones@linaro.org>, MyungJoo Ham <myungjoo.ham@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, Cezary Rojewski <cezary.rojewski@intel.com>, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>, Liam Girdwood <liam.r.girdwood@linux.intel.com>, Jie Yang <yang.jie@linux.intel.com>, Mark Brown <broonie@kernel.org> Cc: Hans de Goede <hdegoede@redhat.com>, patches@opensource.cirrus.com, linux-kernel@vger.kernel.org, Andy Shevchenko <andy.shevchenko@gmail.com>, Charles Keepax <ckeepax@opensource.cirrus.com>, alsa-devel@alsa-project.org Subject: [PATCH v4 resend 03/13] extcon: arizona: Fix various races on driver unbind Date: Thu, 4 Feb 2021 12:24:52 +0100 [thread overview] Message-ID: <20210204112502.88362-4-hdegoede@redhat.com> (raw) In-Reply-To: <20210204112502.88362-1-hdegoede@redhat.com> 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> Tested-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
WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com> To: Lee Jones <lee.jones@linaro.org>, MyungJoo Ham <myungjoo.ham@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, Cezary Rojewski <cezary.rojewski@intel.com>, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>, Liam Girdwood <liam.r.girdwood@linux.intel.com>, Jie Yang <yang.jie@linux.intel.com>, Mark Brown <broonie@kernel.org> Cc: alsa-devel@alsa-project.org, Charles Keepax <ckeepax@opensource.cirrus.com>, patches@opensource.cirrus.com, linux-kernel@vger.kernel.org, Hans de Goede <hdegoede@redhat.com>, Andy Shevchenko <andy.shevchenko@gmail.com> Subject: [PATCH v4 resend 03/13] extcon: arizona: Fix various races on driver unbind Date: Thu, 4 Feb 2021 12:24:52 +0100 [thread overview] Message-ID: <20210204112502.88362-4-hdegoede@redhat.com> (raw) In-Reply-To: <20210204112502.88362-1-hdegoede@redhat.com> 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> Tested-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
next prev parent reply other threads:[~2021-02-04 11:27 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20210204112515epcas1p27a866811ba15a8cd8b0be9a3f7bf86e5@epcas1p2.samsung.com> 2021-02-04 11:24 ` [PATCH v4 resend 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 13:44 ` Lee Jones 2021-02-04 13:44 ` Lee Jones 2021-02-04 11:24 ` [PATCH v4 resend 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` Hans de Goede [this message] 2021-02-04 11:24 ` [PATCH v4 resend 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf, micd-pol") call Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:24 ` [PATCH v4 resend 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede 2021-02-04 11:24 ` Hans de Goede 2021-02-04 11:25 ` [PATCH v4 resend 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede 2021-02-04 11:25 ` Hans de Goede 2021-02-04 11:25 ` [PATCH v4 resend 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede 2021-02-04 11:25 ` Hans de Goede 2021-02-04 11:25 ` [PATCH v4 resend 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support Hans de Goede 2021-02-04 11:25 ` Hans de Goede 2021-02-05 2:00 ` [PATCH v4 resend 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Chanwoo Choi 2021-02-05 2:00 ` Chanwoo Choi 2021-02-05 10:27 ` Hans de Goede 2021-02-05 10:27 ` Hans de Goede 2021-02-08 19:12 ` Hans de Goede 2021-02-08 19:12 ` Hans de Goede 2021-02-09 14:14 ` Lee Jones 2021-02-09 14:14 ` Lee Jones 2021-02-09 14:52 ` Hans de Goede 2021-02-09 14:52 ` Hans de Goede 2021-02-09 15:45 ` Lee Jones 2021-02-09 15:45 ` Lee Jones 2021-02-09 16:34 ` Hans de Goede 2021-02-09 16:34 ` Hans de Goede 2021-02-09 16:44 ` Lee Jones 2021-02-09 16:44 ` Lee Jones 2021-02-10 19:57 ` Mark Brown 2021-02-10 19:57 ` Mark Brown 2021-02-11 8:40 ` Lee Jones 2021-02-11 8:40 ` Lee Jones 2021-03-07 15:17 Hans de Goede 2021-03-07 15:17 ` [PATCH v4 resend 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede 2021-03-07 15:17 ` Hans de Goede
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210204112502.88362-4-hdegoede@redhat.com \ --to=hdegoede@redhat.com \ --cc=alsa-devel@alsa-project.org \ --cc=andy.shevchenko@gmail.com \ --cc=broonie@kernel.org \ --cc=cezary.rojewski@intel.com \ --cc=ckeepax@opensource.cirrus.com \ --cc=cw00.choi@samsung.com \ --cc=lee.jones@linaro.org \ --cc=liam.r.girdwood@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=myungjoo.ham@samsung.com \ --cc=patches@opensource.cirrus.com \ --cc=pierre-louis.bossart@linux.intel.com \ --cc=yang.jie@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.