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

Guenter Roeck
2025-01-05T16:42:09Z

On 1/3/25 11:58, Rob Herring wrote:
> 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.
>

That is a mis-quote, really. I was talking about target-rpm. The property
to be added here is default-pwm, which is completely different.

> 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'.
>

That won't work. Literally each individual fan (even the same model)
runs at a different rpm for a given pwm value, and I am sure the rpm
for a given pwm input changes depending on fan age and temperature.

Quoting from the NCT6796D-S datasheet, for an example:

"The default duty cycle is 7Fh, or 50% for SYSFANOUT, CPUFANOUT, AUXFANOUT0, AUXFANOUT1,
AUXFANOUT2, AUXFANOUT3 and AUXFANOUT4.
Note.The default speed of fan output is specified in registers CR[E0h] ~ CR[E4h] and CR[E7h] of Logical Device B,
and the AUXFANOUT4 default speed of fan output is specified in register CR[E3h] of Logical Device D.
"

Note that the term "default speed" as used in the datasheet refers to pwm.
Again, from the datasheet:

CR E3h. AUXFAN4 Duty Cycle Register
^^^^^^^^^^^^^^^^^^^

Setting the default value for such a register is what we are talking about here.
No rpm involved, just default pwm values. This is completely independent of rpm.

Guenter