1. Patchew
  2. linux
  3. View series

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

Cong Nguyen posted 1 patch 4 weeks, 1 day ago
There is a newer version of this series
drivers/hwmon/gpio-fan.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
Posted by Cong Nguyen 4 weeks, 1 day ago 
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
[PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
Posted by Cong Nguyen 3 weeks, 6 days ago 
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
Re: [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
Posted by Guenter Roeck 3 weeks, 6 days ago 
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
Re: [PATCH v2] hwmon: (gpio-fan) fix pm_runtime imbalance for alarm-only fans
Posted by Nguyễn Công 3 weeks, 4 days ago 
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
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.