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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221011193744.GA4051000-robh@kernel.org>
Date:   Tue, 11 Oct 2022 14:37:44 -0500
From:   Rob Herring <robh@...nel.org>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Naresh Solanki <naresh.solanki@...ements.com>,
        devicetree@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
        Patrick Rudolph <patrick.rudolph@...ements.com>
Subject: Re: [PATCH v2 1/3] dt-bindings: hwmon: fan: Add fan binding to schema

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