linux-kernel - Re: [PATCH 1/2] dt-bindings: hwmon: Add binding for max6639

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: <55a3f673-c204-f2b1-22bc-bf311a505adf@roeck-us.net>
Date:   Wed, 5 Oct 2022 08:45:42 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Naresh Solanki <naresh.solanki@...ements.com>,
        Rob Herring <robh@...nel.org>
Cc:     devicetree@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Roland Stigge <stigge@...com.de>, linux-kernel@...r.kernel.org,
        linux-hwmon@...r.kernel.org,
        Patrick Rudolph <patrick.rudolph@...ements.com>,
        Marcello Sylvester Bauer <sylv@...v.io>
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: Add binding for max6639

On 10/5/22 06:16, Naresh Solanki wrote:
> Hi Rob,
> 
> I can add common fan bindings.
> Below are the properties that I can think of:
> max-rpm, pulse-per-revolution
> Let me know what you think.
> 

For max6650, we have:

     maxim,fan-microvolt = <12000000>;
     maxim,fan-prescale = <4>;
     maxim,fan-target-rpm = <1200>;

The voltage can be handled with a regulator, so that won't be necessary.
Prescale (fan divider) values and target rpm seem relevant, though.
The sysfs ABI also permits a minimum rpm which also seems relevant
(lower speeds would indicate fan failures).

Then there are pwm specific parameters for pwm controlled fans.
     pwm polarity
     pwm frequency
     pwm mode (direct current or pulse-width modulation)

Guenter

> Regards,
> Naresh Solanki
> 
> 
> 
> On Fri, 30 Sept 2022 at 01:02, Rob Herring <robh@...nel.org> wrote:
>>
>> On Thu, Sep 22, 2022 at 07:07:17AM +0200, Naresh Solanki wrote:
>>> From: Marcello Sylvester Bauer <sylv@...v.io>
>>>
>>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>>> monitor with PWM fan-speed controller.
>>>
>>> Signed-off-by: Marcello Sylvester Bauer <sylv@...v.io>
>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@...ements.com>
>>> ---
>>>   .../bindings/hwmon/maxim,max6639.yaml         | 112 ++++++++++++++++++
>>>   1 file changed, 112 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>> new file mode 100644
>>> index 000000000000..c845fb989af2
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>> @@ -0,0 +1,112 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +
>>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Maxim max6639
>>> +
>>> +maintainers:
>>> +  - Roland Stigge <stigge@...com.de>
>>> +
>>> +description: |
>>> +  The MAX6639 is a 2-channel temperature monitor with dual, automatic, PWM
>>> +  fan-speed controller.  It monitors its own temperature and one external
>>> +  diode-connected transistor or the temperatures of two external diode-connected
>>> +  transistors, typically available in CPUs, FPGAs, or GPUs.
>>> +
>>> +  Datasheets:
>>> +    https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - maxim,max6639
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - "fan@0"
>>> +  - "fan@1"
>>> +
>>> +additionalProperties: false
>>> +
>>> +patternProperties:
>>> +  "^fan@[0-1]$":
>>> +    type: object
>>> +    description: |
>>> +      Represents the two fans and their specific configuration.
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description: |
>>> +          The fan number.
>>
>> Addresses are a property of the parent (the fan controller), not the
>> fan.
>>
>>> +        items:
>>> +          minimum: 0
>>> +          maximum: 1
>>> +
>>> +      pwm-polarity:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        enum: [0, 1]
>>> +        default: 1
>>> +        description:
>>> +          PWM output is low at 100% duty cycle when this bit is set to zero. PWM
>>> +          output is high at 100% duty cycle when this bit is set to 1.
>>
>> IIRC, the PWM binding provides for this. The parent should probably be a
>> PWM provider.
>>
>>> +
>>> +      pulses-per-revolution:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        enum: [1, 2, 3, 4]
>>> +        default: 2
>>> +        description:
>>> +          Value specifying the number of pulses per revolution of the controlled
>>> +          FAN.
>>> +
>>> +      maxim,rpm-range:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        enum: [2000, 4000, 8000, 16000]
>>> +        default: 4000
>>> +        description:
>>> +          Scales the tachometer counter by setting the maximum (full-scale) value
>>> +          of the RPM range for max6639.
>>
>> Is this a property of the fan? How is this maxim specific?
>>
>>
>> The bigger issue here is we need a common fan binding. I'm not inclined
>> to accept any more fan controller bindings with fan properties until we
>> do.
>>
>> Rob