linux-kernel - Re: [PATCH v4 1/2] dt-bindings: hwmon: pwm-fan: Document default-pwm property

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: <20250103195810.GA2624225-robh@kernel.org>
Date: Fri, 3 Jan 2025 13:58:10 -0600
From: Rob Herring <robh@...nel.org>
To: Peter Korsgaard <peter@...sgaard.com>
Cc: Guenter Roeck <linux@...ck-us.net>, devicetree@...r.kernel.org,
	linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] dt-bindings: hwmon: pwm-fan: Document default-pwm
 property

On Fri, Jan 03, 2025 at 11:14:47AM +0100, Peter Korsgaard wrote:
> The pwm-fan driver uses full PWM (255) duty cycle at startup, which may not
> always be desirable because of noise or power consumption peaks, so add an
> optional "default-pwm" property that can be used to specify a custom default
> PWM duty cycle (0..255).
> 
> This is somewhat similar to target-rpm from fan-common.yaml, but that cannot
> be used here as target-rpm specifies the target fan speed, whereas this is
> the default pwm to set when the device is instantiated - And the resulting
> fan RPM resulting from a given PWM duty cycle is fan dependent.

I still don't agree. Quoting Guenter:

> The two values are also orthogonal. The fan rpm is fan dependent.
> Each fan will require a different pwm value to reach the target speed.
> Trying to use target-rpm to set a default pwm value would really
> not make much if any sense.

But RPM is ultimately what you care about and is the fan parameter 
that's universal yet independent of the underlying control. RPM is what 
determines noise and power consumption.

There's 2 cases to consider: you have a tach signal and know the fan RPM 
or you don't know the RPM. If you have a tach signal, we probably 
wouldn't be discussing this because target-rpm would be enough. So I'm 
assuming this is the case and you have no idea what RPM the fan runs at. 
The fan-common.yaml binding is a bit incomplete for this. What you need 
is some map of fan speed to PWM duty cycle as most likely it is not 
linear response. I think there are 2 options here:

Use the 'cooling-levels' property. Fan "speed" is the index of the 
array. So you just need a 'default-cooling-level' property that's the 
default index.

The other option is define an array of (fan RPM, PWM duty cycle) tuples. 
Then target-rpm can be used to select the entry. We already have 
something like this with 'gpio-fan,speed-map'.

There's also no definition of the minimum RPM or duty cycle in the 
pwm-fan binding. We have min-rpm in fan-common, but that doesn't work 
without a tach. A map would help here as well

This problem to me is similar to LEDs. Ultimately it's brightness that 
you care about, not the current or PWM duty cycle to get there.

Finally, whatever we end up with, it should go in fan-common.yaml. That 
supports PWMs too, so whatever we end up with is applicable to any PWM 
controlled fan.

Rob