Documentation/hwmon/index.rst | 1 + Documentation/hwmon/sq52205.rst | 44 +++ MAINTAINERS | 5 + drivers/hwmon/Kconfig | 13 + drivers/hwmon/Makefile | 1 + drivers/hwmon/sq52205.c | 558 ++++++++++++++++++++++++++++++++ 6 files changed, 622 insertions(+) create mode 100644 Documentation/hwmon/sq52205.rst create mode 100644 drivers/hwmon/sq52205.c
Thank you for bringing up your questions and suggestions.The SQ52205 is a
part number specific to the Asian region, which is why you might not be
able to find it through a search. I'll provide you the website
(https://us1.silergy.com/zh/productsview/SQ52205FBP).
Some registers of this chip are similar to those of the INA226, but it has
additional registers such as integrators, which is the main reason why I'm
offering a new driver.And I plan add drivers of the same series based on
this. I commit the new patch and look forward to your reply.
Signed-off-by: Wenliang <wenliang202407@163.com>
---
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/sq52205.rst | 44 +++
MAINTAINERS | 5 +
drivers/hwmon/Kconfig | 13 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/sq52205.c | 558 ++++++++++++++++++++++++++++++++
6 files changed, 622 insertions(+)
create mode 100644 Documentation/hwmon/sq52205.rst
create mode 100644 drivers/hwmon/sq52205.c
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 913c11390a45..5130270c8efe 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -219,6 +219,7 @@ Hardware Monitoring Kernel Drivers
smsc47m1
sparx5-temp
spd5118
+ sq52205
stpddc60
surface_fan
sy7636a-hwmon
diff --git a/Documentation/hwmon/sq52205.rst b/Documentation/hwmon/sq52205.rst
new file mode 100644
index 000000000000..e66bf883b42b
--- /dev/null
+++ b/Documentation/hwmon/sq52205.rst
@@ -0,0 +1,44 @@
+SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver sq52205
+====================
+
+Supported chips:
+
+ * Silergy SQ52205
+
+
+ Prefix: 'sq52205'
+ Addresses: I2C 0x40 - 0x4f
+
+ Datasheet: Publicly available at the Silergy website
+
+ https://us1.silergy.com/zh
+
+
+Author: Wenliang Yan <wenliang202407@163.com>
+
+Description
+-----------
+
+The SQ52205 is a high- and low-side current shunt and power monitor with an I2C
+interface. The SQ52205 both shunt drop and supply voltage, with programmable
+calibration value and conversion times. The SQ52205 can also calculate average
+power for use in energy conversion.
+
+
+
+Sysfs entries
+---------------------
+
+======================= ===============================
+in0_input Shunt voltage(mV) channel
+in1_input Bus voltage(mV) channel
+curr1_input Current(mA) measurement channel
+power1_input Power(uW) measurement channel
+shunt_resistor Shunt resistance(uOhm) channel
+update_interval data conversion time; affects number of samples used
+ to average results for shunt and bus voltages.
+calculate_avg_power calculate average power from last reading to the present
+======================= ===============================
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 8766f3e5e87e..4794c703da34 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20822,6 +20822,11 @@ S: Maintained
F: drivers/input/touchscreen/silead.c
F: drivers/platform/x86/touchscreen_dmi.c
+SILERGY HARDWARE MONITOR DRIVER
+M: Wenliang Yan <wenliang202407@163.com>
+S: Maintained
+F: drivers/hwmon/sq52205.c
+
SILICON LABS WIRELESS DRIVERS (for WFxxx series)
M: Jérôme Pouiller <jerome.pouiller@silabs.com>
S: Supported
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..7a6c21c9eee9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2521,6 +2521,19 @@ config SENSORS_INTEL_M10_BMC_HWMON
sensors monitor various telemetry data of different components on the
card, e.g. board temperature, FPGA core temperature/voltage/current.
+config SENSORS_SQ52205
+ tristate "Silergy SQ52205 and compatibles"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ If you say yes here you get support for SQ52205 power monitor chips.
+
+ The SQ52205 driver is configured for the default configuration of
+ the part as described in the datasheet.
+ Default value for Rshunt is 10 mOhms.
+ This driver can also be built as a module. If so, the module
+ will be called sq52205.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index b1c7056c37db..270f88e3c6e6 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -207,6 +207,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
obj-$(CONFIG_SENSORS_SPARX5) += sparx5-temp.o
obj-$(CONFIG_SENSORS_SPD5118) += spd5118.o
+obj-$(CONFIG_SENSORS_SQ52205) += sq52205.o
obj-$(CONFIG_SENSORS_STTS751) += stts751.o
obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o
obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o
diff --git a/drivers/hwmon/sq52205.c b/drivers/hwmon/sq52205.c
new file mode 100644
index 000000000000..a7a674a50289
--- /dev/null
+++ b/drivers/hwmon/sq52205.c
@@ -0,0 +1,558 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for SQ52205 power monitor chip
+ *
+ * Copyright (C) 2024 Wenliang Yan <wenliang202407@163.com>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/jiffies.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+#include <linux/util_macros.h>
+#include <linux/regmap.h>
+
+
+
+/* common register definitions */
+#define SQ522XX_CONFIG 0x00
+#define SQ522XX_SHUNT_VOLTAGE 0x01 /* readonly */
+#define SQ522XX_BUS_VOLTAGE 0x02 /* readonly */
+#define SQ522XX_POWER 0x03 /* readonly */
+#define SQ522XX_CURRENT 0x04 /* readonly */
+#define SQ522XX_CALIBRATION 0x05
+
+/* SQ52205 register definitions */
+#define SQ52205_MASK_ENABLE 0x06
+#define SQ52205_ALERT_LIMIT 0x07
+#define SQ52205_EIN 0x0A
+#define SQ52205_ACCUM_CONFIG 0x0D
+
+/* register count */
+#define SQ52205_REGISTERS 0x0D
+#define SQ522XX_MAX_REGISTERS 0x0D
+
+/* settings - default */
+#define SQ52205_CONFIG_DEFAULT 0x4527 /* averages=16 */
+#define SQ52205_ACCUM_CONFIG_DEFAULT 0x044C
+
+/* worst case is 68.10 ms (~14.6Hz) */
+#define SQ522XX_CONVERSION_RATE 15
+#define SQ522XX_MAX_DELAY 69 /* worst case delay in ms */
+
+#define SQ522XX_RSHUNT_DEFAULT 10000 //10mOhms
+#define SQ52205_BUS_VOLTAGE_LSB 1250 //1.25mV
+
+/* bit mask for reading the averaging setting in the configuration register */
+#define SQ52205_AVG_RD_MASK 0x0E00
+
+#define SQ52205_READ_AVG(reg) (((reg) & SQ52205_AVG_RD_MASK) >> 9)
+#define SQ52205_SHIFT_AVG(val) ((val) << 9)
+
+/* common attrs, sq52205 attrs and NULL */
+#define SQ522XX_MAX_ATTRIBUTE_GROUPS 3
+
+/*
+ * Both bus voltage and shunt voltage conversion times for sq52205 are set
+ * to 0b0100 on POR, which translates to 2200 microseconds in total.
+ */
+#define SQ52205_TOTAL_CONV_TIME_DEFAULT 2200
+
+static const struct regmap_config sq522xx_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+};
+
+enum sq522xx_ids { sq52205 };
+
+struct sq522xx_config {
+ u16 config_default;
+ int calibration_factor;
+ int registers;
+ int shunt_div;
+ int bus_voltage_shift;
+ int bus_voltage_lsb; /* uV */
+ int power_lsb; /* uW */
+};
+
+struct sq522xx_data {
+ const struct sq522xx_config *config;
+
+ long rshunt;
+
+ struct mutex config_lock;
+ struct i2c_client *client;
+ struct regmap *regmap;
+
+ const struct attribute_group *groups[SQ522XX_MAX_ATTRIBUTE_GROUPS];
+};
+
+static const struct sq522xx_config sq522xx_config[] = {
+ [sq52205] = {
+ .config_default = SQ52205_CONFIG_DEFAULT,
+ .calibration_factor = 5120000,
+ .registers = SQ52205_REGISTERS,
+ .shunt_div = 400,
+ .bus_voltage_shift = 0,
+ .bus_voltage_lsb = SQ52205_BUS_VOLTAGE_LSB,
+ .power_lsb = 25000,
+ },
+};
+
+/*
+ * Available averaging rates for sq52205. The indices correspond with
+ * the bit values expected by the chip (according to the sq52205 datasheet)
+ */
+static const int sq52205_avg_tab[] = { 1, 4, 16, 64, 128, 256, 512, 1024 };
+
+static int sq52205_reg_to_interval(u16 config)
+{
+ int avg = sq52205_avg_tab[SQ52205_READ_AVG(config)];
+
+ /*
+ * Multiply the total conversion time by the number of averages.
+ * Return the result in milliseconds.
+ */
+ return DIV_ROUND_CLOSEST(avg * SQ52205_TOTAL_CONV_TIME_DEFAULT, 1000);
+}
+
+/*
+ * Return the new, shifted AVG field value of CONFIG register,
+ * to use with regmap_update_bits
+ */
+static u16 sq52205_interval_to_reg(int interval)
+{
+ int avg, avg_bits;
+
+ avg = DIV_ROUND_CLOSEST(interval * 1000,
+ SQ52205_TOTAL_CONV_TIME_DEFAULT);
+ avg_bits = find_closest(avg, sq52205_avg_tab,
+ ARRAY_SIZE(sq52205_avg_tab));
+
+ return SQ52205_SHIFT_AVG(avg_bits);
+}
+
+/*
+ * Calibration register is set to the best value, which eliminates
+ * truncation errors on calculating current register in hardware.
+ * According to datasheet the best values are 2048 for
+ * sq52205. They are hardcoded as calibration_value.
+ */
+static int sq522xx_calibrate(struct sq522xx_data *data)
+{
+ u16 val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
+ data->rshunt);
+
+ return regmap_write(data->regmap, SQ522XX_CALIBRATION,
+ val);
+}
+
+/*
+ * Initialize the configuration and calibration registers.
+ */
+static int sq522xx_init(struct sq522xx_data *data)
+{
+ int ret = regmap_write(data->regmap, SQ522XX_CONFIG,
+ data->config->config_default);
+ if (ret < 0)
+ return ret;
+
+ return sq522xx_calibrate(data);
+}
+static int sq52205_init(struct sq522xx_data *data)
+{
+ // configure ENABLE register
+ int ret = regmap_write(data->regmap, SQ52205_ACCUM_CONFIG,
+ SQ52205_ACCUM_CONFIG_DEFAULT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int sq522xx_read_reg(struct device *dev, int reg, unsigned int *regval)
+{
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+ int ret, retry;
+
+ dev_dbg(dev, "Starting register %d read\n", reg);
+
+ for (retry = 5; retry; retry--) {
+
+ ret = regmap_read(data->regmap, reg, regval);
+ if (ret < 0)
+ return ret;
+
+ dev_dbg(dev, "read %d, val = 0x%04x\n", reg, *regval);
+
+ /*
+ * If the current value in the calibration register is 0, the
+ * power and current registers will also remain at 0. In case
+ * the chip has been reset let's check the calibration
+ * register and reinitialize if needed.
+ * We do that extra read of the calibration register if there
+ * is some hint of a chip reset.
+ */
+ if (*regval == 0) {
+ unsigned int cal;
+
+ ret = regmap_read(data->regmap, SQ522XX_CALIBRATION,
+ &cal);
+ if (ret < 0)
+ return ret;
+
+ if (cal == 0) {
+ dev_warn(dev, "chip not calibrated, reinitializing\n");
+
+ ret = sq522xx_init(data);
+ if (ret < 0)
+ return ret;
+ /*
+ * Let's make sure the power and current
+ * registers have been updated before trying
+ * again.
+ */
+ msleep(SQ522XX_MAX_DELAY);
+ continue;
+ }
+ }
+ return 0;
+ }
+
+ /*
+ * If we're here then although all write operations succeeded, the
+ * chip still returns 0 in the calibration register. Nothing more we
+ * can do here.
+ */
+ dev_err(dev, "unable to reinitialize the chip\n");
+ return -ENODEV;
+}
+
+static int sq522xx_get_value(struct sq522xx_data *data, u8 reg,
+ unsigned int regval)
+{
+ int val;
+
+ switch (reg) {
+ case SQ522XX_SHUNT_VOLTAGE:
+ /* signed register , value is in mV*/
+ val = DIV_ROUND_CLOSEST((s16)regval, data->config->shunt_div);
+ break;
+ case SQ522XX_BUS_VOLTAGE:
+ /* signed register , value is in mV*/
+ val = (regval >> data->config->bus_voltage_shift)
+ * data->config->bus_voltage_lsb;
+ val = DIV_ROUND_CLOSEST(val, 1000);
+ break;
+ case SQ522XX_POWER:
+ /* value is in uV*/
+ val = regval * data->config->power_lsb;
+ break;
+ case SQ522XX_CURRENT:
+ /* signed register, LSB=1mA (selected), in mA */
+ val = (s16)regval;
+ break;
+ case SQ522XX_CALIBRATION:
+ val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
+ regval);
+ break;
+ default:
+ /* programmer goofed */
+ WARN_ON_ONCE(1);
+ val = 0;
+ break;
+ }
+
+ return val;
+}
+
+static ssize_t sq522xx_value_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+ unsigned int regval;
+
+ int err = sq522xx_read_reg(dev, attr->index, ®val);
+
+ if (err < 0)
+ return err;
+
+ return sysfs_emit(buf, "%d\n", sq522xx_get_value(data, attr->index, regval));
+}
+
+
+/*
+ * In order to keep calibration register value fixed, the product
+ * of current_lsb and shunt_resistor should also be fixed and equal
+ * to shunt_voltage_lsb = 1 / shunt_div multiplied by 10^9 in order
+ * to keep the scale.
+ */
+static ssize_t sq522xx_shunt_store(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ unsigned long val;
+ int status;
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+
+ status = kstrtoul(buf, 10, &val);
+ if (status < 0)
+ return status;
+ /* Values greater than the calibration factor make no sense. */
+ if (val == 0 || val > data->config->calibration_factor)
+ return -EINVAL;
+
+ mutex_lock(&data->config_lock);
+ data->rshunt = val;
+
+ status = sq522xx_calibrate(data);
+ mutex_unlock(&data->config_lock);
+ if (status < 0)
+ return status;
+
+ return count;
+}
+
+static ssize_t sq522xx_shunt_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%li\n", data->rshunt);
+}
+
+
+
+static ssize_t sq52205_interval_store(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+ unsigned long val;
+ int status;
+
+ status = kstrtoul(buf, 10, &val);
+ if (status < 0)
+ return status;
+
+ if (val > INT_MAX || val == 0)
+ return -EINVAL;
+
+ status = regmap_update_bits(data->regmap, SQ522XX_CONFIG,
+ SQ52205_AVG_RD_MASK,
+ sq52205_interval_to_reg(val));
+ if (status < 0)
+ return status;
+
+ return count;
+}
+
+static ssize_t sq52205_interval_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+ int status;
+ unsigned int regval;
+
+ status = regmap_read(data->regmap, SQ522XX_CONFIG, ®val);
+ if (status)
+ return status;
+
+ return sysfs_emit(buf, "%d\n", sq52205_reg_to_interval(regval));
+}
+
+static int sq52205_read_reg48(const struct i2c_client *client, u8 reg,
+ long *accumulator_24, long *sample_count)
+{
+ u8 data[6];
+ int err;
+ *accumulator_24 = 0;
+ *sample_count = 0;
+
+ /* 48-bit register read */
+ err = i2c_smbus_read_i2c_block_data(client, reg, 6, data);
+ if (err < 0)
+ return err;
+ if (err != 6)
+ return -EIO;
+ *accumulator_24 = ((data[3] << 16) |
+ (data[4] << 8) |
+ data[5]);
+ *sample_count = ((data[0] << 16) |
+ (data[1] << 8) |
+ data[2]);
+
+ return 0;
+}
+
+static ssize_t sq52205_avg_power_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sq522xx_data *data = dev_get_drvdata(dev);
+ long sample_count, accumulator_24, regval;
+ int status;
+
+ status = sq52205_read_reg48(data->client, SQ52205_EIN,
+ &accumulator_24, &sample_count);
+ if (status)
+ return status;
+ regval = DIV_ROUND_CLOSEST(accumulator_24, sample_count);
+ regval = regval * data->config->power_lsb;
+
+
+ return sysfs_emit(buf, "%li\n", regval);
+}
+
+/* shunt voltage */
+static SENSOR_DEVICE_ATTR_RO(in0_input, sq522xx_value, SQ522XX_SHUNT_VOLTAGE);
+
+/* bus voltage */
+static SENSOR_DEVICE_ATTR_RO(in1_input, sq522xx_value, SQ522XX_BUS_VOLTAGE);
+
+/* calculated current */
+static SENSOR_DEVICE_ATTR_RO(curr1_input, sq522xx_value, SQ522XX_CURRENT);
+
+/* calculated power */
+static SENSOR_DEVICE_ATTR_RO(power1_input, sq522xx_value, SQ522XX_POWER);
+
+/* shunt resistance */
+static SENSOR_DEVICE_ATTR_RW(shunt_resistor, sq522xx_shunt, SQ522XX_CALIBRATION);
+
+/* update interval (sq52205 only) */
+static SENSOR_DEVICE_ATTR_RW(update_interval, sq52205_interval, 0);
+
+/* calculate_avg_power (sq52205 only) */
+static SENSOR_DEVICE_ATTR_RO(calculate_avg_power, sq52205_avg_power, 0);
+
+
+/* pointers to created device attributes */
+static struct attribute *sq522xx_attrs[] = {
+ &sensor_dev_attr_in0_input.dev_attr.attr,
+ &sensor_dev_attr_in1_input.dev_attr.attr,
+ &sensor_dev_attr_curr1_input.dev_attr.attr,
+ &sensor_dev_attr_power1_input.dev_attr.attr,
+ &sensor_dev_attr_shunt_resistor.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group sq522xx_group = {
+ .attrs = sq522xx_attrs,
+};
+
+static struct attribute *sq52205_attrs[] = {
+ &sensor_dev_attr_update_interval.dev_attr.attr,
+ &sensor_dev_attr_calculate_avg_power.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group sq52205_group = {
+ .attrs = sq52205_attrs,
+};
+
+static const struct i2c_device_id sq522xx_id[];
+
+static int sq522xx_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct sq522xx_data *data;
+ struct device *hwmon_dev;
+ u32 val;
+ int ret, group = 0;
+ enum sq522xx_ids chip;
+
+ if (client->dev.of_node)
+ chip = (uintptr_t)of_device_get_match_data(&client->dev);
+ else
+ chip = i2c_match_id(sq522xx_id, client)->driver_data;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ /* set the device type */
+ data->client = client;
+ data->config = &sq522xx_config[chip];
+ mutex_init(&data->config_lock);
+
+ if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0)
+ val = SQ522XX_RSHUNT_DEFAULT;
+
+
+ if (val <= 0 || val > data->config->calibration_factor)
+ return -ENODEV;
+
+ data->rshunt = val;
+
+ sq522xx_regmap_config.max_register = data->config->registers;
+
+ data->regmap = devm_regmap_init_i2c(client, &sq522xx_regmap_config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(dev, "failed to allocate register map\n");
+ return PTR_ERR(data->regmap);
+ }
+
+
+ ret = sq522xx_init(data);
+ if (ret < 0) {
+ dev_err(dev, "error configuring the device: %d\n", ret);
+ return -ENODEV;
+ }
+ if (chip == sq52205) {
+ ret = sq52205_init(data);
+ if (ret < 0) {
+ dev_err(dev, "error configuring the device cal: %d\n", ret);
+ return -ENODEV;
+ }
+ }
+
+ data->groups[group++] = &sq522xx_group;
+ if (chip == sq52205)
+ data->groups[group++] = &sq52205_group;
+
+ hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
+ data, data->groups);
+ if (IS_ERR(hwmon_dev))
+ return PTR_ERR(hwmon_dev);
+
+ dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
+ client->name, data->rshunt);
+
+ return 0;
+}
+
+static const struct i2c_device_id sq522xx_id[] = {
+ { "sq52205", sq52205 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sq522xx_id);
+
+static const struct of_device_id __maybe_unused sq522xx_of_match[] = {
+ {
+ .compatible = "silergy,sq52205",
+ .data = (void *)sq52205
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sq522xx_of_match);
+
+static struct i2c_driver sq522xx_driver = {
+ .driver = {
+ .name = "sq522xx",
+ .of_match_table = of_match_ptr(sq522xx_of_match),
+ },
+ .probe = sq522xx_probe,
+ .id_table = sq522xx_id,
+};
+
+module_i2c_driver(sq522xx_driver);
+
+MODULE_AUTHOR(" <wenliang202407@163.com> ");
+MODULE_DESCRIPTION("SQ522xx driver");
+MODULE_LICENSE("GPL");
--
2.17.1
Hi Wenliang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linux/master]
url: https://github.com/intel-lab-lkp/linux/commits/Wenliang/hwmon-add-new-hwmon-driver-sq52205/20240912-002906
base: linux/master
patch link: https://lore.kernel.org/r/20240822074426.7241-1-wenliang202407%40163.com
patch subject: [PATCH linux dev 6.11] hwmon:add new hwmon driver sq52205
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240914/202409140727.4pErU6oc-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bf684034844c660b778f0eba103582f582b710c9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409140727.4pErU6oc-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/202409140727.4pErU6oc-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/hwmon/sq52205.c:12:
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:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/hwmon/sq52205.c:12:
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:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/hwmon/sq52205.c:12:
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:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/hwmon/sq52205.c:12:
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:2232:
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/hwmon/sq52205.c:493:37: error: cannot assign to variable 'sq522xx_regmap_config' with const-qualified type 'const struct regmap_config'
493 | sq522xx_regmap_config.max_register = data->config->registers;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
drivers/hwmon/sq52205.c:67:35: note: variable 'sq522xx_regmap_config' declared const here
67 | static const struct regmap_config sq522xx_regmap_config = {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
68 | .reg_bits = 8,
| ~~~~~~~~~~~~~~
69 | .val_bits = 16,
| ~~~~~~~~~~~~~~~
70 | };
| ~
7 warnings and 1 error generated.
vim +493 drivers/hwmon/sq52205.c
460
461 static int sq522xx_probe(struct i2c_client *client)
462 {
463 struct device *dev = &client->dev;
464 struct sq522xx_data *data;
465 struct device *hwmon_dev;
466 u32 val;
467 int ret, group = 0;
468 enum sq522xx_ids chip;
469
470 if (client->dev.of_node)
471 chip = (uintptr_t)of_device_get_match_data(&client->dev);
472 else
473 chip = i2c_match_id(sq522xx_id, client)->driver_data;
474
475 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
476 if (!data)
477 return -ENOMEM;
478
479 /* set the device type */
480 data->client = client;
481 data->config = &sq522xx_config[chip];
482 mutex_init(&data->config_lock);
483
484 if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0)
485 val = SQ522XX_RSHUNT_DEFAULT;
486
487
488 if (val <= 0 || val > data->config->calibration_factor)
489 return -ENODEV;
490
491 data->rshunt = val;
492
> 493 sq522xx_regmap_config.max_register = data->config->registers;
494
495 data->regmap = devm_regmap_init_i2c(client, &sq522xx_regmap_config);
496 if (IS_ERR(data->regmap)) {
497 dev_err(dev, "failed to allocate register map\n");
498 return PTR_ERR(data->regmap);
499 }
500
501
502 ret = sq522xx_init(data);
503 if (ret < 0) {
504 dev_err(dev, "error configuring the device: %d\n", ret);
505 return -ENODEV;
506 }
507 if (chip == sq52205) {
508 ret = sq52205_init(data);
509 if (ret < 0) {
510 dev_err(dev, "error configuring the device cal: %d\n", ret);
511 return -ENODEV;
512 }
513 }
514
515 data->groups[group++] = &sq522xx_group;
516 if (chip == sq52205)
517 data->groups[group++] = &sq52205_group;
518
519 hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
520 data, data->groups);
521 if (IS_ERR(hwmon_dev))
522 return PTR_ERR(hwmon_dev);
523
524 dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
525 client->name, data->rshunt);
526
527 return 0;
528 }
529
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Wenliang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linux/master]
url: https://github.com/intel-lab-lkp/linux/commits/Wenliang/hwmon-add-new-hwmon-driver-sq52205/20240912-002906
base: linux/master
patch link: https://lore.kernel.org/r/20240822074426.7241-1-wenliang202407%40163.com
patch subject: [PATCH linux dev 6.11] hwmon:add new hwmon driver sq52205
config: openrisc-randconfig-r072-20240913 (https://download.01.org/0day-ci/archive/20240913/202409131320.Ne0lQtTj-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240913/202409131320.Ne0lQtTj-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/202409131320.Ne0lQtTj-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/hwmon/sq52205.c: In function 'sq522xx_probe':
>> drivers/hwmon/sq52205.c:493:44: error: assignment of member 'max_register' in read-only object
493 | sq522xx_regmap_config.max_register = data->config->registers;
| ^
vim +/max_register +493 drivers/hwmon/sq52205.c
460
461 static int sq522xx_probe(struct i2c_client *client)
462 {
463 struct device *dev = &client->dev;
464 struct sq522xx_data *data;
465 struct device *hwmon_dev;
466 u32 val;
467 int ret, group = 0;
468 enum sq522xx_ids chip;
469
470 if (client->dev.of_node)
471 chip = (uintptr_t)of_device_get_match_data(&client->dev);
472 else
473 chip = i2c_match_id(sq522xx_id, client)->driver_data;
474
475 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
476 if (!data)
477 return -ENOMEM;
478
479 /* set the device type */
480 data->client = client;
481 data->config = &sq522xx_config[chip];
482 mutex_init(&data->config_lock);
483
484 if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0)
485 val = SQ522XX_RSHUNT_DEFAULT;
486
487
488 if (val <= 0 || val > data->config->calibration_factor)
489 return -ENODEV;
490
491 data->rshunt = val;
492
> 493 sq522xx_regmap_config.max_register = data->config->registers;
494
495 data->regmap = devm_regmap_init_i2c(client, &sq522xx_regmap_config);
496 if (IS_ERR(data->regmap)) {
497 dev_err(dev, "failed to allocate register map\n");
498 return PTR_ERR(data->regmap);
499 }
500
501
502 ret = sq522xx_init(data);
503 if (ret < 0) {
504 dev_err(dev, "error configuring the device: %d\n", ret);
505 return -ENODEV;
506 }
507 if (chip == sq52205) {
508 ret = sq52205_init(data);
509 if (ret < 0) {
510 dev_err(dev, "error configuring the device cal: %d\n", ret);
511 return -ENODEV;
512 }
513 }
514
515 data->groups[group++] = &sq522xx_group;
516 if (chip == sq52205)
517 data->groups[group++] = &sq52205_group;
518
519 hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
520 data, data->groups);
521 if (IS_ERR(hwmon_dev))
522 return PTR_ERR(hwmon_dev);
523
524 dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
525 client->name, data->rshunt);
526
527 return 0;
528 }
529
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Wenliang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linux/master]
url: https://github.com/intel-lab-lkp/linux/commits/Wenliang/hwmon-add-new-hwmon-driver-sq52205/20240826-103932
base: linux/master
patch link: https://lore.kernel.org/r/20240822074426.7241-1-wenliang202407%40163.com
patch subject: [PATCH linux dev 6.11] hwmon:add new hwmon driver sq52205
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240827/202408270025.5A0Q5KZO-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240827/202408270025.5A0Q5KZO-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/202408270025.5A0Q5KZO-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/hwmon/sq52205.c: In function 'sq522xx_probe':
>> drivers/hwmon/sq52205.c:493:44: error: assignment of member 'max_register' in read-only object
493 | sq522xx_regmap_config.max_register = data->config->registers;
| ^
vim +/max_register +493 drivers/hwmon/sq52205.c
460
461 static int sq522xx_probe(struct i2c_client *client)
462 {
463 struct device *dev = &client->dev;
464 struct sq522xx_data *data;
465 struct device *hwmon_dev;
466 u32 val;
467 int ret, group = 0;
468 enum sq522xx_ids chip;
469
470 if (client->dev.of_node)
471 chip = (uintptr_t)of_device_get_match_data(&client->dev);
472 else
473 chip = i2c_match_id(sq522xx_id, client)->driver_data;
474
475 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
476 if (!data)
477 return -ENOMEM;
478
479 /* set the device type */
480 data->client = client;
481 data->config = &sq522xx_config[chip];
482 mutex_init(&data->config_lock);
483
484 if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0)
485 val = SQ522XX_RSHUNT_DEFAULT;
486
487
488 if (val <= 0 || val > data->config->calibration_factor)
489 return -ENODEV;
490
491 data->rshunt = val;
492
> 493 sq522xx_regmap_config.max_register = data->config->registers;
494
495 data->regmap = devm_regmap_init_i2c(client, &sq522xx_regmap_config);
496 if (IS_ERR(data->regmap)) {
497 dev_err(dev, "failed to allocate register map\n");
498 return PTR_ERR(data->regmap);
499 }
500
501
502 ret = sq522xx_init(data);
503 if (ret < 0) {
504 dev_err(dev, "error configuring the device: %d\n", ret);
505 return -ENODEV;
506 }
507 if (chip == sq52205) {
508 ret = sq52205_init(data);
509 if (ret < 0) {
510 dev_err(dev, "error configuring the device cal: %d\n", ret);
511 return -ENODEV;
512 }
513 }
514
515 data->groups[group++] = &sq522xx_group;
516 if (chip == sq52205)
517 data->groups[group++] = &sq52205_group;
518
519 hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
520 data, data->groups);
521 if (IS_ERR(hwmon_dev))
522 return PTR_ERR(hwmon_dev);
523
524 dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
525 client->name, data->rshunt);
526
527 return 0;
528 }
529
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Wenliang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linux/master]
url: https://github.com/intel-lab-lkp/linux/commits/Wenliang/hwmon-add-new-hwmon-driver-sq52205/20240826-103932
base: linux/master
patch link: https://lore.kernel.org/r/20240822074426.7241-1-wenliang202407%40163.com
patch subject: [PATCH linux dev 6.11] hwmon:add new hwmon driver sq52205
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240826/202408261627.lm2xKacD-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240826/202408261627.lm2xKacD-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/202408261627.lm2xKacD-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/hwmon/sq52205.c:493:37: error: cannot assign to variable 'sq522xx_regmap_config' with const-qualified type 'const struct regmap_config'
493 | sq522xx_regmap_config.max_register = data->config->registers;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
drivers/hwmon/sq52205.c:67:35: note: variable 'sq522xx_regmap_config' declared const here
67 | static const struct regmap_config sq522xx_regmap_config = {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
68 | .reg_bits = 8,
| ~~~~~~~~~~~~~~
69 | .val_bits = 16,
| ~~~~~~~~~~~~~~~
70 | };
| ~
1 error generated.
vim +493 drivers/hwmon/sq52205.c
460
461 static int sq522xx_probe(struct i2c_client *client)
462 {
463 struct device *dev = &client->dev;
464 struct sq522xx_data *data;
465 struct device *hwmon_dev;
466 u32 val;
467 int ret, group = 0;
468 enum sq522xx_ids chip;
469
470 if (client->dev.of_node)
471 chip = (uintptr_t)of_device_get_match_data(&client->dev);
472 else
473 chip = i2c_match_id(sq522xx_id, client)->driver_data;
474
475 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
476 if (!data)
477 return -ENOMEM;
478
479 /* set the device type */
480 data->client = client;
481 data->config = &sq522xx_config[chip];
482 mutex_init(&data->config_lock);
483
484 if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0)
485 val = SQ522XX_RSHUNT_DEFAULT;
486
487
488 if (val <= 0 || val > data->config->calibration_factor)
489 return -ENODEV;
490
491 data->rshunt = val;
492
> 493 sq522xx_regmap_config.max_register = data->config->registers;
494
495 data->regmap = devm_regmap_init_i2c(client, &sq522xx_regmap_config);
496 if (IS_ERR(data->regmap)) {
497 dev_err(dev, "failed to allocate register map\n");
498 return PTR_ERR(data->regmap);
499 }
500
501
502 ret = sq522xx_init(data);
503 if (ret < 0) {
504 dev_err(dev, "error configuring the device: %d\n", ret);
505 return -ENODEV;
506 }
507 if (chip == sq52205) {
508 ret = sq52205_init(data);
509 if (ret < 0) {
510 dev_err(dev, "error configuring the device cal: %d\n", ret);
511 return -ENODEV;
512 }
513 }
514
515 data->groups[group++] = &sq522xx_group;
516 if (chip == sq52205)
517 data->groups[group++] = &sq52205_group;
518
519 hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
520 data, data->groups);
521 if (IS_ERR(hwmon_dev))
522 return PTR_ERR(hwmon_dev);
523
524 dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
525 client->name, data->rshunt);
526
527 return 0;
528 }
529
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 8/22/24 00:44, Wenliang wrote: > Thank you for bringing up your questions and suggestions.The SQ52205 is a > part number specific to the Asian region, which is why you might not be > able to find it through a search. I'll provide you the website > (https://us1.silergy.com/zh/productsview/SQ52205FBP). That page does not point to the chip datasheet. The almost identical page at https://us1.silergy.com/productsview/SQ52205FBP does. The datasheet is _not_ "Publicly available" as claimed in this submission. The version I was able to obtain is tagged with "Silergy Confidential For <my employer>", so I am not even sure if I can use it to review this driver submission. > Some registers of this chip are similar to those of the INA226, but it has > additional registers such as integrators, which is the main reason why I'm > offering a new driver.And I plan add drivers of the same series based on That is not a reason to add a separate driver. Look at, for example, lm90.c, which supports a variety of chips in a single driver. The ina2xx driver already does support several chips, and adding another one would be straightforward, even if it is from a different manufacturer. On top of that, only the EIN and ACCUM_CONFIG registers are additional, and the rest appear to be exactly the same as INA226. > this. I commit the new patch and look forward to your reply. > Additonal comments: - Please review and follow Documentation/process/submitting-patches.rst Documentation/hwmon/submitting-patches.rst - As mentioned before, a reworked version of the ina2xx.c driver is available. I am not inclined to accept a new driver for this chip. Even if I were, I would not accept a driver based on deprecated hwmon APIs. I would strongly advise to have a look into the reworked driver. As mentioned before, it is available in the hwmon-staging branch of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git. - "calculate_avg_power" is (unnecessarily) not a standard attribute and therefore unacceptable. Its value (on top of the already averaged power attribute) seems questionable. I would understand an attempt to report the energy, but I fail to understand the value in reporting yet another power average - even more so one that is not well defined in terms of number of samples used to determine the average. Guenter
On 8/25/24 15:50, Guenter Roeck wrote: > On 8/22/24 00:44, Wenliang wrote: >> Thank you for bringing up your questions and suggestions.The SQ52205 is a >> part number specific to the Asian region, which is why you might not be >> able to find it through a search. I'll provide you the website >> (https://us1.silergy.com/zh/productsview/SQ52205FBP). > > That page does not point to the chip datasheet. The almost identical > page at https://us1.silergy.com/productsview/SQ52205FBP does. ... and that page is no longer available. Given that, if you really need support for this chip, you might want to consider adding support for SY24655 and/or SY24657 since public datasheets are available for those chips. The EIN and ACCUM_CONFIG registers in those chips match the definitions in your driver submission, so at least at first glance this should work. I noticed that there is also SY24656, which appears to be register compatible to INA226/INA230. Thanks, Guenter
After listening to your advice, I have adapted SQ52205 by rewriting the
ina2xx driver.At the same time, I would like to clarify that SY24655 and
SQ52205 are different partnumber of the same chip. Therefore, you can
refer to SY24655FBP. I have also changed the naming within the driver to
SY24655, and I hope to receive your response.
Signed-off-by: Wenliang <wenliang202407@163.com>
---
Documentation/hwmon/ina2xx.rst | 24 ++++++++
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/ina2xx.c | 106 +++++++++++++++++++++++++++++++--
3 files changed, 126 insertions(+), 6 deletions(-)
diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
index 27d2e39bc8ac..0bd16a0104a7 100644
--- a/Documentation/hwmon/ina2xx.rst
+++ b/Documentation/hwmon/ina2xx.rst
@@ -53,6 +53,16 @@ Supported chips:
https://www.ti.com/
+ * Silergy SY24655
+
+
+ Prefix: 'sy24655'
+ Addresses: I2C 0x40 - 0x4f
+
+ Datasheet: Publicly available at the Silergy website
+
+ https://us1.silergy.com/
+
Author: Lothar Felten <lothar.felten@gmail.com>
Description
@@ -72,6 +82,11 @@ INA230 and INA231 are high or low side current shunt and power monitors
with an I2C interface. The chips monitor both a shunt voltage drop and
bus supply voltage.
+The SY24655 is a high- and low-side current shunt and power monitor with an I2C
+interface. The SY24655 both shunt drop and supply voltage, with programmable
+calibration value and conversion times. The SY24655 can also calculate average
+power for use in energy conversion.
+
The shunt value in micro-ohms can be set via platform data or device tree at
compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
refer to the Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for bindings
@@ -113,6 +128,15 @@ update_interval data conversion time; affects number of samples used
to average results for shunt and bus voltages.
======================= ====================================================
+Sysfs entries for sy24655 only
+------------------------------------------------
+
+======================= ====================================================
+update_interval data conversion time; affects number of samples used
+ to average results for shunt and bus voltages.
+calculate_avg_power calculate average power from last reading to the present.
+======================= ====================================================
+
.. note::
- Configure `shunt_resistor` before configure `power1_crit`, because power
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..1f9752689ae8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2138,7 +2138,7 @@ config SENSORS_INA2XX
select REGMAP_I2C
help
If you say yes here you get support for INA219, INA220, INA226,
- INA230, and INA231 power monitor chips.
+ INA230, INA231, and Silergy SY24655 power monitor chips.
The INA2xx driver is configured for the default configuration of
the part as described in the datasheet.
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 9ab4205622e2..34d474ac0b66 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -18,6 +18,10 @@
* Bi-directional Current/Power Monitor with I2C Interface
* Datasheet: https://www.ti.com/product/ina230
*
+ * SY24655:
+ * Bi-directional Current/Power Monitor with I2C Interface
+ * Datasheet: https://us1.silergy.com/productsview/SY24655FBP
+ *
* Copyright (C) 2012 Lothar Felten <lothar.felten@gmail.com>
* Thanks to Jan Volkering
*/
@@ -51,15 +55,23 @@
#define INA226_ALERT_LIMIT 0x07
#define INA226_DIE_ID 0xFF
+/* SY24655 register definitions */
+#define SY24655_EIN 0x0A
+#define SY24655_ACCUM_CONFIG 0x0D
+
/* register count */
#define INA219_REGISTERS 6
#define INA226_REGISTERS 8
+#define SY24655_REGISTERS 0x0D
-#define INA2XX_MAX_REGISTERS 8
+#define INA2XX_MAX_REGISTERS 0x0D
/* settings - depend on use case */
#define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
#define INA226_CONFIG_DEFAULT 0x4527 /* averages=16 */
+#define SY24655_CONFIG_DEFAULT 0x4527 /* averages=16 */
+/* (only for sy24655) */
+#define SY24655_ACCUM_CONFIG_DEFAULT 0x044C
/* worst case is 68.10 ms (~14.6Hz, ina219) */
#define INA2XX_CONVERSION_RATE 15
@@ -103,7 +115,7 @@ static struct regmap_config ina2xx_regmap_config = {
.val_bits = 16,
};
-enum ina2xx_ids { ina219, ina226 };
+enum ina2xx_ids { ina219, ina226, sy24655};
struct ina2xx_config {
u16 config_default;
@@ -117,12 +129,13 @@ struct ina2xx_config {
struct ina2xx_data {
const struct ina2xx_config *config;
-
+
long rshunt;
long current_lsb_uA;
long power_lsb_uW;
struct mutex config_lock;
struct regmap *regmap;
+ struct i2c_client *client;
const struct attribute_group *groups[INA2XX_MAX_ATTRIBUTE_GROUPS];
};
@@ -146,6 +159,15 @@ static const struct ina2xx_config ina2xx_config[] = {
.bus_voltage_lsb = 1250,
.power_lsb_factor = 25,
},
+ [sy24655] = {
+ .config_default = SY24655_CONFIG_DEFAULT,
+ .calibration_value = 2048,
+ .registers = SY24655_REGISTERS,
+ .shunt_div = 400,
+ .bus_voltage_shift = 0,
+ .bus_voltage_lsb = 1250,
+ .power_lsb_factor = 25,
+ },
};
/*
@@ -216,6 +238,12 @@ static int ina2xx_init(struct ina2xx_data *data)
return ina2xx_calibrate(data);
}
+static int sy24655_init(struct ina2xx_data *data)
+{
+ return regmap_write(data->regmap, SY24655_ACCUM_CONFIG,
+ SY24655_ACCUM_CONFIG_DEFAULT);
+}
+
static int ina2xx_read_reg(struct device *dev, int reg, unsigned int *regval)
{
struct ina2xx_data *data = dev_get_drvdata(dev);
@@ -551,6 +579,48 @@ static ssize_t ina226_interval_show(struct device *dev,
return sysfs_emit(buf, "%d\n", ina226_reg_to_interval(regval));
}
+static int sy24655_read_reg48(const struct i2c_client *client, u8 reg,
+ long *accumulator_24, long *sample_count)
+{
+ u8 data[6];
+ int err;
+ *accumulator_24 = 0;
+ *sample_count = 0;
+
+ /* 48-bit register read */
+ err = i2c_smbus_read_i2c_block_data(client, reg, 6, data);
+ if (err < 0)
+ return err;
+ if (err != 6)
+ return -EIO;
+ *accumulator_24 = ((data[3] << 16) |
+ (data[4] << 8) |
+ data[5]);
+ *sample_count = ((data[0] << 16) |
+ (data[1] << 8) |
+ data[2]);
+
+ return 0;
+}
+
+static ssize_t sy24655_avg_power_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ long sample_count, accumulator_24, regval;
+ int status;
+
+ status = sy24655_read_reg48(data->client, SY24655_EIN,
+ &accumulator_24, &sample_count);
+ if (status)
+ return status;
+ regval = DIV_ROUND_CLOSEST(accumulator_24, sample_count);
+ regval = regval * data->power_lsb_uW;
+
+
+ return sysfs_emit(buf, "%li\n", regval);
+}
+
/* shunt voltage */
static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
/* shunt voltage over/under voltage alert setting and alarm */
@@ -589,9 +659,13 @@ static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
/* shunt resistance */
static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
-/* update interval (ina226 only) */
+/* update interval (ina226 and sy24655) */
static SENSOR_DEVICE_ATTR_RW(update_interval, ina226_interval, 0);
+/* calculate_avg_power (sy24655 only) */
+static SENSOR_DEVICE_ATTR_RO(calculate_avg_power, sy24655_avg_power, 0);
+
+
/* pointers to created device attributes */
static struct attribute *ina2xx_attrs[] = {
&sensor_dev_attr_in0_input.dev_attr.attr,
@@ -624,6 +698,15 @@ static struct attribute *ina226_attrs[] = {
static const struct attribute_group ina226_group = {
.attrs = ina226_attrs,
};
+static struct attribute *sy24655_attrs[] = {
+ &sensor_dev_attr_update_interval.dev_attr.attr,
+ &sensor_dev_attr_calculate_avg_power.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group sy24655_group = {
+ .attrs = sy24655_attrs,
+};
static int ina2xx_probe(struct i2c_client *client)
{
@@ -641,6 +724,7 @@ static int ina2xx_probe(struct i2c_client *client)
return -ENOMEM;
/* set the device type */
+ data->client = client;
data->config = &ina2xx_config[chip];
mutex_init(&data->config_lock);
@@ -691,10 +775,17 @@ static int ina2xx_probe(struct i2c_client *client)
dev_err(dev, "error configuring the device: %d\n", ret);
return -ENODEV;
}
-
+ if (chip == sy24655)
+ ret = sy24655_init(data);
+ if (ret < 0) {
+ dev_err(dev, "error configuring the accum_reg: %d\n", ret);
+ return -ENODEV;
+ }
data->groups[group++] = &ina2xx_group;
if (chip == ina226)
data->groups[group++] = &ina226_group;
+ else if (chip == sy24655)
+ data->groups[group++] = &sy24655_group;
hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
data, data->groups);
@@ -713,6 +804,7 @@ static const struct i2c_device_id ina2xx_id[] = {
{ "ina226", ina226 },
{ "ina230", ina226 },
{ "ina231", ina226 },
+ { "sy24655", sy24655},
{ }
};
MODULE_DEVICE_TABLE(i2c, ina2xx_id);
@@ -738,6 +830,10 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
.compatible = "ti,ina231",
.data = (void *)ina226
},
+ {
+ .compatible = "silergy,sy24655",
+ .data = (void *)sy24655
+ },
{ },
};
MODULE_DEVICE_TABLE(of, ina2xx_of_match);
--
2.17.1Le 11/09/2024 à 14:25, Wenliang a écrit :
> After listening to your advice, I have adapted SQ52205 by rewriting the
> ina2xx driver.At the same time, I would like to clarify that SY24655 and
> SQ52205 are different partnumber of the same chip. Therefore, you can
> refer to SY24655FBP. I have also changed the naming within the driver to
> SY24655, and I hope to receive your response.
>
> Signed-off-by: Wenliang <wenliang202407@163.com>
> ---
Hi,
...
> @@ -103,7 +115,7 @@ static struct regmap_config ina2xx_regmap_config = {
> .val_bits = 16,
> };
>
> -enum ina2xx_ids { ina219, ina226 };
> +enum ina2xx_ids { ina219, ina226, sy24655};
Nitpick: Missing space before }
>
> struct ina2xx_config {
> u16 config_default;
> @@ -117,12 +129,13 @@ struct ina2xx_config {
>
> struct ina2xx_data {
> const struct ina2xx_config *config;
> -
> +
> long rshunt;
> long current_lsb_uA;
> long power_lsb_uW;
> struct mutex config_lock;
> struct regmap *regmap;
> + struct i2c_client *client;
>
> const struct attribute_group *groups[INA2XX_MAX_ATTRIBUTE_GROUPS];
> };
...
> +static int sy24655_read_reg48(const struct i2c_client *client, u8 reg,
> + long *accumulator_24, long *sample_count)
> +{
> + u8 data[6];
> + int err;
Maybe adding a blank line here?
> + *accumulator_24 = 0;
> + *sample_count = 0;
> +
> + /* 48-bit register read */
> + err = i2c_smbus_read_i2c_block_data(client, reg, 6, data);
> + if (err < 0)
> + return err;
> + if (err != 6)
> + return -EIO;
> + *accumulator_24 = ((data[3] << 16) |
> + (data[4] << 8) |
> + data[5]);
> + *sample_count = ((data[0] << 16) |
> + (data[1] << 8) |
> + data[2]);
> +
> + return 0;
> +}
> +
> +static ssize_t sy24655_avg_power_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> + long sample_count, accumulator_24, regval;
> + int status;
> +
> + status = sy24655_read_reg48(data->client, SY24655_EIN,
> + &accumulator_24, &sample_count);
> + if (status)
> + return status;
> + regval = DIV_ROUND_CLOSEST(accumulator_24, sample_count);
> + regval = regval * data->power_lsb_uW;
> +
> +
Nitpick: unneeded extra empty line
> + return sysfs_emit(buf, "%li\n", regval);
> +}
> +
> /* shunt voltage */
> static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
> /* shunt voltage over/under voltage alert setting and alarm */
> @@ -589,9 +659,13 @@ static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
> /* shunt resistance */
> static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
>
> -/* update interval (ina226 only) */
> +/* update interval (ina226 and sy24655) */
> static SENSOR_DEVICE_ATTR_RW(update_interval, ina226_interval, 0);
>
> +/* calculate_avg_power (sy24655 only) */
> +static SENSOR_DEVICE_ATTR_RO(calculate_avg_power, sy24655_avg_power, 0);
> +
> +
Nitpick: unneeded extra empty line
> /* pointers to created device attributes */
> static struct attribute *ina2xx_attrs[] = {
> &sensor_dev_attr_in0_input.dev_attr.attr,
> @@ -624,6 +698,15 @@ static struct attribute *ina226_attrs[] = {
> static const struct attribute_group ina226_group = {
> .attrs = ina226_attrs,
> };
...
> @@ -691,10 +775,17 @@ static int ina2xx_probe(struct i2c_client *client)
> dev_err(dev, "error configuring the device: %d\n", ret);
> return -ENODEV;
> }
> -
> + if (chip == sy24655)
> + ret = sy24655_init(data);
> + if (ret < 0) {
> + dev_err(dev, "error configuring the accum_reg: %d\n", ret);
> + return -ENODEV;
return ret;?
> + }
> data->groups[group++] = &ina2xx_group;
> if (chip == ina226)
> data->groups[group++] = &ina226_group;
> + else if (chip == sy24655)
> + data->groups[group++] = &sy24655_group;
>
> hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
> data, data->groups);
> @@ -713,6 +804,7 @@ static const struct i2c_device_id ina2xx_id[] = {
> { "ina226", ina226 },
> { "ina230", ina226 },
> { "ina231", ina226 },
> + { "sy24655", sy24655},
Nitpick: missing space before }
> { }
> };
> MODULE_DEVICE_TABLE(i2c, ina2xx_id);
> @@ -738,6 +830,10 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
> .compatible = "ti,ina231",
> .data = (void *)ina226
> },
> + {
> + .compatible = "silergy,sy24655",
> + .data = (void *)sy24655
> + },
> { },
Nitpick: Unrelated, but this comma could be removed.
CJ
> };
> MODULE_DEVICE_TABLE(of, ina2xx_of_match);
On 9/11/24 05:25, Wenliang wrote:
> After listening to your advice, I have adapted SQ52205 by rewriting the
> ina2xx driver.At the same time, I would like to clarify that SY24655 and
> SQ52205 are different partnumber of the same chip. Therefore, you can
> refer to SY24655FBP. I have also changed the naming within the driver to
> SY24655, and I hope to receive your response.
>
This is not an appropriate patch description. The information is useful,
but should be after '---'. Also, this is the second version of your
patch and should be versioned.
Please read and follow Documentation/process/submitting-patches.rst
You did not use the updated version of the ina2xx driver for this patch.
The updated version is now available in linux-next and will be sent upstream
in the next commit window. Consequently your patch is outdated.
Additional comments inline.
> Signed-off-by: Wenliang <wenliang202407@163.com>
> ---
> Documentation/hwmon/ina2xx.rst | 24 ++++++++
> drivers/hwmon/Kconfig | 2 +-
> drivers/hwmon/ina2xx.c | 106 +++++++++++++++++++++++++++++++--
> 3 files changed, 126 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
> index 27d2e39bc8ac..0bd16a0104a7 100644
> --- a/Documentation/hwmon/ina2xx.rst
> +++ b/Documentation/hwmon/ina2xx.rst
> @@ -53,6 +53,16 @@ Supported chips:
>
> https://www.ti.com/
>
> + * Silergy SY24655
> +
> +
> + Prefix: 'sy24655'
> + Addresses: I2C 0x40 - 0x4f
> +
> + Datasheet: Publicly available at the Silergy website
> +
> + https://us1.silergy.com/
> +
> Author: Lothar Felten <lothar.felten@gmail.com>
>
> Description
> @@ -72,6 +82,11 @@ INA230 and INA231 are high or low side current shunt and power monitors
> with an I2C interface. The chips monitor both a shunt voltage drop and
> bus supply voltage.
>
> +The SY24655 is a high- and low-side current shunt and power monitor with an I2C
> +interface. The SY24655 both shunt drop and supply voltage, with programmable
> +calibration value and conversion times. The SY24655 can also calculate average
> +power for use in energy conversion.
> +
> The shunt value in micro-ohms can be set via platform data or device tree at
> compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
> refer to the Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for bindings
> @@ -113,6 +128,15 @@ update_interval data conversion time; affects number of samples used
> to average results for shunt and bus voltages.
> ======================= ====================================================
>
> +Sysfs entries for sy24655 only
> +------------------------------------------------
> +
> +======================= ====================================================
> +update_interval data conversion time; affects number of samples used
> + to average results for shunt and bus voltages.
The above is not for sy24655 only; it also applies to ina226 and compatible chips.
> +calculate_avg_power calculate average power from last reading to the present.
Why not the standard power1_average ? I don't see a reason to introduce a non-standard
attribute.
> +======================= ====================================================
> +
> .. note::
>
> - Configure `shunt_resistor` before configure `power1_crit`, because power
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index b60fe2e58ad6..1f9752689ae8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2138,7 +2138,7 @@ config SENSORS_INA2XX
> select REGMAP_I2C
> help
> If you say yes here you get support for INA219, INA220, INA226,
> - INA230, and INA231 power monitor chips.
> + INA230, INA231, and Silergy SY24655 power monitor chips.
>
> The INA2xx driver is configured for the default configuration of
> the part as described in the datasheet.
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 9ab4205622e2..34d474ac0b66 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -18,6 +18,10 @@
> * Bi-directional Current/Power Monitor with I2C Interface
> * Datasheet: https://www.ti.com/product/ina230
> *
> + * SY24655:
> + * Bi-directional Current/Power Monitor with I2C Interface
> + * Datasheet: https://us1.silergy.com/productsview/SY24655FBP
> + *
> * Copyright (C) 2012 Lothar Felten <lothar.felten@gmail.com>
> * Thanks to Jan Volkering
> */
> @@ -51,15 +55,23 @@
> #define INA226_ALERT_LIMIT 0x07
> #define INA226_DIE_ID 0xFF
>
> +/* SY24655 register definitions */
> +#define SY24655_EIN 0x0A
> +#define SY24655_ACCUM_CONFIG 0x0D
> +
> /* register count */
> #define INA219_REGISTERS 6
> #define INA226_REGISTERS 8
> +#define SY24655_REGISTERS 0x0D
>
> -#define INA2XX_MAX_REGISTERS 8
> +#define INA2XX_MAX_REGISTERS 0x0D
>
> /* settings - depend on use case */
> #define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
> #define INA226_CONFIG_DEFAULT 0x4527 /* averages=16 */
> +#define SY24655_CONFIG_DEFAULT 0x4527 /* averages=16 */
> +/* (only for sy24655) */
> +#define SY24655_ACCUM_CONFIG_DEFAULT 0x044C
>
> /* worst case is 68.10 ms (~14.6Hz, ina219) */
> #define INA2XX_CONVERSION_RATE 15
> @@ -103,7 +115,7 @@ static struct regmap_config ina2xx_regmap_config = {
> .val_bits = 16,
> };
>
> -enum ina2xx_ids { ina219, ina226 };
> +enum ina2xx_ids { ina219, ina226, sy24655};
>
> struct ina2xx_config {
> u16 config_default;
> @@ -117,12 +129,13 @@ struct ina2xx_config {
>
> struct ina2xx_data {
> const struct ina2xx_config *config;
> -
> +
> long rshunt;
> long current_lsb_uA;
> long power_lsb_uW;
> struct mutex config_lock;
> struct regmap *regmap;
> + struct i2c_client *client;
>
> const struct attribute_group *groups[INA2XX_MAX_ATTRIBUTE_GROUPS];
> };
> @@ -146,6 +159,15 @@ static const struct ina2xx_config ina2xx_config[] = {
> .bus_voltage_lsb = 1250,
> .power_lsb_factor = 25,
> },
> + [sy24655] = {
> + .config_default = SY24655_CONFIG_DEFAULT,
> + .calibration_value = 2048,
> + .registers = SY24655_REGISTERS,
> + .shunt_div = 400,
> + .bus_voltage_shift = 0,
> + .bus_voltage_lsb = 1250,
> + .power_lsb_factor = 25,
> + },
> };
>
> /*
> @@ -216,6 +238,12 @@ static int ina2xx_init(struct ina2xx_data *data)
> return ina2xx_calibrate(data);
> }
>
> +static int sy24655_init(struct ina2xx_data *data)
> +{
> + return regmap_write(data->regmap, SY24655_ACCUM_CONFIG,
> + SY24655_ACCUM_CONFIG_DEFAULT);
> +}
> +
> static int ina2xx_read_reg(struct device *dev, int reg, unsigned int *regval)
> {
> struct ina2xx_data *data = dev_get_drvdata(dev);
> @@ -551,6 +579,48 @@ static ssize_t ina226_interval_show(struct device *dev,
> return sysfs_emit(buf, "%d\n", ina226_reg_to_interval(regval));
> }
>
> +static int sy24655_read_reg48(const struct i2c_client *client, u8 reg,
> + long *accumulator_24, long *sample_count)
> +{
> + u8 data[6];
> + int err;
> + *accumulator_24 = 0;
> + *sample_count = 0;
> +
> + /* 48-bit register read */
> + err = i2c_smbus_read_i2c_block_data(client, reg, 6, data);
> + if (err < 0)
> + return err;
> + if (err != 6)
> + return -EIO;
> + *accumulator_24 = ((data[3] << 16) |
> + (data[4] << 8) |
> + data[5]);
> + *sample_count = ((data[0] << 16) |
> + (data[1] << 8) |
> + data[2]);
> +
> + return 0;
> +}
> +
> +static ssize_t sy24655_avg_power_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> + long sample_count, accumulator_24, regval;
> + int status;
> +
> + status = sy24655_read_reg48(data->client, SY24655_EIN,
> + &accumulator_24, &sample_count);
> + if (status)
> + return status;
> + regval = DIV_ROUND_CLOSEST(accumulator_24, sample_count);
Since the sample count is not used anywhere else, it would make sense
to just read and return the average power in a single function. Also
make sure that sample_count isn't 0.
Note that this does not return the "average power from last reading
to the present" as claimed above unless bit 1 of ACCUM_CONFIG is set
to 0. Actually I am not sure what exactly it reports since the sample
count and the accumulator values will overflow at different times.
As far as I can see it returns a more or less random value after an
overflow.
Are you sure this provides any value as implemented ? I really don't see
what that value would be, especially since there is no means to account
for overflows.
> + regval = regval * data->power_lsb_uW;
> +
> +
> + return sysfs_emit(buf, "%li\n", regval);
> +}
> +
> /* shunt voltage */
> static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
> /* shunt voltage over/under voltage alert setting and alarm */
> @@ -589,9 +659,13 @@ static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
> /* shunt resistance */
> static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
>
> -/* update interval (ina226 only) */
> +/* update interval (ina226 and sy24655) */
> static SENSOR_DEVICE_ATTR_RW(update_interval, ina226_interval, 0);
>
> +/* calculate_avg_power (sy24655 only) */
> +static SENSOR_DEVICE_ATTR_RO(calculate_avg_power, sy24655_avg_power, 0);
> +
> +
> /* pointers to created device attributes */
> static struct attribute *ina2xx_attrs[] = {
> &sensor_dev_attr_in0_input.dev_attr.attr,
> @@ -624,6 +698,15 @@ static struct attribute *ina226_attrs[] = {
> static const struct attribute_group ina226_group = {
> .attrs = ina226_attrs,
> };
> +static struct attribute *sy24655_attrs[] = {
> + &sensor_dev_attr_update_interval.dev_attr.attr,
> + &sensor_dev_attr_calculate_avg_power.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group sy24655_group = {
> + .attrs = sy24655_attrs,
> +};
>
> static int ina2xx_probe(struct i2c_client *client)
> {
> @@ -641,6 +724,7 @@ static int ina2xx_probe(struct i2c_client *client)
> return -ENOMEM;
>
> /* set the device type */
> + data->client = client;
> data->config = &ina2xx_config[chip];
> mutex_init(&data->config_lock);
>
> @@ -691,10 +775,17 @@ static int ina2xx_probe(struct i2c_client *client)
> dev_err(dev, "error configuring the device: %d\n", ret);
> return -ENODEV;
> }
> -
> + if (chip == sy24655)
> + ret = sy24655_init(data);
> + if (ret < 0) {
> + dev_err(dev, "error configuring the accum_reg: %d\n", ret);
> + return -ENODEV;
> + }
> data->groups[group++] = &ina2xx_group;
> if (chip == ina226)
> data->groups[group++] = &ina226_group;
> + else if (chip == sy24655)
> + data->groups[group++] = &sy24655_group;
>
> hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
> data, data->groups);
> @@ -713,6 +804,7 @@ static const struct i2c_device_id ina2xx_id[] = {
> { "ina226", ina226 },
> { "ina230", ina226 },
> { "ina231", ina226 },
> + { "sy24655", sy24655},
> { }
> };
> MODULE_DEVICE_TABLE(i2c, ina2xx_id);
> @@ -738,6 +830,10 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
> .compatible = "ti,ina231",
> .data = (void *)ina226
> },
> + {
> + .compatible = "silergy,sy24655",
> + .data = (void *)sy24655
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, ina2xx_of_match);
This change makes the INA2XX driver compatible with sy24655,
involving the reading of average power
Signed-off-by: Wenliang <wenliang202407@163.com>
---
This change uses the latest ina2xx driver under the Linux NEXT branch.
I will explain the operation and significance of the average power
read function. I configure the READ_EIN (bit 10) of the ACCUM_CONFIG
register to 1, so that every time the EIN register is read, it will be
cleared. Therefore, the average power from the current read to the
previous read can be directly calculated. The reason for adopting
this continuous reading mode is to facilitate upper level users to
use more accurate time to convert energy instead of using internal
time intervals.
Documentation/hwmon/ina2xx.rst | 25 +++++++-
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/ina2xx.c | 103 +++++++++++++++++++++++++++++++--
3 files changed, 124 insertions(+), 6 deletions(-)
diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
index 1ce161e6c0bf..eac8bb1deda0 100644
--- a/Documentation/hwmon/ina2xx.rst
+++ b/Documentation/hwmon/ina2xx.rst
@@ -63,6 +63,16 @@ Supported chips:
https://www.ti.com/
+ * Silergy SY24655
+
+
+ Prefix: 'sy24655'
+ Addresses: I2C 0x40 - 0x4f
+
+ Datasheet: Publicly available at the Silergy website
+
+ https://us1.silergy.com/
+
Author: Lothar Felten <lothar.felten@gmail.com>
Description
@@ -85,6 +95,11 @@ bus supply voltage.
INA260 is a high or low side current and power monitor with integrated shunt
resistor.
+The SY24655 is a high- and low-side current shunt and power monitor with an I2C
+interface. The SY24655 both shunt drop and supply voltage, with programmable
+calibration value and conversion times. The SY24655 can also calculate average
+power for use in energy conversion.
+
The shunt value in micro-ohms can be set via platform data or device tree at
compile-time or via the shunt_resistor attribute in sysfs at run-time. Please
refer to the Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for bindings
@@ -108,7 +123,7 @@ power1_input Power(uW) measurement channel
shunt_resistor Shunt resistance(uOhm) channel (not for ina260)
======================= ===============================================
-Additional sysfs entries for ina226, ina230, ina231, and ina260
+Additional sysfs entries for ina226, ina230, ina231, ina260, and sy24655
---------------------------------------------------------------
======================= ====================================================
@@ -130,6 +145,14 @@ update_interval data conversion time; affects number of samples used
to average results for shunt and bus voltages.
======================= ====================================================
+Sysfs entries for sy24655 only
+------------------------------------------------
+
+======================= ====================================================
+power1_average calculate average power from last reading to the
+ present.
+======================= ====================================================
+
.. note::
- Configure `shunt_resistor` before configure `power1_crit`, because power
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6a3385598cc6..11eb1c5aa358 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2167,7 +2167,7 @@ config SENSORS_INA2XX
select REGMAP_I2C
help
If you say yes here you get support for INA219, INA220, INA226,
- INA230, INA231, and INA260 power monitor chips.
+ INA230, INA231, INA260, and SY24655 power monitor chips.
The INA2xx driver is configured for the default configuration of
the part as described in the datasheet.
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index cecc80a41a97..89d2d2ae7f15 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -51,12 +51,25 @@
#define INA226_ALERT_LIMIT 0x07
#define INA226_DIE_ID 0xFF
-#define INA2XX_MAX_REGISTERS 8
+/* SY24655 register definitions */
+#define SY24655_EIN 0x0A
+#define SY24655_ACCUM_CONFIG 0x0D
+
+/* register count */
+#define INA219_REGISTERS 6
+#define INA226_REGISTERS 8
+#define SY24655_REGISTERS 0x0D
+
+#define INA2XX_MAX_REGISTERS 0x0D
/* settings - depend on use case */
#define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
#define INA226_CONFIG_DEFAULT 0x4527 /* averages=16 */
#define INA260_CONFIG_DEFAULT 0x6527 /* averages=16 */
+#define SY24655_CONFIG_DEFAULT 0x4527 /* averages=16 */
+
+/* (only for sy24655) */
+#define SY24655_ACCUM_CONFIG_DEFAULT 0x044C /* continuous mode, clear after read*/
/* worst case is 68.10 ms (~14.6Hz, ina219) */
#define INA2XX_CONVERSION_RATE 15
@@ -84,6 +97,8 @@
#define INA226_ALERT_CONFIG_MASK GENMASK(15, 10)
#define INA226_ALERT_FUNCTION_FLAG BIT(4)
+#define SY24655_EIN_OVERFLOW_FLAG BIT(6)
+
/*
* Both bus voltage and shunt voltage conversion times for ina226 are set
* to 0b0100 on POR, which translates to 2200 microseconds in total.
@@ -97,6 +112,7 @@ static bool ina2xx_writeable_reg(struct device *dev, unsigned int reg)
case INA2XX_CALIBRATION:
case INA226_MASK_ENABLE:
case INA226_ALERT_LIMIT:
+ case SY24655_ACCUM_CONFIG:
return true;
default:
return false;
@@ -110,6 +126,7 @@ static bool ina2xx_volatile_reg(struct device *dev, unsigned int reg)
case INA2XX_BUS_VOLTAGE:
case INA2XX_POWER:
case INA2XX_CURRENT:
+ case SY24655_EIN:
return true;
default:
return false;
@@ -127,7 +144,7 @@ static const struct regmap_config ina2xx_regmap_config = {
.writeable_reg = ina2xx_writeable_reg,
};
-enum ina2xx_ids { ina219, ina226, ina260 };
+enum ina2xx_ids { ina219, ina226, ina260, sy24655 };
struct ina2xx_config {
u16 config_default;
@@ -149,6 +166,8 @@ struct ina2xx_data {
long power_lsb_uW;
struct mutex config_lock;
struct regmap *regmap;
+ struct i2c_client *client;
+
};
static const struct ina2xx_config ina2xx_config[] = {
@@ -181,6 +200,16 @@ static const struct ina2xx_config ina2xx_config[] = {
.has_alerts = true,
.has_ishunt = true,
},
+ [sy24655] = {
+ .config_default = SY24655_CONFIG_DEFAULT,
+ .calibration_value = 2048,
+ .shunt_div = 400,
+ .bus_voltage_shift = 0,
+ .bus_voltage_lsb = 1250,
+ .power_lsb_factor = 25,
+ .has_alerts = false,
+ .has_ishunt = false,
+ },
};
/*
@@ -485,6 +514,49 @@ static int ina2xx_in_read(struct device *dev, u32 attr, int channel, long *val)
return 0;
}
+/*
+ * Configuring the READ_EIN (bit 10) of the ACCUM_CONFIG register to 1
+ * can clear accumulator and sample_count after reading the EIN register.
+ * This way, the average power between the last read and the current
+ * read can be obtained. By combining with accurate time data from
+ * outside, the energy consumption during that period can be calculated.
+ */
+static int sy24655_average_power_read(struct ina2xx_data *data, u8 reg, long *val)
+{
+ u8 template[6];
+ int ret;
+ long accumulator_24, sample_count;
+ unsigned int regval;
+
+ ret = regmap_read_bypassed(data->regmap, INA226_MASK_ENABLE, ®val);
+ if (ret)
+ return ret;
+
+ if (regval & SY24655_EIN_OVERFLOW_FLAG)
+ return -ENOMEM;
+
+ /* 48-bit register read */
+ ret = i2c_smbus_read_i2c_block_data(data->client, reg, 6, template);
+ if (ret < 0)
+ return ret;
+ if (ret != 6)
+ return -EIO;
+ accumulator_24 = ((template[3] << 16) |
+ (template[4] << 8) |
+ template[5]);
+ sample_count = ((template[0] << 16) |
+ (template[1] << 8) |
+ template[2]);
+ if (sample_count <= 0) {
+ *val = 0;
+ return 0;
+ }
+
+ *val = DIV_ROUND_CLOSEST(accumulator_24, sample_count) * data->power_lsb_uW;
+
+ return 0;
+}
+
static int ina2xx_power_read(struct device *dev, u32 attr, long *val)
{
struct ina2xx_data *data = dev_get_drvdata(dev);
@@ -492,6 +564,8 @@ static int ina2xx_power_read(struct device *dev, u32 attr, long *val)
switch (attr) {
case hwmon_power_input:
return ina2xx_read_init(dev, INA2XX_POWER, val);
+ case hwmon_power_average:
+ return sy24655_average_power_read(data, SY24655_EIN, val);
case hwmon_power_crit:
return ina226_alert_limit_read(data, INA226_POWER_OVER_LIMIT_MASK,
INA2XX_POWER, val);
@@ -702,6 +776,8 @@ static umode_t ina2xx_is_visible(const void *_data, enum hwmon_sensor_types type
if (has_alerts)
return 0444;
break;
+ case hwmon_power_average:
+ return 0444;
default:
break;
}
@@ -734,7 +810,8 @@ static const struct hwmon_channel_info * const ina2xx_info[] = {
HWMON_CHANNEL_INFO(curr, HWMON_C_INPUT | HWMON_C_CRIT | HWMON_C_CRIT_ALARM |
HWMON_C_LCRIT | HWMON_C_LCRIT_ALARM),
HWMON_CHANNEL_INFO(power,
- HWMON_P_INPUT | HWMON_P_CRIT | HWMON_P_CRIT_ALARM),
+ HWMON_P_INPUT | HWMON_P_CRIT | HWMON_P_CRIT_ALARM |
+ HWMON_P_AVERAGE),
NULL
};
@@ -840,6 +917,18 @@ static int ina2xx_init(struct device *dev, struct ina2xx_data *data)
FIELD_PREP(INA226_ALERT_POLARITY, active_high));
}
+ if (data->chip == sy24655) {
+ /*
+ * Initialize the power accumulation method to continuous
+ * mode and clear the EIN register after each read of the
+ * EIN register
+ */
+ ret = regmap_write(regmap, SY24655_ACCUM_CONFIG,
+ SY24655_ACCUM_CONFIG_DEFAULT);
+ if (ret < 0)
+ return ret;
+ }
+
if (data->config->has_ishunt)
return 0;
@@ -868,6 +957,7 @@ static int ina2xx_probe(struct i2c_client *client)
return -ENOMEM;
/* set the device type */
+ data->client = client;
data->config = &ina2xx_config[chip];
data->chip = chip;
mutex_init(&data->config_lock);
@@ -906,6 +996,7 @@ static const struct i2c_device_id ina2xx_id[] = {
{ "ina230", ina226 },
{ "ina231", ina226 },
{ "ina260", ina260 },
+ { "sy24655", sy24655 },
{ }
};
MODULE_DEVICE_TABLE(i2c, ina2xx_id);
@@ -935,7 +1026,11 @@ static const struct of_device_id __maybe_unused ina2xx_of_match[] = {
.compatible = "ti,ina260",
.data = (void *)ina260
},
- { },
+ {
+ .compatible = "silergy,sy24655",
+ .data = (void *)sy24655
+ },
+ { }
};
MODULE_DEVICE_TABLE(of, ina2xx_of_match);
--
2.17.1Modified the binding of ina2xx to make it compatible with SY24655.
Signed-off-by: Wenliang <wenliang202407@163.com>
---
SY24655 is a fixed gain power monitor from Silergy, with a power supply
of 2.7-5.5V and communication mode of IIC capable of detecting bus voltage
and voltage on shunt resistors. Its first 5 registers are identical to
ina226, and also have alert and limit functions. So, the sy24655 is
compatible with the ina2xx devices.
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 6ae961732e6b..05a9cb36cd82 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,sy24655
- ti,ina209
- ti,ina219
- ti,ina220
--
2.17.1On 12/10/2024 09:06, Wenliang wrote: > Modified the binding of ina2xx to make it compatible with SY24655. > Your subject is odd. We do not develop v6.11, that's something already old. The cc-list suggest you just want it for next release, so rebase on mainline or next and avoid any unusual patch prefixes. OTOH, v3 is not part of subject. Use b4 or git format-patch -v3. > Signed-off-by: Wenliang <wenliang202407@163.com> > --- > > SY24655 is a fixed gain power monitor from Silergy, with a power supply > of 2.7-5.5V and communication mode of IIC capable of detecting bus voltage > and voltage on shunt resistors. Its first 5 registers are identical to > ina226, and also have alert and limit functions. So, the sy24655 is > compatible with the ina2xx devices. This is hardware description, why it is not part of commit message? Best regards, Krzysztof
On 10/12/24 03:06, Krzysztof Kozlowski wrote: > On 12/10/2024 09:06, Wenliang wrote: >> Modified the binding of ina2xx to make it compatible with SY24655. >> > > Your subject is odd. We do not develop v6.11, that's something already > old. The cc-list suggest you just want it for next release, so rebase on > mainline or next and avoid any unusual patch prefixes. > The submitter also still refuses to use imperative mood. I don't know how often this has to be repeated. I take this as "does not address feedback" and will not even look at the patches anymore until the reported problems have been fixed. Oh, and I just noticed that this version has been sent as response to the previous version, which is also discouraged. On top of that, there is no change log. Guenter
Modified the binding of ina2xx to make it compatible with SY24655.
Signed-off-by: Wenliang <wenliang202407@163.com>
---
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 6ae961732e6b..400e7cefad17 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -29,6 +29,7 @@ properties:
- ti,ina237
- ti,ina238
- ti,ina260
+ - silergy,sy24655
reg:
maxItems: 1
--
2.17.1On Wed, Sep 11, 2024 at 08:25:18AM -0400, Wenliang wrote: > Modified the binding of ina2xx to make it compatible with SY24655. Rather, you should explain why the sy24655 is compatible with the ina2xx devices. > > Signed-off-by: Wenliang <wenliang202407@163.com> > --- > 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 6ae961732e6b..400e7cefad17 100644 > --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml > @@ -29,6 +29,7 @@ properties: > - ti,ina237 > - ti,ina238 > - ti,ina260 > + - silergy,sy24655 Please add this in alphabetical order. Thanks, Conor. > > reg: > maxItems: 1 > -- > 2.17.1 >
On 11/09/2024 19:49, Conor Dooley wrote: > On Wed, Sep 11, 2024 at 08:25:18AM -0400, Wenliang wrote: >> Modified the binding of ina2xx to make it compatible with SY24655. > > Rather, you should explain why the sy24655 is compatible with the ina2xx > devices. > >> This looks like patch for some forked tree, like the BMC folks are doing. At least linux dev suggests it. Best regards, Krzysztof
On Tue, Oct 08, 2024 at 02:35:38PM +0200, Krzysztof Kozlowski wrote: > On 11/09/2024 19:49, Conor Dooley wrote: > > On Wed, Sep 11, 2024 at 08:25:18AM -0400, Wenliang wrote: > >> Modified the binding of ina2xx to make it compatible with SY24655. > > > > Rather, you should explain why the sy24655 is compatible with the ina2xx > > devices. > > > >> > > This looks like patch for some forked tree, like the BMC folks are > doing. At least linux dev suggests it. CC list wouldn't imply that it is.
© 2016 - 2026 Red Hat, Inc.