Re: [PATCH] hwmon: (pwm-fan) Let ctx->pwm_value be assigned only in __set_pwm

[Billy Tsai]

Hi Uwe,

On 2021/11/30, 4:21 PM, "Uwe Kleine-König" <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:

    Hello,

    On Tue, Nov 30, 2021 at 11:00:46AM +0800, Billy Tsai wrote:
    >   > This patch is used to fix the bug when pwm_fan_probe the pwm_value will
    >   > be out of sync with the PWM hardware drivers.
    >   > 
    >   > Fixes: 86585c61972f ("hwmon: (pwm-fan) stop using legacy PWM functions and some cleanups")
    >   > Signed-off-by: Billy Tsai <billy_tsai@xxxxxxxxxxxxxx>
    >   > ---
    >   >  drivers/hwmon/pwm-fan.c | 2 --
    >   >  1 file changed, 2 deletions(-)
    >   > 
    >   > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
    >   > index 17518b4cab1b..f12b9a28a232 100644
    >   > --- a/drivers/hwmon/pwm-fan.c
    >   > +++ b/drivers/hwmon/pwm-fan.c
    >   > @@ -336,8 +336,6 @@ static int pwm_fan_probe(struct platform_device *pdev)
    >   >  			return ret;
    >   >  	}
    >   >  
    >   > -	ctx->pwm_value = MAX_PWM;
    >   > -
    >   >  	pwm_init_state(ctx->pwm, &ctx->pwm_state);

    >   Ah, I see. The effect is that the FAN is supposed to be running at full
    >   speed after probe, but this isn't asserted.

    >   So the patch is fine, but I suggest to make the commit log a bit more
    >   frightening. Something like:

    >   	hwmon: (pwm-fan) Ensure the fan going on in .probe()

    >   	Before commit 86585c61972f ("hwmon: (pwm-fan) stop using legacy
    >   	PWM functions and some cleanups") pwm_apply_state() was called
    >   	unconditionally in pwm_fan_probe(). In this commit this direct
    >   	call was replaced by a call to __set_pwm(ct, MAX_PWM) which
    >   	however is a noop if ctx->pwm_value already matches the value to
    >   	set.

    >   	After probe the fan is supposed to run at full speed, and the
    >   	internal driver state suggests it does, but this isn't asserted
    >   	and depending on bootloader and pwm low-level driver, the fan
    >   	might just be off.

    >   	So drop setting pwm_value to MAXX_PWM to ensure the check in
    >   	__set_pwm doesn't make it exit early and the fan goes on as
    >   	intended.

    >   	Cc: stable@xxxxxxxxxxxxxxx
    >   	Fixes: 86585c61972f ("hwmon: (pwm-fan) stop using legacy PWM functions and some cleanups")
Ok, I will use this commit log and resend the patch.

Best Regards,
Billy Tsai