diff mbox series

dt-bindings: hwmon: Add max6639

Message ID 20240528125122.1129986-1-naresh.solanki@9elements.com (mailing list archive)
State Accepted
Headers show
Series dt-bindings: hwmon: Add max6639 | expand

Commit Message

Naresh Solanki May 28, 2024, 12:51 p.m. UTC 
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         | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml


base-commit: 5fbf8734fb36cf67339f599f0e51747a6aff690c

Comments

Conor Dooley May 28, 2024, 2:26 p.m. UTC | #1 
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.
Guenter Roeck May 28, 2024, 2:41 p.m. UTC | #2 
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
Conor Dooley May 28, 2024, 3:20 p.m. UTC | #3 
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 <conor.dooley@microchip.com>
Thanks,
Conor.
Guenter Roeck May 29, 2024, 10:25 p.m. UTC | #4 
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 <naresh.solanki@9elements.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

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 <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
> +
> +  '#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>;
> +            };
> +        };
> +    };
> +...
diff mbox series

Patch

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 <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
+
+  '#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>;
+            };
+        };
+    };
+...