Re: [PATCH v4 1/6] hwmon: pwm-fan: Refactor fan power on/off

From: Guenter Roeck <linux@roeck-us.net>
To: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: "Jean Delvare" <jdelvare@suse.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Lee Jones" <lee.jones@linaro.org>,
	linux-hwmon@vger.kernel.org, linux-pwm@vger.kernel.org,
	"Markus Niebel" <Markus.Niebel@ew.tq-group.com>
Subject: Re: [PATCH v4 1/6] hwmon: pwm-fan: Refactor fan power on/off
Date: Tue, 30 Aug 2022 06:45:08 -0700	[thread overview]
Message-ID: <20220830134508.GA229842@roeck-us.net> (raw)
In-Reply-To: <20220523110513.407516-2-alexander.stein@ew.tq-group.com>

On Mon, May 23, 2022 at 01:05:08PM +0200, Alexander Stein wrote:
> In preparation for dynamically switching regulator, split the power on
> and power off sequence into separate functions.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/hwmon/pwm-fan.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 6c08551d8d14..831878daffe6 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -82,23 +82,47 @@ static void sample_timer(struct timer_list *t)
>  	mod_timer(&ctx->rpm_timer, jiffies + HZ);
>  }
>  
> -static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
> +static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
>  {
> +	struct pwm_state *state = &ctx->pwm_state;
>  	unsigned long period;
> -	int ret = 0;
> +	int ret;
> +
> +	period = state->period;
> +	state->duty_cycle = DIV_ROUND_UP(ctx->pwm_value * (period - 1), MAX_PWM);
> +	state->enabled = true;
> +	ret = pwm_apply_state(ctx->pwm, state);
> +
> +	return ret;
> +}
> +
> +static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
> +{
>  	struct pwm_state *state = &ctx->pwm_state;
>  
> +	state->enabled = false;
> +	state->duty_cycle = 0;
> +	pwm_apply_state(ctx->pwm, state);
> +
> +	return 0;
> +}
> +
> +static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
> +{
> +	int ret = 0;
> +
>  	mutex_lock(&ctx->lock);
>  	if (ctx->pwm_value == pwm)
>  		goto exit_set_pwm_err;
>  
> -	period = state->period;
> -	state->duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM);
> -	state->enabled = pwm ? true : false;
> +	if (pwm > 0)
> +		ret = pwm_fan_power_on(ctx);
> +	else
> +		ret = pwm_fan_power_off(ctx);
>  
> -	ret = pwm_apply_state(ctx->pwm, state);
>  	if (!ret)
>  		ctx->pwm_value = pwm;
> +
>  exit_set_pwm_err:
>  	mutex_unlock(&ctx->lock);
>  	return ret;