| Message ID | 20200224232647.29213-1-forstmayr.franz@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/3] dt-bindings: hwmon: Add compatible for ti,ina260 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On 2/24/20 3:26 PM, Franz Forstmayr wrote: > Add documentation for INA260, power/current monitor with I2C interface. > Subject should match description here (this patch does not add support for ina2xx). > Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> > --- > Documentation/hwmon/ina2xx.rst | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst > index 94b9a260c518..74267dd433dd 100644 > --- a/Documentation/hwmon/ina2xx.rst > +++ b/Documentation/hwmon/ina2xx.rst > @@ -53,6 +53,16 @@ Supported chips: > > http://www.ti.com/ > > + * Texas Instruments INA260 > + > + Prefix: 'ina260' > + > + Addresses: I2C 0x40 - 0x4f > + > + Datasheet: Publicly available at the Texas Instruments website > + > + http://www.ti.com/ > + > Author: Lothar Felten <lothar.felten@gmail.com> > > Description > @@ -72,14 +82,17 @@ INA230 and INA231 are high or low side current shunt and power monitors > with an I2C interface. The chips monitor both a shunt voltage drop and > bus supply voltage. > > +INA260 is a high or low side current and power monitor with an integrated > +shunt and I2C interface. > + > The shunt value in micro-ohms can be set via platform data or device tree at > compile-time or via the shunt_resistor attribute in sysfs at run-time. Please > refer to the Documentation/devicetree/bindings/hwmon/ina2xx.txt for bindings > if the device tree is used. > > -Additionally ina226 supports update_interval attribute as described in > -Documentation/hwmon/sysfs-interface.rst. Internally the interval is the sum of > -bus and shunt voltage conversion times multiplied by the averaging rate. We > +Additionally ina226 and ina260 supports update_interval attribute as described s/supports/support/ > +in Documentation/hwmon/sysfs-interface.rst. Internally the interval is the sum > +of bus and shunt voltage conversion times multiplied by the averaging rate. We > don't touch the conversion times and only modify the number of averages. The > lower limit of the update_interval is 2 ms, the upper limit is 2253 ms. > The actual programmed interval may vary from the desired value. >
On 2/24/20 3:26 PM, Franz Forstmayr wrote: > Add initial support for INA260 power monitor with integrated shunt. > Registers are different from other INA2xx devices, that's why a small > translation table is used. > > Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> I think the chip is sufficiently different to other chips that a separate driver would make much more sense than adding support to the existing driver. There is no calibration, registers are different, the retry logic is not needed. A new driver could use the with_info API and would be much simpler while at the same time not messing up the existing driver. Guenter
On Tue, 25 Feb 2020 00:26:45 +0100, Franz Forstmayr wrote: > Add initial support for power/current monitor INA260 > > Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> > --- > Documentation/devicetree/bindings/hwmon/ina2xx.txt | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring <robh@kernel.org>
On 26. 02. 20 3:16, Guenter Roeck wrote: > On 2/24/20 3:26 PM, Franz Forstmayr wrote: >> Add initial support for INA260 power monitor with integrated shunt. >> Registers are different from other INA2xx devices, that's why a small >> translation table is used. >> >> Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> > > I think the chip is sufficiently different to other chips that a separate > driver would make much more sense than adding support to the existing > driver. > There is no calibration, registers are different, the retry logic is > not needed. A new driver could use the with_info API and would be much > simpler while at the same time not messing up the existing driver. Isn't it also better to switch to IIO framework? As we discussed in past there are two ina226 drivers. One in hwmon and second based on IIO framework (more advance one?) and would be good to deprecate hwmon one. That's why separate driver is necessary. Thanks, Michal
On 5/18/20 10:21 PM, Michal Simek wrote: > On 26. 02. 20 3:16, Guenter Roeck wrote: >> On 2/24/20 3:26 PM, Franz Forstmayr wrote: >>> Add initial support for INA260 power monitor with integrated shunt. >>> Registers are different from other INA2xx devices, that's why a small >>> translation table is used. >>> >>> Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> >> >> I think the chip is sufficiently different to other chips that a separate >> driver would make much more sense than adding support to the existing >> driver. >> There is no calibration, registers are different, the retry logic is >> not needed. A new driver could use the with_info API and would be much >> simpler while at the same time not messing up the existing driver. > > Isn't it also better to switch to IIO framework? > As we discussed in past there are two ina226 drivers. One in hwmon and > second based on IIO framework (more advance one?) and would be good to > deprecate hwmon one. "More advanced" is relative. The ina2xx driver in iio doesn't support alert limits (which is queued in the hwmon driver for 5.8), and the iio->hwmon bridge doesn't support it either. On top of that, there are existing users of the hwmon driver, which would have to be converted first. As for ina260, it would be up to the implementer to determine if alert limit support is needed or not, and which API would be appropriate for the intended use case. Guenter
On 19. 05. 20 16:14, Guenter Roeck wrote: > On 5/18/20 10:21 PM, Michal Simek wrote: >> On 26. 02. 20 3:16, Guenter Roeck wrote: >>> On 2/24/20 3:26 PM, Franz Forstmayr wrote: >>>> Add initial support for INA260 power monitor with integrated shunt. >>>> Registers are different from other INA2xx devices, that's why a small >>>> translation table is used. >>>> >>>> Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> >>> >>> I think the chip is sufficiently different to other chips that a separate >>> driver would make much more sense than adding support to the existing >>> driver. >>> There is no calibration, registers are different, the retry logic is >>> not needed. A new driver could use the with_info API and would be much >>> simpler while at the same time not messing up the existing driver. >> >> Isn't it also better to switch to IIO framework? >> As we discussed in past there are two ina226 drivers. One in hwmon and >> second based on IIO framework (more advance one?) and would be good to >> deprecate hwmon one. > > "More advanced" is relative. The ina2xx driver in iio doesn't support > alert limits (which is queued in the hwmon driver for 5.8), and the > iio->hwmon bridge doesn't support it either. On top of that, there are > existing users of the hwmon driver, which would have to be converted > first. As for ina260, it would be up to the implementer to determine > if alert limit support is needed or not, and which API would be > appropriate for the intended use case. Good to know. If ina260 is done as separate driver I am fine with it. Thanks, Michal
diff --git a/Documentation/devicetree/bindings/hwmon/ina2xx.txt b/Documentation/devicetree/bindings/hwmon/ina2xx.txt index 02af0d94e921..92983a224109 100644 --- a/Documentation/devicetree/bindings/hwmon/ina2xx.txt +++ b/Documentation/devicetree/bindings/hwmon/ina2xx.txt @@ -8,6 +8,7 @@ Required properties: - "ti,ina226" for ina226 - "ti,ina230" for ina230 - "ti,ina231" for ina231 + - "ti,ina260" for ina260 - reg: I2C address Optional properties:
Add initial support for power/current monitor INA260 Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com> --- Documentation/devicetree/bindings/hwmon/ina2xx.txt | 1 + 1 file changed, 1 insertion(+)