| Message ID | 20231025-ina237-v1-2-a0196119720c@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: add ti,ina237 support to ina238 driver | expand |
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 >
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 > >
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?
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
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 >
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.
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
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
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 >
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
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
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(+)