* [PATCH v3 0/4] hwmon: Add support for INA780
@ 2025-08-29 3:05 Chris Packham
2025-08-29 3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29 3:05 UTC (permalink / raw)
To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham
This series adds support for the INA780 to the existing ina238.c driver.
v2: https://lore.kernel.org/linux-hwmon/20250808030510.552724-1-chris.packham@alliedtelesis.co.nz/
v1: https://lore.kernel.org/linux-hwmon/20250806005127.542298-1-chris.packham@alliedtelesis.co.nz/
One important bit of feedback I've not addressed is this
> Follow-up: I ordered evaluation boards for INA228, INA237, INA238, and INA780A.
> I'll want to see support added for current limits on all chips, using a similar
> approach as the one in the ina2xx driver. After all, the shunt voltage limits
> are really current limits in disguise. With that and appropriate chip specific
> parameters the differences between the chips should become relatively minor.
>
> This should also simplify adding support for INA700 which seems similar to INA780A.
I wasn't sure if that meant you were going to look at adding current limits to
the existing chips and I should wait or if you wanted me to try based on the
code. Given our timezone differences I figured I'd send the series anyway.
Patch 2 is a bugfix that you might want to pick up sooner.
Chris Packham (4):
dt-bindings: hwmon: ti,ina2xx: Add INA780 device
hwmon: (ina238) Correctly clamp temperature
hwmon: (ina238) Add ina238_config fields
hwmon: (ina238) Add support for INA780
.../devicetree/bindings/hwmon/ti,ina2xx.yaml | 1 +
Documentation/hwmon/ina238.rst | 20 ++
drivers/hwmon/ina238.c | 273 ++++++++++++++----
3 files changed, 238 insertions(+), 56 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device
2025-08-29 3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
@ 2025-08-29 3:05 ` Chris Packham
2025-08-31 22:35 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-29 3:05 UTC (permalink / raw)
To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham
Add a compatible string for the INA780 device.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Notes:
Changes in v3:
- Collect ack from Conor
Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index fa68b99ef2e2..980ecba6d811 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,ina780
reg:
maxItems: 1
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
2025-08-29 3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
2025-08-29 3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
@ 2025-08-29 3:05 ` Chris Packham
2025-08-29 9:55 ` Guenter Roeck
2025-08-31 22:34 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
2025-08-29 3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham
3 siblings, 2 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29 3:05 UTC (permalink / raw)
To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham
ina238_write_temp() was attempting to clamp the user input but was
throwing away the result. Ensure that we clamp the value to the
appropriate range before it is converted into a register value.
Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Notes:
Changes in v3:
- New. Split off bugfix from main patch
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 5a394eeff676..4d3dc018ead9 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
return -EOPNOTSUPP;
/* Signed */
- regval = clamp_val(val, -40000, 125000);
+ 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);
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
2025-08-29 3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
2025-08-29 3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
2025-08-29 3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
@ 2025-08-29 3:05 ` Chris Packham
2025-08-29 15:56 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham
3 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-29 3:05 UTC (permalink / raw)
To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham
In preparation for adding INA780 support add some required fields to
ina238_config and set the appropriate values for the existing chips.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Notes:
Changes in v3:
- New. Split config struct changes from main patch
drivers/hwmon/ina238.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 4d3dc018ead9..930e12e64079 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -120,13 +120,17 @@ enum ina238_ids { ina238, ina237, sq52206, ina228 };
struct ina238_config {
bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
+ bool has_shunt; /* has shunt resistor */
bool has_power_highest; /* chip detection power peak */
bool has_energy; /* chip detection energy */
+ bool has_curr_min_max; /* supports COL/CUL */
u8 temp_shift; /* fixed parameters for temp calculate */
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 */
+ int temp_max; /* maximum configurable temp limit in mC */
+ int fixed_power_lsb; /* fixed power LSB value */
};
struct ina238_data {
@@ -141,43 +145,59 @@ struct ina238_data {
static const struct ina238_config ina238_config[] = {
[ina238] = {
.has_20bit_voltage_current = false,
+ .has_shunt = true,
.has_energy = false,
.has_power_highest = false,
+ .has_curr_min_max = 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_max = 125000,
+ .fixed_power_lsb = 0,
},
[ina237] = {
.has_20bit_voltage_current = false,
+ .has_shunt = true,
.has_energy = false,
.has_power_highest = false,
+ .has_curr_min_max = 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_max = 125000,
+ .fixed_power_lsb = 0,
},
[sq52206] = {
.has_20bit_voltage_current = false,
+ .has_shunt = true,
.has_energy = true,
.has_power_highest = true,
+ .has_curr_min_max = false,
.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_max = 125000,
+ .fixed_power_lsb = 0,
},
[ina228] = {
.has_20bit_voltage_current = true,
+ .has_shunt = true,
.has_energy = true,
.has_power_highest = false,
+ .has_curr_min_max = 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_max = 125000,
+ .fixed_power_lsb = 0,
},
};
@@ -572,7 +592,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
return -EOPNOTSUPP;
/* Signed */
- val = clamp_val(val, -40000, 125000);
+ val = clamp_val(val, -40000, data->config->temp_max);
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);
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/4] hwmon: (ina238) Add support for INA780
2025-08-29 3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
` (2 preceding siblings ...)
2025-08-29 3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
@ 2025-08-29 3:05 ` Chris Packham
3 siblings, 0 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29 3:05 UTC (permalink / raw)
To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham
Add support for the TI INA780 Digital Power Monitor. The INA780 uses
EZShunt(tm) technology, which means there are fixed LSB conversions for
a number of fields rather than needing to be calibrated.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Notes:
Changes in v3:
- Rebase on top of master and resolve conflicts with commit fd470f4ed80c
("hwmon: (ina238) Add support for INA228")
- Use INA238_SHUNT_OVER/UNDER_VOLTAGE register definitions with comment
about COL/CUL.
- Move some code to prepartory patches
Documentation/hwmon/ina238.rst | 20 +++
drivers/hwmon/ina238.c | 251 +++++++++++++++++++++++++--------
2 files changed, 216 insertions(+), 55 deletions(-)
diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 9a24da4786a4..220d36d5c947 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -14,6 +14,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'
@@ -69,3 +74,18 @@ energy1_input Energy measurement (uJ)
power1_input_highest Peak Power (uW)
======================= =======================================================
+
+Sysfs differences for ina780
+----------------------------
+
+======================= =======================================================
+in0_input Bus voltage (mV)
+in0_min Minimum bus voltage threshold (mV)
+in0_min_alarm Minimum shunt voltage alarm
+in0_max Maximum bus voltage threshold (mV)
+in0_max_alarm Maximum shunt voltage alarm
+curr1_max Maximum current threshold
+curr1_max_alarm Maximum current alarm
+curr1_min Minimum current threshold
+curr1_min_alarm Minimum current alarm
+======================= =======================================================
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 930e12e64079..ad1b36762ca3 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -2,6 +2,7 @@
/*
* Driver for Texas Instruments INA238 power monitor chip
* Datasheet: https://www.ti.com/product/ina238
+ * https://www.ti.com/product/ina780a
*
* Copyright (C) 2021 Nathan Rossi <nathan.rossi@digi.com>
*/
@@ -31,8 +32,8 @@
#define SQ52206_ENERGY 0x9
#define SQ52206_CHARGE 0xa
#define INA238_DIAG_ALERT 0xb
-#define INA238_SHUNT_OVER_VOLTAGE 0xc
-#define INA238_SHUNT_UNDER_VOLTAGE 0xd
+#define INA238_SHUNT_OVER_VOLTAGE 0xc /* COL on INA780 */
+#define INA238_SHUNT_UNDER_VOLTAGE 0xd /* CUL on INA780 */
#define INA238_BUS_OVER_VOLTAGE 0xe
#define INA238_BUS_UNDER_VOLTAGE 0xf
#define INA238_TEMP_LIMIT 0x10
@@ -45,8 +46,8 @@
#define SQ52206_CONFIG_ADCRANGE_LOW BIT(3)
#define INA238_DIAG_ALERT_TMPOL BIT(7)
-#define INA238_DIAG_ALERT_SHNTOL BIT(6)
-#define INA238_DIAG_ALERT_SHNTUL BIT(5)
+#define INA238_DIAG_ALERT_SHNTOL BIT(6) /* CURRENTOL on INA780 */
+#define INA238_DIAG_ALERT_SHNTUL BIT(5) /* CURRENTUL on INA780 */
#define INA238_DIAG_ALERT_BUSOL BIT(4)
#define INA238_DIAG_ALERT_BUSUL BIT(3)
#define INA238_DIAG_ALERT_POL BIT(2)
@@ -110,13 +111,16 @@
#define SQ52206_DIE_TEMP_LSB 78125 /* 7.8125 mC/lsb */
#define INA228_DIE_TEMP_LSB 78125 /* 7.8125 mC/lsb */
+#define INA780_CURRENT_LSB 2400 /* 2.4 mA/lsb */
+#define INA780_ENERGY_LSB 7680 /* 7.68 mJ/lsb */
+
static const struct regmap_config ina238_regmap_config = {
.max_register = INA238_REGISTERS,
.reg_bits = 8,
.val_bits = 16,
};
-enum ina238_ids { ina238, ina237, sq52206, ina228 };
+enum ina238_ids { ina238, ina237, sq52206, ina228, ina780 };
struct ina238_config {
bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
@@ -199,6 +203,19 @@ static const struct ina238_config ina238_config[] = {
.temp_max = 125000,
.fixed_power_lsb = 0,
},
+ [ina780] = {
+ .has_20bit_voltage_current = false,
+ .has_shunt = false,
+ .has_energy = true,
+ .has_power_highest = false,
+ .temp_shift = 4,
+ .config_default = INA238_CONFIG_DEFAULT,
+ .bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
+ .temp_lsb = INA238_DIE_TEMP_LSB,
+ .temp_max = 150000,
+ .fixed_power_lsb = 480,
+ .has_curr_min_max = true,
+ }
};
static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
@@ -298,12 +315,12 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
long *val)
{
struct ina238_data *data = dev_get_drvdata(dev);
+ bool has_shunt = data->config->has_shunt;
int reg, mask;
int regval;
int err;
- switch (channel) {
- case 0:
+ if (has_shunt && channel == 0) {
switch (attr) {
case hwmon_in_input:
if (data->config->has_20bit_voltage_current)
@@ -327,8 +344,7 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
default:
return -EOPNOTSUPP;
}
- break;
- case 1:
+ } else {
switch (attr) {
case hwmon_in_input:
if (data->config->has_20bit_voltage_current)
@@ -352,9 +368,6 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
default:
return -EOPNOTSUPP;
}
- break;
- default:
- return -EOPNOTSUPP;
}
err = regmap_read(data->regmap, reg, ®val);
@@ -367,7 +380,7 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
case hwmon_in_min:
/* signed register, value in mV */
regval = (s16)regval;
- if (channel == 0)
+ if (has_shunt && channel == 0)
/* gain of 1 -> LSB / 4 */
*val = (regval * INA238_SHUNT_VOLTAGE_LSB) *
data->gain / (1000 * 4);
@@ -387,14 +400,14 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
long val)
{
struct ina238_data *data = dev_get_drvdata(dev);
+ bool has_shunt = data->config->has_shunt;
int regval;
if (attr != hwmon_in_max && attr != hwmon_in_min)
return -EOPNOTSUPP;
/* convert decimal to register value */
- switch (channel) {
- case 0:
+ if (has_shunt && channel == 0) {
/* signed value, clamp to max range +/-163 mV */
regval = clamp_val(val, -163, 163);
regval = (regval * 1000 * 4) /
@@ -411,7 +424,7 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
default:
return -EOPNOTSUPP;
}
- case 1:
+ } else {
/* 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;
@@ -427,8 +440,6 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
default:
return -EOPNOTSUPP;
}
- default:
- return -EOPNOTSUPP;
}
}
@@ -467,9 +478,86 @@ static int ina238_read_current(struct device *dev, u32 attr, long *val)
return 0;
}
+static int ina780_read_current(struct device *dev, u32 attr, long *val)
+{
+ struct ina238_data *data = dev_get_drvdata(dev);
+ unsigned int regval;
+ int reg, mask;
+ int err;
+
+ switch (attr) {
+ case hwmon_curr_input:
+ reg = INA238_CURRENT;
+ break;
+ case hwmon_curr_max:
+ reg = INA238_SHUNT_OVER_VOLTAGE;
+ break;
+ case hwmon_curr_min:
+ reg = INA238_SHUNT_UNDER_VOLTAGE;
+ break;
+ case hwmon_curr_max_alarm:
+ reg = INA238_DIAG_ALERT;
+ mask = INA238_DIAG_ALERT_SHNTOL;
+ break;
+ case hwmon_curr_min_alarm:
+ reg = INA238_DIAG_ALERT;
+ mask = INA238_DIAG_ALERT_SHNTUL;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ switch (attr) {
+ case hwmon_curr_input:
+ case hwmon_curr_max:
+ case hwmon_curr_min:
+ err = regmap_read(data->regmap, reg, ®val);
+ if (err)
+ return err;
+ *val = div_s64((s16)regval * INA780_CURRENT_LSB, 1000);
+ break;
+ case hwmon_curr_max_alarm:
+ case hwmon_curr_min_alarm:
+ err = regmap_read(data->regmap, reg, ®val);
+ if (err)
+ return err;
+ *val = !!(regval & mask);
+ break;
+ }
+
+ return 0;
+}
+
+static int ina780_write_current(struct device *dev, u32 attr, long val)
+{
+ struct ina238_data *data = dev_get_drvdata(dev);
+ bool has_curr_min_max = data->config->has_curr_min_max;
+ unsigned int regval;
+ int reg;
+
+ if (!has_curr_min_max)
+ return -EOPNOTSUPP;
+
+ switch (attr) {
+ case hwmon_curr_max:
+ reg = INA238_SHUNT_OVER_VOLTAGE;
+ break;
+ case hwmon_curr_min:
+ reg = INA238_SHUNT_UNDER_VOLTAGE;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ clamp_val(val, -78643, 78640);
+ regval = div_s64(val * 1000ULL, INA780_CURRENT_LSB);
+
+ return regmap_write(data->regmap, reg, regval);
+}
+
static int ina238_read_power(struct device *dev, u32 attr, long *val)
{
struct ina238_data *data = dev_get_drvdata(dev);
+ long long fixed_power_lsb = data->config->fixed_power_lsb;
long long power;
int regval;
int err;
@@ -480,9 +568,14 @@ 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);
+ if (fixed_power_lsb)
+ power = div_u64(regval * fixed_power_lsb, 1000);
+ else
+ /* 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);
+
/* Clamp value to maximum value of long */
*val = clamp_val(power, 0, LONG_MAX);
break;
@@ -506,8 +599,12 @@ 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);
+ if (fixed_power_lsb)
+ power = div_u64((regval << 8) * fixed_power_lsb * 256, 1000);
+ else
+ power = div_u64((regval << 8) * 1000ULL * INA238_FIXED_SHUNT * data->gain *
+ data->config->power_calculate_factor,
+ 4 * 100 * data->rshunt);
/* Clamp value to maximum value of long */
*val = clamp_val(power, 0, LONG_MAX);
break;
@@ -528,6 +625,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
static int ina238_write_power(struct device *dev, u32 attr, long val)
{
struct ina238_data *data = dev_get_drvdata(dev);
+ int fixed_power_lsb = data->config->fixed_power_lsb;
long regval;
if (attr != hwmon_power_max)
@@ -539,8 +637,12 @@ static int ina238_write_power(struct device *dev, u32 attr, long val)
* register.
*/
regval = clamp_val(val, 0, LONG_MAX);
- regval = div_u64(val * 4 * 100 * data->rshunt, data->config->power_calculate_factor *
- 1000ULL * INA238_FIXED_SHUNT * data->gain);
+ if (fixed_power_lsb)
+ regval = div_u64(val * 1000ULL, fixed_power_lsb);
+ else
+ regval = div_u64(val * 4 * 100 * data->rshunt,
+ data->config->power_calculate_factor *
+ 1000ULL * INA238_FIXED_SHUNT * data->gain);
regval = clamp_val(regval >> 8, 0, U16_MAX);
return regmap_write(data->regmap, INA238_POWER_LIMIT, regval);
@@ -603,6 +705,7 @@ static ssize_t energy1_input_show(struct device *dev,
struct device_attribute *da, char *buf)
{
struct ina238_data *data = dev_get_drvdata(dev);
+ bool has_shunt = data->config->has_shunt;
int ret;
u64 regval;
u64 energy;
@@ -612,8 +715,11 @@ 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);
+ if (has_shunt)
+ energy = div_u64(regval * INA238_FIXED_SHUNT * data->gain * 16 * 10 *
+ data->config->power_calculate_factor, 4 * data->rshunt);
+ else
+ energy = div_u64(regval * INA780_ENERGY_LSB, 1000);
return sysfs_emit(buf, "%llu\n", energy);
}
@@ -621,11 +727,17 @@ static ssize_t energy1_input_show(struct device *dev,
static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
+ struct ina238_data *data = dev_get_drvdata(dev);
+ bool has_curr_min_max = data->config->has_curr_min_max;
+
switch (type) {
case hwmon_in:
return ina238_read_in(dev, attr, channel, val);
case hwmon_curr:
- return ina238_read_current(dev, attr, val);
+ if (has_curr_min_max)
+ return ina780_read_current(dev, attr, val);
+ else
+ return ina238_read_current(dev, attr, val);
case hwmon_power:
return ina238_read_power(dev, attr, val);
case hwmon_temp:
@@ -648,6 +760,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 = ina780_write_current(dev, attr, val);
+ break;
case hwmon_power:
err = ina238_write_power(dev, attr, val);
break;
@@ -669,6 +784,8 @@ 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_curr_min_max = data->config->has_curr_min_max;
+ bool has_shunt = data->config->has_shunt;
switch (type) {
case hwmon_in:
@@ -676,10 +793,14 @@ static umode_t ina238_is_visible(const void *drvdata,
case hwmon_in_input:
case hwmon_in_max_alarm:
case hwmon_in_min_alarm:
- return 0444;
+ if (channel == 0 || has_shunt)
+ return 0444;
+ return 0;
case hwmon_in_max:
case hwmon_in_min:
- return 0644;
+ if (channel == 0 || has_shunt)
+ return 0644;
+ return 0;
default:
return 0;
}
@@ -687,6 +808,13 @@ static umode_t ina238_is_visible(const void *drvdata,
switch (attr) {
case hwmon_curr_input:
return 0444;
+ case hwmon_curr_max:
+ case hwmon_curr_min:
+ case hwmon_curr_max_alarm:
+ case hwmon_curr_min_alarm:
+ if (has_curr_min_max)
+ return 0644;
+ return 0;
default:
return 0;
}
@@ -725,13 +853,16 @@ static umode_t ina238_is_visible(const void *drvdata,
static const struct hwmon_channel_info * const ina238_info[] = {
HWMON_CHANNEL_INFO(in,
- /* 0: shunt voltage */
+ /* 0: shunt voltage (bus voltage for ina780)*/
INA238_HWMON_IN_CONFIG,
- /* 1: bus voltage */
+ /* 1: bus voltage (not present on ina780) */
INA238_HWMON_IN_CONFIG),
HWMON_CHANNEL_INFO(curr,
/* 0: current through shunt */
- HWMON_C_INPUT),
+ HWMON_C_INPUT |
+ /* current limits avialble on ina780*/
+ HWMON_C_MAX | HWMON_C_MAX_ALARM |
+ HWMON_C_MIN | HWMON_C_MIN_ALARM),
HWMON_CHANNEL_INFO(power,
/* 0: power */
HWMON_P_INPUT | HWMON_P_MAX |
@@ -790,21 +921,23 @@ static int ina238_probe(struct i2c_client *client)
return PTR_ERR(data->regmap);
}
- /* 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 (data->rshunt == 0) {
- dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
- return -EINVAL;
- }
+ if (data->config->has_shunt) {
+ /* 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 (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;
+ /* 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 */
@@ -831,12 +964,14 @@ 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;
+ if (data->config->has_shunt) {
+ /* 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 */
@@ -854,8 +989,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->config->has_shunt)
+ dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
+ client->name, data->rshunt, data->gain);
return 0;
}
@@ -865,6 +1001,7 @@ static const struct i2c_device_id ina238_id[] = {
{ "ina237", ina237 },
{ "ina238", ina238 },
{ "sq52206", sq52206 },
+ { "ina780", ina780 },
{ }
};
MODULE_DEVICE_TABLE(i2c, ina238_id);
@@ -886,6 +1023,10 @@ static const struct of_device_id __maybe_unused ina238_of_match[] = {
.compatible = "silergy,sq52206",
.data = (void *)sq52206
},
+ {
+ .compatible = "ti,ina780",
+ .data = (void *)ina780
+ },
{ }
};
MODULE_DEVICE_TABLE(of, ina238_of_match);
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
2025-08-29 3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
@ 2025-08-29 9:55 ` Guenter Roeck
2025-08-31 21:12 ` Chris Packham
2025-08-31 22:34 ` Guenter Roeck
1 sibling, 1 reply; 11+ messages in thread
From: Guenter Roeck @ 2025-08-29 9:55 UTC (permalink / raw)
To: Chris Packham, jdelvare, robh, krzk+dt, conor+dt, corbet,
wenliang202407, jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc
On 8/28/25 20:05, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
>
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
> Changes in v3:
> - New. Split off bugfix from main patch
>
> 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 5a394eeff676..4d3dc018ead9 100644
> --- a/drivers/hwmon/ina238.c
> +++ b/drivers/hwmon/ina238.c
> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
> return -EOPNOTSUPP;
>
> /* Signed */
> - regval = clamp_val(val, -40000, 125000);
> + val = clamp_val(val, -40000, 125000);
That needs another correction: As it turns out, the default register value
is 0x7ff0, or 255875. That means we need to accept that range. The same is
probably true for negative temperatures, but I'll need to see the real chip
to be sure.
Yes, the chips only support a limited temperature range, but that is the
limit register, not the supported range. Other chips have a similar problem.
It is ok to limit the input range if the chip has a reasonable default set,
but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
support writing that value.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
2025-08-29 3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
@ 2025-08-29 15:56 ` Guenter Roeck
0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-29 15:56 UTC (permalink / raw)
To: Chris Packham, jdelvare, robh, krzk+dt, conor+dt, corbet,
wenliang202407, jre
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc
On 8/28/25 20:05, Chris Packham wrote:
> In preparation for adding INA780 support add some required fields to
> ina238_config and set the appropriate values for the existing chips.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
> Changes in v3:
> - New. Split config struct changes from main patch
>
We should have fields for curent_lsb, power_lsb, and energy_lsb in both
struct ina238_config and struct ina238_data, and pre-calculate the values
for chips where it is dynamic. I started writing that code, but while
writing unit test code I found that the driver has more problems similar
to the one you fixed with an earlier patch of this series. We'll have to
address that first. I hope I can find and fix those issues over the weekend.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
2025-08-29 9:55 ` Guenter Roeck
@ 2025-08-31 21:12 ` Chris Packham
2025-08-31 22:09 ` Guenter Roeck
0 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-31 21:12 UTC (permalink / raw)
To: Guenter Roeck, jdelvare@suse.com, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, corbet@lwn.net,
wenliang202407@163.com, jre@pengutronix.de
Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
On 29/08/25 21:55, Guenter Roeck wrote:
> On 8/28/25 20:05, Chris Packham wrote:
>> ina238_write_temp() was attempting to clamp the user input but was
>> throwing away the result. Ensure that we clamp the value to the
>> appropriate range before it is converted into a register value.
>>
>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula
>> to adapt to different chips")
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>> Changes in v3:
>> - New. Split off bugfix from main patch
>>
>> 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 5a394eeff676..4d3dc018ead9 100644
>> --- a/drivers/hwmon/ina238.c
>> +++ b/drivers/hwmon/ina238.c
>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev,
>> u32 attr, long val)
>> return -EOPNOTSUPP;
>> /* Signed */
>> - regval = clamp_val(val, -40000, 125000);
>> + val = clamp_val(val, -40000, 125000);
>
> That needs another correction: As it turns out, the default register
> value
> is 0x7ff0, or 255875. That means we need to accept that range. The
> same is
> probably true for negative temperatures, but I'll need to see the real
> chip
> to be sure.
>
> Yes, the chips only support a limited temperature range, but that is the
> limit register, not the supported range. Other chips have a similar
> problem.
> It is ok to limit the input range if the chip has a reasonable default
> set,
> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
> support writing that value.
I'm guessing that might change my introduction of temp_max in the next
patch. I'll re-order my series to put the bugfix first with the changes
mentioned.
>
> Guenter
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
2025-08-31 21:12 ` Chris Packham
@ 2025-08-31 22:09 ` Guenter Roeck
0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:09 UTC (permalink / raw)
To: Chris Packham, jdelvare@suse.com, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, corbet@lwn.net,
wenliang202407@163.com, jre@pengutronix.de
Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
On 8/31/25 14:12, Chris Packham wrote:
>
> On 29/08/25 21:55, Guenter Roeck wrote:
>> On 8/28/25 20:05, Chris Packham wrote:
>>> ina238_write_temp() was attempting to clamp the user input but was
>>> throwing away the result. Ensure that we clamp the value to the
>>> appropriate range before it is converted into a register value.
>>>
>>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula
>>> to adapt to different chips")
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>> Changes in v3:
>>> - New. Split off bugfix from main patch
>>>
>>> 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 5a394eeff676..4d3dc018ead9 100644
>>> --- a/drivers/hwmon/ina238.c
>>> +++ b/drivers/hwmon/ina238.c
>>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev,
>>> u32 attr, long val)
>>> return -EOPNOTSUPP;
>>> /* Signed */
>>> - regval = clamp_val(val, -40000, 125000);
>>> + val = clamp_val(val, -40000, 125000);
>>
>> That needs another correction: As it turns out, the default register
>> value
>> is 0x7ff0, or 255875. That means we need to accept that range. The
>> same is
>> probably true for negative temperatures, but I'll need to see the real
>> chip
>> to be sure.
>>
>> Yes, the chips only support a limited temperature range, but that is the
>> limit register, not the supported range. Other chips have a similar
>> problem.
>> It is ok to limit the input range if the chip has a reasonable default
>> set,
>> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
>> support writing that value.
> I'm guessing that might change my introduction of temp_max in the next
> patch. I'll re-order my series to put the bugfix first with the changes
> mentioned.
Please wait a bit before resending; I have a series almost ready to send out
with various other changes.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
2025-08-29 3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
2025-08-29 9:55 ` Guenter Roeck
@ 2025-08-31 22:34 ` Guenter Roeck
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:34 UTC (permalink / raw)
To: Chris Packham
Cc: jdelvare, robh, krzk+dt, conor+dt, corbet, wenliang202407, jre,
linux-hwmon, devicetree, linux-kernel, linux-doc
On Fri, Aug 29, 2025 at 03:05:10PM +1200, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
>
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Applied.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device
2025-08-29 3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
@ 2025-08-31 22:35 ` Guenter Roeck
0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:35 UTC (permalink / raw)
To: Chris Packham
Cc: jdelvare, robh, krzk+dt, conor+dt, corbet, wenliang202407, jre,
linux-hwmon, devicetree, linux-kernel, linux-doc
On Fri, Aug 29, 2025 at 03:05:09PM +1200, Chris Packham wrote:
> Add a compatible string for the INA780 device.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-08-31 22:35 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
2025-08-29 3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
2025-08-31 22:35 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
2025-08-29 9:55 ` Guenter Roeck
2025-08-31 21:12 ` Chris Packham
2025-08-31 22:09 ` Guenter Roeck
2025-08-31 22:34 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
2025-08-29 15:56 ` Guenter Roeck
2025-08-29 3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).