All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
@ 2026-08-28 10:50 Cong Nguyen
  2026-08-28 11:01 ` sashiko-bot
  2026-08-30 15:05 ` [PATCH v2] " Cong Nguyen
  0 siblings, 2 replies; 8+ messages in thread
From: Cong Nguyen @ 2026-08-28 10:50 UTC (permalink / raw)
  To: Guenter Roeck, Alexander Stein; +Cc: linux-hwmon, linux-kernel

pm_runtime_enable() runs unconditionally at the end of probe, but the
devm action that calls pm_runtime_disable() is registered only when
control GPIOs are present. An alarm-only fan (no control GPIOs, alarm
GPIO optional) is a valid DT configuration; on unbind the runtime PM
core warns about the missing disable.

Use devm_pm_runtime_enable() so disable is automatic on device release,
and drop the redundant pm_runtime_disable() from gpio_fan_stop().

Fixes: 0d01110e6356 ("hwmon: (gpio-fan) Add regulator support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/hwmon/gpio-fan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 084828e1e281..0bd5561b7a40 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -524,8 +524,6 @@ static void gpio_fan_stop(void *data)
 	mutex_lock(&fan_data->lock);
 	set_fan_speed(data, 0);
 	mutex_unlock(&fan_data->lock);
-
-	pm_runtime_disable(fan_data->dev);
 }
 
 static int gpio_fan_probe(struct platform_device *pdev)
@@ -581,7 +579,9 @@ static int gpio_fan_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_set_suspended(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
+	err = devm_pm_runtime_enable(&pdev->dev);
+	if (err)
+		return err;
 	/* If current GPIO state is active, mark RPM as active as well */
 	if (fan_data->speed_index > 0) {
 		int ret;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-28 10:50 [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans Cong Nguyen
@ 2026-08-28 11:01 ` sashiko-bot
  2026-08-28 15:58   ` Guenter Roeck
  2026-08-30 15:05 ` [PATCH v2] " Cong Nguyen
  1 sibling, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-08-28 11:01 UTC (permalink / raw)
  To: Cong Nguyen; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Converting to `devm_pm_runtime_enable()` and placing it at the end of `probe()` incorrectly reverses the teardown order, causing PM runtime to be disabled too early. This leads to a regulator enable count leak and exposes a sysfs race condition on driver unbind.
--

commit 728a875c515df8244bd554b338f4f008d88b3b9e
Author: Cong Nguyen <congnt264@gmail.com>

hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans

This commit changes the driver to use devm_pm_runtime_enable() during probe
and removes the manual pm_runtime_disable() from gpio_fan_stop(). This ensures
PM runtime is automatically disabled on unbind, fixing an imbalance for
alarm-only fans.

> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 084828e1e2817..0bd5561b7a405 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -524,8 +524,6 @@ static void gpio_fan_stop(void *data)
>  	mutex_lock(&fan_data->lock);
>  	set_fan_speed(data, 0);
>  	mutex_unlock(&fan_data->lock);
> -
> -	pm_runtime_disable(fan_data->dev);
>  }
>  
>  static int gpio_fan_probe(struct platform_device *pdev)
> @@ -581,7 +579,9 @@ static int gpio_fan_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_set_suspended(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> +	err = devm_pm_runtime_enable(&pdev->dev);
> +	if (err)
> +		return err;

[Severity: High]
Does moving the pm_runtime_disable() action to the end of probe reverse the
teardown order in a way that breaks unbind?

Because devm unwinds in LIFO order, registering devm_pm_runtime_enable() at
the end of gpio_fan_probe() means pm_runtime_disable() will execute first
during unbind. The cleanup action for gpio_fan_stop() is registered
earlier in probe via devm_add_action_or_reset(), so it will execute after
PM runtime has already been disabled.

When gpio_fan_stop() calls set_fan_speed(..., 0), it invokes
pm_runtime_put_sync(). Since PM runtime is already disabled, this will
fail with -EACCES. This bypasses the runtime suspend callback, leaving the
hardware regulator permanently enabled and causing a regulator enable count
leak.

>  	/* If current GPIO state is active, mark RPM as active as well */
>  	if (fan_data->speed_index > 0) {
>  		int ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828105018.2392383-1-congnt264@gmail.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-28 11:01 ` sashiko-bot
@ 2026-08-28 15:58   ` Guenter Roeck
  2026-08-30 15:01     ` Nguyễn Công
  0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2026-08-28 15:58 UTC (permalink / raw)
  To: sashiko-reviews, Cong Nguyen; +Cc: linux-hwmon

On 8/28/26 04:01, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Converting to `devm_pm_runtime_enable()` and placing it at the end of `probe()` incorrectly reverses the teardown order, causing PM runtime to be disabled too early. This leads to a regulator enable count leak and exposes a sysfs race condition on driver unbind.
> --
> 
> commit 728a875c515df8244bd554b338f4f008d88b3b9e
> Author: Cong Nguyen <congnt264@gmail.com>
> 
> hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
> 
> This commit changes the driver to use devm_pm_runtime_enable() during probe
> and removes the manual pm_runtime_disable() from gpio_fan_stop(). This ensures
> PM runtime is automatically disabled on unbind, fixing an imbalance for
> alarm-only fans.
> 
>> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
>> index 084828e1e2817..0bd5561b7a405 100644
>> --- a/drivers/hwmon/gpio-fan.c
>> +++ b/drivers/hwmon/gpio-fan.c
>> @@ -524,8 +524,6 @@ static void gpio_fan_stop(void *data)
>>   	mutex_lock(&fan_data->lock);
>>   	set_fan_speed(data, 0);
>>   	mutex_unlock(&fan_data->lock);
>> -
>> -	pm_runtime_disable(fan_data->dev);
>>   }
>>   
>>   static int gpio_fan_probe(struct platform_device *pdev)
>> @@ -581,7 +579,9 @@ static int gpio_fan_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	pm_runtime_set_suspended(&pdev->dev);
>> -	pm_runtime_enable(&pdev->dev);
>> +	err = devm_pm_runtime_enable(&pdev->dev);
>> +	if (err)
>> +		return err;
> 
> [Severity: High]
> Does moving the pm_runtime_disable() action to the end of probe reverse the
> teardown order in a way that breaks unbind?
> 
> Because devm unwinds in LIFO order, registering devm_pm_runtime_enable() at
> the end of gpio_fan_probe() means pm_runtime_disable() will execute first
> during unbind. The cleanup action for gpio_fan_stop() is registered
> earlier in probe via devm_add_action_or_reset(), so it will execute after
> PM runtime has already been disabled.
> 
> When gpio_fan_stop() calls set_fan_speed(..., 0), it invokes
> pm_runtime_put_sync(). Since PM runtime is already disabled, this will
> fail with -EACCES. This bypasses the runtime suspend callback, leaving the
> hardware regulator permanently enabled and causing a regulator enable count
> leak.
> 
Seems to me that this will need a more comprehensive fix.

Thanks,
Guenter

>>   	/* If current GPIO state is active, mark RPM as active as well */
>>   	if (fan_data->speed_index > 0) {
>>   		int ret;
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-28 15:58   ` Guenter Roeck
@ 2026-08-30 15:01     ` Nguyễn Công
  0 siblings, 0 replies; 8+ messages in thread
From: Nguyễn Công @ 2026-08-30 15:01 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: sashiko-reviews, linux-hwmon

On Fri, Aug 28, 2026 at 10:58 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/28/26 04:01, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] Converting to `devm_pm_runtime_enable()` and placing it at the end of `probe()` incorrectly reverses the teardown order, causing PM runtime to be disabled too early. This leads to a regulator enable count leak and exposes a sysfs race condition on driver unbind.
> > --
> >
> > commit 728a875c515df8244bd554b338f4f008d88b3b9e
> > Author: Cong Nguyen <congnt264@gmail.com>
> >
> > hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
> >
> > This commit changes the driver to use devm_pm_runtime_enable() during probe
> > and removes the manual pm_runtime_disable() from gpio_fan_stop(). This ensures
> > PM runtime is automatically disabled on unbind, fixing an imbalance for
> > alarm-only fans.
> >
> >> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> >> index 084828e1e2817..0bd5561b7a405 100644
> >> --- a/drivers/hwmon/gpio-fan.c
> >> +++ b/drivers/hwmon/gpio-fan.c
> >> @@ -524,8 +524,6 @@ static void gpio_fan_stop(void *data)
> >>      mutex_lock(&fan_data->lock);
> >>      set_fan_speed(data, 0);
> >>      mutex_unlock(&fan_data->lock);
> >> -
> >> -    pm_runtime_disable(fan_data->dev);
> >>   }
> >>
> >>   static int gpio_fan_probe(struct platform_device *pdev)
> >> @@ -581,7 +579,9 @@ static int gpio_fan_probe(struct platform_device *pdev)
> >>      }
> >>
> >>      pm_runtime_set_suspended(&pdev->dev);
> >> -    pm_runtime_enable(&pdev->dev);
> >> +    err = devm_pm_runtime_enable(&pdev->dev);
> >> +    if (err)
> >> +            return err;
> >
> > [Severity: High]
> > Does moving the pm_runtime_disable() action to the end of probe reverse the
> > teardown order in a way that breaks unbind?
> >
> > Because devm unwinds in LIFO order, registering devm_pm_runtime_enable() at
> > the end of gpio_fan_probe() means pm_runtime_disable() will execute first
> > during unbind. The cleanup action for gpio_fan_stop() is registered
> > earlier in probe via devm_add_action_or_reset(), so it will execute after
> > PM runtime has already been disabled.
> >
> > When gpio_fan_stop() calls set_fan_speed(..., 0), it invokes
> > pm_runtime_put_sync(). Since PM runtime is already disabled, this will
> > fail with -EACCES. This bypasses the runtime suspend callback, leaving the
> > hardware regulator permanently enabled and causing a regulator enable count
> > leak.
> >
> Seems to me that this will need a more comprehensive fix.

Confirmed, Sashiko's right. v1 registered devm_pm_runtime_enable() after
gpio_fan_stop()'s devm action, so LIFO teardown ran pm_runtime_disable()
first -- gpio_fan_stop()'s pm_runtime_put_sync() got -EACCES and never
reached the regulator disable.

v2 moves devm_pm_runtime_enable() earlier so gpio_fan_stop() runs first
again. Sending shortly.

thanks,
Cong

>
> Thanks,
> Guenter
>
> >>      /* If current GPIO state is active, mark RPM as active as well */
> >>      if (fan_data->speed_index > 0) {
> >>              int ret;
> >
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-28 10:50 [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans Cong Nguyen
  2026-08-28 11:01 ` sashiko-bot
@ 2026-08-30 15:05 ` Cong Nguyen
  2026-08-30 15:21   ` sashiko-bot
  2026-08-30 15:22   ` Guenter Roeck
  1 sibling, 2 replies; 8+ messages in thread
From: Cong Nguyen @ 2026-08-30 15:05 UTC (permalink / raw)
  To: Guenter Roeck, Alexander Stein; +Cc: linux-hwmon, linux-kernel

pm_runtime_enable() runs unconditionally in probe, but the devm cleanup
that calls pm_runtime_disable() is only registered when control GPIOs
are present. Alarm-only fans never get it, so unbind warns about the
missing disable.

Switch to devm_pm_runtime_enable(), registered before gpio_fan_stop()'s
devm action so LIFO teardown runs gpio_fan_stop() first. gpio_fan_stop()
calls set_fan_speed(0) -> pm_runtime_put_sync(), which needs PM still
enabled to reach gpio_fan_runtime_suspend() and turn the regulator off;
registering enable after gpio_fan_stop() (as v1 did) reverses that order
and leaves the regulator on.

Fixes: 0d01110e6356 ("hwmon: (gpio-fan) Add regulator support")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/b9792be3-fcd6-468c-9149-7e33640ee167@roeck-us.net
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
Changes in v2:
- v1's devm_pm_runtime_enable() was registered after gpio_fan_stop()'s
  devm action, reversing LIFO teardown order and leaving the regulator
  enabled on unbind (Guenter/Sashiko). Moved it earlier so gpio_fan_stop()
  registers first.

 drivers/hwmon/gpio-fan.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 084828e1e281..988f52d11ce2 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -524,8 +524,6 @@ static void gpio_fan_stop(void *data)
 	mutex_lock(&fan_data->lock);
 	set_fan_speed(data, 0);
 	mutex_unlock(&fan_data->lock);
-
-	pm_runtime_disable(fan_data->dev);
 }
 
 static int gpio_fan_probe(struct platform_device *pdev)
@@ -553,6 +551,16 @@ static int gpio_fan_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(fan_data->supply),
 				     "Failed to get fan-supply");
 
+	/*
+	 * Register before gpio_fan_stop()'s devm action: LIFO teardown must
+	 * run gpio_fan_stop() (needs PM enabled to disable the regulator)
+	 * before this disables PM.
+	 */
+	pm_runtime_set_suspended(&pdev->dev);
+	err = devm_pm_runtime_enable(&pdev->dev);
+	if (err)
+		return err;
+
 	/* Configure control GPIOs if available. */
 	if (fan_data->gpios && fan_data->num_gpios > 0) {
 		if (!fan_data->speed || fan_data->num_speed <= 1)
@@ -580,8 +588,6 @@ static int gpio_fan_probe(struct platform_device *pdev)
 			return err;
 	}
 
-	pm_runtime_set_suspended(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
 	/* If current GPIO state is active, mark RPM as active as well */
 	if (fan_data->speed_index > 0) {
 		int ret;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-30 15:05 ` [PATCH v2] " Cong Nguyen
@ 2026-08-30 15:21   ` sashiko-bot
  2026-08-30 15:22   ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-30 15:21 UTC (permalink / raw)
  To: Cong Nguyen; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Moving devm_pm_runtime_enable() to the start of probe opens a critical race window causing a permanent PM usage count leak.

Pre-existing issues:
- [Critical] Missing cancel_work_sync() for alarm_work leads to Use-After-Free.
- [High] Missing synchronize_irq() in runtime suspend callback for shared IRQ.
- [High] Probe error paths trigger pm_runtime_put_sync() on an unresumed device, causing a usage count underflow and preventing the regulator from disabling.
- [High] State mismatch and PM usage counter underflow on pm_runtime_put_sync() failure.
- [High] Missing lock acquisition in gpio_fan_shutdown() leads to data races and unbalanced runtime PM.
- [High] Alarm IRQ handler unconditionally returns IRQ_NONE, breaking the shared interrupt line.
--

commit 7341755fa2914776aa77e1819998064fadc8592b
Author: Cong Nguyen <congnt264@gmail.com>

hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans

This commit addresses a runtime PM imbalance in alarm-only fans where
pm_runtime_disable() was skipped on teardown. It replaces
pm_runtime_enable() with devm_pm_runtime_enable() and moves it earlier
in probe to ensure LIFO teardown runs gpio_fan_stop() before disabling
PM.

Link: https://lore.kernel.org/r/b9792be3-fcd6-468c-9149-7e33640ee167@roeck-us.net

> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 084828e1e2817..988f52d11ce2e 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
[ ... ]
> @@ -553,6 +551,16 @@ static int gpio_fan_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(fan_data->supply),
>  				     "Failed to get fan-supply");
>  
> +	/*
> +	 * Register before gpio_fan_stop()'s devm action: LIFO teardown must
> +	 * run gpio_fan_stop() (needs PM enabled to disable the regulator)
> +	 * before this disables PM.
> +	 */
> +	pm_runtime_set_suspended(&pdev->dev);
> +	err = devm_pm_runtime_enable(&pdev->dev);

[Severity: High]
Does moving this earlier open a race condition that could permanently leak a PM
usage count?

If a concurrent sysfs write occurs after hwmon registration exposes sysfs files
(like pwm1, fan1_target) but before probe finishes, set_fan_speed() will
successfully call pm_runtime_resume_and_get().

This increments the usage count to 1 and sets fan_data->speed_index > 0.

When probe reaches its end, it checks if fan_data->speed_index > 0 and
unconditionally calls pm_runtime_resume_and_get() again.

Will this double-increment the counter, preventing the device from ever entering
runtime suspend?

Previously, PM was disabled during this window, causing concurrent sysfs access
to fail cleanly with -EACCES.

[ ... ]

[Severity: High]
This is a pre-existing issue, but could the devm teardown cause a PM usage
count underflow during probe error paths?

In gpio_fan_probe(), the teardown action is registered before the final PM
reference is acquired:

    err = devm_add_action_or_reset(dev, gpio_fan_stop, fan_data);

If probe fails after fan_ctrl_init() sets speed_index > 0, but before
pm_runtime_resume_and_get() is called at the end of probe, the PM usage count
is 0.

The devm rollback calls gpio_fan_stop() which calls set_fan_speed(0). This
invokes pm_runtime_put_sync(), decrementing the usage count from 0 to -1.

Because the count underflows, does the PM core (rpm_suspend) abort with an
error and leave the regulator powered on despite the teardown?


[Severity: High]
This is a pre-existing issue, but are we missing a synchronize_irq() in the
runtime suspend callback?

The PM Subsystem Concurrency and Locking guidelines dictate that drivers using
IRQF_SHARED must call synchronize_irq() before powering down hardware.

In fan_alarm_init(), the alarm interrupt is requested with IRQF_SHARED:

    devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
                     IRQF_SHARED, "GPIO fan alarm", fan_data);

However, in gpio_fan_runtime_suspend(), the hardware regulator is disabled
without first synchronizing the interrupt:

    if (fan_data->supply)
        ret = regulator_disable(fan_data->supply);

Could this race with a shared IRQ executing mid-flight when the device enters
runtime suspend, causing spurious interrupts or data races?


[Severity: Critical]
This is a pre-existing issue, but can the lack of cancel_work_sync() for
alarm_work lead to a Use-After-Free?

In fan_alarm_init(), the work is initialized and the IRQ is requested via
devm_request_irq():

    INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
    return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
                            IRQF_SHARED, "GPIO fan alarm", fan_data);

On driver unbind or probe failure, devm frees the IRQ, but there is no call to
cancel_work_sync() to wait for or cancel already-queued work.

Devm then frees the fan_data structure.

If the asynchronous workqueue eventually executes fan_alarm_notify(), it
dereferences the freed memory:

    sysfs_notify(&fan_data->hwmon_dev->kobj, NULL, "fan1_alarm");

Would a devm_add_action_or_reset() be needed here to cancel the work?


[Severity: High]
This is a pre-existing issue, but does set_fan_speed() cause a state mismatch
and PM counter underflow if pm_runtime_put_sync() fails?

In set_fan_speed():

    ret = pm_runtime_put_sync(fan_data->dev);
    if (ret < 0 && ret != -ENOSYS)
        return ret;
    fan_data->speed_index = speed_index;

pm_runtime_put_sync() unconditionally drops the PM usage count even on error.
If it returns an error, the function returns early without updating
fan_data->speed_index to 0.

Because speed_index incorrectly remains > 0, any subsequent attempt to stop
the fan will invoke pm_runtime_put_sync() again.

Could this lead to multiple decrements of the PM usage counter for a single
PM reference?


[Severity: High]
This is a pre-existing issue, but might gpio_fan_shutdown() race due to missing
lock acquisition?

    static void gpio_fan_shutdown(struct platform_device *pdev)
    {
        struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
        if (fan_data->gpios)
            set_fan_speed(fan_data, 0);
    }

set_fan_speed() explicitly requires fan_data->lock to be held to safely read
and update fan_data->speed_index.

If system shutdown races with a concurrent sysfs write or thermal callback,
both threads might issue set_fan_speed(0) simultaneously.

Could this issue pm_runtime_put_sync() twice and underflow the runtime PM
usage count?


[Severity: High]
This is a pre-existing issue, but does the alarm IRQ handler break the
shared interrupt line by unconditionally returning IRQ_NONE?

    static irqreturn_t fan_alarm_irq_handler(int irq, void *dev_id)
    {
        struct gpio_fan_data *fan_data = dev_id;
        schedule_work(&fan_data->alarm_work);
        return IRQ_NONE;
    }

When requested with IRQF_SHARED, returning IRQ_NONE informs the core IRQ
subsystem that the interrupt was unhandled.

After enough unhandled triggers, won't the kernel's spurious interrupt
detection permanently disable the shared interrupt line, causing a denial of
service for any other device sharing this IRQ?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830150508.1475105-1-congnt264@gmail.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-30 15:05 ` [PATCH v2] " Cong Nguyen
  2026-08-30 15:21   ` sashiko-bot
@ 2026-08-30 15:22   ` Guenter Roeck
  2026-09-01  9:50     ` Nguyễn Công
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2026-08-30 15:22 UTC (permalink / raw)
  To: Cong Nguyen, Alexander Stein; +Cc: linux-hwmon, linux-kernel

On 8/30/26 08:05, Cong Nguyen wrote:
> pm_runtime_enable() runs unconditionally in probe, but the devm cleanup
> that calls pm_runtime_disable() is only registered when control GPIOs
> are present. Alarm-only fans never get it, so unbind warns about the
> missing disable.
> 
> Switch to devm_pm_runtime_enable(), registered before gpio_fan_stop()'s
> devm action so LIFO teardown runs gpio_fan_stop() first. gpio_fan_stop()
> calls set_fan_speed(0) -> pm_runtime_put_sync(), which needs PM still
> enabled to reach gpio_fan_runtime_suspend() and turn the regulator off;
> registering enable after gpio_fan_stop() (as v1 did) reverses that order
> and leaves the regulator on.
> 
> Fixes: 0d01110e6356 ("hwmon: (gpio-fan) Add regulator support")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Link: https://lore.kernel.org/r/b9792be3-fcd6-468c-9149-7e33640ee167@roeck-us.net
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4
> Signed-off-by: Cong Nguyen <congnt264@gmail.com>
> ---

Another instance of a new patch version sent as reply to a previous
version.

It is against guidance in Documentation/process/submitting-patches.rst,
yet it proliferates, and more and more people send new patch revisions
this way. Where is this suggested ?

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
  2026-08-30 15:22   ` Guenter Roeck
@ 2026-09-01  9:50     ` Nguyễn Công
  0 siblings, 0 replies; 8+ messages in thread
From: Nguyễn Công @ 2026-09-01  9:50 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Alexander Stein, linux-hwmon, linux-kernel

On Sun, Aug 30, 2026 at 10:22 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/30/26 08:05, Cong Nguyen wrote:
> > pm_runtime_enable() runs unconditionally in probe, but the devm cleanup
> > that calls pm_runtime_disable() is only registered when control GPIOs
> > are present. Alarm-only fans never get it, so unbind warns about the
> > missing disable.
> >
> > Switch to devm_pm_runtime_enable(), registered before gpio_fan_stop()'s
> > devm action so LIFO teardown runs gpio_fan_stop() first. gpio_fan_stop()
> > calls set_fan_speed(0) -> pm_runtime_put_sync(), which needs PM still
> > enabled to reach gpio_fan_runtime_suspend() and turn the regulator off;
> > registering enable after gpio_fan_stop() (as v1 did) reverses that order
> > and leaves the regulator on.
> >
> > Fixes: 0d01110e6356 ("hwmon: (gpio-fan) Add regulator support")
> > Reported-by: Guenter Roeck <linux@roeck-us.net>
> > Link: https://lore.kernel.org/r/b9792be3-fcd6-468c-9149-7e33640ee167@roeck-us.net
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-4
> > Signed-off-by: Cong Nguyen <congnt264@gmail.com>
> > ---
>
> Another instance of a new patch version sent as reply to a previous
> version.
>
> It is against guidance in Documentation/process/submitting-patches.rst,
> yet it proliferates, and more and more people send new patch revisions
> this way. Where is this suggested ?

Fair, this was my mistake -- won't thread rerolls under the previous version
again. v3 just sent standalone.

>
> Thanks,
> Guenter
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-01  9:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28 10:50 [PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans Cong Nguyen
2026-08-28 11:01 ` sashiko-bot
2026-08-28 15:58   ` Guenter Roeck
2026-08-30 15:01     ` Nguyễn Công
2026-08-30 15:05 ` [PATCH v2] " Cong Nguyen
2026-08-30 15:21   ` sashiko-bot
2026-08-30 15:22   ` Guenter Roeck
2026-09-01  9:50     ` Nguyễn Công

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.