[PATCH] mfd: intel_soc_pmic_crc: balance IRQ wake enable

[PATCH] mfd: intel_soc_pmic_crc: balance IRQ wake enable

[Myeonghun Pak]

The INT33FD Crystal Cove driver enables the parent IRQ as a wake source
after registering its regmap IRQ chip.  When that succeeds, a later
mfd_add_devices() failure or driver removal leaves the wake enable
unbalanced.

Register a managed action only after enable_irq_wake() succeeds.  Since
the action is registered after the managed regmap IRQ chip, reverse devres
order disables IRQ wake before tearing down the IRQ chip.  Keep warning and
continuing when enable_irq_wake() itself fails.

This is limited to the Bay Trail and Cherry Trail Crystal Cove PMIC
variants using the INT33FD ACPI ID.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 516523846006 ("mfd: intel_soc_pmic: Core driver")
Assisted-by: OpenAI:GPT-5.6
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/mfd/intel_soc_pmic_crc.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c
index 627a89334..963dde430 100644
--- a/drivers/mfd/intel_soc_pmic_crc.c
+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -164,6 +164,13 @@ static const struct crystal_cove_config crystal_cove_config_cht_crc = {
 	.irq_chip = &crystal_cove_irq_chip,
 };
 
+static void crystal_cove_disable_irq_wake(void *data)
+{
+	struct intel_soc_pmic *pmic = data;
+
+	disable_irq_wake(pmic->irq);
+}
+
 static int crystal_cove_i2c_probe(struct i2c_client *i2c)
 {
 	const struct crystal_cove_config *config;
@@ -195,8 +202,14 @@ static int crystal_cove_i2c_probe(struct i2c_client *i2c)
 		return ret;
 
 	ret = enable_irq_wake(pmic->irq);
-	if (ret)
+	if (ret) {
 		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
+	} else {
+		ret = devm_add_action_or_reset(dev,
+					       crystal_cove_disable_irq_wake, pmic);
+		if (ret)
+			return ret;
+	}
 
 	/* Add lookup table for crc-pwm */
 	pwm_add_table(crc_pwm_lookup, ARRAY_SIZE(crc_pwm_lookup));
-- 
2.47.1

Re: [PATCH] mfd: intel_soc_pmic_crc: balance IRQ wake enable

[Andy Shevchenko]

On Sat, Sep 12, 2026 at 09:36:15PM -0400, Myeonghun Pak wrote:
> The INT33FD Crystal Cove driver enables the parent IRQ as a wake source
> after registering its regmap IRQ chip.  When that succeeds, a later
> mfd_add_devices() failure or driver removal leaves the wake enable
> unbalanced.
> 
> Register a managed action only after enable_irq_wake() succeeds.  Since
> the action is registered after the managed regmap IRQ chip, reverse devres
> order disables IRQ wake before tearing down the IRQ chip.  Keep warning and
> continuing when enable_irq_wake() itself fails.
> 
> This is limited to the Bay Trail and Cherry Trail Crystal Cove PMIC
> variants using the INT33FD ACPI ID.
> 
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.

...

> Assisted-by: OpenAI:GPT-5.6

Assisted-by: LLM

...

>  	ret = enable_irq_wake(pmic->irq);
> -	if (ret)
> +	if (ret) {
>  		dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> +	} else {
> +		ret = devm_add_action_or_reset(dev,
> +					       crystal_cove_disable_irq_wake, pmic);

Make it a single line.

> +		if (ret)
> +			return ret;
> +	}


Shouldn't disable_irq_wake() be called at .shutdown() as well?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] mfd: intel_soc_pmic_crc: balance IRQ wake enable

[Myeonghun Pak]

Hello Andy,

Thanks for the review. I'll address these in v2, including releasing
the wake-disable action at shutdown only if it was registered.

Best regards,
Myeonghun Pak

2026년 9월 14일 (월) 오전 4:32, Andy Shevchenko <andriy.shevchenko@intel.com>님이 작성:
>
> On Sat, Sep 12, 2026 at 09:36:15PM -0400, Myeonghun Pak wrote:
> > The INT33FD Crystal Cove driver enables the parent IRQ as a wake source
> > after registering its regmap IRQ chip.  When that succeeds, a later
> > mfd_add_devices() failure or driver removal leaves the wake enable
> > unbalanced.
> >
> > Register a managed action only after enable_irq_wake() succeeds.  Since
> > the action is registered after the managed regmap IRQ chip, reverse devres
> > order disables IRQ wake before tearing down the IRQ chip.  Keep warning and
> > continuing when enable_irq_wake() itself fails.
> >
> > This is limited to the Bay Trail and Cherry Trail Crystal Cove PMIC
> > variants using the INT33FD ACPI ID.
> >
> > This issue was identified during our ongoing static-analysis research while
> > reviewing kernel code.
>
> ...
>
> > Assisted-by: OpenAI:GPT-5.6
>
> Assisted-by: LLM
>
> ...
>
> >       ret = enable_irq_wake(pmic->irq);
> > -     if (ret)
> > +     if (ret) {
> >               dev_warn(dev, "Can't enable IRQ as wake source: %d\n", ret);
> > +     } else {
> > +             ret = devm_add_action_or_reset(dev,
> > +                                            crystal_cove_disable_irq_wake, pmic);
>
> Make it a single line.
>
> > +             if (ret)
> > +                     return ret;
> > +     }
>
>
> Shouldn't disable_irq_wake() be called at .shutdown() as well?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>