diff mbox series

[v3,2/2] dt-bindings: hwmon: add renesas,isl28022

Message ID 20230801163546.3170-3-mail@carsten-spiess.de
State Not Applicable, archived
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Carsten Spieß Aug. 1, 2023, 4:35 p.m. UTC 
Add dt-bindings for Renesas ISL28022 power monitor.

Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
---
v3:
- changelog added
v2/v3:
- schema errors fixed
- properties reworked
- shunt-resistor minimum and default value added
---
 .../bindings/hwmon/renesas,isl28022.yaml      | 65 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml

Comments

Conor Dooley Aug. 1, 2023, 8:52 p.m. UTC | #1 
On Tue, Aug 01, 2023 at 06:35:46PM +0200, Carsten Spieß wrote:
> Add dt-bindings for Renesas ISL28022 power monitor.
> 
> Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
> ---
> v3:
> - changelog added
> v2/v3:
> - schema errors fixed
> - properties reworked
> - shunt-resistor minimum and default value added
> ---
>  .../bindings/hwmon/renesas,isl28022.yaml      | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> new file mode 100644
> index 000000000000..1e0971287941
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/renesas,isl28022.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas ISL28022 power monitor
> +
> +maintainers:
> +  - Carsten Spieß <mail@carsten-spiess.de>
> +
> +description: |
> +  The ISL28022 is a power monitor with I2C interface. The device monitors
> +  voltage, current via shunt resistor and calculated power.
> +
> +  Datasheets:
> +    https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,isl28022

You've only got one compatible, why the enum? Will there be more similar
devices that have an incompatible programming model?

> +
> +  reg:
> +    maxItems: 1
> +
> +  shunt-resistor-micro-ohms:
> +    description: |
> +      Shunt resistor value in micro-Ohm
> +    minimum: 800
> +    default: 10000
> +
> +  renesas,shunt-range-microvolt:
> +    description: |

You don't need these |s if you have no formatting to preserve in the
text.
Otherwise, this does look good to me.

Cheers,
Conor.

> +      Maximal shunt voltage range of +/- 40 mV, 80 mV, 160 mV or 320 mV
> +    default: 320000
> +    enum: [40000, 80000, 160000, 320000]
> +
> +  renesas,average-samples:
> +    description: |
> +      Number of samples to be used to report voltage, current and power values.
> +    default: 1
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4, 8, 16, 32, 64, 128]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        power-monitor@40 {
> +            compatible = "renesas,isl28022";
> +            reg = <0x40>;
> +            shunt-resistor-micro-ohms = <8000>;
> +            renesas,shunt-range-microvolt = <40000>;
> +            renesas,average-samples = <128>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b02e3b991676..23b8e8183ece 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11069,6 +11069,7 @@ ISL28022 HARDWARE MONITORING DRIVER
>  M:	Carsten Spieß <mail@carsten-spiess.de>
>  L:	linux-hwmon@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
>  F:	Documentation/hwmon/isl28022.rst
>  F:	drivers/hwmon/isl28022.c
>  
> -- 
> 2.34.1
>
Carsten Spieß Aug. 2, 2023, 7:30 a.m. UTC | #2 
On 8/1/23 22:52, Conor Dooley wrote:
> On Tue, Aug 01, 2023 at 06:35:46PM +0200, Carsten Spieß wrote:
> > Add dt-bindings for Renesas ISL28022 power monitor.
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,isl28022  
> 
> You've only got one compatible, why the enum? Will there be more similar
> devices that have an incompatible programming model?
Yes, there are isl28023 and isl28025 with different register addresses,
might be supported in future releases.

> > +  renesas,shunt-range-microvolt:
> > +    description: |  
> 
> You don't need these |s if you have no formatting to preserve in the
> text.
Will fix in v4.

> Otherwise, this does look good to me.
Thanks, regards
Carsten
Conor Dooley Aug. 2, 2023, 7:43 a.m. UTC | #3 
On Wed, Aug 02, 2023 at 09:30:23AM +0200, Carsten Spieß wrote:
> 
> On 8/1/23 22:52, Conor Dooley wrote:
> > On Tue, Aug 01, 2023 at 06:35:46PM +0200, Carsten Spieß wrote:
> > > Add dt-bindings for Renesas ISL28022 power monitor.
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - renesas,isl28022  
> > 
> > You've only got one compatible, why the enum? Will there be more similar
> > devices that have an incompatible programming model?
> Yes, there are isl28023 and isl28025 with different register addresses,
> might be supported in future releases.

Right. Unless that's a very strong "might", const: will do the trick
here just fine.

Otherwise,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> > > +  renesas,shunt-range-microvolt:
> > > +    description: |  
> > 
> > You don't need these |s if you have no formatting to preserve in the
> > text.
> Will fix in v4.

There's no need to send a v4 for that alone.
Guenter Roeck Aug. 2, 2023, 3:05 p.m. UTC | #4 
On Wed, Aug 02, 2023 at 09:30:23AM +0200, Carsten Spieß wrote:
> 
> On 8/1/23 22:52, Conor Dooley wrote:
> > On Tue, Aug 01, 2023 at 06:35:46PM +0200, Carsten Spieß wrote:
> > > Add dt-bindings for Renesas ISL28022 power monitor.
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - renesas,isl28022  
> > 
> > You've only got one compatible, why the enum? Will there be more similar
> > devices that have an incompatible programming model?
> Yes, there are isl28023 and isl28025 with different register addresses,
> might be supported in future releases.

This is misleading. ISL28023 and ISL28025 are PMBus compatible chips
and would be added as PMBus driver(s) (if needed). Support for those chips
will never be part of the isl28022 driver, and any devicetree properties
of those chips would not be described in this file.

Guenter

> 
> > > +  renesas,shunt-range-microvolt:
> > > +    description: |  
> > 
> > You don't need these |s if you have no formatting to preserve in the
> > text.
> Will fix in v4.
> 
> > Otherwise, this does look good to me.
> Thanks, regards
> Carsten
Guenter Roeck Aug. 2, 2023, 3:07 p.m. UTC | #5 
On Wed, Aug 02, 2023 at 08:43:59AM +0100, Conor Dooley wrote:
> On Wed, Aug 02, 2023 at 09:30:23AM +0200, Carsten Spieß wrote:
> > 
> > On 8/1/23 22:52, Conor Dooley wrote:
> > > On Tue, Aug 01, 2023 at 06:35:46PM +0200, Carsten Spieß wrote:
> > > > Add dt-bindings for Renesas ISL28022 power monitor.
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - renesas,isl28022  
> > > 
> > > You've only got one compatible, why the enum? Will there be more similar
> > > devices that have an incompatible programming model?
> > Yes, there are isl28023 and isl28025 with different register addresses,
> > might be supported in future releases.
> 
> Right. Unless that's a very strong "might", const: will do the trick
> here just fine.

It is a very strong "will never be" for isl28023 and isl28025.

Guenter

> 
> Otherwise,
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> > > > +  renesas,shunt-range-microvolt:
> > > > +    description: |  
> > > 
> > > You don't need these |s if you have no formatting to preserve in the
> > > text.
> > Will fix in v4.
> 
> There's no need to send a v4 for that alone.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
new file mode 100644
index 000000000000..1e0971287941
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
@@ -0,0 +1,65 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/renesas,isl28022.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas ISL28022 power monitor
+
+maintainers:
+  - Carsten Spieß <mail@carsten-spiess.de>
+
+description: |
+  The ISL28022 is a power monitor with I2C interface. The device monitors
+  voltage, current via shunt resistor and calculated power.
+
+  Datasheets:
+    https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
+
+properties:
+  compatible:
+    enum:
+      - renesas,isl28022
+
+  reg:
+    maxItems: 1
+
+  shunt-resistor-micro-ohms:
+    description: |
+      Shunt resistor value in micro-Ohm
+    minimum: 800
+    default: 10000
+
+  renesas,shunt-range-microvolt:
+    description: |
+      Maximal shunt voltage range of +/- 40 mV, 80 mV, 160 mV or 320 mV
+    default: 320000
+    enum: [40000, 80000, 160000, 320000]
+
+  renesas,average-samples:
+    description: |
+      Number of samples to be used to report voltage, current and power values.
+    default: 1
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2, 4, 8, 16, 32, 64, 128]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-monitor@40 {
+            compatible = "renesas,isl28022";
+            reg = <0x40>;
+            shunt-resistor-micro-ohms = <8000>;
+            renesas,shunt-range-microvolt = <40000>;
+            renesas,average-samples = <128>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b02e3b991676..23b8e8183ece 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11069,6 +11069,7 @@  ISL28022 HARDWARE MONITORING DRIVER
 M:	Carsten Spieß <mail@carsten-spiess.de>
 L:	linux-hwmon@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
 F:	Documentation/hwmon/isl28022.rst
 F:	drivers/hwmon/isl28022.c