On 1/7/25 04:43, Wenliang Yan wrote:
...
-#define INA238_REGISTERS 0x11
+#define INA238_MAX_REGISTERS 0x20
Why this change ?
The maximum register address for SQ52206 is 0x20.
That isn't what I referred to. Th value change is ok.
The question was why change INA238_REGISTERS to INA238_MAX_REGISTERS.
That is a personal preference change. I strongly discourage such
changes because if I accept them someone else may come tomorrow
and change the name again to match their preference.
-#define INA238_FIXED_SHUNT 20000
+#define INA238_FIXED_SHUNT 20000
Why this change ?
Also refer to the chip manual provided in the website above.
The value didn't change. The indentation changed without reason.
static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
@@ -197,10 +239,10 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
regval = (s16)regval;
if (channel == 0)
/* gain of 1 -> LSB / 4 */
- *val = (regval * INA238_SHUNT_VOLTAGE_LSB) /
- (1000 * (4 - data->gain + 1));
+ *val = (regval * INA238_SHUNT_VOLTAGE_LSB) *
+ data->gain / (1000 * 4);
Why this change ?
Because INA238 only has two gains of 1 and 4, the previous formula can
be used, but SQ52206 has a gain of 2, so I changed the formula
Ok.
else
- *val = (regval * INA238_BUS_VOLTAGE_LSB) / 1000;
+ *val = (regval * data->config->bus_voltage_lsb) / 1000;
break;
case hwmon_in_max_alarm:
case hwmon_in_min_alarm:
@@ -225,8 +267,8 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
case 0:
/* signed value, clamp to max range +/-163 mV */
regval = clamp_val(val, -163, 163);
- regval = (regval * 1000 * (4 - data->gain + 1)) /
- INA238_SHUNT_VOLTAGE_LSB;
+ regval = (regval * 1000 * 4) /
+ INA238_SHUNT_VOLTAGE_LSB * data->gain;
Why this change ?
Consistent with the reason described in the previous article.
Ok.
data->regmap = devm_regmap_init_i2c(client, &ina238_regmap_config);
@@ -564,48 +673,15 @@ static int ina238_probe(struct i2c_client *client)
/* 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 != 4) {
+ if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
Chip independent changes should be in separate patch(es).
SQ52206 has a gain of 2.
Ok.
Thanks,
Guenter
