[PATCH 00/17] hwmon: (ina238) Various improvements and added chip support

[PATCH 00/17] hwmon: (ina238) Various improvements and added chip support

[Guenter Roeck]

Add support for INA700 and INA780 to the ina238 driver.

To prepare for this, implement various improvements.

- Update documention and Kconfig entry to list all supported chips.

- Drop platform data support. The driver supports device properties,
  and there are no in-tree platform data users.

- Stop checking the attribute value when writing the power_max attribute
  as unnecessary.

- Simplify temperature calculations. Instead of shift and lsb, only
  require the resulution and use it to calculate temperatures.

- Pre-calculate voltage, current, power and energy LSB. The values don't
  change during runtime and can therefore be pre-calculated. Also use the
  equations provided in the dataasheets to calculate power and energy
  LSB from the current LSB instead of calculating it from scratch.

- Use ROUND_CLOSEST operations instead of divide operations to reduce
  rounding errors.

- Improve current dynamic range by matching shunt voltage and current
  register values. With that, the dynamic range is always the full 16 bit
  provided by the ADC.

- Stop using the shunt voltage register. With shunt and current register
  values now always matching, it is unnecessary to read both.

- Provide current limits from shunt voltage limit registers. After all,
  there is no difference for the ADC, so the shunt voltage limits translate
  into current limits.

- Order chip information alphabetically. No functional change, it just
  simplifies adding support for new chips.

- Add 64-bit energy attribute support to the hwmon core.

- Use the hwmon core to report 64-bit energy values.

- Add support for active-high alert polarity

- Limit shunt and calibration register writes to chips requiring/supporting
  it.

- Add support for INA700 and INA780. Both chips have internal shunt
  resistors and do not explicitly report the shunt voltage.

This patch series was inspired by Chris Packham's initial patch set of a
new INA780 driver, by his subsequent patch set adding support for that chip
to the ina238 driver, and by Christian Kahr's submission of a new INA700
driver.

The series was tested with INA228, INA237, INA238, and INA780 evaluation
boards as well as with unit test code.

----------------------------------------------------------------
Guenter Roeck (17):
      hwmon: (ina238) Drop platform data support
      hwmon: (ina238) Update documentation and Kconfig entry
      hwmon: (ina238) Drop pointless power attribute check on attribute writes
      hwmon: (ina238) Rework and simplify temperature calculations
      hwmon: (ina238) Pre-calculate current, power, and energy LSB
      hwmon: (ina238) Simplify voltage register accesses
      hwmon: (ina238) Improve current dynamic range
      hwmon: (ina238) Stop using the shunt voltage register
      hwmon: (ina238) Add support for current limits
      hwmon: (ina238) Order chip information alphabetically
      hwmon: Introduce 64-bit energy attribute support
      hwmon: (ina238) Use the energy64 attribute type to report the energy
      hwmon: (ina238) Support active-high alert polarity
      hwmon: (ina238) Only configure calibration and shunt registers if needed
      hwmon: (ina238) Add support for INA780
      dt-bindings: hwmon: ti,ina2xx: Add INA700
      hwmon: (ina238) Add support for INA700

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml       |   4 +
 Documentation/hwmon/hwmon-kernel-api.rst           |   3 +
 Documentation/hwmon/ina238.rst                     |  64 ++-
 drivers/hwmon/Kconfig                              |   9 +-
 drivers/hwmon/hwmon.c                              |  16 +-
 drivers/hwmon/ina238.c                             | 583 +++++++++++----------
 include/linux/hwmon.h                              |   1 +
 include/trace/events/hwmon.h                       |  10 +-
 8 files changed, 382 insertions(+), 308 deletions(-)

[PATCH 01/17] hwmon: (ina238) Drop platform data support

[Guenter Roeck]

There are no in-tree users of ina2xx platform data. Drop
support for it. The driver already supports device properties
which can be used as alternative if needed.

Also remove reference to the non-existing shunt_resistor sysfs
attribute from the driver documentation.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ina238.rst | 8 ++++----
 drivers/hwmon/ina238.c         | 8 ++------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 9a24da4786a4..9b830e37c986 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -29,10 +29,10 @@ The INA238 is a current shunt, power and temperature monitor with an I2C
 interface. It includes a number of programmable functions including alerts,
 conversion rate, sample averaging and selectable shunt voltage accuracy.
 
-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/ti,ina2xx.yaml for bindings
-if the device tree is used.
+The shunt value in micro-ohms can be set via device properties, either from
+platform code or from device tree data. Please refer to
+Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for bindings if
+device tree is used.
 
 Sysfs entries
 -------------
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 59a2c8889fa2..22e2b862fb33 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -16,8 +16,6 @@
 #include <linux/of.h>
 #include <linux/regmap.h>
 
-#include <linux/platform_data/ina2xx.h>
-
 /* INA238 register definitions */
 #define INA238_CONFIG			0x0
 #define INA238_ADC_CONFIG		0x1
@@ -745,7 +743,6 @@ ATTRIBUTE_GROUPS(ina238);
 
 static int ina238_probe(struct i2c_client *client)
 {
-	struct ina2xx_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device *dev = &client->dev;
 	struct device *hwmon_dev;
 	struct ina238_data *data;
@@ -772,9 +769,8 @@ static int ina238_probe(struct i2c_client *client)
 	}
 
 	/* load shunt value */
-	data->rshunt = INA238_RSHUNT_DEFAULT;
-	if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0 && pdata)
-		data->rshunt = pdata->shunt_uohms;
+	if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0)
+		data->rshunt = INA238_RSHUNT_DEFAULT;
 	if (data->rshunt == 0) {
 		dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
 		return -EINVAL;
-- 
2.45.2

[PATCH 02/17] hwmon: (ina238) Update documentation and Kconfig entry

[Guenter Roeck]

Update driver documentation and Kconfig entry to list all chips supported
by the driver.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ina238.rst | 39 ++++++++++++++++++++++++++--------
 drivers/hwmon/Kconfig          |  9 ++++----
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 9b830e37c986..1ac4e2c419ac 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -5,6 +5,24 @@ Kernel driver ina238
 
 Supported chips:
 
+  * Texas Instruments INA228
+
+    Prefix: 'ina228'
+
+    Addresses: I2C 0x40 - 0x4f
+
+    Datasheet:
+	https://www.ti.com/lit/gpn/ina228
+
+  * Texas Instruments INA237
+
+    Prefix: 'ina237'
+
+    Addresses: I2C 0x40 - 0x4f
+
+    Datasheet:
+	https://www.ti.com/lit/gpn/ina237
+
   * Texas Instruments INA238
 
     Prefix: 'ina238'
@@ -34,6 +52,13 @@ platform code or from device tree data. Please refer to
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for bindings if
 device tree is used.
 
+INA237 is a functionally equivalent variant of INA238 with slightly
+different accuracy. INA228 is another variant of INA238 with higher ADC
+resolution. This chip also reports the energy.
+
+SQ52206 is a mostly compatible chip from Sylergy. It reports the energy
+as well as the peak power consumption.
+
 Sysfs entries
 -------------
 
@@ -53,19 +78,15 @@ in1_max_alarm		Maximum shunt voltage alarm
 power1_input		Power measurement (uW)
 power1_max		Maximum power threshold (uW)
 power1_max_alarm	Maximum power alarm
+power1_input_highest	Peak Power (uW)
+				(SQ52206 only)
 
 curr1_input		Current measurement (mA)
 
+energy1_input		Energy measurement (uJ)
+				(SQ52206 and INA237 only)
+
 temp1_input		Die temperature measurement (mC)
 temp1_max		Maximum die temperature threshold (mC)
 temp1_max_alarm		Maximum die temperature alarm
 ======================= =======================================================
-
-Additional sysfs entries for sq52206
-------------------------------------
-
-======================= =======================================================
-energy1_input		Energy measurement (uJ)
-
-power1_input_highest	Peak Power (uW)
-======================= =======================================================
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9d28fcf7cd2a..23d51e61d993 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2252,13 +2252,14 @@ config SENSORS_INA2XX
 	  will be called ina2xx.
 
 config SENSORS_INA238
-	tristate "Texas Instruments INA238"
+	tristate "Texas Instruments INA238 and compatibles"
 	depends on I2C
 	select REGMAP_I2C
 	help
-	  If you say yes here you get support for the INA238 power monitor
-	  chip. This driver supports voltage, current, power and temperature
-	  measurements as well as alarm configuration.
+	  If you say yes here you get support for INA228, INA237, INA238, and
+	  SQ52206 power monitor chips. This driver supports voltage, current,
+	  power, energy, and temperature measurements as well as alarm
+	  configuration.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called ina238.
-- 
2.45.2

[PATCH 03/17] hwmon: (ina238) Drop pointless power attribute check on attribute writes

[Guenter Roeck]

There is only a single writeable power attribute, and it is very unlikely
that the chips supported by this driver will ever require another one.
That means checking for that attribute during runtime is unnecessary.
Drop the check. Rename the write function from ina238_write_power() to
ina238_write_power_max() to reflect that a single attribute is written.

No functional change intended.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 22e2b862fb33..23195dead74f 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -503,14 +503,11 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 	return 0;
 }
 
-static int ina238_write_power(struct device *dev, u32 attr, long val)
+static int ina238_write_power_max(struct device *dev, long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
 	long regval;
 
-	if (attr != hwmon_power_max)
-		return -EOPNOTSUPP;
-
 	/*
 	 * Unsigned postive values. Compared against the 24-bit power register,
 	 * lower 8-bits are truncated. Same conversion to/from uW as POWER
@@ -628,7 +625,7 @@ static int ina238_write(struct device *dev, enum hwmon_sensor_types type,
 		err = ina238_write_in(dev, attr, channel, val);
 		break;
 	case hwmon_power:
-		err = ina238_write_power(dev, attr, val);
+		err = ina238_write_power_max(dev, val);
 		break;
 	case hwmon_temp:
 		err = ina238_write_temp(dev, attr, val);
-- 
2.45.2

[PATCH 04/17] hwmon: (ina238) Rework and simplify temperature calculations

[Guenter Roeck]

The temperature register is 16 bit wide for all chips. The decimal point
is at the same location (bit 7 = 1 degree C). That means we can use the
resolution to calculate temperatures. Do that to simplify the code.

There is only a single writeable temperature attribute, and it is very
unlikely that the chips supported by this driver will ever require another
one. That means checking for that attribute in the write function is
unnecessary.  Drop the check. Rename the write function from
ina238_write_temp() to ina238_write_temp_max() to reflect that a single
attribute is written.

Also extend the accepted temperature value range to the range supported by
the chip registers. Limiting the accepted value range to the temperature
range supported by the chip would make it impossible to read an
out-of-range limit from the chip and to write the same value back into it.
This is undesirable, especially since the maximum temperature register does
contain the maximum register value after a chip reset, not the temperature
limit supported by the chip.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 52 +++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 23195dead74f..e386a0f83fbb 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -103,10 +103,7 @@
 
 #define INA238_SHUNT_VOLTAGE_LSB	5 /* 5 uV/lsb */
 #define INA238_BUS_VOLTAGE_LSB		3125 /* 3.125 mV/lsb */
-#define INA238_DIE_TEMP_LSB		1250000 /* 125.0000 mC/lsb */
 #define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
-#define SQ52206_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
-#define INA228_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 
 static const struct regmap_config ina238_regmap_config = {
 	.max_register = INA238_REGISTERS,
@@ -120,11 +117,10 @@ struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
 	bool has_power_highest;		/* chip detection power peak */
 	bool has_energy;		/* chip detection energy */
-	u8 temp_shift;			/* fixed parameters for temp calculate */
+	u8 temp_resolution;		/* temperature register resolution in bit */
 	u32 power_calculate_factor;	/* fixed parameters for power calculate */
 	u16 config_default;		/* Power-on default state */
 	int bus_voltage_lsb;		/* use for temperature calculate, uV/lsb */
-	int temp_lsb;			/* use for temperature calculate */
 };
 
 struct ina238_data {
@@ -141,41 +137,37 @@ static const struct ina238_config ina238_config[] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
-		.temp_shift = 4,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
-		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_resolution = 12,
 	},
 	[ina237] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
-		.temp_shift = 4,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
-		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_resolution = 12,
 	},
 	[sq52206] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = true,
 		.has_power_highest = true,
-		.temp_shift = 0,
 		.power_calculate_factor = 24,
 		.config_default = SQ52206_CONFIG_DEFAULT,
 		.bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB,
-		.temp_lsb = SQ52206_DIE_TEMP_LSB,
+		.temp_resolution = 16,
 	},
 	[ina228] = {
 		.has_20bit_voltage_current = true,
 		.has_energy = true,
 		.has_power_highest = false,
-		.temp_shift = 0,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
-		.temp_lsb = INA228_DIE_TEMP_LSB,
+		.temp_resolution = 16,
 	},
 };
 
@@ -522,6 +514,11 @@ static int ina238_write_power_max(struct device *dev, long val)
 	return regmap_write(data->regmap, INA238_POWER_LIMIT, regval);
 }
 
+static int ina238_temp_from_reg(s16 regval, u8 resolution)
+{
+	return ((regval >> (16 - resolution)) * 1000) >> (resolution - 9);
+}
+
 static int ina238_read_temp(struct device *dev, u32 attr, long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
@@ -533,17 +530,14 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val)
 		err = regmap_read(data->regmap, INA238_DIE_TEMP, &regval);
 		if (err)
 			return err;
-		/* Signed, result in mC */
-		*val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) *
-			       (s64)data->config->temp_lsb, 10000);
+		*val = ina238_temp_from_reg(regval, data->config->temp_resolution);
 		break;
 	case hwmon_temp_max:
 		err = regmap_read(data->regmap, INA238_TEMP_LIMIT, &regval);
 		if (err)
 			return err;
 		/* Signed, result in mC */
-		*val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) *
-			       (s64)data->config->temp_lsb, 10000);
+		*val = ina238_temp_from_reg(regval, data->config->temp_resolution);
 		break;
 	case hwmon_temp_max_alarm:
 		err = regmap_read(data->regmap, INA238_DIAG_ALERT, &regval);
@@ -559,19 +553,21 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val)
 	return 0;
 }
 
-static int ina238_write_temp(struct device *dev, u32 attr, long val)
+static u16 ina238_temp_to_reg(long val, u8 resolution)
+{
+	int fraction = 1000 - DIV_ROUND_CLOSEST(1000, BIT(resolution - 9));
+
+	val = clamp_val(val, -255000 - fraction, 255000 + fraction);
+
+	return (DIV_ROUND_CLOSEST(val << (resolution - 9), 1000) << (16 - resolution)) & 0xffff;
+}
+
+static int ina238_write_temp_max(struct device *dev, long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
 	int regval;
 
-	if (attr != hwmon_temp_max)
-		return -EOPNOTSUPP;
-
-	/* Signed */
-	val = clamp_val(val, -40000, 125000);
-	regval = div_s64(val * 10000, data->config->temp_lsb) << data->config->temp_shift;
-	regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift);
-
+	regval = ina238_temp_to_reg(val, data->config->temp_resolution);
 	return regmap_write(data->regmap, INA238_TEMP_LIMIT, regval);
 }
 
@@ -628,7 +624,7 @@ static int ina238_write(struct device *dev, enum hwmon_sensor_types type,
 		err = ina238_write_power_max(dev, val);
 		break;
 	case hwmon_temp:
-		err = ina238_write_temp(dev, attr, val);
+		err = ina238_write_temp_max(dev, val);
 		break;
 	default:
 		err = -EOPNOTSUPP;
-- 
2.45.2

[PATCH 05/17] hwmon: (ina238) Pre-calculate current, power, and energy LSB

[Guenter Roeck]

Current, power, and energy LSB do not change during runtime, so we can
pre-calculate the respective values. The power LSB can be derived from
the current LSB using the equation in the datasheets. Similar, the
energy LSB can be derived from the power LSB.

Also add support for chips with built-in shunt resistor by providing
a chip specific configuration parameter for the current LSB. The
relationship of current -> power -> energy LSB values in those chips
is the same as in chips with external shunt resistor, so configuration
parameters for power and energy LSB are not needed.

Use ROUND_CLOSEST functions instead of divide operations to reduce
rounding errors.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 47 ++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index e386a0f83fbb..316a7dc720f5 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -118,9 +118,10 @@ struct ina238_config {
 	bool has_power_highest;		/* chip detection power peak */
 	bool has_energy;		/* chip detection energy */
 	u8 temp_resolution;		/* temperature register resolution in bit */
-	u32 power_calculate_factor;	/* fixed parameters for power calculate */
+	u32 power_calculate_factor;	/* fixed parameter for power calculation, from datasheet */
 	u16 config_default;		/* Power-on default state */
 	int bus_voltage_lsb;		/* use for temperature calculate, uV/lsb */
+	int current_lsb;		/* current LSB, in uA */
 };
 
 struct ina238_data {
@@ -130,6 +131,9 @@ struct ina238_data {
 	struct regmap *regmap;
 	u32 rshunt;
 	int gain;
+	int current_lsb;		/* current LSB, in uA */
+	int power_lsb;			/* power LSB, in uW */
+	int energy_lsb;			/* energy LSB, in uJ */
 };
 
 static const struct ina238_config ina238_config[] = {
@@ -422,9 +426,8 @@ static int ina238_read_current(struct device *dev, u32 attr, long *val)
 			regval = (s16)regval;
 		}
 
-		/* Signed register, fixed 1mA current lsb. result in mA */
-		*val = div_s64((s64)regval * INA238_FIXED_SHUNT * data->gain,
-			       data->rshunt * 4);
+		/* Signed register. Result in mA */
+		*val = DIV_S64_ROUND_CLOSEST((s64)regval * data->current_lsb, 1000);
 
 		/* Account for 4 bit offset */
 		if (data->config->has_20bit_voltage_current)
@@ -450,9 +453,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 		if (err)
 			return err;
 
-		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT * data->gain *
-				data->config->power_calculate_factor, 4 * 100 * data->rshunt);
+		power = (long long)regval * data->power_lsb;
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -461,9 +462,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 		if (err)
 			return err;
 
-		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT * data->gain *
-				data->config->power_calculate_factor, 4 * 100 * data->rshunt);
+		power = (long long)regval * data->power_lsb;
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -476,8 +475,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 		 * Truncated 24-bit compare register, lower 8-bits are
 		 * truncated. Same conversion to/from uW as POWER register.
 		 */
-		power = div_u64((regval << 8) * 1000ULL * INA238_FIXED_SHUNT * data->gain *
-				data->config->power_calculate_factor, 4 * 100 * data->rshunt);
+		power = ((long long)regval << 8) * data->power_lsb;
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -498,7 +496,6 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 static int ina238_write_power_max(struct device *dev, long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
-	long regval;
 
 	/*
 	 * Unsigned postive values. Compared against the 24-bit power register,
@@ -506,12 +503,11 @@ static int ina238_write_power_max(struct device *dev, long val)
 	 * register.
 	 * The first clamp_val() is to establish a baseline to avoid overflows.
 	 */
-	regval = clamp_val(val, 0, LONG_MAX / 2);
-	regval = div_u64(regval * 4 * 100 * data->rshunt, data->config->power_calculate_factor *
-			1000ULL * INA238_FIXED_SHUNT * data->gain);
-	regval = clamp_val(regval >> 8, 0, U16_MAX);
+	val = clamp_val(val, 0, LONG_MAX / 2);
+	val = DIV_ROUND_CLOSEST(val, data->power_lsb);
+	val = clamp_val(val >> 8, 0, U16_MAX);
 
-	return regmap_write(data->regmap, INA238_POWER_LIMIT, regval);
+	return regmap_write(data->regmap, INA238_POWER_LIMIT, val);
 }
 
 static int ina238_temp_from_reg(s16 regval, u8 resolution)
@@ -584,8 +580,7 @@ static ssize_t energy1_input_show(struct device *dev,
 		return ret;
 
 	/* result in uJ */
-	energy = div_u64(regval * INA238_FIXED_SHUNT * data->gain * 16 * 10 *
-			 data->config->power_calculate_factor, 4 * data->rshunt);
+	energy = regval * data->energy_lsb;
 
 	return sysfs_emit(buf, "%llu\n", energy);
 }
@@ -817,6 +812,18 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	if (data->config->current_lsb)
+		data->current_lsb = data->config->current_lsb;
+	else
+		data->current_lsb = DIV_U64_ROUND_CLOSEST(250ULL * INA238_FIXED_SHUNT * data->gain,
+							  data->rshunt);
+
+	data->power_lsb = DIV_ROUND_CLOSEST(data->current_lsb *
+					    data->config->power_calculate_factor,
+					    100);
+
+	data->energy_lsb = data->power_lsb * 16;
+
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
 							 &ina238_chip_info,
 							 data->config->has_energy ?
-- 
2.45.2

[PATCH 06/17] hwmon: (ina238) Simplify voltage register accesses

[Guenter Roeck]

Calculate voltage LSB values in the probe function and use throughout
the code.

Use a single function to read all voltages, independently of the register
width. Use the pre-calculated LSB values to convert register values to
voltages and do not rely on runtime chip specific code.

Use ROUND_CLOSEST functions instead of divide operations to reduce
rounding errors.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 161 ++++++++++++++---------------------------
 1 file changed, 53 insertions(+), 108 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 316a7dc720f5..ae27ae2582f2 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -101,9 +101,11 @@
 #define INA238_CALIBRATION_VALUE	16384
 #define INA238_FIXED_SHUNT		20000
 
-#define INA238_SHUNT_VOLTAGE_LSB	5 /* 5 uV/lsb */
-#define INA238_BUS_VOLTAGE_LSB		3125 /* 3.125 mV/lsb */
-#define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
+#define INA238_SHUNT_VOLTAGE_LSB	5000	/* 5 uV/lsb, in nV */
+#define INA238_BUS_VOLTAGE_LSB		3125000	/* 3.125 mV/lsb, in nV */
+#define SQ52206_BUS_VOLTAGE_LSB		3750000	/* 3.75 mV/lsb, in nV */
+
+#define NUNIT_PER_MUNIT		1000000	/* n[AV] -> m[AV] */
 
 static const struct regmap_config ina238_regmap_config = {
 	.max_register = INA238_REGISTERS,
@@ -118,9 +120,9 @@ struct ina238_config {
 	bool has_power_highest;		/* chip detection power peak */
 	bool has_energy;		/* chip detection energy */
 	u8 temp_resolution;		/* temperature register resolution in bit */
-	u32 power_calculate_factor;	/* fixed parameter for power calculation, from datasheet */
 	u16 config_default;		/* Power-on default state */
-	int bus_voltage_lsb;		/* use for temperature calculate, uV/lsb */
+	u32 power_calculate_factor;	/* fixed parameter for power calculation, from datasheet */
+	u32 bus_voltage_lsb;		/* bus voltage LSB, in nV */
 	int current_lsb;		/* current LSB, in uA */
 };
 
@@ -131,6 +133,7 @@ struct ina238_data {
 	struct regmap *regmap;
 	u32 rshunt;
 	int gain;
+	u32 voltage_lsb[2];		/* shunt, bus voltage LSB, in nV */
 	int current_lsb;		/* current LSB, in uA */
 	int power_lsb;			/* power LSB, in uW */
 	int energy_lsb;			/* energy LSB, in uJ */
@@ -226,45 +229,28 @@ static int ina238_read_field_s20(const struct i2c_client *client, u8 reg, s32 *v
 	return 0;
 }
 
-static int ina228_read_shunt_voltage(struct device *dev, u32 attr, int channel,
-				     long *val)
+static int ina228_read_voltage(struct ina238_data *data, int channel, long *val)
 {
-	struct ina238_data *data = dev_get_drvdata(dev);
-	int regval;
-	int err;
+	int reg = channel ? INA238_BUS_VOLTAGE : INA238_SHUNT_VOLTAGE;
+	u32 lsb = data->voltage_lsb[channel];
+	u32 factor = NUNIT_PER_MUNIT;
+	int err, regval;
 
-	err = ina238_read_field_s20(data->client, INA238_SHUNT_VOLTAGE, &regval);
-	if (err)
-		return err;
+	if (data->config->has_20bit_voltage_current) {
+		err = ina238_read_field_s20(data->client, reg, &regval);
+		if (err)
+			return err;
+		/* Adjust accuracy: LSB in units of 500 pV */
+		lsb /= 8;
+		factor *= 2;
+	} else {
+		err = regmap_read(data->regmap, reg, &regval);
+		if (err)
+			return err;
+		regval = (s16)regval;
+	}
 
-	/*
-	 * gain of 1 -> LSB / 4
-	 * This field has 16 bit on ina238. ina228 adds another 4 bits of
-	 * precision. ina238 conversion factors can still be applied when
-	 * dividing by 16.
-	 */
-	*val = (regval * INA238_SHUNT_VOLTAGE_LSB) * data->gain / (1000 * 4) / 16;
-	return 0;
-}
-
-static int ina228_read_bus_voltage(struct device *dev, u32 attr, int channel,
-				   long *val)
-{
-	struct ina238_data *data = dev_get_drvdata(dev);
-	int regval;
-	int err;
-
-	err = ina238_read_field_s20(data->client, INA238_BUS_VOLTAGE, &regval);
-	if (err)
-		return err;
-
-	/*
-	 * gain of 1 -> LSB / 4
-	 * This field has 16 bit on ina238. ina228 adds another 4 bits of
-	 * precision. ina238 conversion factors can still be applied when
-	 * dividing by 16.
-	 */
-	*val = (regval * data->config->bus_voltage_lsb) / 1000 / 16;
+	*val = DIV_S64_ROUND_CLOSEST((s64)regval * lsb, factor);
 	return 0;
 }
 
@@ -272,18 +258,16 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 			  long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
-	int reg, mask;
+	int reg, mask = 0;
 	int regval;
 	int err;
 
+	if (attr == hwmon_in_input)
+		return ina228_read_voltage(data, channel, val);
+
 	switch (channel) {
 	case 0:
 		switch (attr) {
-		case hwmon_in_input:
-			if (data->config->has_20bit_voltage_current)
-				return ina228_read_shunt_voltage(dev, attr, channel, val);
-			reg = INA238_SHUNT_VOLTAGE;
-			break;
 		case hwmon_in_max:
 			reg = INA238_SHUNT_OVER_VOLTAGE;
 			break;
@@ -304,11 +288,6 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 		break;
 	case 1:
 		switch (attr) {
-		case hwmon_in_input:
-			if (data->config->has_20bit_voltage_current)
-				return ina228_read_bus_voltage(dev, attr, channel, val);
-			reg = INA238_BUS_VOLTAGE;
-			break;
 		case hwmon_in_max:
 			reg = INA238_BUS_OVER_VOLTAGE;
 			break;
@@ -335,72 +314,35 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 	if (err < 0)
 		return err;
 
-	switch (attr) {
-	case hwmon_in_input:
-	case hwmon_in_max:
-	case hwmon_in_min:
-		/* signed register, value in mV */
-		regval = (s16)regval;
-		if (channel == 0)
-			/* gain of 1 -> LSB / 4 */
-			*val = (regval * INA238_SHUNT_VOLTAGE_LSB) *
-				data->gain / (1000 * 4);
-		else
-			*val = (regval * data->config->bus_voltage_lsb) / 1000;
-		break;
-	case hwmon_in_max_alarm:
-	case hwmon_in_min_alarm:
+	if (mask)
 		*val = !!(regval & mask);
-		break;
-	}
+	else
+		*val = DIV_S64_ROUND_CLOSEST((s64)(s16)regval * data->voltage_lsb[channel],
+					     NUNIT_PER_MUNIT);
 
 	return 0;
 }
 
-static int ina238_write_in(struct device *dev, u32 attr, int channel,
-			   long val)
+static int ina238_write_in(struct device *dev, u32 attr, int channel, long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	static const int low_limits[2] = {-164, 0};
+	static const int high_limits[2] = {164, 150000};
+	static const u8 low_regs[2] = {INA238_SHUNT_UNDER_VOLTAGE, INA238_BUS_UNDER_VOLTAGE};
+	static const u8 high_regs[2] = {INA238_SHUNT_OVER_VOLTAGE, INA238_BUS_OVER_VOLTAGE};
 	int regval;
 
-	if (attr != hwmon_in_max && attr != hwmon_in_min)
-		return -EOPNOTSUPP;
+	/* Initial clamp to avoid overflows */
+	val = clamp_val(val, low_limits[channel], high_limits[channel]);
+	val = DIV_S64_ROUND_CLOSEST((s64)val * NUNIT_PER_MUNIT, data->voltage_lsb[channel]);
+	/* Final clamp to register limits */
+	regval = clamp_val(val, S16_MIN, S16_MAX) & 0xffff;
 
-	/* convert decimal to register value */
-	switch (channel) {
-	case 0:
-		/* signed value, clamp to max range +/-163 mV */
-		regval = clamp_val(val, -163, 163);
-		regval = (regval * 1000 * 4) /
-			 (INA238_SHUNT_VOLTAGE_LSB * data->gain);
-		regval = clamp_val(regval, S16_MIN, S16_MAX) & 0xffff;
-
-		switch (attr) {
-		case hwmon_in_max:
-			return regmap_write(data->regmap,
-					    INA238_SHUNT_OVER_VOLTAGE, regval);
-		case hwmon_in_min:
-			return regmap_write(data->regmap,
-					    INA238_SHUNT_UNDER_VOLTAGE, regval);
-		default:
-			return -EOPNOTSUPP;
-		}
-	case 1:
-		/* signed value, positive values only. Clamp to max 102.396 V */
-		regval = clamp_val(val, 0, 102396);
-		regval = (regval * 1000) / data->config->bus_voltage_lsb;
-		regval = clamp_val(regval, 0, S16_MAX);
-
-		switch (attr) {
-		case hwmon_in_max:
-			return regmap_write(data->regmap,
-					    INA238_BUS_OVER_VOLTAGE, regval);
-		case hwmon_in_min:
-			return regmap_write(data->regmap,
-					    INA238_BUS_UNDER_VOLTAGE, regval);
-		default:
-			return -EOPNOTSUPP;
-		}
+	switch (attr) {
+	case hwmon_in_min:
+		return regmap_write(data->regmap, low_regs[channel], regval);
+	case hwmon_in_max:
+		return regmap_write(data->regmap, high_regs[channel], regval);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -812,6 +754,9 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB * data->gain / 4;
+	data->voltage_lsb[1] = data->config->bus_voltage_lsb;
+
 	if (data->config->current_lsb)
 		data->current_lsb = data->config->current_lsb;
 	else
-- 
2.45.2

[PATCH 07/17] hwmon: (ina238) Improve current dynamic range

[Guenter Roeck]

The best possible dynamic range for current measurements is achieved
if the shunt register value matches the current register value. Adjust
the calibration register as well as fixed and default shunt resistor
values accordingly to achieve this range.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 51 ++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index ae27ae2582f2..c04481a8c643 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -51,7 +51,7 @@
 
 #define INA238_REGISTERS		0x20
 
-#define INA238_RSHUNT_DEFAULT		10000 /* uOhm */
+#define INA238_RSHUNT_DEFAULT		2500	/* uOhm */
 
 /* Default configuration of device on reset. */
 #define INA238_CONFIG_DEFAULT		0
@@ -67,39 +67,26 @@
  * relative to the shunt resistor value within the driver. This is similar to
  * how the ina2xx driver handles current/power scaling.
  *
- * The end result of this is that increasing shunt values (from a fixed 20 mOhm
- * shunt) increase the effective current/power accuracy whilst limiting the
- * range and decreasing shunt values decrease the effective accuracy but
- * increase the range.
+ * To achieve the best possible dynamic range, the value of the shunt voltage
+ * register should match the value of the current register. With that, the shunt
+ * voltage of 0x7fff = 32,767 uV = 163,785 uV matches the maximum current,
+ * and no accuracy is lost. Experiments with a real chip show that this is
+ * achieved by setting the SHUNT_CAL register to a value of 0x1000 = 4,096.
+ * Per datasheet,
+ *  SHUNT_CAL = 819.2 x 10^6 x CURRENT_LSB x Rshunt
+ *            = 819,200,000 x CURRENT_LSB x Rshunt
+ * With SHUNT_CAL set to 4,096, we get
+ *  CURRENT_LSB = 4,096 / (819,200,000 x Rshunt)
+ * Assuming an Rshunt value of 5 mOhm, we get
+ *  CURRENT_LSB = 4,096 / (819,200,000 x 0.005) = 1mA
+ * and thus a dynamic range of 1mA ... 32,767mA, which is sufficient for most
+ * applications. The actual dynamic range is of course determined by the actual
+ * shunt resistor value.
  *
- * The value of the Current register is calculated given the following:
- *   Current (A) = (shunt voltage register * 5) * calibration / 81920
- *
- * The maximum shunt voltage is 163.835 mV (0x7fff, ADC_RANGE = 0, gain = 4).
- * With the maximum current value of 0x7fff and a fixed shunt value results in
- * a calibration value of 16384 (0x4000).
- *
- *   0x7fff = (0x7fff * 5) * calibration / 81920
- *   calibration = 0x4000
- *
- * Equivalent calibration is applied for the Power register (maximum value for
- * bus voltage is 102396.875 mV, 0x7fff), where the maximum power that can
- * occur is ~16776192 uW (register value 0x147a8):
- *
- * This scaling means the resulting values for Current and Power registers need
- * to be scaled by the difference between the fixed shunt resistor and the
- * actual shunt resistor:
- *
- *  shunt = 0x4000 / (819.2 * 10^6) / 0.001 = 20000 uOhms (with 1mA/lsb)
- *
- *  Current (mA) = register value * 20000 / rshunt / 4 * gain
- *  Power (mW) = 0.2 * register value * 20000 / rshunt / 4 * gain
- *  (Specific for SQ52206)
- *  Power (mW) = 0.24 * register value * 20000 / rshunt / 4 * gain
- *  Energy (uJ) = 16 * 0.24 * register value * 20000 / rshunt / 4 * gain * 1000
+ * Power and energy values are scaled accordingly.
  */
-#define INA238_CALIBRATION_VALUE	16384
-#define INA238_FIXED_SHUNT		20000
+#define INA238_CALIBRATION_VALUE	4096
+#define INA238_FIXED_SHUNT		5000
 
 #define INA238_SHUNT_VOLTAGE_LSB	5000	/* 5 uV/lsb, in nV */
 #define INA238_BUS_VOLTAGE_LSB		3125000	/* 3.125 mV/lsb, in nV */
-- 
2.45.2

[PATCH 08/17] hwmon: (ina238) Stop using the shunt voltage register

[Guenter Roeck]

Since the value of the current register and the value of the shunt register
now match each other, it is no longer necessary to read the shunt voltage
register in the first place. Read the current register instead and use it
to calculate the shunt voltage.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index c04481a8c643..9dc94eccb750 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -218,7 +218,7 @@ static int ina238_read_field_s20(const struct i2c_client *client, u8 reg, s32 *v
 
 static int ina228_read_voltage(struct ina238_data *data, int channel, long *val)
 {
-	int reg = channel ? INA238_BUS_VOLTAGE : INA238_SHUNT_VOLTAGE;
+	int reg = channel ? INA238_BUS_VOLTAGE : INA238_CURRENT;
 	u32 lsb = data->voltage_lsb[channel];
 	u32 factor = NUNIT_PER_MUNIT;
 	int err, regval;
-- 
2.45.2

[PATCH 09/17] hwmon: (ina238) Add support for current limits

[Guenter Roeck]

Since the shunt voltage register and the current register now report the
same values, use the shunt voltage limit registers to report and adjust
current limits, using the same LSB as the LSB used for the actual current
register.

Handle current register accuracy differences in separate function to
improve code readability.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ina238.rst |   4 ++
 drivers/hwmon/ina238.c         | 105 ++++++++++++++++++++++++++-------
 2 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 1ac4e2c419ac..3c7db4a47056 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -82,6 +82,10 @@ power1_input_highest	Peak Power (uW)
 				(SQ52206 only)
 
 curr1_input		Current measurement (mA)
+curr1_min		Minimum current threshold (mA)
+curr1_min_alarm		Minimum current alarm
+curr1_max		Maximum current threshold (mA)
+curr1_max_alarm		Maximum current alarm
 
 energy1_input		Energy measurement (uJ)
 				(SQ52206 and INA237 only)
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 9dc94eccb750..97f12efcaef4 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -335,40 +335,92 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel, long val)
 	}
 }
 
-static int ina238_read_current(struct device *dev, u32 attr, long *val)
+static int __ina238_read_curr(struct ina238_data *data, long *val)
+{
+	u32 lsb = data->current_lsb;
+	int err, regval;
+
+	if (data->config->has_20bit_voltage_current) {
+		err = ina238_read_field_s20(data->client, INA238_CURRENT, &regval);
+		if (err)
+			return err;
+		lsb /= 16;	/* Adjust accuracy */
+	} else {
+		err = regmap_read(data->regmap, INA238_CURRENT, &regval);
+		if (err)
+			return err;
+		regval = (s16)regval;
+	}
+
+	*val = DIV_S64_ROUND_CLOSEST((s64)regval * lsb, 1000);
+	return 0;
+}
+
+static int ina238_read_curr(struct device *dev, u32 attr, long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	int reg, mask = 0;
 	int regval;
 	int err;
 
+	if (attr == hwmon_curr_input)
+		return __ina238_read_curr(data, val);
+
 	switch (attr) {
-	case hwmon_curr_input:
-		if (data->config->has_20bit_voltage_current) {
-			err = ina238_read_field_s20(data->client, INA238_CURRENT, &regval);
-			if (err)
-				return err;
-		} else {
-			err = regmap_read(data->regmap, INA238_CURRENT, &regval);
-			if (err < 0)
-				return err;
-			/* sign-extend */
-			regval = (s16)regval;
-		}
-
-		/* Signed register. Result in mA */
-		*val = DIV_S64_ROUND_CLOSEST((s64)regval * data->current_lsb, 1000);
-
-		/* Account for 4 bit offset */
-		if (data->config->has_20bit_voltage_current)
-			*val /= 16;
+	case hwmon_curr_min:
+		reg = INA238_SHUNT_UNDER_VOLTAGE;
+		break;
+	case hwmon_curr_min_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA238_DIAG_ALERT_SHNTUL;
+		break;
+	case hwmon_curr_max:
+		reg = INA238_SHUNT_OVER_VOLTAGE;
+		break;
+	case hwmon_curr_max_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA238_DIAG_ALERT_SHNTOL;
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
 
+	err = regmap_read(data->regmap, reg, &regval);
+	if (err < 0)
+		return err;
+
+	if (mask)
+		*val = !!(regval & mask);
+	else
+		*val = DIV_S64_ROUND_CLOSEST((s64)(s16)regval * data->current_lsb, 1000);
+
 	return 0;
 }
 
+static int ina238_write_curr(struct device *dev, u32 attr, long val)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	int regval;
+
+	/* Set baseline range to avoid over/underflows */
+	val = clamp_val(val, -1000000, 1000000);
+	/* Scale */
+	val = DIV_ROUND_CLOSEST(val * 1000, data->current_lsb);
+	/* Clamp to register size */
+	regval = clamp_val(val, S16_MIN, S16_MAX) & 0xffff;
+
+	switch (attr) {
+	case hwmon_curr_min:
+		return regmap_write(data->regmap, INA238_SHUNT_UNDER_VOLTAGE,
+				    regval);
+	case hwmon_curr_max:
+		return regmap_write(data->regmap, INA238_SHUNT_OVER_VOLTAGE,
+				    regval);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int ina238_read_power(struct device *dev, u32 attr, long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
@@ -521,7 +573,7 @@ static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
 	case hwmon_in:
 		return ina238_read_in(dev, attr, channel, val);
 	case hwmon_curr:
-		return ina238_read_current(dev, attr, val);
+		return ina238_read_curr(dev, attr, val);
 	case hwmon_power:
 		return ina238_read_power(dev, attr, val);
 	case hwmon_temp:
@@ -544,6 +596,9 @@ static int ina238_write(struct device *dev, enum hwmon_sensor_types type,
 	case hwmon_in:
 		err = ina238_write_in(dev, attr, channel, val);
 		break;
+	case hwmon_curr:
+		err = ina238_write_curr(dev, attr, val);
+		break;
 	case hwmon_power:
 		err = ina238_write_power_max(dev, val);
 		break;
@@ -582,7 +637,12 @@ static umode_t ina238_is_visible(const void *drvdata,
 	case hwmon_curr:
 		switch (attr) {
 		case hwmon_curr_input:
+		case hwmon_curr_max_alarm:
+		case hwmon_curr_min_alarm:
 			return 0444;
+		case hwmon_curr_max:
+		case hwmon_curr_min:
+			return 0644;
 		default:
 			return 0;
 		}
@@ -627,7 +687,8 @@ static const struct hwmon_channel_info * const ina238_info[] = {
 			   INA238_HWMON_IN_CONFIG),
 	HWMON_CHANNEL_INFO(curr,
 			   /* 0: current through shunt */
-			   HWMON_C_INPUT),
+			   HWMON_C_INPUT | HWMON_C_MIN | HWMON_C_MIN_ALARM |
+			   HWMON_C_MAX | HWMON_C_MAX_ALARM),
 	HWMON_CHANNEL_INFO(power,
 			   /* 0: power */
 			   HWMON_P_INPUT | HWMON_P_MAX |
-- 
2.45.2

[PATCH 10/17] hwmon: (ina238) Order chip information alphabetically

[Guenter Roeck]

Order chip type enum and chip configuration data alphabetically
to simplify adding support for additional chips.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 97f12efcaef4..4681325f58f0 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -100,7 +100,7 @@ static const struct regmap_config ina238_regmap_config = {
 	.val_bits = 16,
 };
 
-enum ina238_ids { ina238, ina237, sq52206, ina228 };
+enum ina238_ids { ina228, ina237, ina238, sq52206 };
 
 struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
@@ -127,7 +127,16 @@ struct ina238_data {
 };
 
 static const struct ina238_config ina238_config[] = {
-	[ina238] = {
+	[ina228] = {
+		.has_20bit_voltage_current = true,
+		.has_energy = true,
+		.has_power_highest = false,
+		.power_calculate_factor = 20,
+		.config_default = INA238_CONFIG_DEFAULT,
+		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
+		.temp_resolution = 16,
+	},
+	[ina237] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
@@ -136,7 +145,7 @@ static const struct ina238_config ina238_config[] = {
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_resolution = 12,
 	},
-	[ina237] = {
+	[ina238] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
@@ -154,15 +163,6 @@ static const struct ina238_config ina238_config[] = {
 		.bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB,
 		.temp_resolution = 16,
 	},
-	[ina228] = {
-		.has_20bit_voltage_current = true,
-		.has_energy = true,
-		.has_power_highest = false,
-		.power_calculate_factor = 20,
-		.config_default = INA238_CONFIG_DEFAULT,
-		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
-		.temp_resolution = 16,
-	},
 };
 
 static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
-- 
2.45.2

[PATCH 11/17] hwmon: Introduce 64-bit energy attribute support

[Guenter Roeck]

Many chips require 64-bit variables to display the accumulated energy,
even more so since the energy units are micro-Joule. Add new sensor type
"energy64" to support reporting the chip energy as 64-bit values.

Changing the entire hardware monitoring API is not feasible, and it is only
really necessary to support reading 64-bit values for the "energyX_input"
attribute. For this reason, keep the API as-is and use type casts on both
ends to pass 64-bit pointers when reading the accumulated energy. On the
write side (which is only useful for the energyX_enable attribute), keep
passing the written value as long.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/hwmon-kernel-api.rst |  3 +++
 drivers/hwmon/hwmon.c                    | 16 +++++++++++-----
 include/linux/hwmon.h                    |  1 +
 include/trace/events/hwmon.h             | 10 +++++-----
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index e47fc757e63e..fcc61171f36e 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -159,6 +159,7 @@ It contains following fields:
      hwmon_curr		Current sensor
      hwmon_power		Power sensor
      hwmon_energy	Energy sensor
+     hwmon_energy64	Energy sensor, reported as 64-bit signed value
      hwmon_humidity	Humidity sensor
      hwmon_fan		Fan speed sensor
      hwmon_pwm		PWM control
@@ -288,6 +289,8 @@ Parameters:
 		The sensor channel number.
 	val:
 		Pointer to attribute value.
+		For hwmon_energy64, 'val' is passed as long * but needs
+		a typecast to s64 *.
 
 Return value:
 	0 on success, a negative error number otherwise.
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 1688c210888a..2e17f3a4c59b 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -426,18 +426,22 @@ static ssize_t hwmon_attr_show(struct device *dev,
 			       struct device_attribute *devattr, char *buf)
 {
 	struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
+	s64 val64;
 	long val;
 	int ret;
 
 	ret = hattr->ops->read(dev, hattr->type, hattr->attr, hattr->index,
-			       &val);
+			       (hattr->type == hwmon_energy64) ? (long *)&val64 : &val);
 	if (ret < 0)
 		return ret;
 
-	trace_hwmon_attr_show(hattr->index + hwmon_attr_base(hattr->type),
-			      hattr->name, val);
+	if (hattr->type != hwmon_energy64)
+		val64 = val;
 
-	return sprintf(buf, "%ld\n", val);
+	trace_hwmon_attr_show(hattr->index + hwmon_attr_base(hattr->type),
+			      hattr->name, val64);
+
+	return sprintf(buf, "%lld\n", val64);
 }
 
 static ssize_t hwmon_attr_show_string(struct device *dev,
@@ -478,7 +482,7 @@ static ssize_t hwmon_attr_store(struct device *dev,
 		return ret;
 
 	trace_hwmon_attr_store(hattr->index + hwmon_attr_base(hattr->type),
-			       hattr->name, val);
+			       hattr->name, (s64)val);
 
 	return count;
 }
@@ -734,6 +738,7 @@ static const char * const *__templates[] = {
 	[hwmon_curr] = hwmon_curr_attr_templates,
 	[hwmon_power] = hwmon_power_attr_templates,
 	[hwmon_energy] = hwmon_energy_attr_templates,
+	[hwmon_energy64] = hwmon_energy_attr_templates,
 	[hwmon_humidity] = hwmon_humidity_attr_templates,
 	[hwmon_fan] = hwmon_fan_attr_templates,
 	[hwmon_pwm] = hwmon_pwm_attr_templates,
@@ -747,6 +752,7 @@ static const int __templates_size[] = {
 	[hwmon_curr] = ARRAY_SIZE(hwmon_curr_attr_templates),
 	[hwmon_power] = ARRAY_SIZE(hwmon_power_attr_templates),
 	[hwmon_energy] = ARRAY_SIZE(hwmon_energy_attr_templates),
+	[hwmon_energy64] = ARRAY_SIZE(hwmon_energy_attr_templates),
 	[hwmon_humidity] = ARRAY_SIZE(hwmon_humidity_attr_templates),
 	[hwmon_fan] = ARRAY_SIZE(hwmon_fan_attr_templates),
 	[hwmon_pwm] = ARRAY_SIZE(hwmon_pwm_attr_templates),
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 3a63dff62d03..886fc90b2d25 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -24,6 +24,7 @@ enum hwmon_sensor_types {
 	hwmon_curr,
 	hwmon_power,
 	hwmon_energy,
+	hwmon_energy64,
 	hwmon_humidity,
 	hwmon_fan,
 	hwmon_pwm,
diff --git a/include/trace/events/hwmon.h b/include/trace/events/hwmon.h
index d1ff560cd9b5..3865098f21f1 100644
--- a/include/trace/events/hwmon.h
+++ b/include/trace/events/hwmon.h
@@ -9,14 +9,14 @@
 
 DECLARE_EVENT_CLASS(hwmon_attr_class,
 
-	TP_PROTO(int index, const char *attr_name, long val),
+	TP_PROTO(int index, const char *attr_name, long long val),
 
 	TP_ARGS(index, attr_name, val),
 
 	TP_STRUCT__entry(
 		__field(int, index)
 		__string(attr_name, attr_name)
-		__field(long, val)
+		__field(long long, val)
 	),
 
 	TP_fast_assign(
@@ -25,20 +25,20 @@ DECLARE_EVENT_CLASS(hwmon_attr_class,
 		__entry->val = val;
 	),
 
-	TP_printk("index=%d, attr_name=%s, val=%ld",
+	TP_printk("index=%d, attr_name=%s, val=%lld",
 		  __entry->index,  __get_str(attr_name), __entry->val)
 );
 
 DEFINE_EVENT(hwmon_attr_class, hwmon_attr_show,
 
-	TP_PROTO(int index, const char *attr_name, long val),
+	TP_PROTO(int index, const char *attr_name, long long val),
 
 	TP_ARGS(index, attr_name, val)
 );
 
 DEFINE_EVENT(hwmon_attr_class, hwmon_attr_store,
 
-	TP_PROTO(int index, const char *attr_name, long val),
+	TP_PROTO(int index, const char *attr_name, long long val),
 
 	TP_ARGS(index, attr_name, val)
 );
-- 
2.45.2

[PATCH 12/17] hwmon: (ina238) Use the energy64 attribute type to report the energy

[Guenter Roeck]

Use the energy64 attribute type instead of locally defined sysfs attributes
to report the accumulated energy.

No functional change intended.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 4681325f58f0..4d5b383b2521 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -548,22 +548,19 @@ static int ina238_write_temp_max(struct device *dev, long val)
 	return regmap_write(data->regmap, INA238_TEMP_LIMIT, regval);
 }
 
-static ssize_t energy1_input_show(struct device *dev,
-				  struct device_attribute *da, char *buf)
+static int ina238_read_energy(struct device *dev, s64 *energy)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
-	int ret;
 	u64 regval;
-	u64 energy;
+	int ret;
 
 	ret = ina238_read_reg40(data->client, SQ52206_ENERGY, &regval);
 	if (ret)
 		return ret;
 
 	/* result in uJ */
-	energy = regval * data->energy_lsb;
-
-	return sysfs_emit(buf, "%llu\n", energy);
+	*energy = regval * data->energy_lsb;
+	return 0;
 }
 
 static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
@@ -576,6 +573,8 @@ static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
 		return ina238_read_curr(dev, attr, val);
 	case hwmon_power:
 		return ina238_read_power(dev, attr, val);
+	case hwmon_energy64:
+		return ina238_read_energy(dev, (s64 *)val);
 	case hwmon_temp:
 		return ina238_read_temp(dev, attr, val);
 	default:
@@ -620,6 +619,7 @@ static umode_t ina238_is_visible(const void *drvdata,
 {
 	const struct ina238_data *data = drvdata;
 	bool has_power_highest = data->config->has_power_highest;
+	bool has_energy = data->config->has_energy;
 
 	switch (type) {
 	case hwmon_in:
@@ -660,6 +660,11 @@ static umode_t ina238_is_visible(const void *drvdata,
 		default:
 			return 0;
 		}
+	case hwmon_energy64:
+		/* hwmon_energy_input */
+		if (has_energy)
+			return 0444;
+		return 0;
 	case hwmon_temp:
 		switch (attr) {
 		case hwmon_temp_input:
@@ -693,6 +698,8 @@ static const struct hwmon_channel_info * const ina238_info[] = {
 			   /* 0: power */
 			   HWMON_P_INPUT | HWMON_P_MAX |
 			   HWMON_P_MAX_ALARM | HWMON_P_INPUT_HIGHEST),
+	HWMON_CHANNEL_INFO(energy64,
+			   HWMON_E_INPUT),
 	HWMON_CHANNEL_INFO(temp,
 			   /* 0: die temperature */
 			   HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_ALARM),
@@ -710,15 +717,6 @@ static const struct hwmon_chip_info ina238_chip_info = {
 	.info = ina238_info,
 };
 
-/* energy attributes are 5 bytes wide so we need u64 */
-static DEVICE_ATTR_RO(energy1_input);
-
-static struct attribute *ina238_attrs[] = {
-	&dev_attr_energy1_input.attr,
-	NULL,
-};
-ATTRIBUTE_GROUPS(ina238);
-
 static int ina238_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -818,9 +816,7 @@ static int ina238_probe(struct i2c_client *client)
 	data->energy_lsb = data->power_lsb * 16;
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
-							 &ina238_chip_info,
-							 data->config->has_energy ?
-								ina238_groups : NULL);
+							 &ina238_chip_info, NULL);
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
-- 
2.45.2

[PATCH 13/17] hwmon: (ina238) Support active-high alert polarity

[Guenter Roeck]

All chips supported by this driver support configurable active-high
alert priority. This is already documented in the devicetree description.
Add support for it to the driver.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 4d5b383b2521..24e396c69ae2 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -60,6 +60,7 @@
 #define INA238_ADC_CONFIG_DEFAULT	0xfb6a
 /* Configure alerts to be based on averaged value (SLOWALERT) */
 #define INA238_DIAG_ALERT_DEFAULT	0x2000
+#define INA238_DIAG_ALERT_APOL		BIT(12)
 /*
  * This driver uses a fixed calibration value in order to scale current/power
  * based on a fixed shunt resistor value. This allows for conversion within the
@@ -793,8 +794,11 @@ static int ina238_probe(struct i2c_client *client)
 	}
 
 	/* Setup alert/alarm configuration */
-	ret = regmap_write(data->regmap, INA238_DIAG_ALERT,
-			   INA238_DIAG_ALERT_DEFAULT);
+	config = INA238_DIAG_ALERT_DEFAULT;
+	if (device_property_read_bool(dev, "ti,alert-polarity-active-high"))
+		config |= INA238_DIAG_ALERT_APOL;
+
+	ret = regmap_write(data->regmap, INA238_DIAG_ALERT, config);
 	if (ret < 0) {
 		dev_err(dev, "error configuring the device: %d\n", ret);
 		return -ENODEV;
-- 
2.45.2

[PATCH 14/17] hwmon: (ina238) Only configure calibration and shunt registers if needed

[Guenter Roeck]

Prepare for supporting chips with internal shunt resistor by only setting
calibration and shunt resistor registers if no current LSB is configured.

Do not display a log message during probe if a chip does not have shunt
and gain registers since those would otherwise display 0, and a message
just indicating that the driver was loaded would be just noise.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina238.c | 82 +++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 24e396c69ae2..da5b43184dd1 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -745,32 +745,48 @@ static int ina238_probe(struct i2c_client *client)
 		return PTR_ERR(data->regmap);
 	}
 
-	/* load shunt value */
-	if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0)
-		data->rshunt = INA238_RSHUNT_DEFAULT;
-	if (data->rshunt == 0) {
-		dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
-		return -EINVAL;
-	}
-
-	/* load shunt gain value */
-	if (device_property_read_u32(dev, "ti,shunt-gain", &data->gain) < 0)
-		data->gain = 4; /* Default of ADCRANGE = 0 */
-	if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
-		dev_err(dev, "invalid shunt gain value %u\n", data->gain);
-		return -EINVAL;
-	}
-
 	/* Setup CONFIG register */
 	config = data->config->config_default;
-	if (chip == sq52206) {
-		if (data->gain == 1)
-			config |= SQ52206_CONFIG_ADCRANGE_HIGH; /* ADCRANGE = 10/11 is /1 */
-		else if (data->gain == 2)
-			config |= SQ52206_CONFIG_ADCRANGE_LOW; /* ADCRANGE = 01 is /2 */
-	} else if (data->gain == 1) {
-		config |= INA238_CONFIG_ADCRANGE; /* ADCRANGE = 1 is /1 */
+	if (data->config->current_lsb) {
+		data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB;
+		data->current_lsb = data->config->current_lsb;
+	} else {
+		/* load shunt value */
+		if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0)
+			data->rshunt = INA238_RSHUNT_DEFAULT;
+		if (data->rshunt == 0) {
+			dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
+			return -EINVAL;
+		}
+
+		/* load shunt gain value */
+		if (device_property_read_u32(dev, "ti,shunt-gain", &data->gain) < 0)
+			data->gain = 4;	/* Default of ADCRANGE = 0 */
+		if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
+			dev_err(dev, "invalid shunt gain value %u\n", data->gain);
+			return -EINVAL;
+		}
+
+		/* Setup SHUNT_CALIBRATION register with fixed value */
+		ret = regmap_write(data->regmap, INA238_SHUNT_CALIBRATION,
+				   INA238_CALIBRATION_VALUE);
+		if (ret < 0) {
+			dev_err(dev, "error configuring the device: %d\n", ret);
+			return -ENODEV;
+		}
+		if (chip == sq52206) {
+			if (data->gain == 1)		/* ADCRANGE = 10/11 is /1 */
+				config |= SQ52206_CONFIG_ADCRANGE_HIGH;
+			else if (data->gain == 2)	/* ADCRANGE = 01 is /2 */
+				config |= SQ52206_CONFIG_ADCRANGE_LOW;
+		} else if (data->gain == 1) {		/* ADCRANGE = 1 is /1 */
+			config |= INA238_CONFIG_ADCRANGE;
+		}
+		data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB * data->gain / 4;
+		data->current_lsb = DIV_U64_ROUND_CLOSEST(250ULL * INA238_FIXED_SHUNT * data->gain,
+							  data->rshunt);
 	}
+
 	ret = regmap_write(data->regmap, INA238_CONFIG, config);
 	if (ret < 0) {
 		dev_err(dev, "error configuring the device: %d\n", ret);
@@ -785,14 +801,6 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
-	/* Setup SHUNT_CALIBRATION register with fixed value */
-	ret = regmap_write(data->regmap, INA238_SHUNT_CALIBRATION,
-			   INA238_CALIBRATION_VALUE);
-	if (ret < 0) {
-		dev_err(dev, "error configuring the device: %d\n", ret);
-		return -ENODEV;
-	}
-
 	/* Setup alert/alarm configuration */
 	config = INA238_DIAG_ALERT_DEFAULT;
 	if (device_property_read_bool(dev, "ti,alert-polarity-active-high"))
@@ -804,15 +812,8 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
-	data->voltage_lsb[0] = INA238_SHUNT_VOLTAGE_LSB * data->gain / 4;
 	data->voltage_lsb[1] = data->config->bus_voltage_lsb;
 
-	if (data->config->current_lsb)
-		data->current_lsb = data->config->current_lsb;
-	else
-		data->current_lsb = DIV_U64_ROUND_CLOSEST(250ULL * INA238_FIXED_SHUNT * data->gain,
-							  data->rshunt);
-
 	data->power_lsb = DIV_ROUND_CLOSEST(data->current_lsb *
 					    data->config->power_calculate_factor,
 					    100);
@@ -824,8 +825,9 @@ static int ina238_probe(struct i2c_client *client)
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
-	dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
-		 client->name, data->rshunt, data->gain);
+	if (data->rshunt)
+		dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
+			 client->name, data->rshunt, data->gain);
 
 	return 0;
 }
-- 
2.45.2

[PATCH 15/17] hwmon: (ina238) Add support for INA780

[Guenter Roeck]

INA780 is similar to the other chips in the series, but does not
support the shunt voltage register. Shunt voltage limit registers
have been renamed to current limit registers, but are otherwise
identical.

While the chip does not directly report the shunt voltage, report
it anyway by calculating its value from the current register.

Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ina238.rst | 10 +++++++++-
 drivers/hwmon/Kconfig          |  6 +++---
 drivers/hwmon/ina238.c         | 17 ++++++++++++++++-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 3c7db4a47056..722760961821 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -32,6 +32,11 @@ Supported chips:
     Datasheet:
 	https://www.ti.com/lit/gpn/ina238
 
+  * Texas Instruments INA780
+
+    Datasheet:
+	https://www.ti.com/product/ina780a
+
   * Silergy SQ52206
 
     Prefix: 'SQ52206'
@@ -56,6 +61,9 @@ INA237 is a functionally equivalent variant of INA238 with slightly
 different accuracy. INA228 is another variant of INA238 with higher ADC
 resolution. This chip also reports the energy.
 
+INA780 is a variant of the chip series with built-in shunt resistor.
+It also reports the energy.
+
 SQ52206 is a mostly compatible chip from Sylergy. It reports the energy
 as well as the peak power consumption.
 
@@ -88,7 +96,7 @@ curr1_max		Maximum current threshold (mA)
 curr1_max_alarm		Maximum current alarm
 
 energy1_input		Energy measurement (uJ)
-				(SQ52206 and INA237 only)
+				(SQ52206, INA237, and INA780 only)
 
 temp1_input		Die temperature measurement (mC)
 temp1_max		Maximum die temperature threshold (mC)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 23d51e61d993..b0f440011478 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2256,9 +2256,9 @@ config SENSORS_INA238
 	depends on I2C
 	select REGMAP_I2C
 	help
-	  If you say yes here you get support for INA228, INA237, INA238, and
-	  SQ52206 power monitor chips. This driver supports voltage, current,
-	  power, energy, and temperature measurements as well as alarm
+	  If you say yes here you get support for INA228, INA237, INA238,
+	  INA780, and SQ52206 power monitor chips. This driver supports voltage,
+	  current, power, energy, and temperature measurements as well as alarm
 	  configuration.
 
 	  This driver can also be built as a module. If so, the module
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index da5b43184dd1..98255619adeb 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -101,7 +101,7 @@ static const struct regmap_config ina238_regmap_config = {
 	.val_bits = 16,
 };
 
-enum ina238_ids { ina228, ina237, ina238, sq52206 };
+enum ina238_ids { ina228, ina237, ina238, ina780, sq52206 };
 
 struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
@@ -155,6 +155,16 @@ static const struct ina238_config ina238_config[] = {
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_resolution = 12,
 	},
+	[ina780] = {
+		.has_20bit_voltage_current = false,
+		.has_energy = true,
+		.has_power_highest = false,
+		.power_calculate_factor = 20,
+		.config_default = INA238_CONFIG_DEFAULT,
+		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
+		.temp_resolution = 12,
+		.current_lsb = 2400,
+	},
 	[sq52206] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = true,
@@ -836,6 +846,7 @@ static const struct i2c_device_id ina238_id[] = {
 	{ "ina228", ina228 },
 	{ "ina237", ina237 },
 	{ "ina238", ina238 },
+	{ "ina780", ina780 },
 	{ "sq52206", sq52206 },
 	{ }
 };
@@ -854,6 +865,10 @@ static const struct of_device_id __maybe_unused ina238_of_match[] = {
 		.compatible = "ti,ina238",
 		.data = (void *)ina238
 	},
+	{
+		.compatible = "ti,ina780",
+		.data = (void *)ina780
+	},
 	{
 		.compatible = "silergy,sq52206",
 		.data = (void *)sq52206
-- 
2.45.2

[PATCH 16/17] dt-bindings: hwmon: ti,ina2xx: Add INA700

[Guenter Roeck]

Add a compatible string for INA700. The chip is register compatible with
INA780 but implements different ADC ranges and thus needs a separate
compatible entry.

Cc: Christian Kahr <christian.kahr@sie.at>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index 8b491be9c49d..d3cde8936686 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -32,6 +32,7 @@ properties:
       - ti,ina237
       - ti,ina238
       - ti,ina260
+      - ti,ina700
       - ti,ina780
 
   reg:
@@ -115,6 +116,7 @@ allOf:
               - ti,ina237
               - ti,ina238
               - ti,ina260
+              - ti,ina700
               - ti,ina780
     then:
       properties:
@@ -133,6 +135,7 @@ allOf:
               - ti,ina230
               - ti,ina231
               - ti,ina260
+              - ti,ina700
               - ti,ina780
     then:
       properties:
@@ -143,6 +146,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - ti,ina700
               - ti,ina780
     then:
       properties:
-- 
2.45.2

[PATCH 17/17] hwmon: (ina238) Add support for INA700

[Guenter Roeck]

INA700 is register compatible to INA780 but has different current, power,
and energy LSB values.

While the chip does not directly report the shunt voltage, report
it anyway by calculating its value from the current register.

Cc: Christian Kahr <christian.kahr@sie.at>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ina238.rst |  9 +++++++--
 drivers/hwmon/Kconfig          |  6 +++---
 drivers/hwmon/ina238.c         | 17 ++++++++++++++++-
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 722760961821..43950d1ec551 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -32,6 +32,11 @@ Supported chips:
     Datasheet:
 	https://www.ti.com/lit/gpn/ina238
 
+  * Texas Instruments INA700
+
+    Datasheet:
+	https://www.ti.com/product/ina700
+
   * Texas Instruments INA780
 
     Datasheet:
@@ -61,8 +66,8 @@ INA237 is a functionally equivalent variant of INA238 with slightly
 different accuracy. INA228 is another variant of INA238 with higher ADC
 resolution. This chip also reports the energy.
 
-INA780 is a variant of the chip series with built-in shunt resistor.
-It also reports the energy.
+INA700 and INA780 are variants of the chip series with built-in shunt resistor.
+They also report the energy.
 
 SQ52206 is a mostly compatible chip from Sylergy. It reports the energy
 as well as the peak power consumption.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b0f440011478..af3107c9ca4a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2257,9 +2257,9 @@ config SENSORS_INA238
 	select REGMAP_I2C
 	help
 	  If you say yes here you get support for INA228, INA237, INA238,
-	  INA780, and SQ52206 power monitor chips. This driver supports voltage,
-	  current, power, energy, and temperature measurements as well as alarm
-	  configuration.
+	  INA700, INA780, and SQ52206 power monitor chips. This driver supports
+	  voltage, current, power, energy, and temperature measurements as well
+	  as alarm configuration.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called ina238.
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 98255619adeb..356d19b7675c 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -101,7 +101,7 @@ static const struct regmap_config ina238_regmap_config = {
 	.val_bits = 16,
 };
 
-enum ina238_ids { ina228, ina237, ina238, ina780, sq52206 };
+enum ina238_ids { ina228, ina237, ina238, ina700, ina780, sq52206 };
 
 struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
@@ -155,6 +155,16 @@ static const struct ina238_config ina238_config[] = {
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_resolution = 12,
 	},
+	[ina700] = {
+		.has_20bit_voltage_current = false,
+		.has_energy = true,
+		.has_power_highest = false,
+		.power_calculate_factor = 20,
+		.config_default = INA238_CONFIG_DEFAULT,
+		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
+		.temp_resolution = 12,
+		.current_lsb = 480,
+	},
 	[ina780] = {
 		.has_20bit_voltage_current = false,
 		.has_energy = true,
@@ -846,6 +856,7 @@ static const struct i2c_device_id ina238_id[] = {
 	{ "ina228", ina228 },
 	{ "ina237", ina237 },
 	{ "ina238", ina238 },
+	{ "ina700", ina700 },
 	{ "ina780", ina780 },
 	{ "sq52206", sq52206 },
 	{ }
@@ -865,6 +876,10 @@ static const struct of_device_id __maybe_unused ina238_of_match[] = {
 		.compatible = "ti,ina238",
 		.data = (void *)ina238
 	},
+	{
+		.compatible = "ti,ina700",
+		.data = (void *)ina700
+	},
 	{
 		.compatible = "ti,ina780",
 		.data = (void *)ina780
-- 
2.45.2

Re: [PATCH 00/17] hwmon: (ina238) Various improvements and added chip support

[Chris Packham]

Hi Guenter,

On 06/09/2025 08:41, Guenter Roeck wrote:
> Add support for INA700 and INA780 to the ina238 driver.
>
> To prepare for this, implement various improvements.
>
> - Update documention and Kconfig entry to list all supported chips.
>
> - Drop platform data support. The driver supports device properties,
>    and there are no in-tree platform data users.
>
> - Stop checking the attribute value when writing the power_max attribute
>    as unnecessary.
>
> - Simplify temperature calculations. Instead of shift and lsb, only
>    require the resulution and use it to calculate temperatures.
>
> - Pre-calculate voltage, current, power and energy LSB. The values don't
>    change during runtime and can therefore be pre-calculated. Also use the
>    equations provided in the dataasheets to calculate power and energy
>    LSB from the current LSB instead of calculating it from scratch.
>
> - Use ROUND_CLOSEST operations instead of divide operations to reduce
>    rounding errors.
>
> - Improve current dynamic range by matching shunt voltage and current
>    register values. With that, the dynamic range is always the full 16 bit
>    provided by the ADC.
>
> - Stop using the shunt voltage register. With shunt and current register
>    values now always matching, it is unnecessary to read both.
>
> - Provide current limits from shunt voltage limit registers. After all,
>    there is no difference for the ADC, so the shunt voltage limits translate
>    into current limits.
>
> - Order chip information alphabetically. No functional change, it just
>    simplifies adding support for new chips.
>
> - Add 64-bit energy attribute support to the hwmon core.
>
> - Use the hwmon core to report 64-bit energy values.
>
> - Add support for active-high alert polarity
>
> - Limit shunt and calibration register writes to chips requiring/supporting
>    it.
>
> - Add support for INA700 and INA780. Both chips have internal shunt
>    resistors and do not explicitly report the shunt voltage.
>
> This patch series was inspired by Chris Packham's initial patch set of a
> new INA780 driver, by his subsequent patch set adding support for that chip
> to the ina238 driver, and by Christian Kahr's submission of a new INA700
> driver.
>
> The series was tested with INA228, INA237, INA238, and INA780 evaluation
> boards as well as with unit test code.
>
> ----------------------------------------------------------------
> Guenter Roeck (17):
>        hwmon: (ina238) Drop platform data support
>        hwmon: (ina238) Update documentation and Kconfig entry
>        hwmon: (ina238) Drop pointless power attribute check on attribute writes
>        hwmon: (ina238) Rework and simplify temperature calculations
>        hwmon: (ina238) Pre-calculate current, power, and energy LSB
>        hwmon: (ina238) Simplify voltage register accesses
>        hwmon: (ina238) Improve current dynamic range
>        hwmon: (ina238) Stop using the shunt voltage register
>        hwmon: (ina238) Add support for current limits
>        hwmon: (ina238) Order chip information alphabetically
>        hwmon: Introduce 64-bit energy attribute support
>        hwmon: (ina238) Use the energy64 attribute type to report the energy
>        hwmon: (ina238) Support active-high alert polarity
>        hwmon: (ina238) Only configure calibration and shunt registers if needed
>        hwmon: (ina238) Add support for INA780
>        dt-bindings: hwmon: ti,ina2xx: Add INA700
>        hwmon: (ina238) Add support for INA700
>
>   .../devicetree/bindings/hwmon/ti,ina2xx.yaml       |   4 +
>   Documentation/hwmon/hwmon-kernel-api.rst           |   3 +
>   Documentation/hwmon/ina238.rst                     |  64 ++-
>   drivers/hwmon/Kconfig                              |   9 +-
>   drivers/hwmon/hwmon.c                              |  16 +-
>   drivers/hwmon/ina238.c                             | 583 +++++++++++----------
>   include/linux/hwmon.h                              |   1 +
>   include/trace/events/hwmon.h                       |  10 +-
>   8 files changed, 382 insertions(+), 308 deletions(-)

For the series

Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz> # INA780

Re: [PATCH 00/17] hwmon: (ina238) Various improvements and added chip support

[Guenter Roeck]

Hi Chris,

On 9/7/25 16:00, Chris Packham wrote:
> Hi Guenter,
> 
> On 06/09/2025 08:41, Guenter Roeck wrote:
>> Add support for INA700 and INA780 to the ina238 driver.
>>
>> To prepare for this, implement various improvements.
>>
>> - Update documention and Kconfig entry to list all supported chips.
>>
>> - Drop platform data support. The driver supports device properties,
>>     and there are no in-tree platform data users.
>>
>> - Stop checking the attribute value when writing the power_max attribute
>>     as unnecessary.
>>
>> - Simplify temperature calculations. Instead of shift and lsb, only
>>     require the resulution and use it to calculate temperatures.
>>
>> - Pre-calculate voltage, current, power and energy LSB. The values don't
>>     change during runtime and can therefore be pre-calculated. Also use the
>>     equations provided in the dataasheets to calculate power and energy
>>     LSB from the current LSB instead of calculating it from scratch.
>>
>> - Use ROUND_CLOSEST operations instead of divide operations to reduce
>>     rounding errors.
>>
>> - Improve current dynamic range by matching shunt voltage and current
>>     register values. With that, the dynamic range is always the full 16 bit
>>     provided by the ADC.
>>
>> - Stop using the shunt voltage register. With shunt and current register
>>     values now always matching, it is unnecessary to read both.
>>
>> - Provide current limits from shunt voltage limit registers. After all,
>>     there is no difference for the ADC, so the shunt voltage limits translate
>>     into current limits.
>>
>> - Order chip information alphabetically. No functional change, it just
>>     simplifies adding support for new chips.
>>
>> - Add 64-bit energy attribute support to the hwmon core.
>>
>> - Use the hwmon core to report 64-bit energy values.
>>
>> - Add support for active-high alert polarity
>>
>> - Limit shunt and calibration register writes to chips requiring/supporting
>>     it.
>>
>> - Add support for INA700 and INA780. Both chips have internal shunt
>>     resistors and do not explicitly report the shunt voltage.
>>
>> This patch series was inspired by Chris Packham's initial patch set of a
>> new INA780 driver, by his subsequent patch set adding support for that chip
>> to the ina238 driver, and by Christian Kahr's submission of a new INA700
>> driver.
>>
>> The series was tested with INA228, INA237, INA238, and INA780 evaluation
>> boards as well as with unit test code.
>>
>> ----------------------------------------------------------------
>> Guenter Roeck (17):
>>         hwmon: (ina238) Drop platform data support
>>         hwmon: (ina238) Update documentation and Kconfig entry
>>         hwmon: (ina238) Drop pointless power attribute check on attribute writes
>>         hwmon: (ina238) Rework and simplify temperature calculations
>>         hwmon: (ina238) Pre-calculate current, power, and energy LSB
>>         hwmon: (ina238) Simplify voltage register accesses
>>         hwmon: (ina238) Improve current dynamic range
>>         hwmon: (ina238) Stop using the shunt voltage register
>>         hwmon: (ina238) Add support for current limits
>>         hwmon: (ina238) Order chip information alphabetically
>>         hwmon: Introduce 64-bit energy attribute support
>>         hwmon: (ina238) Use the energy64 attribute type to report the energy
>>         hwmon: (ina238) Support active-high alert polarity
>>         hwmon: (ina238) Only configure calibration and shunt registers if needed
>>         hwmon: (ina238) Add support for INA780
>>         dt-bindings: hwmon: ti,ina2xx: Add INA700
>>         hwmon: (ina238) Add support for INA700
>>
>>    .../devicetree/bindings/hwmon/ti,ina2xx.yaml       |   4 +
>>    Documentation/hwmon/hwmon-kernel-api.rst           |   3 +
>>    Documentation/hwmon/ina238.rst                     |  64 ++-
>>    drivers/hwmon/Kconfig                              |   9 +-
>>    drivers/hwmon/hwmon.c                              |  16 +-
>>    drivers/hwmon/ina238.c                             | 583 +++++++++++----------
>>    include/linux/hwmon.h                              |   1 +
>>    include/trace/events/hwmon.h                       |  10 +-
>>    8 files changed, 382 insertions(+), 308 deletions(-)
> 
> For the series
> 
> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz> # INA780

Thanks a lot, appreciate it!

Guenter