| 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: <481b00b4-c3b7-49c4-80fe-0c0fb8448673@amperemail.onmicrosoft.com>
Date: Wed, 14 Aug 2024 14:31:43 +0700
From: Chanh Nguyen <chanh@...eremail.onmicrosoft.com>
To: Conor Dooley <conor@...nel.org>, 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 13/08/2024 23:16, Conor Dooley wrote:
> 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!
>
Thank Guenter and Conor for your comments!
I will add me to maintainers list. I'm going to push the patch v4 with
this update soon. It will be as below:
maintainers:
- Guenter Roeck <linux@...ck-us.net>
- Chanh Nguyen <chanh@...amperecomputing.com>
I think I can support to reviewing any max31790 binding update later.
Thanks,
Chanh
>>
>>>> +
>>>> +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
>>>>
>>
Powered by blists - more mailing lists