linux-kernel - Re: [PATCH 3/4] hwmon: (oxp-sensors) Fix wording in code comment

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: <51006199-1de4-4bda-b579-181e19bd66e4@roeck-us.net>
Date: Thu, 26 Dec 2024 12:52:23 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: tjakobi@...h.uni-bielefeld.de
Cc: Derek John Clark <derekjohn.clark@...il.com>,
	Joaquín Ignacio Aramendía <samsagax@...il.com>,
	Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] hwmon: (oxp-sensors) Fix wording in code comment

On Thu, Dec 26, 2024 at 06:00:18PM +0100, tjakobi@...h.uni-bielefeld.de wrote:
> From: Tobias Jakobi <tjakobi@...h.uni-bielefeld.de>
> 
> Despite what the current comment says, the register is used
> both for reading and writing the PWM value.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@...h.uni-bielefeld.de>
> ---
>  drivers/hwmon/oxp-sensors.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> index fbd1463d1494..8089349fa508 100644
> --- a/drivers/hwmon/oxp-sensors.c
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -46,14 +46,14 @@ static bool unlock_global_acpi_lock(void)
>  #define OXP_SENSOR_FAN_REG             0x76 /* Fan reading is 2 registers long */
>  #define OXP_2_SENSOR_FAN_REG           0x58 /* Fan reading is 2 registers long */
>  #define OXP_SENSOR_PWM_ENABLE_REG      0x4A /* PWM enable is 1 register long */
> -#define OXP_SENSOR_PWM_REG             0x4B /* PWM reading is 1 register long */
> +#define OXP_SENSOR_PWM_REG             0x4B /* PWM control is 1 register long */

I think that, if anything, this is more confusing than before.
"control" is, for example, enabling or disabling pwm management,
setting automatic or manual mode, or setting the pwm polarity.
Together ith the next two defines, "control" would suggest that
PWM_MODE_AUTO and PWM_MODE_MANUAL are set through that register -
which is not the case. "value" maybe, but "control" is just wrong.

Guenter

>  #define PWM_MODE_AUTO                  0x00
>  #define PWM_MODE_MANUAL                0x01
>  
>  /* OrangePi fan reading and PWM */
>  #define ORANGEPI_SENSOR_FAN_REG        0x78 /* Fan reading is 2 registers long */
>  #define ORANGEPI_SENSOR_PWM_ENABLE_REG 0x40 /* PWM enable is 1 register long */
> -#define ORANGEPI_SENSOR_PWM_REG        0x38 /* PWM reading is 1 register long */
> +#define ORANGEPI_SENSOR_PWM_REG        0x38 /* PWM control is 1 register long */
>  
>  /* Turbo button takeover function
>   * Different boards have different values and EC registers
> -- 
> 2.45.2
> 
>