diff mbox series

hwmon: pwmfan: do not force disable pwm controller

Message ID 20240816063656.275918-1-mailinglist1@johanneskirchmair.de (mailing list archive)
State Changes Requested
Headers show
Series hwmon: pwmfan: do not force disable pwm controller | expand

Commit Message

mailinglist1@johanneskirchmair.de Aug. 16, 2024, 6:36 a.m. UTC
From: Johannes Kirchmair <johannes.kirchmair@skidata.com>

The pwm1_enable attribute of the pwmfan driver influences the mode of
operation, especially in case of a requested pwm1 duty cycle of zero.
Especially setting pwm1_enable to two, should keep the pwm controller
enabled even if the duty cycle is set to zero [1].

This is not the case at the moment, as the pwm controller is disabled
always if pwm1 is set to zero.

This commit tries to fix this behavior.

[1] https://docs.kernel.org/hwmon/pwm-fan.html

Signed-off-by: Johannes Kirchmair <johannes.kirchmair@skidata.com>
---
 drivers/hwmon/pwm-fan.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Guenter Roeck Aug. 16, 2024, 2:20 p.m. UTC | #1
On 8/15/24 23:36, mailinglist1@johanneskirchmair.de wrote:
> From: Johannes Kirchmair <johannes.kirchmair@skidata.com>
> 
> The pwm1_enable attribute of the pwmfan driver influences the mode of
> operation, especially in case of a requested pwm1 duty cycle of zero.
> Especially setting pwm1_enable to two, should keep the pwm controller
> enabled even if the duty cycle is set to zero [1].
> 
> This is not the case at the moment, as the pwm controller is disabled
> always if pwm1 is set to zero.
> 
> This commit tries to fix this behavior.
> 
> [1] https://docs.kernel.org/hwmon/pwm-fan.html
> 
> Signed-off-by: Johannes Kirchmair <johannes.kirchmair@skidata.com>
> ---
>   drivers/hwmon/pwm-fan.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index a1712649b07e..10c4e9bcf10c 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -167,7 +167,7 @@ static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
>   	return ret;
>   }
>   
> -static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
> +static int pwm_fan_power_off(struct pwm_fan_ctx *ctx, int force_disable)

Please use bool.

Guenter
mailinglist1@johanneskirchmair.de Aug. 19, 2024, 7:09 a.m. UTC | #2
Hey

> -----Ursprüngliche Nachricht-----
> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Freitag, 16. August 2024 16:21
> An: mailinglist1@johanneskirchmair.de; linux-hwmon@vger.kernel.org
> Cc: jdelvare@suse.com; Johannes Kirchmair
> <johannes.kirchmair@skidata.com>
> Betreff: Re: [PATCH] hwmon: pwmfan: do not force disable pwm controller
> 
> On 8/15/24 23:36, mailinglist1@johanneskirchmair.de wrote:
> > From: Johannes Kirchmair <johannes.kirchmair@skidata.com>
> >
> > The pwm1_enable attribute of the pwmfan driver influences the mode of
> > operation, especially in case of a requested pwm1 duty cycle of zero.
> > Especially setting pwm1_enable to two, should keep the pwm controller
> > enabled even if the duty cycle is set to zero [1].
> >
> > This is not the case at the moment, as the pwm controller is disabled
> > always if pwm1 is set to zero.
> >
> > This commit tries to fix this behavior.
> >
> > [1] https://docs.kernel.org/hwmon/pwm-fan.html
> >
> > Signed-off-by: Johannes Kirchmair <johannes.kirchmair@skidata.com>
> > ---
> >   drivers/hwmon/pwm-fan.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index
> > a1712649b07e..10c4e9bcf10c 100644
> > --- a/drivers/hwmon/pwm-fan.c
> > +++ b/drivers/hwmon/pwm-fan.c
> > @@ -167,7 +167,7 @@ static int pwm_fan_power_on(struct pwm_fan_ctx
> *ctx)
> >   	return ret;
> >   }
> >
> > -static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
> > +static int pwm_fan_power_off(struct pwm_fan_ctx *ctx, int
> > +force_disable)
> 
> Please use bool.
Just send v2.

Regards,
Johannes
> 
> Guenter
diff mbox series

Patch

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index a1712649b07e..10c4e9bcf10c 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -167,7 +167,7 @@  static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
 	return ret;
 }
 
-static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
+static int pwm_fan_power_off(struct pwm_fan_ctx *ctx, int force_disable)
 {
 	struct pwm_state *state = &ctx->pwm_state;
 	bool enable_regulator = false;
@@ -180,7 +180,8 @@  static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
 				    state,
 				    &enable_regulator);
 
-	state->enabled = false;
+	if (force_disable)
+		state->enabled = false;
 	state->duty_cycle = 0;
 	ret = pwm_apply_might_sleep(ctx->pwm, state);
 	if (ret) {
@@ -213,7 +214,7 @@  static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
 			return ret;
 		ret = pwm_fan_power_on(ctx);
 	} else {
-		ret = pwm_fan_power_off(ctx);
+		ret = pwm_fan_power_off(ctx, 0);
 	}
 	if (!ret)
 		ctx->pwm_value = pwm;
@@ -468,7 +469,7 @@  static void pwm_fan_cleanup(void *__ctx)
 	del_timer_sync(&ctx->rpm_timer);
 	/* Switch off everything */
 	ctx->enable_mode = pwm_disable_reg_disable;
-	pwm_fan_power_off(ctx);
+	pwm_fan_power_off(ctx, 1);
 }
 
 static int pwm_fan_probe(struct platform_device *pdev)
@@ -661,7 +662,7 @@  static int pwm_fan_suspend(struct device *dev)
 {
 	struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
 
-	return pwm_fan_power_off(ctx);
+	return pwm_fan_power_off(ctx, 1);
 }
 
 static int pwm_fan_resume(struct device *dev)