> From: Rob Herring <robh@xxxxxxxxxx> > Sent: Monday, July 12, 2021 7:27 PM > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > Cc: linux-hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > Guenter Roeck <linux@xxxxxxxxxxxx>; Jean Delvare > <jdelvare@xxxxxxxx> > Subject: Re: [RFC PATCH 3/6] dt-bindings: axi-fan-control: add tacho > properties > > [External] > > On Thu, Jul 08, 2021 at 02:01:08PM +0200, Nuno Sá wrote: > > Add the bindings for the tacho signal evaluation parameters which > depend > > on the FAN being used. > > > > Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx> > > --- > > .../bindings/hwmon/adi,axi-fan-control.yaml | 12 > ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,axi-fan- > control.yaml b/Documentation/devicetree/bindings/hwmon/adi,axi- > fan-control.yaml > > index 6747b870f297..0481eb34d9f1 100644 > > --- a/Documentation/devicetree/bindings/hwmon/adi,axi-fan- > control.yaml > > +++ b/Documentation/devicetree/bindings/hwmon/adi,axi-fan- > control.yaml > > @@ -37,6 +37,18 @@ properties: > > $ref: /schemas/types.yaml#/definitions/uint32 > > enum: [1, 2, 4] > > > > + adi,tacho-25-us: > > + description: Expected tacho signal when the PWM is at 25%. > > + > > + adi,tacho-50-us: > > + description: Expected tacho signal when the PWM is at 50%. > > + > > + adi,tacho-75-us: > > + description: Expected tacho signal when the PWM is at 75%. > > + > > + adi,tacho-100-us: > > + description: Expected tacho signal when the PWM is at 100%. > > This looks like it should be common. But having PWM percents in the > property names doesn't scale. This is also a property of the fan, not > the fan controller. Yes, I see that these parameters are definitely a property of the attached fan but the evaluation of these timings are very specific to this controller (I think). The way it works is that the HW can fully operate without any runtime SW configuration. In this case, it will use the values in these registers to evaluate the tacho signal coming from the FAN. And the HW really uses the evaluation points like this (0, 25%, 50% and 100%). It has some predefined values that work for the FAN that was used to develop the IP but naturally the evaluation will fail as soon as other FAN is attached (resulting in fan fault interrupts). And yes, writing these parameters is already SW configuration but what I mean with "runtime" is after probe :). So, I honestly do not know how we could name this better... Maybe a 'tacho-eval-points-us' array? The question would be the min/max elements? Do you have any suggestion for a more generic property? > There's only so many ways a fan can be controlled and I'm going to > keep > telling folks to make a common fan binding. There's some start to it, > but it needs some work. You mean the pwm-fan.txt? I gave a look to the driver and I don't think it fully fits this controller. I'm ok in sending an fan.yaml binding if you prefer it but I'm just not sure what we would need there... Maybe pulses-per-revolution and the above property (whatever the decided name) would be a starting point? - Nuno Sá
