> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter > Roeck > Sent: Saturday, July 17, 2021 7:23 PM > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > Cc: linux-hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Rob > Herring <robh+dt@xxxxxxxxxx>; Jean Delvare <jdelvare@xxxxxxxx> > 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: > > The HW has some predefined points where it will associate a PWM > value. > > 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 > > 0%; > > * pwm1_auto_point2_temp: temperature threshold above which > PWM should be > > 25%; > > * pwm1_auto_point3_temp: temperature threshold below which > PWM should be > > 25%; > > * pwm1_auto_point4_temp: temperature threshold above which > PWM should be > > 50%; > > * pwm1_auto_point5_temp: temperature threshold below which > PWM should be > > 50%; > > * pwm1_auto_point6_temp: temperature threshold above which > PWM should be > > 75%; > > * pwm1_auto_point7_temp: temperature threshold below which > PWM should be > > 75%; > > * pwm1_auto_point8_temp: temperature threshold above which > PWM should be > > 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á
