[PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields

Chris Packham posted 4 patches 1 month ago
[PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
Posted by Chris Packham 1 month ago
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
Re: [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
Posted by Guenter Roeck 1 month ago
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