[v2,1/4] dt-bindings: hwmon: adi,ltc2945: Add binding

Message ID 20221220000457.1163446-2-jcormier@criticallink.com
State New
Headers
Series hwmon: ltc2945: Add binding and shunt resistor support |

Commit Message

Jonathan Cormier Dec. 20, 2022, 12:04 a.m. UTC
  Create initial binding for the LTC2945 I2C power monitor.
Also adds shunt-resistor-micro-ohms parameter

Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com>
---
 .../bindings/hwmon/adi,ltc2945.yaml           | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml

--
2.25.1
  

Comments

Krzysztof Kozlowski Dec. 20, 2022, 10:15 a.m. UTC | #1
On 20/12/2022 01:04, Cormier, Jonathan wrote:
> Create initial binding for the LTC2945 I2C power monitor.
> Also adds shunt-resistor-micro-ohms parameter

The last sentence does not make sense. I propose to skip it.

> 
> Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com>
> ---
>  .../bindings/hwmon/adi,ltc2945.yaml           | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> new file mode 100644
> index 000000000000..f90d40919ee6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2945 wide range i2c power monitor
> +
> +maintainers:
> +  - Guenter Roeck <linux@roeck-us.net>

Maintainer of binding is person interested in the device, e.g. having
the hardware or datasheet. Not the subsystem maintainer. Unless by
coincidence this is the same person here?

> +
> +description: |
> +  Analog Devices LTC2945 wide range i2c power monitor over I2C.
> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ltc2945
> +
> +  reg:
> +    maxItems: 1
> +
> +  shunt-resistor-micro-ohms:
> +    description:
> +      Shunt resistor value in micro-Ohms
> +    default: 1000
> +
> +required:
> +  - compatible
> +  - reg
> +
> +
This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +additionalProperties: false
> +

Best regards,
Krzysztof
  
Jonathan Cormier Dec. 20, 2022, 2:35 p.m. UTC | #2
On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/12/2022 01:04, Cormier, Jonathan wrote:
> > Create initial binding for the LTC2945 I2C power monitor.
> > Also adds shunt-resistor-micro-ohms parameter
>
> The last sentence does not make sense. I propose to skip it.
Ok
>
> >
> > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com>
> > ---
> >  .../bindings/hwmon/adi,ltc2945.yaml           | 50 +++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > new file mode 100644
> > index 000000000000..f90d40919ee6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices LTC2945 wide range i2c power monitor
> > +
> > +maintainers:
> > +  - Guenter Roeck <linux@roeck-us.net>
>
> Maintainer of binding is person interested in the device, e.g. having
> the hardware or datasheet. Not the subsystem maintainer. Unless by
> coincidence this is the same person here?
What do you do with a basic kernel driver which hasn't been touched
since it was introduced except for various refactors? He seems to be
the one who introduced it and most consistently made changes to it.
>
>
> > +
> > +description: |
> > +  Analog Devices LTC2945 wide range i2c power monitor over I2C.
> > +
> > +  https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ltc2945
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  shunt-resistor-micro-ohms:
> > +    description:
> > +      Shunt resistor value in micro-Ohms
> > +    default: 1000
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +
> This is a friendly reminder during the review process.
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
My bad,
>
> Thank you.
>
> > +additionalProperties: false
> > +
>
> Best regards,
> Krzysztof
>
  
Guenter Roeck Dec. 20, 2022, 2:46 p.m. UTC | #3
On Tue, Dec 20, 2022 at 09:35:38AM -0500, Jon Cormier wrote:
> On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 20/12/2022 01:04, Cormier, Jonathan wrote:
> > > Create initial binding for the LTC2945 I2C power monitor.
> > > Also adds shunt-resistor-micro-ohms parameter
> >
> > The last sentence does not make sense. I propose to skip it.
> Ok
> >
> > >
> > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com>
> > > ---
> > >  .../bindings/hwmon/adi,ltc2945.yaml           | 50 +++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > > new file mode 100644
> > > index 000000000000..f90d40919ee6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > > @@ -0,0 +1,50 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices LTC2945 wide range i2c power monitor
> > > +
> > > +maintainers:
> > > +  - Guenter Roeck <linux@roeck-us.net>
> >
> > Maintainer of binding is person interested in the device, e.g. having
> > the hardware or datasheet. Not the subsystem maintainer. Unless by
> > coincidence this is the same person here?
> What do you do with a basic kernel driver which hasn't been touched
> since it was introduced except for various refactors? He seems to be
> the one who introduced it and most consistently made changes to it.

FWIW, if I was not ok with being listed as maintainer I would have objected.

Having said that, it is just as fine with me to list someone else.

Guenter

> >
> >
> > > +
> > > +description: |
> > > +  Analog Devices LTC2945 wide range i2c power monitor over I2C.
> > > +
> > > +  https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,ltc2945
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  shunt-resistor-micro-ohms:
> > > +    description:
> > > +      Shunt resistor value in micro-Ohms
> > > +    default: 1000
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +
> > This is a friendly reminder during the review process.
> >
> > It seems my previous comments were not fully addressed. Maybe my
> > feedback got lost between the quotes, maybe you just forgot to apply it.
> > Please go back to the previous discussion and either implement all
> > requested changes or keep discussing them.
> My bad,
> >
> > Thank you.
> >
> > > +additionalProperties: false
> > > +
> >
> > Best regards,
> > Krzysztof
> >
> 
> 
> -- 
> Jonathan Cormier
> Software Engineer
> 
> Voice:  315.425.4045 x222
> 
> 
> 
> http://www.CriticalLink.com
> 6712 Brooklawn Parkway, Syracuse, NY 13211
  
Jonathan Cormier Dec. 20, 2022, 9:47 p.m. UTC | #4
Okay so for V3, I see only 2 changes. Please let me know if I've
overlooked something else.
- Remove "Also adds shunt-resistor-micro-ohms parameter" from commit description
- Remove extra newline from binding doc

Resending in plaintext...


On Tue, Dec 20, 2022 at 9:46 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Dec 20, 2022 at 09:35:38AM -0500, Jon Cormier wrote:
> > On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 20/12/2022 01:04, Cormier, Jonathan wrote:
> > > > Create initial binding for the LTC2945 I2C power monitor.
> > > > Also adds shunt-resistor-micro-ohms parameter
> > >
> > > The last sentence does not make sense. I propose to skip it.
> > Ok
> > >
> > > >
> > > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com>
> > > > ---
> > > >  .../bindings/hwmon/adi,ltc2945.yaml           | 50 +++++++++++++++++++
> > > >  1 file changed, 50 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > > > new file mode 100644
> > > > index 000000000000..f90d40919ee6
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
> > > > @@ -0,0 +1,50 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Analog Devices LTC2945 wide range i2c power monitor
> > > > +
> > > > +maintainers:
> > > > +  - Guenter Roeck <linux@roeck-us.net>
> > >
> > > Maintainer of binding is person interested in the device, e.g. having
> > > the hardware or datasheet. Not the subsystem maintainer. Unless by
> > > coincidence this is the same person here?
> > What do you do with a basic kernel driver which hasn't been touched
> > since it was introduced except for various refactors? He seems to be
> > the one who introduced it and most consistently made changes to it.
>
> FWIW, if I was not ok with being listed as maintainer I would have objected.
>
> Having said that, it is just as fine with me to list someone else.
>
> Guenter
>
> > >
> > >
> > > > +
> > > > +description: |
> > > > +  Analog Devices LTC2945 wide range i2c power monitor over I2C.
> > > > +
> > > > +  https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - adi,ltc2945
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  shunt-resistor-micro-ohms:
> > > > +    description:
> > > > +      Shunt resistor value in micro-Ohms
> > > > +    default: 1000
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +
> > > > +
> > > This is a friendly reminder during the review process.
> > >
> > > It seems my previous comments were not fully addressed. Maybe my
> > > feedback got lost between the quotes, maybe you just forgot to apply it.
> > > Please go back to the previous discussion and either implement all
> > > requested changes or keep discussing them.
> > My bad,
> > >
> > > Thank you.
> > >
> > > > +additionalProperties: false
> > > > +
> > >
> > > Best regards,
> > > Krzysztof
> > >
> >
> >
> > --
> > Jonathan Cormier
> > Software Engineer
> >
> > Voice:  315.425.4045 x222
> >
> >
> >
> > http://www.CriticalLink.com
> > 6712 Brooklawn Parkway, Syracuse, NY 13211
  

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
new file mode 100644
index 000000000000..f90d40919ee6
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
@@ -0,0 +1,50 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2945 wide range i2c power monitor
+
+maintainers:
+  - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+  Analog Devices LTC2945 wide range i2c power monitor over I2C.
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ltc2945
+
+  reg:
+    maxItems: 1
+
+  shunt-resistor-micro-ohms:
+    description:
+      Shunt resistor value in micro-Ohms
+    default: 1000
+
+required:
+  - compatible
+  - reg
+
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-monitor@6e {
+            compatible = "adi,ltc2945";
+            reg = <0x6e>;
+            /* 10 milli-Ohm shunt resistor */
+            shunt-resistor-micro-ohms = <10000>;
+        };
+    };
+...