linux-kernel - Re: [PATCH 3/3] hwmon: (nct6775) Fix fan speed set failure in automatic mode

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <949b2ae6-d631-449a-958c-a7d97fed57f6@roeck-us.net>
Date:   Thu, 16 Nov 2023 00:12:39 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Xing Tong Wu <xingtong_wu@....com>
Cc:     Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org, xingtong.wu@...mens.com,
        tobias.schaffner@...mens.com, gerd.haeussler.ext@...mens.com
Subject: Re: [PATCH 3/3] hwmon: (nct6775) Fix fan speed set failure in
 automatic mode

On Thu, Nov 16, 2023 at 10:23:30AM +0800, Xing Tong Wu wrote:
> From: Xing Tong Wu <xingtong.wu@...mens.com>
> 
> Setting the fan speed is only valid in manual mode; it is not possible
> to set the fan's speed in automatic mode.
> Return error and show error message when attempting to set the fan
> speed in automatic mode.
> 
> Signed-off-by: Xing Tong Wu <xingtong.wu@...mens.com>
> ---
>  drivers/hwmon/nct6775-core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
> index 575db6cb96e9..3fb9896c61ce 100644
> --- a/drivers/hwmon/nct6775-core.c
> +++ b/drivers/hwmon/nct6775-core.c
> @@ -2551,6 +2551,14 @@ store_pwm(struct device *dev, struct device_attribute *attr, const char *buf,
>  	int err;
>  	u16 reg;
>  
> +	if (index == 0 && data->pwm_enable[nr] != manual) {
> +		dev_err(dev,
> +			"The pwm%d doesn't support manual fan speed control in automatic mode.\n",
> +			nr + 1);
> +		dev_err(dev, "Please set pwm%d_enable to manual mode.\n", nr + 1);

No kernel log messages as reponse to user input, please. This
could and likely would clog the kernel log if, for example, some automated
script tries to write into the register.

Please add a comment describing why this is blocked.

> +		return -EOPNOTSUPP;

Wrong error code. I would suggest -EBUSY; that is what is used in the it87
driver.

Guenter

> +	}
> +
>  	err = kstrtoul(buf, 10, &val);
>  	if (err < 0)
>  		return err;
> -- 
> 2.25.1
>