2024-05-28 12:51:42

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH] dt-bindings: hwmon: Add max6639

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

Signed-off-by: Naresh Solanki <[email protected]>
---
.../bindings/hwmon/maxim,max6639.yaml | 92 +++++++++++++++++++
1 file changed, 92 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..a2e37f7329b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
@@ -0,0 +1,92 @@
+# 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:
+ - Naresh Solanki <[email protected]>
+
+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
+
+ '#pwm-cells':
+ const: 3
+
+required:
+ - compatible
+ - reg
+
+patternProperties:
+ "^fan@[0-1]$":
+ type: object
+ description: |
+ Represents the two fans and their specific configuration.
+
+ $ref: fan-common.yaml#
+
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description: |
+ The fan number.
+
+ required:
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fan1: fan-controller@10 {
+ compatible = "maxim,max6639";
+ reg = <0x10>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #pwm-cells = <3>;
+
+ fan@0 {
+ reg = <0x0>;
+ pulses-per-revolution = <2>;
+ max-rpm = <4000>;
+ target-rpm = <1000>;
+ pwms = <&fan1 0 25000 0>;
+ };
+
+ fan@1 {
+ reg = <0x1>;
+ pulses-per-revolution = <2>;
+ max-rpm = <8000>;
+ pwms = <&fan1 1 25000 0>;
+ };
+ };
+ };
+...

base-commit: 5fbf8734fb36cf67339f599f0e51747a6aff690c
--
2.42.0



2024-05-28 14:27:14

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: hwmon: Add max6639

On Tue, May 28, 2024 at 06:21:21PM +0530, Naresh Solanki wrote:
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.

Other than unneeded |s where your descriptions don't have any
formatting, this patch looks fine. That said - where's your dts or your
driver? A binding on its own is unusual.

Thanks,
Conor.


Attachments:
(No filename) (376.00 B)
signature.asc (235.00 B)
Download all attachments

2024-05-28 14:42:11

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: hwmon: Add max6639

On 5/28/24 07:26, Conor Dooley wrote:
> On Tue, May 28, 2024 at 06:21:21PM +0530, Naresh Solanki wrote:
>> Add Devicetree binding documentation for Maxim MAX6639 temperature
>> monitor with PWM fan-speed controller.
>
> Other than unneeded |s where your descriptions don't have any
> formatting, this patch looks fine. That said - where's your dts or your
> driver? A binding on its own is unusual.
>

The driver is drivers/hwmon/max6639.c which needs other unrelated changes/
cleanups. Holding those up until the bindings are accepted would not make
sense, so my guess is that Naresh decided to pursue the other changes for now
and add devicetree support to the driver after the devicetree properties
have been approved. On the other side, adding devicetree support does depend
on the other changes, so trying to do that before the other changes are
complete would be difficult.

That is just my guess, though, and I am ok with it.

Thanks,
Guenter


2024-05-28 15:20:38

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: hwmon: Add max6639

On Tue, May 28, 2024 at 07:41:58AM -0700, Guenter Roeck wrote:
> On 5/28/24 07:26, Conor Dooley wrote:
> > On Tue, May 28, 2024 at 06:21:21PM +0530, Naresh Solanki wrote:
> > > Add Devicetree binding documentation for Maxim MAX6639 temperature
> > > monitor with PWM fan-speed controller.
> >
> > Other than unneeded |s where your descriptions don't have any
> > formatting, this patch looks fine. That said - where's your dts or your
> > driver? A binding on its own is unusual.
> >
>
> The driver is drivers/hwmon/max6639.c which needs other unrelated changes/
> cleanups. Holding those up until the bindings are accepted would not make
> sense, so my guess is that Naresh decided to pursue the other changes for now
> and add devicetree support to the driver after the devicetree properties
> have been approved. On the other side, adding devicetree support does depend
> on the other changes, so trying to do that before the other changes are
> complete would be difficult.
>
> That is just my guess, though, and I am ok with it.

Well, I think the binding is fine, so it is up to you whether you want
to merge the binding without having the driver implementation nailed
down.

Ideally with the |s removed,
Reviewed-by: Conor Dooley <[email protected]>
Thanks,
Conor.


Attachments:
(No filename) (1.28 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-29 22:25:32

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: hwmon: Add max6639

On Tue, May 28, 2024 at 06:21:21PM +0530, Naresh Solanki wrote:
> Add Devicetree binding documentation for Maxim MAX6639 temperature
> monitor with PWM fan-speed controller.
>
> Signed-off-by: Naresh Solanki <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>

Applied (and I'll drop the '|' after description:).

Thanks,
Guenter

> ---
> .../bindings/hwmon/maxim,max6639.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
>
>
> base-commit: 5fbf8734fb36cf67339f599f0e51747a6aff690c
>
> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> new file mode 100644
> index 000000000000..a2e37f7329b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml
> @@ -0,0 +1,92 @@
> +# 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:
> + - Naresh Solanki <[email protected]>
> +
> +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
> +
> + '#pwm-cells':
> + const: 3
> +
> +required:
> + - compatible
> + - reg
> +
> +patternProperties:
> + "^fan@[0-1]$":
> + type: object
> + description: |
> + Represents the two fans and their specific configuration.
> +
> + $ref: fan-common.yaml#
> +
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + description: |
> + The fan number.
> +
> + required:
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan1: fan-controller@10 {
> + compatible = "maxim,max6639";
> + reg = <0x10>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #pwm-cells = <3>;
> +
> + fan@0 {
> + reg = <0x0>;
> + pulses-per-revolution = <2>;
> + max-rpm = <4000>;
> + target-rpm = <1000>;
> + pwms = <&fan1 0 25000 0>;
> + };
> +
> + fan@1 {
> + reg = <0x1>;
> + pulses-per-revolution = <2>;
> + max-rpm = <8000>;
> + pwms = <&fan1 1 25000 0>;
> + };
> + };
> + };
> +...