Re: [PATCH v2 1/3] dt-bindings: hwmon: fan: Add fan binding to schema

[Rob Herring <robh@kernel.org>]

On Tue, Oct 11, 2022 at 09:46:08AM -0700, Guenter Roeck wrote:
> On 10/11/22 09:12, Naresh Solanki wrote:
> > Hi Guenter,
> > 
> > fan-common is intended for fan properties. i.e., those derived from
> > fan datasheets.
> > For min-rpm, some fans have minimum non zero rpm like 1900rpm below which
> > the fan cannot run.
> > 
> 
> I would argue the properties are for fan controllers, not for fans.
> Fans don't have or depend on specific pwm frequencies. Fan controllers
> do.

Presumably fan controllers can produce a range of frequencies. If they 
need a specific frequency, then why are they programmable? Something 
outside the fan controller must have the constraint.

> Fans don't have a configurable pwm polarity. Fan controllers do,
> to match the hardware on a board.

We don't model an inverter, so it's got to go somewhere.

> Fans don't have or rely on
> a target rpm. That is a system property, configured into the
> fan controller. And so on.

Yes, but it is per fan. per fan properties/settings should go in the fan 
node IMO.

> If this is for fans, we'll need another set of properties for
> fan controllers. The driver for max6639, being a fan controller,
> would need to implement those properties.
> 
> Also, as implemented in the MAX6639, max-rpm is the fan's
> rpm range, not the actual rpm. Your implementation is just
> confusing, including the example in the bindings. Valid values
> should be what the chip accepts, not some random value.

A fan would have some design maximum RPM depending on its mechanical 
design and lifetime requirements. A controller would have some maximum 
in terms of electrical pulse frequency or register bit sizes (depending 
how RPM or pulse counts are exposed to s/w. That should all be implied 
from the controller part and not in DT. It's the mechanical limits of 
the fan we should be defining here.

> Really, I don't understand where you are going with this.

Certainly it needs more thought for different cases.

Rob