[PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206

[PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206

[Wenliang Yan]

Add support for Silergy i2c power monitor SQ52206 to the ina238
driver as those two are similar.

Signed-off-by: Wenliang Yan <wenliang202407@163.com>
---

Add new chip SQ52206, the datasheet depends on 
https://us1.silergy.com/cloud/index/uniqid/676b659b4a503
The password is fx6NEe.

Changes in v2:
- Explain why sq52206 compatibility has been added to ina2xx.yaml.
- addressed various review comments
- Link to v1: https://lore.kernel.org/linux-hwmon/20241224063559.391061-1-wenliang202407@163.com/

Wenliang Yan (2):
  dt-bindings:Add SQ52206 to ina2xx devicetree bindings
  hwmon:(ina238)Add support for SQ52206

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml  |   1 +
 Documentation/hwmon/ina238.rst                |  15 ++
 drivers/hwmon/ina238.c                        | 207 +++++++++++++++---
 3 files changed, 191 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings

[Wenliang Yan]

Add the sq52206 compatible to the ina2xx.yaml

Signed-off-by: Wenliang Yan <wenliang202407@163.com>
---

The new features added to sq52206 compared to ina238 do not
affect the differences in hardware properties.The properties
described in the ina2xx.yaml are applicable to the sq52206.

 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 05a9cb36cd82..f0b7758ab29f 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -20,6 +20,7 @@ description: |
 properties:
   compatible:
     enum:
+      - silergy,sq52206
       - silergy,sy24655
       - ti,ina209
       - ti,ina219
-- 
2.43.0

[PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[Wenliang Yan]

Add support for SQ52206 to the Ina238 driver. Add registers,
add calculation formulas, increase compatibility, add
compatibility programs for multiple chips.

Signed-off-by: Wenliang Yan <wenliang202407@163.com>
---

Incorporate four additional registers to the original register
set of SQ52206 beyond INA238.

The ADC measurement range of SQ52206 is divided into 1/2/4, so
change the configuration of INA238_ADC_CONFIG.

SQ52206's calculation of power read values is different from
INA238.Add new value of BUS_VOLTAGE_LSB and DIE-TEMP_LSB for
SQ52206. As a result of these changes, modify both the power
and temperature read and write operations.

Add new parameters in struct ina238_data to save the chip type
and different configurations for each chip type, promoting
program reusability.

Due to the temperature reading of SQ52206 being a signed 16 bit
value, while INA238 is a 12 bit value. So we changed the
temperature reading function.

Extract the chip initialization process into a separate function
named ina238_init to facilitate adjustments for various chips.

Add a corresponding compatible to the driver.

Add a 40 bit data reading function to prepare for energy reading.

Energy attributes are 5bytes wide, so modified the function for
energy1_input to use u64.

Add HWMON_P_INPUT_HIGHEST for power.

 Documentation/hwmon/ina238.rst |  15 +++
 drivers/hwmon/ina238.c         | 207 ++++++++++++++++++++++++++++-----
 2 files changed, 190 insertions(+), 32 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index d9f479984420..d1b93cf8627f 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -14,6 +14,12 @@ Supported chips:
     Datasheet:
 	https://www.ti.com/lit/gpn/ina238
 
+  * Silergy SQ52206
+
+    Prefix: 'SQ52206'
+
+    Addresses: I2C 0x40 - 0x4f
+
 Author: Nathan Rossi <nathan.rossi@digi.com>
 
 Description
@@ -54,3 +60,12 @@ 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 (mJ)
+
+power1_input_highest	Peak Power (uW)
+======================= =======================================================
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 2d9f12f68d50..6c5bc10c3a1f 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -21,11 +21,14 @@
 #define INA238_CONFIG			0x0
 #define INA238_ADC_CONFIG		0x1
 #define INA238_SHUNT_CALIBRATION	0x2
+#define SQ52206_SHUNT_TEMPCO		0x3
 #define INA238_SHUNT_VOLTAGE		0x4
 #define INA238_BUS_VOLTAGE		0x5
 #define INA238_DIE_TEMP			0x6
 #define INA238_CURRENT			0x7
 #define INA238_POWER			0x8
+#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
@@ -33,9 +36,12 @@
 #define INA238_BUS_UNDER_VOLTAGE	0xf
 #define INA238_TEMP_LIMIT		0x10
 #define INA238_POWER_LIMIT		0x11
+#define SQ52206_POWER_PEAK		0x20
 #define INA238_DEVICE_ID		0x3f /* not available on INA237 */
 
 #define INA238_CONFIG_ADCRANGE		BIT(4)
+#define SQ52206_CONFIG_ADCRANGE_HIGH	BIT(4)
+#define SQ52206_CONFIG_ADCRANGE_LOW		BIT(3)
 
 #define INA238_DIAG_ALERT_TMPOL		BIT(7)
 #define INA238_DIAG_ALERT_SHNTOL	BIT(6)
@@ -44,12 +50,13 @@
 #define INA238_DIAG_ALERT_BUSUL		BIT(3)
 #define INA238_DIAG_ALERT_POL		BIT(2)
 
-#define INA238_REGISTERS		0x11
+#define INA238_REGISTERS		0x20
 
 #define INA238_RSHUNT_DEFAULT		10000 /* uOhm */
 
 /* Default configuration of device on reset. */
 #define INA238_CONFIG_DEFAULT		0
+#define SQ52206_CONFIG_DEFAULT		0x0005
 /* 16 sample averaging, 1052us conversion time, continuous mode */
 #define INA238_ADC_CONFIG_DEFAULT	0xfb6a
 /* Configure alerts to be based on averaged value (SLOWALERT) */
@@ -87,14 +94,19 @@
  *  shunt = 0x4000 / (819.2 * 10^6) / 0.001 = 20000 uOhms (with 1mA/lsb)
  *
  *  Current (mA) = register value * 20000 / rshunt / 4 * gain
- *  Power (W) = 0.2 * 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 (mJ) = 16 * 0.24 * register value * 20000 / rshunt / 4 * gain
  */
 #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 INA238_DIE_TEMP_LSB		125 /* 125 mC/lsb */
+#define INA238_DIE_TEMP_LSB			1250000 /* 125 mC/lsb */
+#define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
+#define SQ52206_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 
 static const struct regmap_config ina238_regmap_config = {
 	.max_register = INA238_REGISTERS,
@@ -102,7 +114,20 @@ static const struct regmap_config ina238_regmap_config = {
 	.val_bits = 16,
 };
 
+enum ina238_ids { ina238, ina237, sq52206 };
+
+struct ina238_config {
+	bool has_power_highest;		/* chip detection power peak */
+	bool has_energy;		/* chip detection energy */
+	u8 temp_shift;
+	u32 power_calculate_factor;		/*fixed parameters for power calculate*/
+	u16 config_default;
+	int bus_voltage_lsb;    /* uV */
+	int temp_lsb;   /* mC */
+};
+
 struct ina238_data {
+	const struct ina238_config *config;
 	struct i2c_client *client;
 	struct mutex config_lock;
 	struct regmap *regmap;
@@ -110,6 +135,36 @@ struct ina238_data {
 	int gain;
 };
 
+static const struct ina238_config ina238_config[] = {
+	[ina238] = {
+		.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,
+	},
+	[ina237] = {
+		.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,
+	},
+	[sq52206] = {
+		.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,
+	},
+};
+
 static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
 {
 	u8 data[3];
@@ -126,6 +181,24 @@ static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
 	return 0;
 }
 
+static int ina238_read_reg40(const struct i2c_client *client, u8 reg, u64 *val)
+{
+	u8 data[5];
+	u32 low;
+	int err;
+
+	/* 40-bit register read */
+	err = i2c_smbus_read_i2c_block_data(client, reg, 5, data);
+	if (err < 0)
+		return err;
+	if (err != 5)
+		return -EIO;
+	low = (data[1] << 24) | (data[2] << 16) | (data[3] << 8) | data[4];
+	*val = ((long long)data[0] << 32) | low;
+
+	return 0;
+}
+
 static int ina238_read_in(struct device *dev, u32 attr, int channel,
 			  long *val)
 {
@@ -197,10 +270,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);
 		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 +298,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;
 		regval = clamp_val(regval, S16_MIN, S16_MAX);
 
 		switch (attr) {
@@ -242,7 +315,7 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
 	case 1:
 		/* signed value, positive values only. Clamp to max 102.396 V */
 		regval = clamp_val(val, 0, 102396);
-		regval = (regval * 1000) / INA238_BUS_VOLTAGE_LSB;
+		regval = (regval * 1000) / data->config->bus_voltage_lsb;
 		regval = clamp_val(regval, 0, S16_MAX);
 
 		switch (attr) {
@@ -297,8 +370,19 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 			return err;
 
 		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT *
-				data->gain, 20 * data->rshunt);
+		power = div_u64(regval * data->config->power_calculate_factor * 50ULL *
+				INA238_FIXED_SHUNT * data->gain, 20 * data->rshunt);
+		/* Clamp value to maximum value of long */
+		*val = clamp_val(power, 0, LONG_MAX);
+		break;
+	case hwmon_power_input_highest:
+		err = ina238_read_reg24(data->client, SQ52206_POWER_PEAK, &regval);
+		if (err)
+			return err;
+
+		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
+		power = div_u64(regval * data->config->power_calculate_factor * 50ULL *
+				INA238_FIXED_SHUNT * data->gain, 20 * data->rshunt);
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -311,8 +395,8 @@ 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, 20 * data->rshunt);
+		power = div_u64((regval << 8) * data->config->power_calculate_factor *
+		50ULL * INA238_FIXED_SHUNT * data->gain, 20 * data->rshunt);
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -344,8 +428,8 @@ static int ina238_write_power(struct device *dev, u32 attr, long val)
 	 * register.
 	 */
 	regval = clamp_val(val, 0, LONG_MAX);
-	regval = div_u64(val * 20ULL * data->rshunt,
-			 1000ULL * INA238_FIXED_SHUNT * data->gain);
+	regval = div_u64(val * data->config->power_calculate_factor * data->rshunt,
+			1000ULL * INA238_FIXED_SHUNT * data->gain);
 	regval = clamp_val(regval >> 8, 0, U16_MAX);
 
 	return regmap_write(data->regmap, INA238_POWER_LIMIT, regval);
@@ -362,17 +446,17 @@ 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, bits 15-4 of register, result in mC */
-		*val = ((s16)regval >> 4) * INA238_DIE_TEMP_LSB;
+		/* Signed, result in mC */
+		*val = div_s64(((s16)regval >> data->config->temp_shift) *
+						data->config->temp_lsb, 10000);
 		break;
 	case hwmon_temp_max:
 		err = regmap_read(data->regmap, INA238_TEMP_LIMIT, &regval);
 		if (err)
 			return err;
-
-		/* Signed, bits 15-4 of register, result in mC */
-		*val = ((s16)regval >> 4) * INA238_DIE_TEMP_LSB;
+		/* Signed, result in mC */
+		*val = div_s64(((s16)regval >> data->config->temp_shift) *
+						data->config->temp_lsb, 10000);
 		break;
 	case hwmon_temp_max_alarm:
 		err = regmap_read(data->regmap, INA238_DIAG_ALERT, &regval);
@@ -396,13 +480,31 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
 	if (attr != hwmon_temp_max)
 		return -EOPNOTSUPP;
 
-	/* Signed, bits 15-4 of register */
-	regval = (val / INA238_DIE_TEMP_LSB) << 4;
-	regval = clamp_val(regval, S16_MIN, S16_MAX) & 0xfff0;
+	/* Signed */
+	regval = div_u64(val*10000, data->config->temp_lsb) << data->config->temp_shift;
+	regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift);
 
 	return regmap_write(data->regmap, INA238_TEMP_LIMIT, regval);
 }
 
+static ssize_t energy1_input_show(struct device *dev,
+				  struct device_attribute *da, char *buf)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	int ret;
+	u64 val;
+
+	ret = ina238_read_reg40(data->client, SQ52206_ENERGY, &val);
+	if (ret)
+		return ret;
+
+	/* result in microJoule */
+	val = div_u64(val * 96 * INA238_FIXED_SHUNT * data->gain,
+			       data->rshunt * 100);
+
+	return sprintf(buf, "%llu\n", val);
+}
+
 static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
 		       u32 attr, int channel, long *val)
 {
@@ -452,6 +554,9 @@ static umode_t ina238_is_visible(const void *drvdata,
 				 enum hwmon_sensor_types type,
 				 u32 attr, int channel)
 {
+	const struct ina238_data *data = drvdata;
+	bool has_power_highest = data->config->has_power_highest;
+
 	switch (type) {
 	case hwmon_in:
 		switch (attr) {
@@ -479,6 +584,10 @@ static umode_t ina238_is_visible(const void *drvdata,
 			return 0444;
 		case hwmon_power_max:
 			return 0644;
+		case hwmon_power_input_highest:
+			if (has_power_highest)
+				return 0444;
+			break;
 		default:
 			return 0;
 		}
@@ -512,7 +621,8 @@ static const struct hwmon_channel_info * const ina238_info[] = {
 			   HWMON_C_INPUT),
 	HWMON_CHANNEL_INFO(power,
 			   /* 0: power */
-			   HWMON_P_INPUT | HWMON_P_MAX | HWMON_P_MAX_ALARM),
+			   HWMON_P_INPUT | HWMON_P_MAX |
+			   HWMON_P_MAX_ALARM | HWMON_P_INPUT_HIGHEST),
 	HWMON_CHANNEL_INFO(temp,
 			   /* 0: die temperature */
 			   HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_ALARM),
@@ -530,6 +640,15 @@ static const struct hwmon_chip_info ina238_chip_info = {
 	.info = ina238_info,
 };
 
+/* energy attributes are 5bytes 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 ina2xx_platform_data *pdata = dev_get_platdata(&client->dev);
@@ -537,13 +656,19 @@ static int ina238_probe(struct i2c_client *client)
 	struct device *hwmon_dev;
 	struct ina238_data *data;
 	int config;
+	enum ina238_ids chip;
 	int ret;
 
+	chip = (uintptr_t)i2c_get_match_data(client);
+
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
 	data->client = client;
+	/* set the device type */
+	data->config = &ina238_config[chip];
+
 	mutex_init(&data->config_lock);
 
 	data->regmap = devm_regmap_init_i2c(client, &ina238_regmap_config);
@@ -564,14 +689,19 @@ 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) {
 		dev_err(dev, "invalid shunt gain value %u\n", data->gain);
 		return -EINVAL;
 	}
 
 	/* Setup CONFIG register */
-	config = INA238_CONFIG_DEFAULT;
-	if (data->gain == 1)
+	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 */
 	ret = regmap_write(data->regmap, INA238_CONFIG, config);
 	if (ret < 0) {
@@ -605,7 +735,8 @@ static int ina238_probe(struct i2c_client *client)
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
 							 &ina238_chip_info,
-							 NULL);
+							 data->config->has_energy ?
+								NULL : ina238_groups);
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
@@ -616,14 +747,26 @@ static int ina238_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ina238_id[] = {
-	{ "ina238" },
+	{ "ina237", ina237 },
+	{ "ina238", ina238 },
+	{ "sq52206", sq52206 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ina238_id);
 
 static const struct of_device_id __maybe_unused ina238_of_match[] = {
-	{ .compatible = "ti,ina237" },
-	{ .compatible = "ti,ina238" },
+	{
+		.compatible = "silergy,sq52206",
+		.data = (void *)sq52206
+	},
+	{
+		.compatible = "ti,ina237",
+		.data = (void *)ina237
+	},
+	{
+		.compatible = "ti,ina238",
+		.data = (void *)ina238
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ina238_of_match);
-- 
2.43.0

Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[kernel test robot]

Hi Wenliang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.13-rc7 next-20250110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenliang-Yan/dt-bindings-Add-SQ52206-to-ina2xx-devicetree-bindings/20250113-115457
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20250113035023.365697-3-wenliang202407%40163.com
patch subject: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206
config: i386-buildonly-randconfig-004-20250113 (https://download.01.org/0day-ci/archive/20250113/202501131420.UQWnurXD-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250113/202501131420.UQWnurXD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501131420.UQWnurXD-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/hwmon/ina238.c:11:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/hwmon/ina238.c:594:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
     594 |         case hwmon_temp:
         |         ^
   drivers/hwmon/ina238.c:594:2: note: insert '__attribute__((fallthrough));' to silence this warning
     594 |         case hwmon_temp:
         |         ^
         |         __attribute__((fallthrough)); 
   drivers/hwmon/ina238.c:594:2: note: insert 'break;' to avoid fall-through
     594 |         case hwmon_temp:
         |         ^
         |         break; 
>> drivers/hwmon/ina238.c:702:3: warning: add explicit braces to avoid dangling else [-Wdangling-else]
     702 |                 else if (data->gain == 2)
         |                 ^
   3 warnings generated.


vim +594 drivers/hwmon/ina238.c

eacb52f010a807 Nathan Rossi        2021-11-02  552  
eacb52f010a807 Nathan Rossi        2021-11-02  553  static umode_t ina238_is_visible(const void *drvdata,
eacb52f010a807 Nathan Rossi        2021-11-02  554  				 enum hwmon_sensor_types type,
eacb52f010a807 Nathan Rossi        2021-11-02  555  				 u32 attr, int channel)
eacb52f010a807 Nathan Rossi        2021-11-02  556  {
f08436905922ae Wenliang Yan        2025-01-13  557  	const struct ina238_data *data = drvdata;
f08436905922ae Wenliang Yan        2025-01-13  558  	bool has_power_highest = data->config->has_power_highest;
f08436905922ae Wenliang Yan        2025-01-13  559  
eacb52f010a807 Nathan Rossi        2021-11-02  560  	switch (type) {
eacb52f010a807 Nathan Rossi        2021-11-02  561  	case hwmon_in:
eacb52f010a807 Nathan Rossi        2021-11-02  562  		switch (attr) {
eacb52f010a807 Nathan Rossi        2021-11-02  563  		case hwmon_in_input:
eacb52f010a807 Nathan Rossi        2021-11-02  564  		case hwmon_in_max_alarm:
eacb52f010a807 Nathan Rossi        2021-11-02  565  		case hwmon_in_min_alarm:
eacb52f010a807 Nathan Rossi        2021-11-02  566  			return 0444;
eacb52f010a807 Nathan Rossi        2021-11-02  567  		case hwmon_in_max:
eacb52f010a807 Nathan Rossi        2021-11-02  568  		case hwmon_in_min:
eacb52f010a807 Nathan Rossi        2021-11-02  569  			return 0644;
eacb52f010a807 Nathan Rossi        2021-11-02  570  		default:
eacb52f010a807 Nathan Rossi        2021-11-02  571  			return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  572  		}
eacb52f010a807 Nathan Rossi        2021-11-02  573  	case hwmon_curr:
eacb52f010a807 Nathan Rossi        2021-11-02  574  		switch (attr) {
eacb52f010a807 Nathan Rossi        2021-11-02  575  		case hwmon_curr_input:
eacb52f010a807 Nathan Rossi        2021-11-02  576  			return 0444;
eacb52f010a807 Nathan Rossi        2021-11-02  577  		default:
eacb52f010a807 Nathan Rossi        2021-11-02  578  			return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  579  		}
eacb52f010a807 Nathan Rossi        2021-11-02  580  	case hwmon_power:
eacb52f010a807 Nathan Rossi        2021-11-02  581  		switch (attr) {
eacb52f010a807 Nathan Rossi        2021-11-02  582  		case hwmon_power_input:
eacb52f010a807 Nathan Rossi        2021-11-02  583  		case hwmon_power_max_alarm:
eacb52f010a807 Nathan Rossi        2021-11-02  584  			return 0444;
eacb52f010a807 Nathan Rossi        2021-11-02  585  		case hwmon_power_max:
eacb52f010a807 Nathan Rossi        2021-11-02  586  			return 0644;
f08436905922ae Wenliang Yan        2025-01-13  587  		case hwmon_power_input_highest:
f08436905922ae Wenliang Yan        2025-01-13  588  			if (has_power_highest)
f08436905922ae Wenliang Yan        2025-01-13  589  				return 0444;
f08436905922ae Wenliang Yan        2025-01-13  590  			break;
eacb52f010a807 Nathan Rossi        2021-11-02  591  		default:
eacb52f010a807 Nathan Rossi        2021-11-02  592  			return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  593  		}
eacb52f010a807 Nathan Rossi        2021-11-02 @594  	case hwmon_temp:
eacb52f010a807 Nathan Rossi        2021-11-02  595  		switch (attr) {
eacb52f010a807 Nathan Rossi        2021-11-02  596  		case hwmon_temp_input:
eacb52f010a807 Nathan Rossi        2021-11-02  597  		case hwmon_temp_max_alarm:
eacb52f010a807 Nathan Rossi        2021-11-02  598  			return 0444;
eacb52f010a807 Nathan Rossi        2021-11-02  599  		case hwmon_temp_max:
eacb52f010a807 Nathan Rossi        2021-11-02  600  			return 0644;
eacb52f010a807 Nathan Rossi        2021-11-02  601  		default:
eacb52f010a807 Nathan Rossi        2021-11-02  602  			return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  603  		}
eacb52f010a807 Nathan Rossi        2021-11-02  604  	default:
eacb52f010a807 Nathan Rossi        2021-11-02  605  		return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  606  	}
eacb52f010a807 Nathan Rossi        2021-11-02  607  }
eacb52f010a807 Nathan Rossi        2021-11-02  608  
eacb52f010a807 Nathan Rossi        2021-11-02  609  #define INA238_HWMON_IN_CONFIG (HWMON_I_INPUT | \
eacb52f010a807 Nathan Rossi        2021-11-02  610  				HWMON_I_MAX | HWMON_I_MAX_ALARM | \
eacb52f010a807 Nathan Rossi        2021-11-02  611  				HWMON_I_MIN | HWMON_I_MIN_ALARM)
eacb52f010a807 Nathan Rossi        2021-11-02  612  
bf9b7f92df7376 Krzysztof Kozlowski 2023-04-06  613  static const struct hwmon_channel_info * const ina238_info[] = {
eacb52f010a807 Nathan Rossi        2021-11-02  614  	HWMON_CHANNEL_INFO(in,
eacb52f010a807 Nathan Rossi        2021-11-02  615  			   /* 0: shunt voltage */
eacb52f010a807 Nathan Rossi        2021-11-02  616  			   INA238_HWMON_IN_CONFIG,
eacb52f010a807 Nathan Rossi        2021-11-02  617  			   /* 1: bus voltage */
eacb52f010a807 Nathan Rossi        2021-11-02  618  			   INA238_HWMON_IN_CONFIG),
eacb52f010a807 Nathan Rossi        2021-11-02  619  	HWMON_CHANNEL_INFO(curr,
eacb52f010a807 Nathan Rossi        2021-11-02  620  			   /* 0: current through shunt */
eacb52f010a807 Nathan Rossi        2021-11-02  621  			   HWMON_C_INPUT),
eacb52f010a807 Nathan Rossi        2021-11-02  622  	HWMON_CHANNEL_INFO(power,
eacb52f010a807 Nathan Rossi        2021-11-02  623  			   /* 0: power */
f08436905922ae Wenliang Yan        2025-01-13  624  			   HWMON_P_INPUT | HWMON_P_MAX |
f08436905922ae Wenliang Yan        2025-01-13  625  			   HWMON_P_MAX_ALARM | HWMON_P_INPUT_HIGHEST),
eacb52f010a807 Nathan Rossi        2021-11-02  626  	HWMON_CHANNEL_INFO(temp,
eacb52f010a807 Nathan Rossi        2021-11-02  627  			   /* 0: die temperature */
eacb52f010a807 Nathan Rossi        2021-11-02  628  			   HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_ALARM),
eacb52f010a807 Nathan Rossi        2021-11-02  629  	NULL
eacb52f010a807 Nathan Rossi        2021-11-02  630  };
eacb52f010a807 Nathan Rossi        2021-11-02  631  
eacb52f010a807 Nathan Rossi        2021-11-02  632  static const struct hwmon_ops ina238_hwmon_ops = {
eacb52f010a807 Nathan Rossi        2021-11-02  633  	.is_visible = ina238_is_visible,
eacb52f010a807 Nathan Rossi        2021-11-02  634  	.read = ina238_read,
eacb52f010a807 Nathan Rossi        2021-11-02  635  	.write = ina238_write,
eacb52f010a807 Nathan Rossi        2021-11-02  636  };
eacb52f010a807 Nathan Rossi        2021-11-02  637  
eacb52f010a807 Nathan Rossi        2021-11-02  638  static const struct hwmon_chip_info ina238_chip_info = {
eacb52f010a807 Nathan Rossi        2021-11-02  639  	.ops = &ina238_hwmon_ops,
eacb52f010a807 Nathan Rossi        2021-11-02  640  	.info = ina238_info,
eacb52f010a807 Nathan Rossi        2021-11-02  641  };
eacb52f010a807 Nathan Rossi        2021-11-02  642  
f08436905922ae Wenliang Yan        2025-01-13  643  /* energy attributes are 5bytes wide so we need u64 */
f08436905922ae Wenliang Yan        2025-01-13  644  static DEVICE_ATTR_RO(energy1_input);
f08436905922ae Wenliang Yan        2025-01-13  645  
f08436905922ae Wenliang Yan        2025-01-13  646  static struct attribute *ina238_attrs[] = {
f08436905922ae Wenliang Yan        2025-01-13  647  	&dev_attr_energy1_input.attr,
f08436905922ae Wenliang Yan        2025-01-13  648  	NULL,
f08436905922ae Wenliang Yan        2025-01-13  649  };
f08436905922ae Wenliang Yan        2025-01-13  650  ATTRIBUTE_GROUPS(ina238);
f08436905922ae Wenliang Yan        2025-01-13  651  
eacb52f010a807 Nathan Rossi        2021-11-02  652  static int ina238_probe(struct i2c_client *client)
eacb52f010a807 Nathan Rossi        2021-11-02  653  {
eacb52f010a807 Nathan Rossi        2021-11-02  654  	struct ina2xx_platform_data *pdata = dev_get_platdata(&client->dev);
eacb52f010a807 Nathan Rossi        2021-11-02  655  	struct device *dev = &client->dev;
eacb52f010a807 Nathan Rossi        2021-11-02  656  	struct device *hwmon_dev;
eacb52f010a807 Nathan Rossi        2021-11-02  657  	struct ina238_data *data;
eacb52f010a807 Nathan Rossi        2021-11-02  658  	int config;
f08436905922ae Wenliang Yan        2025-01-13  659  	enum ina238_ids chip;
eacb52f010a807 Nathan Rossi        2021-11-02  660  	int ret;
eacb52f010a807 Nathan Rossi        2021-11-02  661  
f08436905922ae Wenliang Yan        2025-01-13  662  	chip = (uintptr_t)i2c_get_match_data(client);
f08436905922ae Wenliang Yan        2025-01-13  663  
eacb52f010a807 Nathan Rossi        2021-11-02  664  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
eacb52f010a807 Nathan Rossi        2021-11-02  665  	if (!data)
eacb52f010a807 Nathan Rossi        2021-11-02  666  		return -ENOMEM;
eacb52f010a807 Nathan Rossi        2021-11-02  667  
eacb52f010a807 Nathan Rossi        2021-11-02  668  	data->client = client;
f08436905922ae Wenliang Yan        2025-01-13  669  	/* set the device type */
f08436905922ae Wenliang Yan        2025-01-13  670  	data->config = &ina238_config[chip];
f08436905922ae Wenliang Yan        2025-01-13  671  
eacb52f010a807 Nathan Rossi        2021-11-02  672  	mutex_init(&data->config_lock);
eacb52f010a807 Nathan Rossi        2021-11-02  673  
eacb52f010a807 Nathan Rossi        2021-11-02  674  	data->regmap = devm_regmap_init_i2c(client, &ina238_regmap_config);
eacb52f010a807 Nathan Rossi        2021-11-02  675  	if (IS_ERR(data->regmap)) {
eacb52f010a807 Nathan Rossi        2021-11-02  676  		dev_err(dev, "failed to allocate register map\n");
eacb52f010a807 Nathan Rossi        2021-11-02  677  		return PTR_ERR(data->regmap);
eacb52f010a807 Nathan Rossi        2021-11-02  678  	}
eacb52f010a807 Nathan Rossi        2021-11-02  679  
eacb52f010a807 Nathan Rossi        2021-11-02  680  	/* load shunt value */
eacb52f010a807 Nathan Rossi        2021-11-02  681  	data->rshunt = INA238_RSHUNT_DEFAULT;
eacb52f010a807 Nathan Rossi        2021-11-02  682  	if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0 && pdata)
eacb52f010a807 Nathan Rossi        2021-11-02  683  		data->rshunt = pdata->shunt_uohms;
eacb52f010a807 Nathan Rossi        2021-11-02  684  	if (data->rshunt == 0) {
eacb52f010a807 Nathan Rossi        2021-11-02  685  		dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
eacb52f010a807 Nathan Rossi        2021-11-02  686  		return -EINVAL;
eacb52f010a807 Nathan Rossi        2021-11-02  687  	}
eacb52f010a807 Nathan Rossi        2021-11-02  688  
eacb52f010a807 Nathan Rossi        2021-11-02  689  	/* load shunt gain value */
eacb52f010a807 Nathan Rossi        2021-11-02  690  	if (device_property_read_u32(dev, "ti,shunt-gain", &data->gain) < 0)
eacb52f010a807 Nathan Rossi        2021-11-02  691  		data->gain = 4; /* Default of ADCRANGE = 0 */
f08436905922ae Wenliang Yan        2025-01-13  692  	if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
eacb52f010a807 Nathan Rossi        2021-11-02  693  		dev_err(dev, "invalid shunt gain value %u\n", data->gain);
eacb52f010a807 Nathan Rossi        2021-11-02  694  		return -EINVAL;
eacb52f010a807 Nathan Rossi        2021-11-02  695  	}
eacb52f010a807 Nathan Rossi        2021-11-02  696  
eacb52f010a807 Nathan Rossi        2021-11-02  697  	/* Setup CONFIG register */
f08436905922ae Wenliang Yan        2025-01-13  698  	config = data->config->config_default;
f08436905922ae Wenliang Yan        2025-01-13  699  	if (chip == sq52206)
eacb52f010a807 Nathan Rossi        2021-11-02  700  		if (data->gain == 1)
f08436905922ae Wenliang Yan        2025-01-13  701  			config |= SQ52206_CONFIG_ADCRANGE_HIGH; /* ADCRANGE = 10/11 is /1 */
f08436905922ae Wenliang Yan        2025-01-13 @702  		else if (data->gain == 2)
f08436905922ae Wenliang Yan        2025-01-13  703  			config |= SQ52206_CONFIG_ADCRANGE_LOW; /* ADCRANGE = 01 is /2 */
f08436905922ae Wenliang Yan        2025-01-13  704  	else if (data->gain == 1)
eacb52f010a807 Nathan Rossi        2021-11-02  705  		config |= INA238_CONFIG_ADCRANGE; /* ADCRANGE = 1 is /1 */
eacb52f010a807 Nathan Rossi        2021-11-02  706  	ret = regmap_write(data->regmap, INA238_CONFIG, config);
eacb52f010a807 Nathan Rossi        2021-11-02  707  	if (ret < 0) {
eacb52f010a807 Nathan Rossi        2021-11-02  708  		dev_err(dev, "error configuring the device: %d\n", ret);
eacb52f010a807 Nathan Rossi        2021-11-02  709  		return -ENODEV;
eacb52f010a807 Nathan Rossi        2021-11-02  710  	}
eacb52f010a807 Nathan Rossi        2021-11-02  711  
eacb52f010a807 Nathan Rossi        2021-11-02  712  	/* Setup ADC_CONFIG register */
eacb52f010a807 Nathan Rossi        2021-11-02  713  	ret = regmap_write(data->regmap, INA238_ADC_CONFIG,
eacb52f010a807 Nathan Rossi        2021-11-02  714  			   INA238_ADC_CONFIG_DEFAULT);
eacb52f010a807 Nathan Rossi        2021-11-02  715  	if (ret < 0) {
eacb52f010a807 Nathan Rossi        2021-11-02  716  		dev_err(dev, "error configuring the device: %d\n", ret);
eacb52f010a807 Nathan Rossi        2021-11-02  717  		return -ENODEV;
eacb52f010a807 Nathan Rossi        2021-11-02  718  	}
eacb52f010a807 Nathan Rossi        2021-11-02  719  
eacb52f010a807 Nathan Rossi        2021-11-02  720  	/* Setup SHUNT_CALIBRATION register with fixed value */
eacb52f010a807 Nathan Rossi        2021-11-02  721  	ret = regmap_write(data->regmap, INA238_SHUNT_CALIBRATION,
eacb52f010a807 Nathan Rossi        2021-11-02  722  			   INA238_CALIBRATION_VALUE);
eacb52f010a807 Nathan Rossi        2021-11-02  723  	if (ret < 0) {
eacb52f010a807 Nathan Rossi        2021-11-02  724  		dev_err(dev, "error configuring the device: %d\n", ret);
eacb52f010a807 Nathan Rossi        2021-11-02  725  		return -ENODEV;
eacb52f010a807 Nathan Rossi        2021-11-02  726  	}
eacb52f010a807 Nathan Rossi        2021-11-02  727  
eacb52f010a807 Nathan Rossi        2021-11-02  728  	/* Setup alert/alarm configuration */
eacb52f010a807 Nathan Rossi        2021-11-02  729  	ret = regmap_write(data->regmap, INA238_DIAG_ALERT,
eacb52f010a807 Nathan Rossi        2021-11-02  730  			   INA238_DIAG_ALERT_DEFAULT);
eacb52f010a807 Nathan Rossi        2021-11-02  731  	if (ret < 0) {
eacb52f010a807 Nathan Rossi        2021-11-02  732  		dev_err(dev, "error configuring the device: %d\n", ret);
eacb52f010a807 Nathan Rossi        2021-11-02  733  		return -ENODEV;
eacb52f010a807 Nathan Rossi        2021-11-02  734  	}
eacb52f010a807 Nathan Rossi        2021-11-02  735  
eacb52f010a807 Nathan Rossi        2021-11-02  736  	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
eacb52f010a807 Nathan Rossi        2021-11-02  737  							 &ina238_chip_info,
f08436905922ae Wenliang Yan        2025-01-13  738  							 data->config->has_energy ?
f08436905922ae Wenliang Yan        2025-01-13  739  								NULL : ina238_groups);
eacb52f010a807 Nathan Rossi        2021-11-02  740  	if (IS_ERR(hwmon_dev))
eacb52f010a807 Nathan Rossi        2021-11-02  741  		return PTR_ERR(hwmon_dev);
eacb52f010a807 Nathan Rossi        2021-11-02  742  
eacb52f010a807 Nathan Rossi        2021-11-02  743  	dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
eacb52f010a807 Nathan Rossi        2021-11-02  744  		 client->name, data->rshunt, data->gain);
eacb52f010a807 Nathan Rossi        2021-11-02  745  
eacb52f010a807 Nathan Rossi        2021-11-02  746  	return 0;
eacb52f010a807 Nathan Rossi        2021-11-02  747  }
eacb52f010a807 Nathan Rossi        2021-11-02  748  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

On Mon, Jan 13, 2025 at 11:50:22AM +0800, Wenliang Yan wrote:
> Add the sq52206 compatible to the ina2xx.yaml
> 
> Signed-off-by: Wenliang Yan <wenliang202407@163.com>
> ---
> 
> The new features added to sq52206 compared to ina238 do not
> affect the differences in hardware properties.The properties
> described in the ina2xx.yaml are applicable to the sq52206.

This blurb is a bit confusing, as it makes it seem like the devices are
compatible. The driver patch, however, suggests otherwise. Please
mention in your commit message what differs between the new device
you're adding and existing ones.

> 
>  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 05a9cb36cd82..f0b7758ab29f 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> @@ -20,6 +20,7 @@ description: |
>  properties:
>    compatible:
>      enum:
> +      - silergy,sq52206
>        - silergy,sy24655
>        - ti,ina209
>        - ti,ina219
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[Guenter Roeck]

On Mon, Jan 13, 2025 at 11:50:23AM +0800, Wenliang Yan wrote:
> Add support for SQ52206 to the Ina238 driver. Add registers,
> add calculation formulas, increase compatibility, add
> compatibility programs for multiple chips.
> 
> Signed-off-by: Wenliang Yan <wenliang202407@163.com>
> ---
...
> @@ -452,6 +554,9 @@ static umode_t ina238_is_visible(const void *drvdata,
>  				 enum hwmon_sensor_types type,
>  				 u32 attr, int channel)
>  {
> +	const struct ina238_data *data = drvdata;
> +	bool has_power_highest = data->config->has_power_highest;
> +
>  	switch (type) {
>  	case hwmon_in:
>  		switch (attr) {
> @@ -479,6 +584,10 @@ static umode_t ina238_is_visible(const void *drvdata,
>  			return 0444;
>  		case hwmon_power_max:
>  			return 0644;
> +		case hwmon_power_input_highest:
> +			if (has_power_highest)
> +				return 0444;
> +			break;

This doesn't work as intended. The break; results in the code
entering the hwmon_temp: case. It has to be "return 0;" or
the entire function needs to be rewritten to use "break;"
to exit the switch statements, and "return 0;" at the end
of the function.

>  static int ina238_probe(struct i2c_client *client)
>  {
...
> -	if (data->gain == 1)
> +	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)

The "else" here is the else from "if (data->gain == 2)" which is wrong.
The entire if/else block from "if (chip == sq52206)" needs to be in {}
to avoid that.

>  		config |= INA238_CONFIG_ADCRANGE; /* ADCRANGE = 1 is /1 */
>  	ret = regmap_write(data->regmap, INA238_CONFIG, config);
>  	if (ret < 0) {
> @@ -605,7 +735,8 @@ static int ina238_probe(struct i2c_client *client)
>  
>  	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
>  							 &ina238_chip_info,
> -							 NULL);
> +							 data->config->has_energy ?
> +								NULL : ina238_groups);

This seems to be wrong check. I would assume that ina238_groups is passed
if data->config->has_energy is true, not if it is false.

Guenter

Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[kernel test robot]

Hi Wenliang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.13-rc7 next-20250113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenliang-Yan/dt-bindings-Add-SQ52206-to-ina2xx-devicetree-bindings/20250113-115457
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20250113035023.365697-3-wenliang202407%40163.com
patch subject: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206
config: csky-randconfig-002-20250114 (https://download.01.org/0day-ci/archive/20250114/202501140230.5s2Uytod-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250114/202501140230.5s2Uytod-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501140230.5s2Uytod-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/err.h:5,
                    from drivers/hwmon/ina238.c:9:
   drivers/hwmon/ina238.c: In function 'ina238_probe':
>> include/linux/compiler.h:55:26: warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else]
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                          ^
   drivers/hwmon/ina238.c:699:9: note: in expansion of macro 'if'
     699 |         if (chip == sq52206)
         |         ^~
   drivers/hwmon/ina238.c: In function 'ina238_is_visible':
>> drivers/hwmon/ina238.c:581:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
     581 |                 switch (attr) {
         |                 ^~~~~~
   drivers/hwmon/ina238.c:594:9: note: here
     594 |         case hwmon_temp:
         |         ^~~~


vim +581 drivers/hwmon/ina238.c

eacb52f010a807 Nathan Rossi 2021-11-02  552  
eacb52f010a807 Nathan Rossi 2021-11-02  553  static umode_t ina238_is_visible(const void *drvdata,
eacb52f010a807 Nathan Rossi 2021-11-02  554  				 enum hwmon_sensor_types type,
eacb52f010a807 Nathan Rossi 2021-11-02  555  				 u32 attr, int channel)
eacb52f010a807 Nathan Rossi 2021-11-02  556  {
f08436905922ae Wenliang Yan 2025-01-13  557  	const struct ina238_data *data = drvdata;
f08436905922ae Wenliang Yan 2025-01-13  558  	bool has_power_highest = data->config->has_power_highest;
f08436905922ae Wenliang Yan 2025-01-13  559  
eacb52f010a807 Nathan Rossi 2021-11-02  560  	switch (type) {
eacb52f010a807 Nathan Rossi 2021-11-02  561  	case hwmon_in:
eacb52f010a807 Nathan Rossi 2021-11-02  562  		switch (attr) {
eacb52f010a807 Nathan Rossi 2021-11-02  563  		case hwmon_in_input:
eacb52f010a807 Nathan Rossi 2021-11-02  564  		case hwmon_in_max_alarm:
eacb52f010a807 Nathan Rossi 2021-11-02  565  		case hwmon_in_min_alarm:
eacb52f010a807 Nathan Rossi 2021-11-02  566  			return 0444;
eacb52f010a807 Nathan Rossi 2021-11-02  567  		case hwmon_in_max:
eacb52f010a807 Nathan Rossi 2021-11-02  568  		case hwmon_in_min:
eacb52f010a807 Nathan Rossi 2021-11-02  569  			return 0644;
eacb52f010a807 Nathan Rossi 2021-11-02  570  		default:
eacb52f010a807 Nathan Rossi 2021-11-02  571  			return 0;
eacb52f010a807 Nathan Rossi 2021-11-02  572  		}
eacb52f010a807 Nathan Rossi 2021-11-02  573  	case hwmon_curr:
eacb52f010a807 Nathan Rossi 2021-11-02  574  		switch (attr) {
eacb52f010a807 Nathan Rossi 2021-11-02  575  		case hwmon_curr_input:
eacb52f010a807 Nathan Rossi 2021-11-02  576  			return 0444;
eacb52f010a807 Nathan Rossi 2021-11-02  577  		default:
eacb52f010a807 Nathan Rossi 2021-11-02  578  			return 0;
eacb52f010a807 Nathan Rossi 2021-11-02  579  		}
eacb52f010a807 Nathan Rossi 2021-11-02  580  	case hwmon_power:
eacb52f010a807 Nathan Rossi 2021-11-02 @581  		switch (attr) {
eacb52f010a807 Nathan Rossi 2021-11-02  582  		case hwmon_power_input:
eacb52f010a807 Nathan Rossi 2021-11-02  583  		case hwmon_power_max_alarm:
eacb52f010a807 Nathan Rossi 2021-11-02  584  			return 0444;
eacb52f010a807 Nathan Rossi 2021-11-02  585  		case hwmon_power_max:
eacb52f010a807 Nathan Rossi 2021-11-02  586  			return 0644;
f08436905922ae Wenliang Yan 2025-01-13  587  		case hwmon_power_input_highest:
f08436905922ae Wenliang Yan 2025-01-13  588  			if (has_power_highest)
f08436905922ae Wenliang Yan 2025-01-13  589  				return 0444;
f08436905922ae Wenliang Yan 2025-01-13  590  			break;
eacb52f010a807 Nathan Rossi 2021-11-02  591  		default:
eacb52f010a807 Nathan Rossi 2021-11-02  592  			return 0;
eacb52f010a807 Nathan Rossi 2021-11-02  593  		}
eacb52f010a807 Nathan Rossi 2021-11-02  594  	case hwmon_temp:
eacb52f010a807 Nathan Rossi 2021-11-02  595  		switch (attr) {
eacb52f010a807 Nathan Rossi 2021-11-02  596  		case hwmon_temp_input:
eacb52f010a807 Nathan Rossi 2021-11-02  597  		case hwmon_temp_max_alarm:
eacb52f010a807 Nathan Rossi 2021-11-02  598  			return 0444;
eacb52f010a807 Nathan Rossi 2021-11-02  599  		case hwmon_temp_max:
eacb52f010a807 Nathan Rossi 2021-11-02  600  			return 0644;
eacb52f010a807 Nathan Rossi 2021-11-02  601  		default:
eacb52f010a807 Nathan Rossi 2021-11-02  602  			return 0;
eacb52f010a807 Nathan Rossi 2021-11-02  603  		}
eacb52f010a807 Nathan Rossi 2021-11-02  604  	default:
eacb52f010a807 Nathan Rossi 2021-11-02  605  		return 0;
eacb52f010a807 Nathan Rossi 2021-11-02  606  	}
eacb52f010a807 Nathan Rossi 2021-11-02  607  }
eacb52f010a807 Nathan Rossi 2021-11-02  608  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[Wenliang Yan]

At 2025-01-14 01:12:32, "Conor Dooley" <conor@kernel.org> wrote:
>On Mon, Jan 13, 2025 at 11:50:22AM +0800, Wenliang Yan wrote:
>> Add the sq52206 compatible to the ina2xx.yaml
>> 
>> Signed-off-by: Wenliang Yan <wenliang202407@163.com>
>> ---
>> 
>> The new features added to sq52206 compared to ina238 do not
>> affect the differences in hardware properties.The properties
>> described in the ina2xx.yaml are applicable to the sq52206.
>
>This blurb is a bit confusing, as it makes it seem like the devices are
>compatible. The driver patch, however, suggests otherwise. Please
>mention in your commit message what differs between the new device
>you're adding and existing ones.
>

Sorry for not mentioning their differences.

sq52206 has a difference, the shunt gain value 1 is mapped to
ADCRANGE=10/11, the value 2 is mapped to ADCRANGE=01, and the value 2 is
mapped to ADCRANGE=00.

>> 
>>  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 05a9cb36cd82..f0b7758ab29f 100644
>> --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>> @@ -20,6 +20,7 @@ description: |
>>  properties:
>>    compatible:
>>      enum:
>> +      - silergy,sq52206
>>        - silergy,sy24655
>>        - ti,ina209
>>        - ti,ina219
>> -- 
>> 2.43.0
>> 

Thanks,
Wenliang Yan

Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206

[Wenliang Yan]

At 2025-01-14 01:45:48, "Guenter Roeck" <linux@roeck-us.net> wrote:
>On Mon, Jan 13, 2025 at 11:50:23AM +0800, Wenliang Yan wrote:
>> Add support for SQ52206 to the Ina238 driver. Add registers,
>> add calculation formulas, increase compatibility, add
>> compatibility programs for multiple chips.
>> 
>> Signed-off-by: Wenliang Yan <wenliang202407@163.com>
>> ---
>...
>> @@ -452,6 +554,9 @@ static umode_t ina238_is_visible(const void *drvdata,
>>  				 enum hwmon_sensor_types type,
>>  				 u32 attr, int channel)
>>  {
>> +	const struct ina238_data *data = drvdata;
>> +	bool has_power_highest = data->config->has_power_highest;
>> +
>>  	switch (type) {
>>  	case hwmon_in:
>>  		switch (attr) {
>> @@ -479,6 +584,10 @@ static umode_t ina238_is_visible(const void *drvdata,
>>  			return 0444;
>>  		case hwmon_power_max:
>>  			return 0644;
>> +		case hwmon_power_input_highest:
>> +			if (has_power_highest)
>> +				return 0444;
>> +			break;
>
>This doesn't work as intended. The break; results in the code
>entering the hwmon_temp: case. It has to be "return 0;" or
>the entire function needs to be rewritten to use "break;"
>to exit the switch statements, and "return 0;" at the end
>of the function.
>

Yes, I made a mistake with the nested hierarchy. I will change
'break' to 'return 0'

>>  static int ina238_probe(struct i2c_client *client)
>>  {
>...
>> -	if (data->gain == 1)
>> +	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)
>
>The "else" here is the else from "if (data->gain == 2)" which is wrong.
>The entire if/else block from "if (chip == sq52206)" needs to be in {}
>to avoid that.
>

Agree.

>>  		config |= INA238_CONFIG_ADCRANGE; /* ADCRANGE = 1 is /1 */
>>  	ret = regmap_write(data->regmap, INA238_CONFIG, config);
>>  	if (ret < 0) {
>> @@ -605,7 +735,8 @@ static int ina238_probe(struct i2c_client *client)
>>  
>>  	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
>>  							 &ina238_chip_info,
>> -							 NULL);
>> +							 data->config->has_energy ?
>> +								NULL : ina238_groups);
>
>This seems to be wrong check. I would assume that ina238_groups is passed
>if data->config->has_energy is true, not if it is false.
>

Yes, the logic here is reversed. Should exchange NULL and ina238_groups.

>Guenter

Thanks,
Wenliang Yan