RE: [RFC PATCH 3/6] dt-bindings: axi-fan-control: add tacho properties

-----Original Message-----
From: Guenter Roeck <redacted> On Behalf Of Guenter
Roeck
Sent: Friday, July 16, 2021 5:04 PM
To: Sa, Nuno <Nuno.Sa@analog.com>
Cc: Rob Herring <robh@kernel.org>; linux-hwmon@vger.kernel.org;
devicetree@vger.kernel.org; Jean Delvare [off-list ref]
Subject: Re: [RFC PATCH 3/6] dt-bindings: axi-fan-control: add tacho
properties

[External]

On 7/16/21 12:44 AM, Sa, Nuno wrote:
[ ... ]

quoted

quoted

Are you sure you can ever get this stable ? Each fan has its own
properties
and tolerances. If you replace a fan in a given system, you might get
different RPM numbers. The RPM will differ widely from system to
system
and from fan to fan. Anything that assumes a specific RPM in
devicetree
data seems to be quite vulnerable to failures. I have experienced

that

quoted

quoted

recently with a different chip which also tries to correlate RPM and
PWM
and fails quite miserably.

In my experience, anything other than minimum fan speed is really

a

quoted

quoted

recipe
for instability and sporadic false failures. Even setting a minimum

fan

quoted

quoted

speed
is tricky because it depends a lot on the fan.

I see what you mean. So, I had to go through this process when

testing

quoted

this changes because the fan I'm using is different from the default

one

quoted

used to develop and stablish the default values in the IP core. The

core

Exactly my point.

quoted

provides you with a register which contains the tacho measurements

in

quoted

clock cycles. You can read that for all the PWM points of interest
(with devmem2 for example) and make your own "calibration". I

assume

quoted

that people have to go through this process before putting some

values

quoted

in the devicetree. I'm aware this is not the neatest process but I

guess it's

quoted

acceptable...

Do you really expect everyone using a system with this chip to go
through
this process and update its devicetree configuration, and then repeat it
whenever a fan is changed ? Given how dynamic this is, I really wonder
if that information should be in devicetree in the first place.

My naive assumption was that we would only do this work at evaluation
time. After that and after we settled with a fan for some system, I expected
that changing to a different fan is not that likely. From your inputs, I guess
this is not really the case which makes this process more cumbersome (as it
also implies recompiling the devicetree for your system).

However, even if we export these as runtime parameters, services/daemons
will also have an hard time doing this "calibration" in a dynamic way. The reason
is because the way the controller works is that it only accepts a new PWM
request if it is an higher value than whatever the HW has at that moment. Thus,
going through the calibration points might be very cumbersome. I can see some
ways of handling this though but not very neat...

Since this is a FPGA core, we might have some flexibility here. Something that
came to my mind would be to have a calibration mode in the HW that would
allow us to freely control the PWM values. In that way we could go freely over
the calibration points. I guess, for safety reasons, this calibration mode would
expire after some reasonable time (that give us enough time for doing the whole
thing). The best place for doing the calibration, I guess it would be directly in the
driver since we do receive the interrupts about new tacho measurements making
things easier to sync and handle. However, given the time that takes for a new
PWM to settle + new tacho measurements, it would not be very acceptable to do this 
during probe which is definitely also not ideal (we could defer this to a worker/timer).

I'm not sure if the above makes much sense to you and it also depends on the HW
guys being on board with this mechanism.

- Nuno Sá