* [PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206 @ 2025-01-13 3:50 Wenliang Yan 2025-01-13 3:50 ` [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings Wenliang Yan 2025-01-13 3:50 ` Wenliang Yan 0 siblings, 2 replies; 9+ messages in thread From: Wenliang Yan @ 2025-01-13 3:50 UTC (permalink / raw) To: linux, jdelvare Cc: Wenliang Yan, robh, krzk+dt, conor+dt, corbet, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings 2025-01-13 3:50 [PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206 Wenliang Yan @ 2025-01-13 3:50 ` Wenliang Yan 2025-01-13 17:12 ` Conor Dooley 2025-01-13 3:50 ` Wenliang Yan 1 sibling, 1 reply; 9+ messages in thread From: Wenliang Yan @ 2025-01-13 3:50 UTC (permalink / raw) To: linux, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Wenliang Yan, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings 2025-01-13 3:50 ` [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings Wenliang Yan @ 2025-01-13 17:12 ` Conor Dooley 2025-01-14 7:43 ` [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 Wenliang Yan 0 siblings, 1 reply; 9+ messages in thread From: Conor Dooley @ 2025-01-13 17:12 UTC (permalink / raw) To: Wenliang Yan Cc: linux, Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 17:12 ` Conor Dooley @ 2025-01-14 7:43 ` Wenliang Yan 0 siblings, 0 replies; 9+ messages in thread From: Wenliang Yan @ 2025-01-14 7:43 UTC (permalink / raw) To: conor Cc: conor+dt, corbet, devicetree, jdelvare, krzk+dt, linux-hwmon, linux-kernel, linux, robh, wenliang202407 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 3:50 [PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206 Wenliang Yan 2025-01-13 3:50 ` [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings Wenliang Yan @ 2025-01-13 3:50 ` Wenliang Yan 2025-01-13 6:52 ` kernel test robot ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Wenliang Yan @ 2025-01-13 3:50 UTC (permalink / raw) To: linux, Jean Delvare Cc: Wenliang Yan, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-hwmon, linux-kernel 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, ®val); + 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, ®val); 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, ®val); 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, ®val); @@ -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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 3:50 ` Wenliang Yan @ 2025-01-13 6:52 ` kernel test robot 2025-01-13 17:45 ` Guenter Roeck 2025-01-13 18:30 ` kernel test robot 2 siblings, 0 replies; 9+ messages in thread From: kernel test robot @ 2025-01-13 6:52 UTC (permalink / raw) To: Wenliang Yan, linux, Jean Delvare Cc: llvm, oe-kbuild-all, Wenliang Yan, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 3:50 ` Wenliang Yan 2025-01-13 6:52 ` kernel test robot @ 2025-01-13 17:45 ` Guenter Roeck 2025-01-14 7:44 ` Wenliang Yan 2025-01-13 18:30 ` kernel test robot 2 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2025-01-13 17:45 UTC (permalink / raw) To: Wenliang Yan Cc: Jean Delvare, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 17:45 ` Guenter Roeck @ 2025-01-14 7:44 ` Wenliang Yan 0 siblings, 0 replies; 9+ messages in thread From: Wenliang Yan @ 2025-01-14 7:44 UTC (permalink / raw) To: linux Cc: conor+dt, corbet, jdelvare, krzk+dt, linux-hwmon, linux-kernel, robh, wenliang202407 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 2025-01-13 3:50 ` Wenliang Yan 2025-01-13 6:52 ` kernel test robot 2025-01-13 17:45 ` Guenter Roeck @ 2025-01-13 18:30 ` kernel test robot 2 siblings, 0 replies; 9+ messages in thread From: kernel test robot @ 2025-01-13 18:30 UTC (permalink / raw) To: Wenliang Yan, linux, Jean Delvare Cc: oe-kbuild-all, Wenliang Yan, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, linux-hwmon, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-01-14 7:46 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-13 3:50 [PATCH v2 0/2] hwmon:(ina238)Add support for SQ52206 Wenliang Yan 2025-01-13 3:50 ` [PATCH v2 1/2] dt-bindings:Add SQ52206 to ina2xx devicetree bindings Wenliang Yan 2025-01-13 17:12 ` Conor Dooley 2025-01-14 7:43 ` [PATCH v2 2/2] hwmon:(ina238)Add support for SQ52206 Wenliang Yan 2025-01-13 3:50 ` Wenliang Yan 2025-01-13 6:52 ` kernel test robot 2025-01-13 17:45 ` Guenter Roeck 2025-01-14 7:44 ` Wenliang Yan 2025-01-13 18:30 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox