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: <24dc65c2-b6c9-4909-a784-fb81d9299f1c@roeck-us.net>
Date: Sun, 27 Oct 2024 11:16:54 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: "Derek J. Clark" <derekjohn.clark@...il.com>
Cc: Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
 Cryolita PukNgae <cryolitia@...il.com>, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC] hwmon: pwm_enable clarification

On 10/27/24 10:48, Derek J. Clark wrote:
> Greetings all,
> 
> I am working with Cryolita to fix up the GPD driver she submitted recently:
> https://lore.kernel.org/all/20240718-gpd_fan-v4-0-116e5431a9fe@gmail.com/
> 
> We are currently having a discussion about the meaning of this part of the
> documentation and are seeking some guidance from upstream.
>  >> pwm[1-*]_enable
>>      	Fan speed control method:
>>      	0: no fan speed control (i.e. fan at full speed)
>>      	1: manual fan speed control enabled (using pwm[1-*])
>>      	2+: automatic fan speed control enabled
>>      	Check individual chip documentation files for automatic mode
>>      	details.
>>      	RW
> 
> In oxp-sensors we took 0 to mean "no kernel control" so a setting of 0 is
> technically "automatic" but fully controlled by the hardware with no
> interaction from the driver. In her original driver draft she had taken this

That is wrong. It should be (or have been) 2 or higher. Ah yes, I can see that
the code sets fan control to automatic when oxp_pwm_disable() is called.
Again, that is wrong. Congratulations to the submitters, you sneaked that by
my review. That doesn't make it better. It is still wrong, and I call it "sneaky"
because the function is not called "oxp_pwm_automatic()" or similar, it is
called "oxp_pwm_disable(). Setting fan control to automatic does not disable
fan control.

My bad, I should have paid closer attention, and/or maybe not have trusted
the submitters as much as I did. I guess I'll have to pay closer attention
in the future.

> literally to have the driver set the fan speed to 100% on this setting rather
> then give back control to the hardware. My question is simply what is the
> correct interpretation here? Ideally I would like to see this interface match

It seems to me that the above text is well defined.

> as existing userspace software is expecting 0 as hardware controlled and 1 as
> manually controlled, but we also want to ensure this is correct before we
> submit a v5.
> 

Any such userspace expectations are simply wrong. The ABI definition is above
and, again, it is well defined.

	0: no fan speed control (i.e. fan at full speed)

I don't really see any ambiguity in this text. This isn't about kernel control,
it is an absolute statement. There is no "kernel" in "no fan speed control".
"fan at full speed" should make this even more obvious.

Guenter


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ