[PATCH 0/2] Add max6639 regulator and devicetree support

[PATCH 0/2] Add max6639 regulator and devicetree support

[Naresh Solanki]

These patches add devicetree support for MAX6639.

Changes:
- add dt-binding
- add max6639 specific property i.e., maxim,rpm-range

Marcello Sylvester Bauer (2):
  dt-bindings: hwmon: Add binding for max6639
  hwmon: (max6639) Change from pdata to dt configuration

 .../bindings/hwmon/maxim,max6639.yaml         | 112 ++++++++++++
 drivers/hwmon/max6639.c                       | 162 +++++++++++++-----
 include/linux/platform_data/max6639.h         |  15 --
 3 files changed, 231 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
 delete mode 100644 include/linux/platform_data/max6639.h


base-commit: 9f6ca00b5be47e471e9703e6b017e1e2cfa7f604
-- 
2.35.3

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

[Naresh Solanki]

From: Marcello Sylvester Bauer <sylv@sylv.io>

Add Devicetree binding documentation for Maxim MAX6639 temperature
monitor with PWM fan-speed controller.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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.
+        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.
+
+      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.
+
+    required:
+      - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max6639@10 {
+        compatible = "maxim,max6639";
+        reg = <0x10>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        fan@0 {
+          reg = <0x0>;
+          pwm-polarity = <1>;
+          pulses-per-revolution = <2>;
+          maxim,rpm-range = <4000>;
+        };
+
+        fan@1 {
+          reg = <0x1>;
+          pwm-polarity = <1>;
+          pulses-per-revolution = <2>;
+          maxim,rpm-range = <4000>;
+        };
+      };
+    };
+...
-- 
2.35.3

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

[Krzysztof Kozlowski]

On 22/09/2022 07:07, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <sylv@sylv.io>
> 
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.de>

Ack from Roland is needed here.

> +
> +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:

This goes after properties.

> +  "^fan@[0-1]$":
> +    type: object
> +    description: |
> +      Represents the two fans and their specific configuration.
> +
> +    properties:
> +      reg:
> +        description: |
> +          The fan number.
> +        items:

Skip items.

> +          minimum: 0
> +          maximum: 1
> +
> +      pwm-polarity:

Why is this property of fan, not of PWM source?

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1]

type: boolean

> +        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.
> +
> +      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.
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      max6639@10 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +        compatible = "maxim,max6639";
> +        reg = <0x10>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        fan@0 {
> +          reg = <0x0>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +
> +        fan@1 {
> +          reg = <0x1>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +      };
> +    };
> +...

Best regards,
Krzysztof

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

[Guenter Roeck]

On 9/21/22 23:34, Krzysztof Kozlowski wrote:
> On 22/09/2022 07:07, Naresh Solanki wrote:
>> From: Marcello Sylvester Bauer <sylv@sylv.io>
>>
>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>> monitor with PWM fan-speed controller.
>>
>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.de>
> 
> Ack from Roland is needed here.
> 
>> +
>> +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:
> 
> This goes after properties.
> 
>> +  "^fan@[0-1]$":
>> +    type: object
>> +    description: |
>> +      Represents the two fans and their specific configuration.
>> +
>> +    properties:
>> +      reg:
>> +        description: |
>> +          The fan number.
>> +        items:
> 
> Skip items.
> 
>> +          minimum: 0
>> +          maximum: 1
>> +
>> +      pwm-polarity:
> 
> Why is this property of fan, not of PWM source?
> 

The chip provides pwm output to the fan. That typical for fan
controller chips. Typically they also have options to configure
the pwm frequency and either pwm or DC output (max6639 does not
have the latter option).

Guenter

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

[Naresh Solanki]

Thanks Guenter, to summarize below properties that can be configured
based on input from
fan datasheet:
pwm controlled fan:
1.  max-rpm : Defines maximum rpm the fan supports
2.  tach-pulse/pulse-per-revolution( also referred as prescale) :
Determines the number of
 pulses received from the sensor/tach pin of fan per revolution.
3.  rpm-init (also ref. as fan-target-rpm): Determines the fan rpm
driver needs to configure
during probe.
4. pwm-input-polarity(normal or inverse) : Specifies fan pwm input
polarity as specified in
 fan datasheet. a fan controller driver can use this data to configure
pwm output polarity
accordingly during driver probe.

For a DC output controlled fan, voltage across fan is controlled to control rpm.
Below is what I can think of:
5. voltage-range : Specifies valid voltage range for rpm control
example for 3V to 5V range:
     voltage-range=<3000 5000>;

Above mentioned properties are within the scope of fan.
Fan controllers like max6639/max6650 etc can have additional
properties specific to the
feather supported by the chip.

Let me know if I can go ahead with this.

Thanks,
Naresh Solanki

Regards,
Naresh Solanki



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email:  naresh.solanki@9elements.com
Mobile:  +91 9538631477

Sitz der Gesellschaft: Bochum
Handelsregister: Amtsgericht Bochum, HRB 17519
Geschäftsführung: Sebastian Deutsch, Eray Basar

Datenschutzhinweise nach Art. 13 DSGVO


On Thu, 6 Oct 2022 at 00:10, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 9/21/22 23:34, Krzysztof Kozlowski wrote:
> > On 22/09/2022 07:07, Naresh Solanki wrote:
> >> From: Marcello Sylvester Bauer <sylv@sylv.io>
> >>
> >> Add Devicetree binding documentation for Maxim MAX6639 temperature
> >> monitor with PWM fan-speed controller.
> >>
> >> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.de>
> >
> > Ack from Roland is needed here.
> >
> >> +
> >> +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:
> >
> > This goes after properties.
> >
> >> +  "^fan@[0-1]$":
> >> +    type: object
> >> +    description: |
> >> +      Represents the two fans and their specific configuration.
> >> +
> >> +    properties:
> >> +      reg:
> >> +        description: |
> >> +          The fan number.
> >> +        items:
> >
> > Skip items.
> >
> >> +          minimum: 0
> >> +          maximum: 1
> >> +
> >> +      pwm-polarity:
> >
> > Why is this property of fan, not of PWM source?
> >
>
> The chip provides pwm output to the fan. That typical for fan
> controller chips. Typically they also have options to configure
> the pwm frequency and either pwm or DC output (max6639 does not
> have the latter option).
>
> Guenter

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

[Guenter Roeck]

On 10/5/22 23:30, Naresh Solanki wrote:
> Thanks Guenter, to summarize below properties that can be configured
> based on input from
> fan datasheet:
> pwm controlled fan:
> 1.  max-rpm : Defines maximum rpm the fan supports
> 2.  tach-pulse/pulse-per-revolution( also referred as prescale) :
> Determines the number of
>   pulses received from the sensor/tach pin of fan per revolution.

There are two different parameters: pulse-per-revolution is the number
of pulses per revolution from the fan. Then there is a chip internal
divider, used to ensure that the chip fan counter does not exceed
its limits. Both are needed and orthogonal.

 From the max6650 description:

- maxim,fan-prescale  : Pre-scaling value, as per datasheet [1]. Lower values
                         allow more fine-grained control of slower fans.
                         Valid: 1, 2, 4, 8, 16.

This has nothing to do with pulse-per-revolution (there are no fans
with 16 pulses per revolution). The range for pulse-per-revolution
is typically 1..4.

Also see /sys/class/hwmon/hwmonX/fanY_pulses and /sys/class/hwmon/hwmonX/fanY_div
in Documentation/ABI/testing/sysfs-class-hwmon                                                                                                                                                            <216,37>-46 24%.

> 3.  rpm-init (also ref. as fan-target-rpm): Determines the fan rpm
> driver needs to configure
> during probe.

This has nothing to do with an "init" value. It is the fan target rpm.
Also see /sys/class/hwmon/hwmonX/fanY_target.

> 4. pwm-input-polarity(normal or inverse) : Specifies fan pwm input
> polarity as specified in
>   fan datasheet. a fan controller driver can use this data to configure
> pwm output polarity

This has nothing to do with fan requirements. It is the output polarity
from the chip, period. There can be an inverter between pwm output and the
fan input, which is the most likely reason why the output has to be inverted.

> accordingly during driver probe.
> 
> For a DC output controlled fan, voltage across fan is controlled to control rpm.
> Below is what I can think of:
> 5. voltage-range : Specifies valid voltage range for rpm control
> example for 3V to 5V range:
>       voltage-range=<3000 5000>;

I never heard about that option. Usually fan controllers convert the provided
or calculated pwm value into a voltage if DC mode is configured. What may be
needed is the fan supply voltage, which can be provided by a regulator.

What is also needed is a means to switch the fan controller between
DC mode and PWM mode (matching /sys/class/hwmon/hwmonX/pwmY_mode).

I would also expect the ability to set the pwm frequency (matching
/sys/class/hwmon/hwmonX/pwmY_freq).

Thanks,
Guenter

> 
> Above mentioned properties are within the scope of fan.
> Fan controllers like max6639/max6650 etc can have additional
> properties specific to the
> feather supported by the chip.
> 
> Let me know if I can go ahead with this.
> 
> Thanks,
> Naresh Solanki
> 
> Regards,
> Naresh Solanki
> 
> 
> 
> 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
> Email:  naresh.solanki@9elements.com
> Mobile:  +91 9538631477
> 
> Sitz der Gesellschaft: Bochum
> Handelsregister: Amtsgericht Bochum, HRB 17519
> Geschäftsführung: Sebastian Deutsch, Eray Basar
> 
> Datenschutzhinweise nach Art. 13 DSGVO
> 
> 
> On Thu, 6 Oct 2022 at 00:10, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 9/21/22 23:34, Krzysztof Kozlowski wrote:
>>> On 22/09/2022 07:07, Naresh Solanki wrote:
>>>> From: Marcello Sylvester Bauer <sylv@sylv.io>
>>>>
>>>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>>>> monitor with PWM fan-speed controller.
>>>>
>>>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.de>
>>>
>>> Ack from Roland is needed here.
>>>
>>>> +
>>>> +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:
>>>
>>> This goes after properties.
>>>
>>>> +  "^fan@[0-1]$":
>>>> +    type: object
>>>> +    description: |
>>>> +      Represents the two fans and their specific configuration.
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        description: |
>>>> +          The fan number.
>>>> +        items:
>>>
>>> Skip items.
>>>
>>>> +          minimum: 0
>>>> +          maximum: 1
>>>> +
>>>> +      pwm-polarity:
>>>
>>> Why is this property of fan, not of PWM source?
>>>
>>
>> The chip provides pwm output to the fan. That typical for fan
>> controller chips. Typically they also have options to configure
>> the pwm frequency and either pwm or DC output (max6639 does not
>> have the latter option).
>>
>> Guenter

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

[Patrick Rudolph]

On Thu, Sep 22, 2022 at 7:07 AM Naresh Solanki
<naresh.solanki@9elements.com> wrote:
>
> From: Marcello Sylvester Bauer <sylv@sylv.io>
>
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.
>
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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"
Why are both fan children required? The driver doesn't care and uses
the default settings if not present.

> +
> +additionalProperties: false
> +
> +patternProperties:
> +  "^fan@[0-1]$":
> +    type: object
> +    description: |
> +      Represents the two fans and their specific configuration.
> +
> +    properties:
> +      reg:
> +        description: |
> +          The fan number.
> +        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.
> +
> +      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.
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      max6639@10 {
> +        compatible = "maxim,max6639";
> +        reg = <0x10>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        fan@0 {
> +          reg = <0x0>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +
> +        fan@1 {
> +          reg = <0x1>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +      };
> +    };
> +...
> --
> 2.35.3
>

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

[Rob Herring]

On Thu, Sep 22, 2022 at 07:07:17AM +0200, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <sylv@sylv.io>
> 
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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

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

[Naresh Solanki]

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.

Regards,
Naresh Solanki



On Fri, 30 Sept 2022 at 01:02, Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Sep 22, 2022 at 07:07:17AM +0200, Naresh Solanki wrote:
> > From: Marcello Sylvester Bauer <sylv@sylv.io>
> >
> > Add Devicetree binding documentation for Maxim MAX6639 temperature
> > monitor with PWM fan-speed controller.
> >
> > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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

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

[Guenter Roeck]

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@kernel.org> wrote:
>>
>> On Thu, Sep 22, 2022 at 07:07:17AM +0200, Naresh Solanki wrote:
>>> From: Marcello Sylvester Bauer <sylv@sylv.io>
>>>
>>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>>> monitor with PWM fan-speed controller.
>>>
>>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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

[PATCH 2/2] hwmon: (max6639) Change from pdata to dt configuration

[Naresh Solanki]

From: Marcello Sylvester Bauer <sylv@sylv.io>

max6639_platform_data is not used by any in-kernel driver and does not
address the MAX6639 channels separately. Move to device tree
configuration with explicit properties to configure each channel.

Non-DT platform can still use this module with its default
configuration.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/hwmon/max6639.c               | 162 +++++++++++++++++++-------
 include/linux/platform_data/max6639.h |  15 ---
 2 files changed, 119 insertions(+), 58 deletions(-)
 delete mode 100644 include/linux/platform_data/max6639.h

diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
index 14bb7726f8d7..4e8c3d3ec4fd 100644
--- a/drivers/hwmon/max6639.c
+++ b/drivers/hwmon/max6639.c
@@ -19,7 +19,6 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
-#include <linux/platform_data/max6639.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
@@ -85,8 +84,9 @@ struct max6639_data {
 	u8 temp_ot[2];		/* OT Temperature, 0..255 C (->_emergency) */
 
 	/* Register values initialized only once */
-	u8 ppr;			/* Pulses per rotation 0..3 for 1..4 ppr */
-	u8 rpm_range;		/* Index in above rpm_ranges table */
+	u8 ppr[2];		/* Pulses per rotation 0..3 for 1..4 ppr */
+	u8 rpm_range[2];	/* Index in above rpm_ranges table */
+	u8 pwm_polarity[2];	/* Fans PWM polarity, 0..1 */
 
 	/* Optional regulator for FAN supply */
 	struct regulator *reg;
@@ -319,7 +319,7 @@ static ssize_t fan_input_show(struct device *dev,
 		return PTR_ERR(data);
 
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index],
-		       data->rpm_range));
+		       data->rpm_range[attr->index]));
 }
 
 static ssize_t alarm_show(struct device *dev,
@@ -386,28 +386,26 @@ static struct attribute *max6639_attrs[] = {
 ATTRIBUTE_GROUPS(max6639);
 
 /*
- *  returns respective index in rpm_ranges table
- *  1 by default on invalid range
+ *  Get respective index in rpm_ranges table
  */
-static int rpm_range_to_reg(int range)
+static int rpm_range_to_index(u8 *index, int range)
 {
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) {
-		if (rpm_ranges[i] == range)
-			return i;
+		if (rpm_ranges[i] == range) {
+			*index = i;
+			return 0;
+		}
 	}
 
-	return 1; /* default: 4000 RPM */
+	return -EINVAL;
 }
 
 static int max6639_init_client(struct i2c_client *client,
 			       struct max6639_data *data)
 {
-	struct max6639_platform_data *max6639_info =
-		dev_get_platdata(&client->dev);
 	int i;
-	int rpm_range = 1; /* default: 4000 RPM */
 	int err;
 
 	/* Reset chip to default values, see below for GCONFIG setup */
@@ -416,43 +414,25 @@ static int max6639_init_client(struct i2c_client *client,
 	if (err)
 		goto exit;
 
-	/* Fans pulse per revolution is 2 by default */
-	if (max6639_info && max6639_info->ppr > 0 &&
-			max6639_info->ppr < 5)
-		data->ppr = max6639_info->ppr;
-	else
-		data->ppr = 2;
-	data->ppr -= 1;
-
-	if (max6639_info)
-		rpm_range = rpm_range_to_reg(max6639_info->rpm_range);
-	data->rpm_range = rpm_range;
-
 	for (i = 0; i < 2; i++) {
 
 		/* Set Fan pulse per revolution */
 		err = i2c_smbus_write_byte_data(client,
 				MAX6639_REG_FAN_PPR(i),
-				data->ppr << 6);
+				data->ppr[i] << 6);
 		if (err)
 			goto exit;
 
 		/* Fans config PWM, RPM */
 		err = i2c_smbus_write_byte_data(client,
 			MAX6639_REG_FAN_CONFIG1(i),
-			MAX6639_FAN_CONFIG1_PWM | rpm_range);
+			MAX6639_FAN_CONFIG1_PWM | data->rpm_range[i]);
 		if (err)
 			goto exit;
 
-		/* Fans PWM polarity high by default */
-		if (max6639_info && max6639_info->pwm_polarity == 0)
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x00);
-		else
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x02);
-		if (err)
-			goto exit;
+		/* Fans PWM polarity */
+		err = i2c_smbus_write_byte_data(client,
+			MAX6639_REG_FAN_CONFIG2a(i), data->pwm_polarity[i] ? 0x00 : 0x02);
 
 		/*
 		 * /THERM full speed enable,
@@ -464,10 +444,6 @@ static int max6639_init_client(struct i2c_client *client,
 		if (err)
 			goto exit;
 
-		/* Max. temp. 80C/90C/100C */
-		data->temp_therm[i] = 80;
-		data->temp_alert[i] = 90;
-		data->temp_ot[i] = 100;
 		err = i2c_smbus_write_byte_data(client,
 				MAX6639_REG_THERM_LIMIT(i),
 				data->temp_therm[i]);
@@ -483,8 +459,6 @@ static int max6639_init_client(struct i2c_client *client,
 		if (err)
 			goto exit;
 
-		/* PWM 120/120 (i.e. 100%) */
-		data->pwm[i] = 120;
 		err = i2c_smbus_write_byte_data(client,
 				MAX6639_REG_TARGTDUTY(i), data->pwm[i]);
 		if (err)
@@ -524,12 +498,87 @@ static void max6639_regulator_disable(void *data)
 	regulator_disable(data);
 }
 
+static int max6639_probe_child_from_dt(struct i2c_client *client,
+				      struct device_node *child,
+				      struct max6639_data *data)
+
+{
+	struct device *dev = &client->dev;
+	u32 i, val;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &i);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+
+	if (i >= 2) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(child, "pulses-per-revolution", &val);
+	if (!err) {
+		if (val < 0 || val > 5) {
+			dev_err(dev, "invalid pulses-per-revolution %d of %pOFn\n", val, child);
+			return -EINVAL;
+		}
+		data->ppr[i] = val;
+	}
+
+	err = of_property_read_u32(child, "maxim,rpm-range", &val);
+	if (!err) {
+		err = rpm_range_to_index(&data->rpm_range[i], val);
+		if (err) {
+			dev_err(dev, "invalid maxim,rpm-range %d of %pOFn\n", val, child);
+			return err;
+		}
+	}
+
+	err = of_property_read_u32(child, "pwm-polarity", &val);
+	if (!err) {
+		if (val > 1) {
+			dev_err(dev, "invalid pwm-polarity %d of %pOFn\n", val, child);
+			return -EINVAL;
+		}
+		data->pwm_polarity[i] = val;
+	}
+
+	return 0;
+}
+
+static int max6639_probe_from_dt(struct i2c_client *client, struct max6639_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	/* Compatible with non-DT platforms */
+	if (!np)
+		return 0;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "channel"))
+			continue;
+
+		err = max6639_probe_child_from_dt(client, child, data);
+		if (err) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
 static int max6639_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct max6639_data *data;
 	struct device *hwmon_dev;
-	int err;
+	int err, i;
 
 	data = devm_kzalloc(dev, sizeof(struct max6639_data), GFP_KERNEL);
 	if (!data)
@@ -560,6 +609,24 @@ static int max6639_probe(struct i2c_client *client)
 
 	mutex_init(&data->update_lock);
 
+	/* default values */
+	for (i = 0; i < 2; i++) {
+		/* 4000 RPM */
+		data->rpm_range[i] = 1;
+		data->ppr[i] = 2;
+		data->pwm_polarity[i] = 1;
+		/* Max. temp. 80C/90C/100C */
+		data->temp_therm[i] = 80;
+		data->temp_alert[i] = 90;
+		data->temp_ot[i] = 100;
+		/* PWM 120/120 (i.e. 100%) */
+		data->pwm[i] = 120;
+	}
+
+	err = max6639_probe_from_dt(client, data);
+	if (err)
+		return err;
+
 	/* Initialize the max6639 chip */
 	err = max6639_init_client(client, data);
 	if (err < 0)
@@ -618,6 +685,14 @@ static const struct i2c_device_id max6639_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, max6639_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id maxim_of_platform_match[] = {
+	{.compatible = "maxim,max6639"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, maxim_of_platform_match);
+#endif
+
 static SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
 
 static struct i2c_driver max6639_driver = {
@@ -625,6 +700,7 @@ static struct i2c_driver max6639_driver = {
 	.driver = {
 		   .name = "max6639",
 		   .pm = &max6639_pm_ops,
+		   .of_match_table = of_match_ptr(maxim_of_platform_match),
 		   },
 	.probe_new = max6639_probe,
 	.id_table = max6639_id,
diff --git a/include/linux/platform_data/max6639.h b/include/linux/platform_data/max6639.h
deleted file mode 100644
index 65bfdb4fdc15..000000000000
--- a/include/linux/platform_data/max6639.h
+++ /dev/null
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _LINUX_MAX6639_H
-#define _LINUX_MAX6639_H
-
-#include <linux/types.h>
-
-/* platform data for the MAX6639 temperature sensor and fan control */
-
-struct max6639_platform_data {
-	bool pwm_polarity;	/* Polarity low (0) or high (1, default) */
-	int ppr;		/* Pulses per rotation 1..4 (default == 2) */
-	int rpm_range;		/* 2000, 4000 (default), 8000 or 16000 */
-};
-
-#endif /* _LINUX_MAX6639_H */
-- 
2.35.3

Re: [PATCH 2/2] hwmon: (max6639) Change from pdata to dt configuration

[Patrick Rudolph]

On Thu, Sep 22, 2022 at 7:07 AM Naresh Solanki
<naresh.solanki@9elements.com> wrote:
>
> From: Marcello Sylvester Bauer <sylv@sylv.io>
>
> max6639_platform_data is not used by any in-kernel driver and does not
> address the MAX6639 channels separately. Move to device tree
> configuration with explicit properties to configure each channel.
>
> Non-DT platform can still use this module with its default
> configuration.
>
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>  drivers/hwmon/max6639.c               | 162 +++++++++++++++++++-------
>  include/linux/platform_data/max6639.h |  15 ---
>  2 files changed, 119 insertions(+), 58 deletions(-)
>  delete mode 100644 include/linux/platform_data/max6639.h
>
> diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
> index 14bb7726f8d7..4e8c3d3ec4fd 100644
> --- a/drivers/hwmon/max6639.c
> +++ b/drivers/hwmon/max6639.c
> @@ -19,7 +19,6 @@
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/err.h>
>  #include <linux/mutex.h>
> -#include <linux/platform_data/max6639.h>
>
>  /* Addresses to scan */
>  static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
> @@ -85,8 +84,9 @@ struct max6639_data {
>         u8 temp_ot[2];          /* OT Temperature, 0..255 C (->_emergency) */
>
>         /* Register values initialized only once */
> -       u8 ppr;                 /* Pulses per rotation 0..3 for 1..4 ppr */
> -       u8 rpm_range;           /* Index in above rpm_ranges table */
> +       u8 ppr[2];              /* Pulses per rotation 0..3 for 1..4 ppr */
> +       u8 rpm_range[2];        /* Index in above rpm_ranges table */
> +       u8 pwm_polarity[2];     /* Fans PWM polarity, 0..1 */
>
>         /* Optional regulator for FAN supply */
>         struct regulator *reg;
> @@ -319,7 +319,7 @@ static ssize_t fan_input_show(struct device *dev,
>                 return PTR_ERR(data);
>
>         return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index],
> -                      data->rpm_range));
> +                      data->rpm_range[attr->index]));
>  }
>
>  static ssize_t alarm_show(struct device *dev,
> @@ -386,28 +386,26 @@ static struct attribute *max6639_attrs[] = {
>  ATTRIBUTE_GROUPS(max6639);
>
>  /*
> - *  returns respective index in rpm_ranges table
> - *  1 by default on invalid range
> + *  Get respective index in rpm_ranges table
>   */
> -static int rpm_range_to_reg(int range)
> +static int rpm_range_to_index(u8 *index, int range)
>  {
>         int i;
>
>         for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) {
> -               if (rpm_ranges[i] == range)
> -                       return i;
> +               if (rpm_ranges[i] == range) {
> +                       *index = i;
> +                       return 0;
> +               }
>         }
>
> -       return 1; /* default: 4000 RPM */
> +       return -EINVAL;
>  }
>
>  static int max6639_init_client(struct i2c_client *client,
>                                struct max6639_data *data)
>  {
> -       struct max6639_platform_data *max6639_info =
> -               dev_get_platdata(&client->dev);
>         int i;
> -       int rpm_range = 1; /* default: 4000 RPM */
>         int err;
>
>         /* Reset chip to default values, see below for GCONFIG setup */
> @@ -416,43 +414,25 @@ static int max6639_init_client(struct i2c_client *client,
>         if (err)
>                 goto exit;
>
> -       /* Fans pulse per revolution is 2 by default */
> -       if (max6639_info && max6639_info->ppr > 0 &&
> -                       max6639_info->ppr < 5)
> -               data->ppr = max6639_info->ppr;
> -       else
> -               data->ppr = 2;
> -       data->ppr -= 1;
> -
> -       if (max6639_info)
> -               rpm_range = rpm_range_to_reg(max6639_info->rpm_range);
> -       data->rpm_range = rpm_range;
> -
>         for (i = 0; i < 2; i++) {
>
>                 /* Set Fan pulse per revolution */
>                 err = i2c_smbus_write_byte_data(client,
>                                 MAX6639_REG_FAN_PPR(i),
> -                               data->ppr << 6);
> +                               data->ppr[i] << 6);
>                 if (err)
>                         goto exit;
>
>                 /* Fans config PWM, RPM */
>                 err = i2c_smbus_write_byte_data(client,
>                         MAX6639_REG_FAN_CONFIG1(i),
> -                       MAX6639_FAN_CONFIG1_PWM | rpm_range);
> +                       MAX6639_FAN_CONFIG1_PWM | data->rpm_range[i]);
>                 if (err)
>                         goto exit;
>
> -               /* Fans PWM polarity high by default */
> -               if (max6639_info && max6639_info->pwm_polarity == 0)
> -                       err = i2c_smbus_write_byte_data(client,
> -                               MAX6639_REG_FAN_CONFIG2a(i), 0x00);
> -               else
> -                       err = i2c_smbus_write_byte_data(client,
> -                               MAX6639_REG_FAN_CONFIG2a(i), 0x02);
> -               if (err)
> -                       goto exit;
> +               /* Fans PWM polarity */
> +               err = i2c_smbus_write_byte_data(client,
> +                       MAX6639_REG_FAN_CONFIG2a(i), data->pwm_polarity[i] ? 0x00 : 0x02);
>
>                 /*
>                  * /THERM full speed enable,
> @@ -464,10 +444,6 @@ static int max6639_init_client(struct i2c_client *client,
>                 if (err)
>                         goto exit;
>
> -               /* Max. temp. 80C/90C/100C */
> -               data->temp_therm[i] = 80;
> -               data->temp_alert[i] = 90;
> -               data->temp_ot[i] = 100;
>                 err = i2c_smbus_write_byte_data(client,
>                                 MAX6639_REG_THERM_LIMIT(i),
>                                 data->temp_therm[i]);
> @@ -483,8 +459,6 @@ static int max6639_init_client(struct i2c_client *client,
>                 if (err)
>                         goto exit;
>
> -               /* PWM 120/120 (i.e. 100%) */
> -               data->pwm[i] = 120;
>                 err = i2c_smbus_write_byte_data(client,
>                                 MAX6639_REG_TARGTDUTY(i), data->pwm[i]);
>                 if (err)
> @@ -524,12 +498,87 @@ static void max6639_regulator_disable(void *data)
>         regulator_disable(data);
>  }
>
> +static int max6639_probe_child_from_dt(struct i2c_client *client,
> +                                     struct device_node *child,
> +                                     struct max6639_data *data)
> +
> +{
> +       struct device *dev = &client->dev;
> +       u32 i, val;
> +       int err;
> +
> +       err = of_property_read_u32(child, "reg", &i);
> +       if (err) {
> +               dev_err(dev, "missing reg property of %pOFn\n", child);
> +               return err;
> +       }
> +
> +       if (i >= 2) {
> +               dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
> +               return -EINVAL;
> +       }
> +
> +       err = of_property_read_u32(child, "pulses-per-revolution", &val);
> +       if (!err) {
> +               if (val < 0 || val > 5) {
> +                       dev_err(dev, "invalid pulses-per-revolution %d of %pOFn\n", val, child);
> +                       return -EINVAL;
> +               }
> +               data->ppr[i] = val;
> +       }
> +
> +       err = of_property_read_u32(child, "maxim,rpm-range", &val);
> +       if (!err) {
> +               err = rpm_range_to_index(&data->rpm_range[i], val);
> +               if (err) {
> +                       dev_err(dev, "invalid maxim,rpm-range %d of %pOFn\n", val, child);
> +                       return err;
> +               }
> +       }
> +
> +       err = of_property_read_u32(child, "pwm-polarity", &val);
> +       if (!err) {
> +               if (val > 1) {
> +                       dev_err(dev, "invalid pwm-polarity %d of %pOFn\n", val, child);
> +                       return -EINVAL;
> +               }
> +               data->pwm_polarity[i] = val;
> +       }
> +
> +       return 0;
> +}
> +
> +static int max6639_probe_from_dt(struct i2c_client *client, struct max6639_data *data)
> +{
> +       struct device *dev = &client->dev;
> +       const struct device_node *np = dev->of_node;
> +       struct device_node *child;
> +       int err;
> +
> +       /* Compatible with non-DT platforms */
> +       if (!np)
> +               return 0;
> +
> +       for_each_child_of_node(np, child) {
> +               if (strcmp(child->name, "channel"))

According to the dt-binding this should be "fan" instead of "channel".

> +                       continue;
> +
> +               err = max6639_probe_child_from_dt(client, child, data);
> +               if (err) {
> +                       of_node_put(child);
> +                       return err;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static int max6639_probe(struct i2c_client *client)
>  {
>         struct device *dev = &client->dev;
>         struct max6639_data *data;
>         struct device *hwmon_dev;
> -       int err;
> +       int err, i;
>
>         data = devm_kzalloc(dev, sizeof(struct max6639_data), GFP_KERNEL);
>         if (!data)
> @@ -560,6 +609,24 @@ static int max6639_probe(struct i2c_client *client)
>
>         mutex_init(&data->update_lock);
>
> +       /* default values */
> +       for (i = 0; i < 2; i++) {
> +               /* 4000 RPM */
> +               data->rpm_range[i] = 1;
> +               data->ppr[i] = 2;
> +               data->pwm_polarity[i] = 1;
> +               /* Max. temp. 80C/90C/100C */
> +               data->temp_therm[i] = 80;
> +               data->temp_alert[i] = 90;
> +               data->temp_ot[i] = 100;
> +               /* PWM 120/120 (i.e. 100%) */
> +               data->pwm[i] = 120;
> +       }
> +
> +       err = max6639_probe_from_dt(client, data);
> +       if (err)
> +               return err;
> +
>         /* Initialize the max6639 chip */
>         err = max6639_init_client(client, data);
>         if (err < 0)
> @@ -618,6 +685,14 @@ static const struct i2c_device_id max6639_id[] = {
>
>  MODULE_DEVICE_TABLE(i2c, max6639_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id maxim_of_platform_match[] = {
> +       {.compatible = "maxim,max6639"},
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, maxim_of_platform_match);
> +#endif
> +
>  static SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
>
>  static struct i2c_driver max6639_driver = {
> @@ -625,6 +700,7 @@ static struct i2c_driver max6639_driver = {
>         .driver = {
>                    .name = "max6639",
>                    .pm = &max6639_pm_ops,
> +                  .of_match_table = of_match_ptr(maxim_of_platform_match),
>                    },
>         .probe_new = max6639_probe,
>         .id_table = max6639_id,
> diff --git a/include/linux/platform_data/max6639.h b/include/linux/platform_data/max6639.h
> deleted file mode 100644
> index 65bfdb4fdc15..000000000000
> --- a/include/linux/platform_data/max6639.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _LINUX_MAX6639_H
> -#define _LINUX_MAX6639_H
> -
> -#include <linux/types.h>
> -
> -/* platform data for the MAX6639 temperature sensor and fan control */
> -
> -struct max6639_platform_data {
> -       bool pwm_polarity;      /* Polarity low (0) or high (1, default) */
> -       int ppr;                /* Pulses per rotation 1..4 (default == 2) */
> -       int rpm_range;          /* 2000, 4000 (default), 8000 or 16000 */
> -};
> -
> -#endif /* _LINUX_MAX6639_H */
> --
> 2.35.3
>

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

[Naresh Solanki]

Add Devicetree binding documentation for Maxim MAX6639 temperature
monitor with PWM fan-speed controller.

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 .../bindings/hwmon/maxim,max6639.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 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..20b28cd36555
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
@@ -0,0 +1,55 @@
+# 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 Dual fan controller
+
+maintainers:
+  - Naresh Solanki <Naresh.Solanki@9elements.com>
+
+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
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max6639@10 {
+        compatible = "maxim,max6639";
+        reg = <0x10>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+      };
+    };
+...

base-commit: b8610e0fe08f1476895f141be60f8dd842f0adac
-- 
2.39.1

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

[Krzysztof Kozlowski]

On 18/04/2023 13:32, Naresh Solanki wrote:
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.

Subject: drop second/last, redundant "binding for". The "dt-bindings"
prefix is already stating that these are bindings.

> 
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>  .../bindings/hwmon/maxim,max6639.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 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..20b28cd36555
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +

Drop blank line

This is v9. Where is the changelog? Where is previous authorship? At
least some parts of it? Why this has less properties than old one? Why
this has more mistakes than the old one? Go to previous patch... or fix
everything which was already fixed.

Best regards,
Krzysztof

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

[Naresh Solanki]

Hi Krysztof,

On 18-04-2023 09:47 pm, Krzysztof Kozlowski wrote:
> On 18/04/2023 13:32, Naresh Solanki wrote:
>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>> monitor with PWM fan-speed controller.
> 
> Subject: drop second/last, redundant "binding for". The "dt-bindings"
> prefix is already stating that these are bindings.
Ack
> 
>>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>> ---
>>   .../bindings/hwmon/maxim,max6639.yaml         | 55 +++++++++++++++++++
>>   1 file changed, 55 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..20b28cd36555
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>> @@ -0,0 +1,55 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +
> 
> Drop blank line
Ack
> 
> This is v9. Where is the changelog? Where is previous authorship? At
> least some parts of it? Why this has less properties than old one? Why
> this has more mistakes than the old one? Go to previous patch... or fix
> everything which was already fixed.
This patch is to add basic dt support only hence no additional properties.

> 
> Best regards,
> Krzysztof
> 
Regards,
Naresh

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

[Krzysztof Kozlowski]

On 19/04/2023 11:00, Naresh Solanki wrote:
> Hi Krysztof,
> 
> On 18-04-2023 09:47 pm, Krzysztof Kozlowski wrote:
>> On 18/04/2023 13:32, Naresh Solanki wrote:
>>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>>> monitor with PWM fan-speed controller.
>>
>> Subject: drop second/last, redundant "binding for". The "dt-bindings"
>> prefix is already stating that these are bindings.
> Ack
>>
>>>
>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>> ---
>>>   .../bindings/hwmon/maxim,max6639.yaml         | 55 +++++++++++++++++++
>>>   1 file changed, 55 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..20b28cd36555
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>> @@ -0,0 +1,55 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +
>>
>> Drop blank line
> Ack
>>
>> This is v9. Where is the changelog? Where is previous authorship? At
>> least some parts of it? Why this has less properties than old one? Why
>> this has more mistakes than the old one? Go to previous patch... or fix
>> everything which was already fixed.
> This patch is to add basic dt support only hence no additional properties.

How does that answer my questions?

Best regards,
Krzysztof

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

[Naresh Solanki]

Hi

On 19-04-2023 02:34 pm, Krzysztof Kozlowski wrote:
> On 19/04/2023 11:00, Naresh Solanki wrote:
>> Hi Krysztof,
>>
>> On 18-04-2023 09:47 pm, Krzysztof Kozlowski wrote:
>>> On 18/04/2023 13:32, Naresh Solanki wrote:
>>>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>>>> monitor with PWM fan-speed controller.
>>>
>>> Subject: drop second/last, redundant "binding for". The "dt-bindings"
>>> prefix is already stating that these are bindings.
>> Ack
>>>
>>>>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>> ---
>>>>    .../bindings/hwmon/maxim,max6639.yaml         | 55 +++++++++++++++++++
>>>>    1 file changed, 55 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..20b28cd36555
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>>>> @@ -0,0 +1,55 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +
>>>
>>> Drop blank line
>> Ack
>>>
>>> This is v9. Where is the changelog? Where is previous authorship? At
>>> least some parts of it? Why this has less properties than old one? Why
>>> this has more mistakes than the old one? Go to previous patch... or fix
>>> everything which was already fixed.
>> This patch is to add basic dt support only hence no additional properties.
> 
> How does that answer my questions?
This patch is intended to add dt support for the max6639 driver, and I 
did not take it from previous patches. As this is a basic dt binding, it 
has fewer properties compared to the old one.

I have used v8 as a reference for this patch, and I will review it again 
to make sure I have not missed anything. Will ensure to fix them in the 
next patch.

Although I cannot go back to the previous patch, I will address and fix 
any issues in this patchset.
> 
> Best regards,
> Krzysztof
> 
Regards,
Naresh

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

[Naresh Solanki]

From: Marcello Sylvester Bauer <sylv@sylv.io>

Add Devicetree binding documentation for Maxim MAX6639 temperature
monitor with PWM fan-speed controller.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.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@antcom.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.
+        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.
+
+      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.
+
+    required:
+      - reg
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max6639@10 {
+        compatible = "maxim,max6639";
+        reg = <0x10>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        fan@0 {
+          reg = <0x0>;
+          pwm-polarity = <1>;
+          pulses-per-revolution = <2>;
+          maxim,rpm-range = <4000>;
+        };
+
+        fan@1 {
+          reg = <0x1>;
+          pwm-polarity = <1>;
+          pulses-per-revolution = <2>;
+          maxim,rpm-range = <4000>;
+        };
+      };
+    };
+...
-- 
2.35.3

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

[Guenter Roeck]

On 7/7/22 09:12, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <sylv@sylv.io>
> 
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>

You didn't copy devicetree maintainers, so you won't get the necessary
Reviewed-by: tags.

Guenter

> ---
>   .../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@antcom.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.
> +        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.
> +
> +      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.
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      max6639@10 {
> +        compatible = "maxim,max6639";
> +        reg = <0x10>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        fan@0 {
> +          reg = <0x0>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +
> +        fan@1 {
> +          reg = <0x1>;
> +          pwm-polarity = <1>;
> +          pulses-per-revolution = <2>;
> +          maxim,rpm-range = <4000>;
> +        };
> +      };
> +    };
> +...