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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <706A57A6-C34C-425B-B432-2CDF18A344B1@aspeedtech.com>
Date:   Tue, 30 Nov 2021 09:12:15 +0000
From:   Billy Tsai <billy_tsai@...eedtech.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
CC:     "b.zolnierkie@...sung.com" <b.zolnierkie@...sung.com>,
        "jdelvare@...e.com" <jdelvare@...e.com>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [PATCH] hwmon: (pwm-fan) Let ctx->pwm_value be assigned only in
 __set_pwm

Hi Uwe,

On 2021/11/30, 4:21 PM, "Uwe Kleine-König" <u.kleine-koenig@...gutronix.de> 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@...eedtech.com>
    >   > ---
    >   >  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@...r.kernel.org
    >   	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