2023-08-01 17:42:27

by Carsten Spieß

[permalink] [raw]
Subject: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022

Add dt-bindings for Renesas ISL28022 power monitor.

Signed-off-by: Carsten Spieß <[email protected]>
---
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ß <[email protected]>
+
+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ß <[email protected]>
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
F: Documentation/hwmon/isl28022.rst
F: drivers/hwmon/isl28022.c

--
2.34.1



2023-08-01 21:26:00

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022

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? <[email protected]>
> ---
> 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? <[email protected]>
> +
> +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? <[email protected]>
> L: [email protected]
> S: Maintained
> +F: Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> F: Documentation/hwmon/isl28022.rst
> F: drivers/hwmon/isl28022.c
>
> --
> 2.34.1
>


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

2023-08-02 08:06:11

by Carsten Spieß

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022


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


Attachments:
(No filename) (849.00 B)
Digitale Signatur von OpenPGP

2023-08-02 09:40:11

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022

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 <[email protected]>

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


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

2023-08-02 16:32:18

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022

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 <[email protected]>
>
> > > > + 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.



2023-08-02 16:54:10

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] dt-bindings: hwmon: add renesas,isl28022

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