[PATCH 0/4] hwmon: add ti,ina237 support to ina238 driver

[PATCH 0/4] hwmon: add ti,ina237 support to ina238 driver

[Richard Leitner]

This series adds support for the ina237 power monitor to the ina238
driver as those two are very similar.

As the driver missed MAINTAINERS and dt-bindings I've also added them.
If this is incorrect please just ignore the patches or drop me a line so
I can provide a v2.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
Richard Leitner (4):
      MAINTAINERS: Add entry for ina238 driver
      dt-bindings: hwmon: add ti,ina238
      hwmon: ina238: add ina237 support
      dt-bindings: hwmon: ti,ina238: add ti,ina237

 .../devicetree/bindings/hwmon/ti,ina238.yaml       | 47 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 ++++
 drivers/hwmon/ina238.c                             |  3 +-
 3 files changed, 56 insertions(+), 1 deletion(-)
---
base-commit: 4f82870119a46b0d04d91ef4697ac4977a255a9d
change-id: 20231025-ina237-0f1a9c14fd5d

Best regards,
-- 
Richard Leitner <richard.leitner@linux.dev>

[PATCH 1/4] MAINTAINERS: Add entry for ina238 driver

[Richard Leitner]

The ina238 driver is available since 2021 but lacks a MAINTAINERS entry
file. Therefore add the missing entry now.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 668d1e24452d..28f91c8a2e1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10267,6 +10267,12 @@ F:	Documentation/hwmon/ina2xx.rst
 F:	drivers/hwmon/ina2xx.c
 F:	include/linux/platform_data/ina2xx.h
 
+INA238 HARDWARE MONITOR DRIVER
+M:	Guenter Roeck <linux@roeck-us.net>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	drivers/hwmon/ina238.c
+
 INDEX OF FURTHER KERNEL DOCUMENTATION
 M:	Carlos Bilbao <carlos.bilbao@amd.com>
 S:	Maintained

-- 
2.40.1

Re: [PATCH 1/4] MAINTAINERS: Add entry for ina238 driver

[Guenter Roeck]

On Wed, Oct 25, 2023 at 10:34:11AM +0000, Richard Leitner wrote:
> The ina238 driver is available since 2021 but lacks a MAINTAINERS entry
> file. Therefore add the missing entry now.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>

NACK. I am already the hwmon maintainer, and there is no need
to list me as maintainer for every hwmon driver.

Guenter

> ---
>  MAINTAINERS | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 668d1e24452d..28f91c8a2e1c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10267,6 +10267,12 @@ F:	Documentation/hwmon/ina2xx.rst
>  F:	drivers/hwmon/ina2xx.c
>  F:	include/linux/platform_data/ina2xx.h
>  
> +INA238 HARDWARE MONITOR DRIVER
> +M:	Guenter Roeck <linux@roeck-us.net>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Maintained
> +F:	drivers/hwmon/ina238.c
> +
>  INDEX OF FURTHER KERNEL DOCUMENTATION
>  M:	Carlos Bilbao <carlos.bilbao@amd.com>
>  S:	Maintained

Re: [PATCH 1/4] MAINTAINERS: Add entry for ina238 driver

[Richard Leitner]

On Wed, Oct 25, 2023 at 06:03:23PM -0700, Guenter Roeck wrote:
> On Wed, Oct 25, 2023 at 10:34:11AM +0000, Richard Leitner wrote:
> > The ina238 driver is available since 2021 but lacks a MAINTAINERS entry
> > file. Therefore add the missing entry now.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> 
> NACK. I am already the hwmon maintainer, and there is no need
> to list me as maintainer for every hwmon driver.

Thanks for the feedback. Understood. Will remove the entry in v2.

regards;rl

> 
> Guenter
> 
> > ---
> >  MAINTAINERS | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 668d1e24452d..28f91c8a2e1c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10267,6 +10267,12 @@ F:	Documentation/hwmon/ina2xx.rst
> >  F:	drivers/hwmon/ina2xx.c
> >  F:	include/linux/platform_data/ina2xx.h
> >  
> > +INA238 HARDWARE MONITOR DRIVER
> > +M:	Guenter Roeck <linux@roeck-us.net>
> > +L:	linux-hwmon@vger.kernel.org
> > +S:	Maintained
> > +F:	drivers/hwmon/ina238.c
> > +
> >  INDEX OF FURTHER KERNEL DOCUMENTATION
> >  M:	Carlos Bilbao <carlos.bilbao@amd.com>
> >  S:	Maintained

[PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Richard Leitner]

The ina238 driver is available since 2021 but lacks a dt-bindings file.
Therefore add the missing file now.

Mention Jean Delvare and Guenter Roeck as maintainers as reported by the
get_maintainer.pl script.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 .../devicetree/bindings/hwmon/ti,ina238.yaml       | 46 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 47 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
new file mode 100644
index 000000000000..aba89e5f34b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/ti,ina238.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments INA238 power/voltage monitors
+
+maintainers:
+  - Jean Delvare <jdelvare@suse.com>
+  - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+  The INA238 is an ultra-precise digital power monitor with a
+  16-bit delta-sigma ADC specifically designed for current-sensing
+  applications.
+
+  Datasheets:
+    https://www.ti.com/lit/ds/symlink/ina238.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,ina238
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-sensor@40 {
+            compatible = "ti,ina238";
+            reg = <0x40>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 28f91c8a2e1c..13858bd6a3d4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10271,6 +10271,7 @@ INA238 HARDWARE MONITOR DRIVER
 M:	Guenter Roeck <linux@roeck-us.net>
 L:	linux-hwmon@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
 F:	drivers/hwmon/ina238.c
 
 INDEX OF FURTHER KERNEL DOCUMENTATION

-- 
2.40.1

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]

On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> The ina238 driver is available since 2021 but lacks a dt-bindings file.
> Therefore add the missing file now.

Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml

Cheers,
Conor.

> 
> Mention Jean Delvare and Guenter Roeck as maintainers as reported by the
> get_maintainer.pl script.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
>  .../devicetree/bindings/hwmon/ti,ina238.yaml       | 46 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> new file mode 100644
> index 000000000000..aba89e5f34b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/ti,ina238.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments INA238 power/voltage monitors
> +
> +maintainers:
> +  - Jean Delvare <jdelvare@suse.com>
> +  - Guenter Roeck <linux@roeck-us.net>
> +
> +description: |
> +  The INA238 is an ultra-precise digital power monitor with a
> +  16-bit delta-sigma ADC specifically designed for current-sensing
> +  applications.
> +
> +  Datasheets:
> +    https://www.ti.com/lit/ds/symlink/ina238.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ina238
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        power-sensor@40 {
> +            compatible = "ti,ina238";
> +            reg = <0x40>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 28f91c8a2e1c..13858bd6a3d4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10271,6 +10271,7 @@ INA238 HARDWARE MONITOR DRIVER
>  M:	Guenter Roeck <linux@roeck-us.net>
>  L:	linux-hwmon@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
>  F:	drivers/hwmon/ina238.c
>  
>  INDEX OF FURTHER KERNEL DOCUMENTATION
> 
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Richard Leitner]

On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> > The ina238 driver is available since 2021 but lacks a dt-bindings file.
> > Therefore add the missing file now.
> 
> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml

Thanks for the feedback. True. So is it fine if it's left there or
should it be removed from ti,ina2xxx.yml as this is a separate driver
with different properties?

> 
> Cheers,
> Conor.
> 
> > 
> > Mention Jean Delvare and Guenter Roeck as maintainers as reported by the
> > get_maintainer.pl script.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> >  .../devicetree/bindings/hwmon/ti,ina238.yaml       | 46 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> > new file mode 100644
> > index 000000000000..aba89e5f34b3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/hwmon/ti,ina238.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Texas Instruments INA238 power/voltage monitors
> > +
> > +maintainers:
> > +  - Jean Delvare <jdelvare@suse.com>
> > +  - Guenter Roeck <linux@roeck-us.net>
> > +
> > +description: |
> > +  The INA238 is an ultra-precise digital power monitor with a
> > +  16-bit delta-sigma ADC specifically designed for current-sensing
> > +  applications.
> > +
> > +  Datasheets:
> > +    https://www.ti.com/lit/ds/symlink/ina238.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,ina238
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        power-sensor@40 {
> > +            compatible = "ti,ina238";
> > +            reg = <0x40>;
> > +        };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 28f91c8a2e1c..13858bd6a3d4 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10271,6 +10271,7 @@ INA238 HARDWARE MONITOR DRIVER
> >  M:	Guenter Roeck <linux@roeck-us.net>
> >  L:	linux-hwmon@vger.kernel.org
> >  S:	Maintained
> > +F:	Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> >  F:	drivers/hwmon/ina238.c
> >  
> >  INDEX OF FURTHER KERNEL DOCUMENTATION
> > 
> > -- 
> > 2.40.1
> > 

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
> > On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> > > The ina238 driver is available since 2021 but lacks a dt-bindings file.
> > > Therefore add the missing file now.
> > 
> > Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> 
> Thanks for the feedback. True. So is it fine if it's left there or
> should it be removed from ti,ina2xxx.yml as this is a separate driver
> with different properties?

Merging them would seem like the most straightforward thing to do, no?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Krzysztof Kozlowski]

On 25/10/2023 16:11, Conor Dooley wrote:
> On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
>> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
>>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
>>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
>>>> Therefore add the missing file now.
>>>
>>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>>
>> Thanks for the feedback. True. So is it fine if it's left there or
>> should it be removed from ti,ina2xxx.yml as this is a separate driver
>> with different properties?
> 
> Merging them would seem like the most straightforward thing to do, no?

Sorry folks, I don't quite get what do you want to merge or move and
why. Drivers are not related to bindings. The point is the compatible is
already documented, so is anything wrong with existing documentation?

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Richard Leitner]

On Wed, Oct 25, 2023 at 04:18:31PM +0200, Krzysztof Kozlowski wrote:
> On 25/10/2023 16:11, Conor Dooley wrote:
> > On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
> >> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
> >>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> >>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
> >>>> Therefore add the missing file now.
> >>>
> >>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> >>
> >> Thanks for the feedback. True. So is it fine if it's left there or
> >> should it be removed from ti,ina2xxx.yml as this is a separate driver
> >> with different properties?
> > 
> > Merging them would seem like the most straightforward thing to do, no?
> 
> Sorry folks, I don't quite get what do you want to merge or move and
> why. Drivers are not related to bindings. The point is the compatible is
> already documented, so is anything wrong with existing documentation?

ina238 is a separate driver which doesn't evaluate the same properties as
the ina2xx driver. So I thought it would be reasonable to split those
bindings and therefore reflect the drivers capabilities.

If it's fine if there are additional properties in the dt-bindings which
are not evaluated by the driver then it's of course fine with me to just
add the ina327 compatible in ina2xx.yaml.

Thanks for the feedback & regards;rl

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Krzysztof Kozlowski]

On 25/10/2023 16:23, Richard Leitner wrote:
> On Wed, Oct 25, 2023 at 04:18:31PM +0200, Krzysztof Kozlowski wrote:
>> On 25/10/2023 16:11, Conor Dooley wrote:
>>> On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
>>>> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
>>>>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
>>>>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
>>>>>> Therefore add the missing file now.
>>>>>
>>>>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>>>>
>>>> Thanks for the feedback. True. So is it fine if it's left there or
>>>> should it be removed from ti,ina2xxx.yml as this is a separate driver
>>>> with different properties?
>>>
>>> Merging them would seem like the most straightforward thing to do, no?
>>
>> Sorry folks, I don't quite get what do you want to merge or move and
>> why. Drivers are not related to bindings. The point is the compatible is
>> already documented, so is anything wrong with existing documentation?
> 
> ina238 is a separate driver which doesn't evaluate the same properties as
> the ina2xx driver. So I thought it would be reasonable to split those
> bindings and therefore reflect the drivers capabilities.

I do not see different properties in the bindings, so what do you mean
that it evaluates something else?

Anyway, whatever driver does is rarely good argument for change in
bindings, because we focus here on the hardware, not on one, chosen OS
implementation.

> 
> If it's fine if there are additional properties in the dt-bindings which

Where are they? Or rather which additional properties?

> are not evaluated by the driver then it's of course fine with me to just
> add the ina327 compatible in ina2xx.yaml.

Depends. What driver does, might not matter in some cases. What matters
is if these properties are applicable to this hardware.

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Richard Leitner]

On Wed, Oct 25, 2023 at 04:27:18PM +0200, Krzysztof Kozlowski wrote:
> On 25/10/2023 16:23, Richard Leitner wrote:
> > On Wed, Oct 25, 2023 at 04:18:31PM +0200, Krzysztof Kozlowski wrote:
> >> On 25/10/2023 16:11, Conor Dooley wrote:
> >>> On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
> >>>> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
> >>>>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> >>>>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
> >>>>>> Therefore add the missing file now.
> >>>>>
> >>>>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> >>>>
> >>>> Thanks for the feedback. True. So is it fine if it's left there or
> >>>> should it be removed from ti,ina2xxx.yml as this is a separate driver
> >>>> with different properties?
> >>>
> >>> Merging them would seem like the most straightforward thing to do, no?
> >>
> >> Sorry folks, I don't quite get what do you want to merge or move and
> >> why. Drivers are not related to bindings. The point is the compatible is
> >> already documented, so is anything wrong with existing documentation?
> > 
> > ina238 is a separate driver which doesn't evaluate the same properties as
> > the ina2xx driver. So I thought it would be reasonable to split those
> > bindings and therefore reflect the drivers capabilities.
> 
> I do not see different properties in the bindings, so what do you mean
> that it evaluates something else?
> 
> Anyway, whatever driver does is rarely good argument for change in
> bindings, because we focus here on the hardware, not on one, chosen OS
> implementation.

Understood.

> 
> > 
> > If it's fine if there are additional properties in the dt-bindings which
> 
> Where are they? Or rather which additional properties?

For example the "shunt-resistor" property described in ina2xx.yaml is
not evaluated in ina238.c.

> 
> > are not evaluated by the driver then it's of course fine with me to just
> > add the ina327 compatible in ina2xx.yaml.
> 
> Depends. What driver does, might not matter in some cases. What matters
> is if these properties are applicable to this hardware.

Thanks for that explanation. That makes things clearer to me.

The properties described in the ina2xx.yaml are applicable to the ina237
and ina238 hardware, but are not implemented in the ina238.c driver.

So I will just add the ina237 compatible to the ina2xx.yaml and drop the
ina238.yaml from that series. Would that be fine from your side?

regards;rl

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Krzysztof Kozlowski]

On 25/10/2023 16:32, Richard Leitner wrote:
>>> are not evaluated by the driver then it's of course fine with me to just
>>> add the ina327 compatible in ina2xx.yaml.
>>
>> Depends. What driver does, might not matter in some cases. What matters
>> is if these properties are applicable to this hardware.
> 
> Thanks for that explanation. That makes things clearer to me.
> 
> The properties described in the ina2xx.yaml are applicable to the ina237
> and ina238 hardware, but are not implemented in the ina238.c driver.
> 
> So I will just add the ina237 compatible to the ina2xx.yaml and drop the
> ina238.yaml from that series. Would that be fine from your side?


Yes, thanks.

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

On Wed, Oct 25, 2023 at 04:18:31PM +0200, Krzysztof Kozlowski wrote:
> On 25/10/2023 16:11, Conor Dooley wrote:
> > On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
> >> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
> >>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
> >>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
> >>>> Therefore add the missing file now.
> >>>
> >>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> >>
> >> Thanks for the feedback. True. So is it fine if it's left there or
> >> should it be removed from ti,ina2xxx.yml as this is a separate driver
> >> with different properties?
> > 
> > Merging them would seem like the most straightforward thing to do, no?
> 
> Sorry folks, I don't quite get what do you want to merge or move and
> why. Drivers are not related to bindings. The point is the compatible is
> already documented, so is anything wrong with existing documentation?

If it was not clear, I meant merging the bindings, not the drivers. IOW,
adding the new compatible for the ina237 to the existing ina2xx.yaml
binding.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/4] dt-bindings: hwmon: add ti,ina238

[Krzysztof Kozlowski]

On 25/10/2023 16:25, Conor Dooley wrote:
> On Wed, Oct 25, 2023 at 04:18:31PM +0200, Krzysztof Kozlowski wrote:
>> On 25/10/2023 16:11, Conor Dooley wrote:
>>> On Wed, Oct 25, 2023 at 04:07:31PM +0200, Richard Leitner wrote:
>>>> On Wed, Oct 25, 2023 at 03:00:01PM +0100, Conor Dooley wrote:
>>>>> On Wed, Oct 25, 2023 at 10:34:12AM +0000, Richard Leitner wrote:
>>>>>> The ina238 driver is available since 2021 but lacks a dt-bindings file.
>>>>>> Therefore add the missing file now.
>>>>>
>>>>> Seemingly it is documented in Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>>>>
>>>> Thanks for the feedback. True. So is it fine if it's left there or
>>>> should it be removed from ti,ina2xxx.yml as this is a separate driver
>>>> with different properties?
>>>
>>> Merging them would seem like the most straightforward thing to do, no?
>>
>> Sorry folks, I don't quite get what do you want to merge or move and
>> why. Drivers are not related to bindings. The point is the compatible is
>> already documented, so is anything wrong with existing documentation?
> 
> If it was not clear, I meant merging the bindings, not the drivers. IOW,
> adding the new compatible for the ina237 to the existing ina2xx.yaml
> binding.

Yes, this sounds good. Maybe ina2xx is indeed not matching all of this
hardware (e.g. supplies, shunt resistors), but then we need to talk
about specifics.

Best regards,
Krzysztof

[PATCH 3/4] hwmon: ina238: add ina237 support

[Richard Leitner]

The INA237 "85-V, 16-Bit, Precision Power Monitor With I2C Interface" is
basically the same as INA328. Therefore add a corresponding compatible
to the driver.

According to the datasheet the main difference is the current and power
monitoring accuracy:

+------------------------+---------------+---------------+
|                        | INA238        | INA237        |
+------------------------+---------------+---------------+
| Offset voltage         | +/- 5µV       | +/- 50µV      |
| Offset drift           | +/- 0.02µV/°C | +/- 0.02µV/°C |
| Gain error             | +/- 0.1%      | +/- 0.3%      |
| Gain error drift       | +/- 25ppm/°C  | +/- 50ppm/°C  |
| Common mode rejection  | 140dB         | 120dB         |
| Power accuracy         | 0.7%          | 1.6%          |
+------------------------+---------------+---------------+

As well as the missing DEVICE_ID register at 0x3F, which is currently
not in use by the driver.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 drivers/hwmon/ina238.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index f519c22d3907..ca9f5d2c811b 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -33,7 +33,7 @@
 #define INA238_BUS_UNDER_VOLTAGE	0xf
 #define INA238_TEMP_LIMIT		0x10
 #define INA238_POWER_LIMIT		0x11
-#define INA238_DEVICE_ID		0x3f
+#define INA238_DEVICE_ID		0x3f /* not available on INA237 */
 
 #define INA238_CONFIG_ADCRANGE		BIT(4)
 
@@ -622,6 +622,7 @@ static const struct i2c_device_id ina238_id[] = {
 MODULE_DEVICE_TABLE(i2c, ina238_id);
 
 static const struct of_device_id __maybe_unused ina238_of_match[] = {
+	{ .compatible = "ti,ina237" },
 	{ .compatible = "ti,ina238" },
 	{ },
 };

-- 
2.40.1

[PATCH 4/4] dt-bindings: hwmon: ti,ina238: add ti,ina237

[Richard Leitner]

Add ti,ina237 binding to ti,ina238 as they share the same driver.

Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
---
 Documentation/devicetree/bindings/hwmon/ti,ina238.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
index aba89e5f34b3..17408076696c 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
@@ -22,6 +22,7 @@ description: |
 properties:
   compatible:
     enum:
+      - ti,ina237
       - ti,ina238
 
   reg:

-- 
2.40.1

Re: [PATCH 4/4] dt-bindings: hwmon: ti,ina238: add ti,ina237

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

On Wed, Oct 25, 2023 at 10:34:14AM +0000, Richard Leitner wrote:
> Add ti,ina237 binding to ti,ina238 as they share the same driver.
> 
> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> ---
>  Documentation/devicetree/bindings/hwmon/ti,ina238.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> index aba89e5f34b3..17408076696c 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> @@ -22,6 +22,7 @@ description: |
>  properties:
>    compatible:
>      enum:
> +      - ti,ina237

The driver patch you have done implies no difference between the
programming model for both of these devices. It'd seem to make more sense
for the ina237 to fall back to the ina238, thereby requiring no change in
the driver to support it.

Cheers,
Conor.

>        - ti,ina238
>  
>    reg:
> 
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/4] dt-bindings: hwmon: ti,ina238: add ti,ina237

[Richard Leitner]

On Wed, Oct 25, 2023 at 02:58:44PM +0100, Conor Dooley wrote:
> On Wed, Oct 25, 2023 at 10:34:14AM +0000, Richard Leitner wrote:
> > Add ti,ina237 binding to ti,ina238 as they share the same driver.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
> > ---
> >  Documentation/devicetree/bindings/hwmon/ti,ina238.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> > index aba89e5f34b3..17408076696c 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
> > @@ -22,6 +22,7 @@ description: |
> >  properties:
> >    compatible:
> >      enum:
> > +      - ti,ina237
> 
> The driver patch you have done implies no difference between the
> programming model for both of these devices. It'd seem to make more sense
> for the ina237 to fall back to the ina238, thereby requiring no change in
> the driver to support it.

Thanks for the quick feedback, Conor.

I first thought of just mentioning the ina237 in the documentation as
"compatible" to the ina238. But IMHO it is better understandable if it's
listed as compatible.

And I would strongly encourage mentioning it somewhere (documentation or
compatible). So other people using the ina237 are able to find it and
don't have to compare the datasheets by themselves to find the right
driver.

> 
> Cheers,
> Conor.
> 
> >        - ti,ina238
> >  
> >    reg:
> > 
> > -- 
> > 2.40.1
> > 

Re: [PATCH 4/4] dt-bindings: hwmon: ti,ina238: add ti,ina237

[Krzysztof Kozlowski]

On 25/10/2023 16:13, Richard Leitner wrote:
> On Wed, Oct 25, 2023 at 02:58:44PM +0100, Conor Dooley wrote:
>> On Wed, Oct 25, 2023 at 10:34:14AM +0000, Richard Leitner wrote:
>>> Add ti,ina237 binding to ti,ina238 as they share the same driver.
>>>
>>> Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
>>> ---
>>>  Documentation/devicetree/bindings/hwmon/ti,ina238.yaml | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
>>> index aba89e5f34b3..17408076696c 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina238.yaml
>>> @@ -22,6 +22,7 @@ description: |
>>>  properties:
>>>    compatible:
>>>      enum:
>>> +      - ti,ina237
>>
>> The driver patch you have done implies no difference between the
>> programming model for both of these devices. It'd seem to make more sense
>> for the ina237 to fall back to the ina238, thereby requiring no change in
>> the driver to support it.
> 
> Thanks for the quick feedback, Conor.
> 
> I first thought of just mentioning the ina237 in the documentation as
> "compatible" to the ina238. But IMHO it is better understandable if it's
> listed as compatible.

Conor did not oppose listing it. The point is to use fall-back.

> 
> And I would strongly encourage mentioning it somewhere (documentation or
> compatible). So other people using the ina237 are able to find it and
> don't have to compare the datasheets by themselves to find the right
> driver.

Sure, there is plenty of space in the driver code (.c or Kconfig) to
document whatever you wish. We focus here on the bindings, though.

Best regards,
Krzysztof