| 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
| ||
|
Message-ID: <20240813-extruding-unfunded-0e14a5c161e1@spud>
Date: Tue, 13 Aug 2024 17:16:38 +0100
From: Conor Dooley <conor@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Chanh Nguyen <chanh@...amperecomputing.com>,
Jean Delvare <jdelvare@...e.com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Justin Ledford <justinledford@...gle.com>,
devicetree@...r.kernel.org, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org,
OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
Open Source Submission <patches@...erecomputing.com>,
Phong Vo <phong@...amperecomputing.com>,
Thang Nguyen <thang@...amperecomputing.com>,
Quan Nguyen <quan@...amperecomputing.com>
Subject: Re: [PATCH v3 1/1] dt-bindings: hwmon: Add maxim max31790
On Tue, Aug 13, 2024 at 08:52:22AM -0700, Guenter Roeck wrote:
> On 8/13/24 08:33, Conor Dooley wrote:
> > On Tue, Aug 13, 2024 at 08:41:52AM +0000, Chanh Nguyen wrote:
> > > Add device tree bindings and an example for max31790 device.
> > >
> > > Signed-off-by: Chanh Nguyen <chanh@...amperecomputing.com>
> > > ---
> > > Changes in v2:
> > > - Update filename of the maxim,max31790.yaml [Krzysztof]
> > > - Add the common fan schema to $ref [Krzysztof]
> > > - Update the node name to "fan-controller" in maxim,max31790.yaml [Krzysztof]
> > > - Drop "driver" in commit title [Krzysztof]
> > > Changes in v3:
> > > - Drop redundant "bindings" in commit title [Krzysztof]
> > > - Add the clocks and resets property in example [Krzysztof]
> > > - Add child node refer to fan-common.yaml [Krzysztof, Conor]
> > > ---
> > > .../bindings/hwmon/maxim,max31790.yaml | 81 +++++++++++++++++++
> > > 1 file changed, 81 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
> > > new file mode 100644
> > > index 000000000000..d28a6373edd3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/maxim,max31790.yaml
> > > @@ -0,0 +1,81 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/maxim,max31790.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: The Maxim MAX31790 Fan Controller
> > > +
> > > +maintainers:
> > > + - Guenter Roeck <linux@...ck-us.net>
> >
> > Why Guenter and not you?
> >
>
> Fine with me, actually. We don't expect individual driver maintainers
> in the hardware monitoring subsystem, and this chip doesn't have an
> explicit maintainer. Forcing people to act as maintainer for .yaml
> files they submit can only result in fewer submissions. I prefer to be
> listed as maintainer over having no devicetree bindings.
Sure, if you're happy with it. Having someone that knows about how the
particular model works is usually preferred however!
>
> > > +
> > > +description: >
> > > + The MAX31790 controls the speeds of up to six fans using six
> > > + independent PWM outputs. The desired fan speeds (or PWM duty cycles)
> > > + are written through the I2C interface.
> > > +
> > > + Datasheets:
> > > + https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf
> > > +
> > > +properties:
> > > + compatible:
> > > + const: maxim,max31790
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + resets:
> > > + maxItems: 1
> > > +
> > > + "#pwm-cells":
> > > + const: 1
> > > +
> > > +patternProperties:
> > > + "^fan-[0-9]+$":
> > > + $ref: fan-common.yaml#
> > > + unevaluatedProperties: false
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + i2c {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + fan-controller@21 {
> > > + compatible = "maxim,max31790";
> > > + reg = <0x21>;
> > > + clocks = <&sys_clk>;
> > > + resets = <&reset 0>;
> > > + };
> > > + };
> >
> > What does this example demonstrate? The one below seems useful, this one
> > I don't quite understand - what's the point of a fan controller with no
> > fans connected to it? What am I missing?
> >
>
> Just guessing, but maybe this is supposed to reflect a system which only monitors fan
> speeds but does not implement fan control.
Even without any control, I would expect to see fan-# child nodes, just
no pwms property in them. Without the child nodes, how does software
determine which fan is being monitored by which channel?
Cheers,
Conor.
> > > + - |
> > > + i2c {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + pwm_provider: fan-controller@20 {
> > > + compatible = "maxim,max31790";
> > > + reg = <0x20>;
> > > + clocks = <&sys_clk>;
> > > + resets = <&reset 0>;
> > > + #pwm-cells = <1>;
> > > +
> > > + fan-0 {
> > > + pwms = <&pwm_provider 1>;
> > > + };
> > > +
> > > + fan-1 {
> > > + pwms = <&pwm_provider 2>;
> > > + };
> > > + };
> > > + };
> > > +
> > > --
> > > 2.43.0
> > >
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists