RE: [RFC PATCH 6/6] hwmon: axi-fan-control: support temperature vs pwm points

From: Guenter Roeck <redacted> On Behalf Of Guenter
Roeck
Sent: Saturday, July 17, 2021 7:23 PM
To: Sa, Nuno <Nuno.Sa@analog.com>
Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; Rob
Herring [off-list ref]; Jean Delvare [off-list ref]
Subject: Re: [RFC PATCH 6/6] hwmon: axi-fan-control: support
temperature vs pwm points

[External]

On Thu, Jul 08, 2021 at 02:01:11PM +0200, Nuno Sá wrote:

quoted

The HW has some predefined points where it will associate a PWM

value.

quoted

However some users might want to better set these points to their
usecases. This patch exposes these points as pwm auto_points:

 * pwm1_auto_point1_temp: temperature threshold below which

PWM should be

quoted

   0%;
 * pwm1_auto_point2_temp: temperature threshold above which

PWM should be

quoted

   25%;
 * pwm1_auto_point3_temp: temperature threshold below which

PWM should be

quoted

   25%;
 * pwm1_auto_point4_temp: temperature threshold above which

PWM should be

quoted

   50%;
 * pwm1_auto_point5_temp: temperature threshold below which

PWM should be

quoted

   50%;
 * pwm1_auto_point6_temp: temperature threshold above which

PWM should be

quoted

   75%;
 * pwm1_auto_point7_temp: temperature threshold below which

PWM should be

quoted

   75%;
 * pwm1_auto_point8_temp: temperature threshold above which

PWM should be

quoted

   100%;

If I understand those correctly, half of those are really hysteresis
points.
I think it would be better to express this with
	pwm1_auto_pointX_temp
	pwm1_auto_pointX_temp_hyst

where the hysteresis point is the temperature where the previous
pwm value
is activated. In other words, change attribute names as follows:
    for 25%:
	pwm1_auto_point1_temp -> pwm1_auto_point1_temp_hyst
	pwm1_auto_point2_temp -> pwm1_auto_point1_temp
    for 50%:
	pwm1_auto_point3_temp -> pwm1_auto_point2_temp_hyst
	pwm1_auto_point4_temp -> pwm1_auto_point2_temp
    for 75%:
	pwm1_auto_point5_temp -> pwm1_auto_point3_temp_hyst
	pwm1_auto_point6_temp -> pwm1_auto_point3_temp
    for 100%:
	pwm1_auto_point7_temp -> pwm1_auto_point4_temp_hyst
	pwm1_auto_point8_temp -> pwm1_auto_point4_temp

Agree, it makes sense.

- Nuno Sá