Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Document start from dead stop properties

[Marek Vasut <marex@denx.de>]

On 11/6/24 1:34 AM, Guenter Roeck wrote:
> On 11/5/24 10:53, Marek Vasut wrote:
>> On 11/5/24 3:11 PM, Guenter Roeck wrote:
>>> On 11/5/24 05:52, Marek Vasut wrote:
>>>> Delta AFC0612DB-F00 fan has to be set to at least 30% PWM duty cycle
>>>> to spin up from a dead stop, and can be afterward throttled down to
>>>> lower PWM duty cycle. Introduce support for operating such fans which
>>>
>>> Doesn't this imply that a minimum pwm value is needed as well ?
>>
>> It depends. For this fan, yes, it does stop at around 8% PWM duty cycle.
>>
>>> Super-IO chips such as the NCT67xx series typically have two separate
>>> registers, one for the pwm start value and one for the minimum pwm 
>>> value.
>>
>> I use plain SoC PWM output to operate the fan. This one needs to be 
>> set to higher PWM duty cycle first, to spin up, and can be reduced to 
>> lower PWM duty cycle afterward without stopping.
>>
> 
> Yes, exactly. That is what many fans require.
> 
>>>> need to start at higher PWM duty cycle first and can slow down next.
>>>>
>>>> Document two new DT properties, "fan-dead-stop-start-percent" and
>>>> "fan-dead-stop-start-usec". The former describes the minimum percent
>>>> of fan RPM at which it will surely spin up from dead stop. This value
>>>> can be found in the fan datasheet and can be converted to PWM duty
>>>> cycle easily. The "fan-dead-stop-start-usec" describes the minimum
>>>> time in microseconds for which the fan has to be set to dead stop
>>>> start RPM for the fan to surely spin up.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>>> Cc: Jean Delvare <jdelvare@suse.com>
>>>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: devicetree@vger.kernel.org
>>>> Cc: linux-hwmon@vger.kernel.org
>>>> ---
>>>>   Documentation/devicetree/bindings/hwmon/pwm-fan.yaml | 11 +++++++++++
>>>>   1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/ 
>>>> Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>>>> index 4e5abf7580cc6..f1042471b5176 100644
>>>> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>>>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>>>> @@ -31,6 +31,17 @@ properties:
>>>>         it must be self resetting edge interrupts.
>>>>       maxItems: 1
>>>> +  fan-dead-stop-start-percent:
>>>
>>> Personally I don't think that "dead-stop" in the property name adds 
>>> any value.
>>> On the contrary, I think it leads to confusion. I head to read the 
>>> description
>>> to understand.
>>
>> The documentation refers to this behavior as a "dead stop" , hence the 
>> property name. I can change it to fan-stop-to-start-percent ?
> 
> I do not understand the need for that much complexity in the property name,
> and I don't think it makes sense to name a property based on a specific
> chip documentation. I have seen that before, where different vendors use
> different names for the same functionality. That doesn't mean that the
> vendor-determined name has to make it into the property name.
> 
> As an example, Nuvoton calls the values "Start-Up Value" and "Stop Value".
> ITE calls the start value "start PWM value" (and as far as I can see 
> doesn't
> have a separate stop value). I am sure pretty much every vendor uses a
> different description.
> 
> I am personally not a friend of long property names. Having said that,
> I'll let you use whatever DT maintainers accept. They may have a different
> opinion.
Do you have a different suggestion for the property name ? Else I'll 
just send a V2 .