[PATCH v6 0/3] hwmon: (isl28022) new driver for ISL28022 power monitor

[PATCH v6 0/3] hwmon: (isl28022) new driver for ISL28022 power monitor

[Delphine CC Chiu]

Driver for Renesas ISL28022 power monitor chip.
Found e.g. on Ubiquiti Edgerouter ER-6P

v6: shunt voltage in mV and revise code
v5: review comments incorporated
v4: property compatible fixed
v3: changelog added
v2: properties reworked
v2: calculations fixed
v2: shunt voltage input moved to debugfs
v2: documentation and devicetree schema reworked
v1: created

Carsten Spieß (2):
  hwmon: (isl28022) new driver for ISL28022 power monitor
  dt-bindings: hwmon: add renesas,isl28022

Delphine CC Chiu (1):
  hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

 .../bindings/hwmon/renesas,isl28022.yaml      |  64 +++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/isl28022.rst              |  63 +++
 MAINTAINERS                                   |   8 +
 drivers/hwmon/Kconfig                         |  11 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/isl28022.c                      | 510 ++++++++++++++++++
 7 files changed, 658 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
 create mode 100644 Documentation/hwmon/isl28022.rst
 create mode 100644 drivers/hwmon/isl28022.c

-- 
2.25.1

[PATCH v6 1/3] hwmon: (isl28022) new driver for ISL28022 power monitor

[Delphine CC Chiu]

From: Carsten Spieß <mail@carsten-spiess.de>

Driver for Renesas ISL28022 power monitor with I2C interface.
The device monitors voltage, current via shunt resistor
and calculated power.

Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
---
 Documentation/hwmon/index.rst    |   1 +
 Documentation/hwmon/isl28022.rst |  63 +++++
 MAINTAINERS                      |   7 +
 drivers/hwmon/Kconfig            |  11 +
 drivers/hwmon/Makefile           |   1 +
 drivers/hwmon/isl28022.c         | 471 +++++++++++++++++++++++++++++++
 6 files changed, 554 insertions(+)
 create mode 100644 Documentation/hwmon/isl28022.rst
 create mode 100644 drivers/hwmon/isl28022.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 913c11390a45..ba297c43d902 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -96,6 +96,7 @@ Hardware Monitoring Kernel Drivers
    ir35221
    ir38064
    ir36021
+   isl28022
    isl68137
    it87
    jc42
diff --git a/Documentation/hwmon/isl28022.rst b/Documentation/hwmon/isl28022.rst
new file mode 100644
index 000000000000..8d4422a2dacd
--- /dev/null
+++ b/Documentation/hwmon/isl28022.rst
@@ -0,0 +1,63 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver isl28022
+======================
+
+Supported chips:
+
+  * Renesas ISL28022
+
+    Prefix: 'isl28022'
+
+    Addresses scanned: none
+
+    Datasheet: Publicly available at the Renesas website
+
+	       https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
+
+Author:
+    Carsten Spieß <mail@carsten-spiess.de>
+
+Description
+-----------
+
+The ISL28022 is a power monitor with I2C interface. The device monitors
+voltage, current via shunt resistor and calculated power.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate the
+device explicitly. Please see Documentation/i2c/instantiating-devices.rst for
+details.
+
+The shunt value in micro-ohms, shunt voltage range and averaging can be set
+with device properties.
+Please refer to the Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml
+for bindings if the device tree is used.
+
+The driver supports only shunt and bus continuous ADC mode at 15bit resolution.
+Averaging can be set from 1 to 128 samples (power of 2) on both channels.
+Shunt voltage range of 40, 80, 160 or 320mV is allowed
+The bus voltage range is 60V fixed.
+
+Sysfs entries
+-------------
+
+The following attributes are supported. All attributes are read-only.
+
+======================= =======================================================
+in0_input		bus voltage (milli Volt)
+
+curr1_input		current (milli Ampere)
+power1_input		power (micro Watt)
+======================= =======================================================
+
+Debugfs entries
+---------------
+
+The following attributes are supported. All attributes are read-only.
+
+======================= =======================================================
+shunt_voltage		shunt voltage (micro Volt)
+======================= =======================================================
diff --git a/MAINTAINERS b/MAINTAINERS
index fe83ba7194ea..d39199ed51da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11932,6 +11932,13 @@ F:	drivers/isdn/Makefile
 F:	drivers/isdn/hardware/
 F:	drivers/isdn/mISDN/
 
+ISL28022 HARDWARE MONITORING DRIVER
+M:	Carsten Spieß <mail@carsten-spiess.de>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/isl28022.rst
+F:	drivers/hwmon/isl28022.c
+
 ISOFS FILESYSTEM
 M:	Jan Kara <jack@suse.cz>
 L:	linux-fsdevel@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..adbbbb128afc 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -853,6 +853,17 @@ config SENSORS_CORETEMP
 	  sensor inside your CPU. Most of the family 6 CPUs
 	  are supported. Check Documentation/hwmon/coretemp.rst for details.
 
+config SENSORS_ISL28022
+	tristate "Renesas ISL28022"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for ISL28022 power monitor.
+	  Check Documentation/hwmon/isl28022.rst for details.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called isl28022.
+
 config SENSORS_IT87
 	tristate "ITE IT87xx and compatibles"
 	depends on HAS_IOPORT
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index b1c7056c37db..51132b2b2c07 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
 obj-$(CONFIG_SENSORS_INA238)	+= ina238.o
 obj-$(CONFIG_SENSORS_INA3221)	+= ina3221.o
 obj-$(CONFIG_SENSORS_INTEL_M10_BMC_HWMON) += intel-m10-bmc-hwmon.o
+obj-$(CONFIG_SENSORS_ISL28022)	+= isl28022.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_JC42)	+= jc42.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
new file mode 100644
index 000000000000..f0494c3bd483
--- /dev/null
+++ b/drivers/hwmon/isl28022.c
@@ -0,0 +1,471 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * isl28022.c - driver for Renesas ISL28022 power monitor chip monitoring
+ *
+ * Copyright (c) 2023 Carsten Spieß <mail@carsten-spiess.de>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* ISL28022 registers */
+#define ISL28022_REG_CONFIG	0x00
+#define ISL28022_REG_SHUNT	0x01
+#define ISL28022_REG_BUS	0x02
+#define ISL28022_REG_POWER	0x03
+#define ISL28022_REG_CURRENT	0x04
+#define ISL28022_REG_CALIB	0x05
+#define ISL28022_REG_SHUNT_THR	0x06
+#define ISL28022_REG_BUS_THR	0x07
+#define ISL28022_REG_INT	0x08
+#define ISL28022_REG_AUX	0x09
+#define ISL28022_REG_MAX	ISL28022_REG_AUX
+
+/* ISL28022 config flags */
+/* mode flags */
+#define ISL28022_MODE_SHIFT	0
+#define ISL28022_MODE_MASK	0x0007
+
+#define ISL28022_MODE_PWR_DOWN	0x0
+#define ISL28022_MODE_TRG_S	0x1
+#define ISL28022_MODE_TRG_B	0x2
+#define ISL28022_MODE_TRG_SB	0x3
+#define ISL28022_MODE_ADC_OFF	0x4
+#define ISL28022_MODE_CONT_S	0x5
+#define ISL28022_MODE_CONT_B	0x6
+#define ISL28022_MODE_CONT_SB	0x7
+
+/* shunt ADC settings */
+#define ISL28022_SADC_SHIFT	3
+#define ISL28022_SADC_MASK	0x0078
+
+#define ISL28022_BADC_SHIFT	7
+#define ISL28022_BADC_MASK	0x0780
+
+#define ISL28022_ADC_12		0x0	/* 12 bit ADC */
+#define ISL28022_ADC_13		0x1	/* 13 bit ADC */
+#define ISL28022_ADC_14		0x2	/* 14 bit ADC */
+#define ISL28022_ADC_15		0x3	/* 15 bit ADC */
+#define ISL28022_ADC_15_1	0x8	/* 15 bit ADC, 1 sample */
+#define ISL28022_ADC_15_2	0x9	/* 15 bit ADC, 2 samples */
+#define ISL28022_ADC_15_4	0xA	/* 15 bit ADC, 4 samples */
+#define ISL28022_ADC_15_8	0xB	/* 15 bit ADC, 8 samples */
+#define ISL28022_ADC_15_16	0xC	/* 15 bit ADC, 16 samples */
+#define ISL28022_ADC_15_32	0xD	/* 15 bit ADC, 32 samples */
+#define ISL28022_ADC_15_64	0xE	/* 15 bit ADC, 64 samples */
+#define ISL28022_ADC_15_128	0xF	/* 15 bit ADC, 128 samples */
+
+/* shunt voltage range */
+#define ISL28022_PG_SHIFT	11
+#define ISL28022_PG_MASK	0x1800
+
+#define ISL28022_PG_40		0x0	/* +/-40 mV */
+#define ISL28022_PG_80		0x1	/* +/-80 mV */
+#define ISL28022_PG_160		0x2	/* +/-160 mV */
+#define ISL28022_PG_320		0x3	/* +/-3200 mV */
+
+/* bus voltage range */
+#define ISL28022_BRNG_SHIFT	13
+#define ISL28022_BRNG_MASK	0x6000
+
+#define ISL28022_BRNG_16	0x0	/* 16 V */
+#define ISL28022_BRNG_32	0x1	/* 32 V */
+#define ISL28022_BRNG_60	0x3	/* 60 V */
+
+/* reset */
+#define ISL28022_RESET		0x8000
+
+struct isl28022_data {
+	struct regmap		*regmap;
+	u32			shunt;
+	u32			gain;
+	u32			average;
+	u16			config;
+	u16			calib;
+};
+
+static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
+			 u32 attr, int channel, long *val)
+{
+	struct isl28022_data *data = dev_get_drvdata(dev);
+	unsigned int regval;
+	int err;
+
+	switch (type) {
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_input:
+			err = regmap_read(data->regmap,
+					  ISL28022_REG_BUS, &regval);
+			if (err < 0)
+				return err;
+			/* driver supports only 60V mode (BRNG 11) */
+			*val = (long)(((u16)regval) & 0xFFFC);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case hwmon_curr:
+		switch (attr) {
+		case hwmon_curr_input:
+			err = regmap_read(data->regmap,
+					  ISL28022_REG_CURRENT, &regval);
+			if (err < 0)
+				return err;
+			*val = ((long)regval * 1250L * (long)data->gain) /
+				(long)data->shunt;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case hwmon_power:
+		switch (attr) {
+		case hwmon_power_input:
+			err = regmap_read(data->regmap,
+					  ISL28022_REG_POWER, &regval);
+			if (err < 0)
+				return err;
+			*val = ((51200000L * ((long)data->gain)) /
+				(long)data->shunt) * (long)regval;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static umode_t isl28022_is_visible(const void *data, enum hwmon_sensor_types type,
+				   u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_input:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	case hwmon_curr:
+		switch (attr) {
+		case hwmon_curr_input:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	case hwmon_power:
+		switch (attr) {
+		case hwmon_power_input:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	return 0;
+}
+
+static const struct hwmon_channel_info *isl28022_info[] = {
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT),	/* channel 0: bus voltage (mV) */
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_INPUT),	/* channel 1: current (mA) */
+	HWMON_CHANNEL_INFO(power,
+			   HWMON_P_INPUT),	/* channel 1: power (µW) */
+	NULL
+};
+
+static const struct hwmon_ops isl28022_hwmon_ops = {
+	.is_visible = isl28022_is_visible,
+	.read = isl28022_read,
+};
+
+static const struct hwmon_chip_info isl28022_chip_info = {
+	.ops = &isl28022_hwmon_ops,
+	.info = isl28022_info,
+};
+
+static bool isl28022_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ISL28022_REG_CONFIG:
+	case ISL28022_REG_CALIB:
+	case ISL28022_REG_SHUNT_THR:
+	case ISL28022_REG_BUS_THR:
+	case ISL28022_REG_INT:
+	case ISL28022_REG_AUX:
+		return true;
+	}
+
+	return false;
+}
+
+static bool isl28022_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ISL28022_REG_CONFIG:
+	case ISL28022_REG_SHUNT:
+	case ISL28022_REG_BUS:
+	case ISL28022_REG_POWER:
+	case ISL28022_REG_CURRENT:
+	case ISL28022_REG_INT:
+	case ISL28022_REG_AUX:
+		return true;
+	}
+	return true;
+}
+
+static const struct regmap_config isl28022_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = ISL28022_REG_MAX,
+	.writeable_reg = isl28022_is_writeable_reg,
+	.volatile_reg = isl28022_is_volatile_reg,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+	.cache_type = REGCACHE_RBTREE,
+	.use_single_read = true,
+	.use_single_write = true,
+};
+
+static const struct i2c_device_id isl28022_ids[] = {
+	{ "isl28022", 0},
+	{ /* LIST END */ }
+};
+MODULE_DEVICE_TABLE(i2c, isl28022_ids);
+
+static int shunt_voltage_show(struct seq_file *seqf, void *unused)
+{
+	struct isl28022_data *data = seqf->private;
+	unsigned int regval;
+	int err;
+
+	err = regmap_read(data->regmap,
+			  ISL28022_REG_SHUNT, &regval);
+	if (err)
+		return err;
+
+	/* print shunt voltage in micro volt  */
+	seq_printf(seqf, "%d\n", regval * 10);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(shunt_voltage);
+
+static struct dentry *isl28022_debugfs_root;
+
+static void isl28022_debugfs_remove(void *res)
+{
+	debugfs_remove_recursive(res);
+}
+
+static void isl28022_debugfs_init(struct i2c_client *client, struct isl28022_data *data)
+{
+	char name[16];
+	struct dentry *debugfs;
+
+	scnprintf(name, sizeof(name), "%d-%04hx", client->adapter->nr, client->addr);
+
+	debugfs = debugfs_create_dir(name, isl28022_debugfs_root);
+	debugfs_create_file("shunt_voltage", 0444, debugfs, data, &shunt_voltage_fops);
+
+	devm_add_action_or_reset(&client->dev, isl28022_debugfs_remove, debugfs);
+}
+
+/*
+ * read property values and make consistency checks.
+ *
+ * following values for shunt range and resistor are allowed:
+ *   40 mV -> gain 1, shunt min.  800 micro ohms
+ *   80 mV -> gain 2, shunt min. 1600 micro ohms
+ *  160 mV -> gain 4, shunt min. 3200 micro ohms
+ *  320 mV -> gain 8, shunt min. 6400 micro ohms
+ */
+static int isl28022_read_properties(struct device *dev, struct isl28022_data *data)
+{
+	u32 val;
+	int err;
+
+	err = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &val);
+	if (err == -EINVAL)
+		val = 10000;
+	else if (err < 0)
+		return err;
+	data->shunt = val;
+
+	err = device_property_read_u32(dev, "renesas,shunt-range-microvolt", &val);
+	if (err == -EINVAL)
+		val = 320000;
+	else if (err < 0)
+		return err;
+
+	switch (val) {
+	case 40000:
+		data->gain = 1;
+		if (data->shunt < 800)
+			goto shunt_invalid;
+		break;
+	case 80000:
+		data->gain = 2;
+		if (data->shunt < 1600)
+			goto shunt_invalid;
+		break;
+	case 160000:
+		data->gain = 4;
+		if (data->shunt < 3200)
+			goto shunt_invalid;
+		break;
+	case 320000:
+		data->gain = 8;
+		if (data->shunt < 6400)
+			goto shunt_invalid;
+		break;
+	default:
+		dev_err(dev, "renesas,shunt-range-microvolt invalid value %d\n", val);
+		return -EINVAL;
+	}
+
+	err = device_property_read_u32(dev, "renesas,average-samples", &val);
+	if (err == -EINVAL)
+		val = 1;
+	else if (err < 0)
+		return err;
+	if (val > 128 || hweight32(val) != 1) {
+		dev_err(dev, "renesas,average-samples invalid value %d\n", val);
+		return -EINVAL;
+	}
+	data->average = val;
+
+	return 0;
+
+shunt_invalid:
+	dev_err(dev, "renesas,shunt-resistor-microvolt invalid value %d\n", data->shunt);
+	return -EINVAL;
+}
+
+/*
+ * write configuration and calibration registers
+ *
+ * The driver supports only shunt and bus continuous ADC mode at 15bit resolution
+ * with averaging from 1 to 128 samples (pow of 2) on both channels.
+ * Shunt voltage gain 1,2,4 or 8 is allowed.
+ * The bus voltage range is 60V fixed.
+ */
+static int isl28022_config(struct isl28022_data *data)
+{
+	int err;
+
+	data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
+			(ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT) |
+			(__ffs(data->gain) << ISL28022_PG_SHIFT) |
+			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_SADC_SHIFT) |
+			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_BADC_SHIFT);
+
+	data->calib = data->shunt ? 0x8000 / data->gain : 0;
+
+	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config);
+	if (err < 0)
+		return err;
+
+	err = regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int isl28022_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct device *hwmon_dev;
+	struct isl28022_data *data;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = isl28022_read_properties(dev, data);
+	if (err)
+		return err;
+
+	data->regmap = devm_regmap_init_i2c(client, &isl28022_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	err = isl28022_config(data);
+	if (err)
+		return err;
+
+	isl28022_debugfs_init(client, data);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon",
+							 data, &isl28022_chip_info, NULL);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), client->name);
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused isl28022_of_match[] = {
+	{ .compatible = "renesas,isl28022"},
+	{ /* LIST END */ }
+};
+MODULE_DEVICE_TABLE(of, isl28022_of_match);
+
+static struct i2c_driver isl28022_driver = {
+	.class		= I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "isl28022",
+	},
+	.probe_new	= isl28022_probe,
+	.id_table	= isl28022_ids,
+};
+
+static int __init
+isl28022_init(void)
+{
+	int err;
+
+	isl28022_debugfs_root = debugfs_create_dir("isl28022", NULL);
+	err = i2c_add_driver(&isl28022_driver);
+	if (!err)
+		return 0;
+
+	debugfs_remove_recursive(isl28022_debugfs_root);
+	return err;
+}
+
+static void __exit
+isl28022_exit(void)
+{
+	i2c_del_driver(&isl28022_driver);
+	debugfs_remove_recursive(isl28022_debugfs_root);
+}
+
+module_init(isl28022_init);
+module_exit(isl28022_exit);
+
+MODULE_AUTHOR("Carsten Spieß <mail@carsten-spiess.de>");
+MODULE_DESCRIPTION("ISL28022 driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v6 2/3] dt-bindings: hwmon: add renesas,isl28022

[Delphine CC Chiu]

From: Carsten Spieß <mail@carsten-spiess.de>

Add dt-bindings for Renesas ISL28022 power monitor.

Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/hwmon/renesas,isl28022.yaml      | 64 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
new file mode 100644
index 000000000000..dd82a80e4115
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/renesas,isl28022.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas ISL28022 power monitor
+
+maintainers:
+  - Carsten Spieß <mail@carsten-spiess.de>
+
+description: |
+  The ISL28022 is a power monitor with I2C interface. The device monitors
+  voltage, current via shunt resistor and calculated power.
+
+  Datasheets:
+    https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
+
+properties:
+  compatible:
+    const: renesas,isl28022
+
+  reg:
+    maxItems: 1
+
+  shunt-resistor-micro-ohms:
+    description:
+      Shunt resistor value in micro-Ohm
+    minimum: 800
+    default: 10000
+
+  renesas,shunt-range-microvolt:
+    description:
+      Maximal shunt voltage range of +/- 40 mV, 80 mV, 160 mV or 320 mV
+    default: 320000
+    enum: [40000, 80000, 160000, 320000]
+
+  renesas,average-samples:
+    description:
+      Number of samples to be used to report voltage, current and power values.
+    default: 1
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2, 4, 8, 16, 32, 64, 128]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-monitor@40 {
+            compatible = "renesas,isl28022";
+            reg = <0x40>;
+            shunt-resistor-micro-ohms = <8000>;
+            renesas,shunt-range-microvolt = <40000>;
+            renesas,average-samples = <128>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index d39199ed51da..d5809cf181ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11936,6 +11936,7 @@ ISL28022 HARDWARE MONITORING DRIVER
 M:	Carsten Spieß <mail@carsten-spiess.de>
 L:	linux-hwmon@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
 F:	Documentation/hwmon/isl28022.rst
 F:	drivers/hwmon/isl28022.c
 
-- 
2.25.1

[PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

[Delphine CC Chiu]

Added support reading shunt voltage in mV and revise code
for Renesas ISL28022.

Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
---
 drivers/hwmon/isl28022.c | 93 ++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
index f0494c3bd483..01220fad813d 100644
--- a/drivers/hwmon/isl28022.c
+++ b/drivers/hwmon/isl28022.c
@@ -85,8 +85,6 @@ struct isl28022_data {
 	u32			shunt;
 	u32			gain;
 	u32			average;
-	u16			config;
-	u16			calib;
 };
 
 static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
@@ -95,20 +93,61 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
 	struct isl28022_data *data = dev_get_drvdata(dev);
 	unsigned int regval;
 	int err;
+	u16 sign_bit;
 
 	switch (type) {
 	case hwmon_in:
-		switch (attr) {
-		case hwmon_in_input:
-			err = regmap_read(data->regmap,
-					  ISL28022_REG_BUS, &regval);
-			if (err < 0)
-				return err;
-			/* driver supports only 60V mode (BRNG 11) */
-			*val = (long)(((u16)regval) & 0xFFFC);
+		switch (channel) {
+		case 0:
+			switch (attr) {
+			case hwmon_in_input:
+				err = regmap_read(data->regmap,
+						  ISL28022_REG_BUS, &regval);
+				if (err < 0)
+					return err;
+				/* driver supports only 60V mode (BRNG 11) */
+				*val = (long)(((u16)regval) & 0xFFFC);
+				break;
+			default:
+				return -EOPNOTSUPP;
+			}
+			break;
+		case 1:
+			switch (attr) {
+			case hwmon_in_input:
+				err = regmap_read(data->regmap,
+						  ISL28022_REG_SHUNT, &regval);
+				if (err < 0)
+					return err;
+			switch (data->gain) {
+			case 8:
+				sign_bit = (regval >> 15) & 0x01;
+				*val = (long)((((u16)regval) & 0x7FFF) -
+					   (sign_bit * 32768)) / 100;
+				break;
+			case 4:
+				sign_bit = (regval >> 14) & 0x01;
+				*val = (long)((((u16)regval) & 0x3FFF) -
+					   (sign_bit * 16384)) / 100;
+				break;
+			case 2:
+				sign_bit = (regval >> 13) & 0x01;
+				*val = (long)((((u16)regval) & 0x1FFF) -
+					   (sign_bit * 8192)) / 100;
+				break;
+			case 1:
+				sign_bit = (regval >> 12) & 0x01;
+				*val = (long)((((u16)regval) & 0x0FFF) -
+					   (sign_bit * 4096)) / 100;
+				break;
+			}
+			break;
+			default:
+				return -EOPNOTSUPP;
+			}
 			break;
 		default:
-			return -EINVAL;
+			return -EOPNOTSUPP;
 		}
 		break;
 	case hwmon_curr:
@@ -122,7 +161,7 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
 				(long)data->shunt;
 			break;
 		default:
-			return -EINVAL;
+			return -EOPNOTSUPP;
 		}
 		break;
 	case hwmon_power:
@@ -136,11 +175,11 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
 				(long)data->shunt) * (long)regval;
 			break;
 		default:
-			return -EINVAL;
+			return -EOPNOTSUPP;
 		}
 		break;
 	default:
-		return -EINVAL;
+		return -EOPNOTSUPP;
 	}
 
 	return 0;
@@ -182,7 +221,8 @@ static umode_t isl28022_is_visible(const void *data, enum hwmon_sensor_types typ
 
 static const struct hwmon_channel_info *isl28022_info[] = {
 	HWMON_CHANNEL_INFO(in,
-			   HWMON_I_INPUT),	/* channel 0: bus voltage (mV) */
+			   HWMON_I_INPUT,	/* channel 0: bus voltage (mV) */
+			   HWMON_I_INPUT),	/* channel 1: shunt voltage (mV) */
 	HWMON_CHANNEL_INFO(curr,
 			   HWMON_C_INPUT),	/* channel 1: current (mA) */
 	HWMON_CHANNEL_INFO(power,
@@ -368,24 +408,22 @@ static int isl28022_read_properties(struct device *dev, struct isl28022_data *da
 static int isl28022_config(struct isl28022_data *data)
 {
 	int err;
+	u16 config;
+	u16 calib;
 
-	data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
+	config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
 			(ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT) |
 			(__ffs(data->gain) << ISL28022_PG_SHIFT) |
 			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_SADC_SHIFT) |
 			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_BADC_SHIFT);
 
-	data->calib = data->shunt ? 0x8000 / data->gain : 0;
-
-	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config);
-	if (err < 0)
-		return err;
+	calib = data->shunt ? 0x8000 / data->gain : 0;
 
-	err = regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib);
+	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, config);
 	if (err < 0)
 		return err;
 
-	return 0;
+	return regmap_write(data->regmap, ISL28022_REG_CALIB, calib);
 }
 
 static int isl28022_probe(struct i2c_client *client)
@@ -396,8 +434,8 @@ static int isl28022_probe(struct i2c_client *client)
 	int err;
 
 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_BYTE_DATA |
-				     I2C_FUNC_SMBUS_WORD_DATA))
+					 I2C_FUNC_SMBUS_BYTE_DATA |
+					 I2C_FUNC_SMBUS_WORD_DATA))
 		return -ENODEV;
 
 	data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL);
@@ -418,7 +456,7 @@ static int isl28022_probe(struct i2c_client *client)
 
 	isl28022_debugfs_init(client, data);
 
-	hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon",
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
 							 data, &isl28022_chip_info, NULL);
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
@@ -437,8 +475,9 @@ static struct i2c_driver isl28022_driver = {
 	.class		= I2C_CLASS_HWMON,
 	.driver = {
 		.name	= "isl28022",
+		.of_match_table = of_match_ptr(isl28022_of_match),
 	},
-	.probe_new	= isl28022_probe,
+	.probe	= isl28022_probe,
 	.id_table	= isl28022_ids,
 };
 
-- 
2.25.1

RE: [PATCH v6 2/3] dt-bindings: hwmon: add renesas,isl28022

[Biju Das]

Hi Delphine CC Chiu,

I guess binding should be first patch, otherwise you get undocumented warnings for driver patch
as it is the first patch. Then you could move MAINTAINERS from this patch to driver patch

Also, it is missing your SoB.

Cheers,
Biju

> -----Original Message-----
> From: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> Sent: Friday, September 6, 2024 7:14 AM
> Subject: [PATCH v6 2/3] dt-bindings: hwmon: add renesas,isl28022
> 
> From: Carsten Spieß <mail@carsten-spiess.de>
> 
> Add dt-bindings for Renesas ISL28022 power monitor.
> 
> Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/hwmon/renesas,isl28022.yaml      | 64 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> new file mode 100644
> index 000000000000..dd82a80e4115
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/renesas,isl28022.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas ISL28022 power monitor
> +
> +maintainers:
> +  - Carsten Spieß <mail@carsten-spiess.de>
> +
> +description: |
> +  The ISL28022 is a power monitor with I2C interface. The device
> +monitors
> +  voltage, current via shunt resistor and calculated power.
> +
> +  Datasheets:
> +    https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
> +
> +properties:
> +  compatible:
> +    const: renesas,isl28022
> +
> +  reg:
> +    maxItems: 1
> +
> +  shunt-resistor-micro-ohms:
> +    description:
> +      Shunt resistor value in micro-Ohm
> +    minimum: 800
> +    default: 10000
> +
> +  renesas,shunt-range-microvolt:
> +    description:
> +      Maximal shunt voltage range of +/- 40 mV, 80 mV, 160 mV or 320 mV
> +    default: 320000
> +    enum: [40000, 80000, 160000, 320000]
> +
> +  renesas,average-samples:
> +    description:
> +      Number of samples to be used to report voltage, current and power values.
> +    default: 1
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4, 8, 16, 32, 64, 128]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        power-monitor@40 {
> +            compatible = "renesas,isl28022";
> +            reg = <0x40>;
> +            shunt-resistor-micro-ohms = <8000>;
> +            renesas,shunt-range-microvolt = <40000>;
> +            renesas,average-samples = <128>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d39199ed51da..d5809cf181ff 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11936,6 +11936,7 @@ ISL28022 HARDWARE MONITORING DRIVER
>  M:	Carsten Spieß <mail@carsten-spiess.de>
>  L:	linux-hwmon@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml
>  F:	Documentation/hwmon/isl28022.rst
>  F:	drivers/hwmon/isl28022.c
> 
> --
> 2.25.1
> 

RE: [PATCH v6 1/3] hwmon: (isl28022) new driver for ISL28022 power monitor

[Biju Das]

Hi Delphine CC Chiu,

Thanks for the patch.

> -----Original Message-----
> From: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> Sent: Friday, September 6, 2024 7:14 AM
> Subject: [PATCH v6 1/3] hwmon: (isl28022) new driver for ISL28022 power monitor
> 
> From: Carsten Spieß <mail@carsten-spiess.de>
> 
> Driver for Renesas ISL28022 power monitor with I2C interface.
> The device monitors voltage, current via shunt resistor and calculated power.
> 
> Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>

It is missing your SoB.

> ---
>  Documentation/hwmon/index.rst    |   1 +
>  Documentation/hwmon/isl28022.rst |  63 +++++
>  MAINTAINERS                      |   7 +
>  drivers/hwmon/Kconfig            |  11 +
>  drivers/hwmon/Makefile           |   1 +
>  drivers/hwmon/isl28022.c         | 471 +++++++++++++++++++++++++++++++
>  6 files changed, 554 insertions(+)
>  create mode 100644 Documentation/hwmon/isl28022.rst  create mode 100644 drivers/hwmon/isl28022.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index
> 913c11390a45..ba297c43d902 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -96,6 +96,7 @@ Hardware Monitoring Kernel Drivers
>     ir35221
>     ir38064
>     ir36021
> +   isl28022
>     isl68137
>     it87
>     jc42
> diff --git a/Documentation/hwmon/isl28022.rst b/Documentation/hwmon/isl28022.rst
> new file mode 100644
> index 000000000000..8d4422a2dacd
> --- /dev/null
> +++ b/Documentation/hwmon/isl28022.rst
> @@ -0,0 +1,63 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver isl28022
> +======================
> +
> +Supported chips:
> +
> +  * Renesas ISL28022
> +
> +    Prefix: 'isl28022'
> +
> +    Addresses scanned: none
> +
> +    Datasheet: Publicly available at the Renesas website
> +
> +	       https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf
> +
> +Author:
> +    Carsten Spieß <mail@carsten-spiess.de>
> +
> +Description
> +-----------
> +
> +The ISL28022 is a power monitor with I2C interface. The device monitors
> +voltage, current via shunt resistor and calculated power.
> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate
> +the device explicitly. Please see
> +Documentation/i2c/instantiating-devices.rst for details.
> +
> +The shunt value in micro-ohms, shunt voltage range and averaging can be
> +set with device properties.
> +Please refer to the
> +Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml
> +for bindings if the device tree is used.
> +
> +The driver supports only shunt and bus continuous ADC mode at 15bit resolution.
> +Averaging can be set from 1 to 128 samples (power of 2) on both channels.
> +Shunt voltage range of 40, 80, 160 or 320mV is allowed The bus voltage
> +range is 60V fixed.
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported. All attributes are read-only.
> +
> +======================= =======================================================
> +in0_input		bus voltage (milli Volt)
> +
> +curr1_input		current (milli Ampere)
> +power1_input		power (micro Watt)
> +=======================
> +=======================================================
> +
> +Debugfs entries
> +---------------
> +
> +The following attributes are supported. All attributes are read-only.
> +
> +======================= =======================================================
> +shunt_voltage		shunt voltage (micro Volt)
> +=======================
> +=======================================================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe83ba7194ea..d39199ed51da 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11932,6 +11932,13 @@ F:	drivers/isdn/Makefile
>  F:	drivers/isdn/hardware/
>  F:	drivers/isdn/mISDN/
> 
> +ISL28022 HARDWARE MONITORING DRIVER
> +M:	Carsten Spieß <mail@carsten-spiess.de>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/hwmon/isl28022.rst
> +F:	drivers/hwmon/isl28022.c
> +
>  ISOFS FILESYSTEM
>  M:	Jan Kara <jack@suse.cz>
>  L:	linux-fsdevel@vger.kernel.org
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index b60fe2e58ad6..adbbbb128afc 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -853,6 +853,17 @@ config SENSORS_CORETEMP
>  	  sensor inside your CPU. Most of the family 6 CPUs
>  	  are supported. Check Documentation/hwmon/coretemp.rst for details.
> 
> +config SENSORS_ISL28022
> +	tristate "Renesas ISL28022"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  If you say yes here you get support for ISL28022 power monitor.
> +	  Check Documentation/hwmon/isl28022.rst for details.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called isl28022.
> +
>  config SENSORS_IT87
>  	tristate "ITE IT87xx and compatibles"
>  	depends on HAS_IOPORT
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index b1c7056c37db..51132b2b2c07 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
>  obj-$(CONFIG_SENSORS_INA238)	+= ina238.o
>  obj-$(CONFIG_SENSORS_INA3221)	+= ina3221.o
>  obj-$(CONFIG_SENSORS_INTEL_M10_BMC_HWMON) += intel-m10-bmc-hwmon.o
> +obj-$(CONFIG_SENSORS_ISL28022)	+= isl28022.o
>  obj-$(CONFIG_SENSORS_IT87)	+= it87.o
>  obj-$(CONFIG_SENSORS_JC42)	+= jc42.o
>  obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
> diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c new file mode 100644 index
> 000000000000..f0494c3bd483
> --- /dev/null
> +++ b/drivers/hwmon/isl28022.c
> @@ -0,0 +1,471 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * isl28022.c - driver for Renesas ISL28022 power monitor chip
> +monitoring
> + *
> + * Copyright (c) 2023 Carsten Spieß <mail@carsten-spiess.de>  */
> +
> +#include <linux/debugfs.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>

Do you need slab.h ??

> +
> +/* ISL28022 registers */
> +#define ISL28022_REG_CONFIG	0x00
> +#define ISL28022_REG_SHUNT	0x01
> +#define ISL28022_REG_BUS	0x02
> +#define ISL28022_REG_POWER	0x03
> +#define ISL28022_REG_CURRENT	0x04
> +#define ISL28022_REG_CALIB	0x05
> +#define ISL28022_REG_SHUNT_THR	0x06
> +#define ISL28022_REG_BUS_THR	0x07
> +#define ISL28022_REG_INT	0x08
> +#define ISL28022_REG_AUX	0x09
> +#define ISL28022_REG_MAX	ISL28022_REG_AUX
> +
> +/* ISL28022 config flags */
> +/* mode flags */
> +#define ISL28022_MODE_SHIFT	0
> +#define ISL28022_MODE_MASK	0x0007
> +
> +#define ISL28022_MODE_PWR_DOWN	0x0
> +#define ISL28022_MODE_TRG_S	0x1
> +#define ISL28022_MODE_TRG_B	0x2
> +#define ISL28022_MODE_TRG_SB	0x3
> +#define ISL28022_MODE_ADC_OFF	0x4
> +#define ISL28022_MODE_CONT_S	0x5
> +#define ISL28022_MODE_CONT_B	0x6
> +#define ISL28022_MODE_CONT_SB	0x7
> +
> +/* shunt ADC settings */
> +#define ISL28022_SADC_SHIFT	3
> +#define ISL28022_SADC_MASK	0x0078
> +
> +#define ISL28022_BADC_SHIFT	7
> +#define ISL28022_BADC_MASK	0x0780
> +
> +#define ISL28022_ADC_12		0x0	/* 12 bit ADC */
> +#define ISL28022_ADC_13		0x1	/* 13 bit ADC */
> +#define ISL28022_ADC_14		0x2	/* 14 bit ADC */
> +#define ISL28022_ADC_15		0x3	/* 15 bit ADC */
> +#define ISL28022_ADC_15_1	0x8	/* 15 bit ADC, 1 sample */
> +#define ISL28022_ADC_15_2	0x9	/* 15 bit ADC, 2 samples */
> +#define ISL28022_ADC_15_4	0xA	/* 15 bit ADC, 4 samples */
> +#define ISL28022_ADC_15_8	0xB	/* 15 bit ADC, 8 samples */
> +#define ISL28022_ADC_15_16	0xC	/* 15 bit ADC, 16 samples */
> +#define ISL28022_ADC_15_32	0xD	/* 15 bit ADC, 32 samples */
> +#define ISL28022_ADC_15_64	0xE	/* 15 bit ADC, 64 samples */
> +#define ISL28022_ADC_15_128	0xF	/* 15 bit ADC, 128 samples */
> +
> +/* shunt voltage range */
> +#define ISL28022_PG_SHIFT	11
> +#define ISL28022_PG_MASK	0x1800
> +
> +#define ISL28022_PG_40		0x0	/* +/-40 mV */
> +#define ISL28022_PG_80		0x1	/* +/-80 mV */
> +#define ISL28022_PG_160		0x2	/* +/-160 mV */
> +#define ISL28022_PG_320		0x3	/* +/-3200 mV */
> +
> +/* bus voltage range */
> +#define ISL28022_BRNG_SHIFT	13
> +#define ISL28022_BRNG_MASK	0x6000
> +
> +#define ISL28022_BRNG_16	0x0	/* 16 V */
> +#define ISL28022_BRNG_32	0x1	/* 32 V */
> +#define ISL28022_BRNG_60	0x3	/* 60 V */
> +
> +/* reset */
> +#define ISL28022_RESET		0x8000
> +
> +struct isl28022_data {
> +	struct regmap		*regmap;
> +	u32			shunt;
> +	u32			gain;
> +	u32			average;
> +	u16			config;
> +	u16			calib;
> +};
> +
> +static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel, long *val)
> +{
> +	struct isl28022_data *data = dev_get_drvdata(dev);
> +	unsigned int regval;
> +	int err;
> +
> +	switch (type) {
> +	case hwmon_in:
> +		switch (attr) {
> +		case hwmon_in_input:
> +			err = regmap_read(data->regmap,
> +					  ISL28022_REG_BUS, &regval);
> +			if (err < 0)
> +				return err;
> +			/* driver supports only 60V mode (BRNG 11) */
> +			*val = (long)(((u16)regval) & 0xFFFC);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case hwmon_curr:
> +		switch (attr) {
> +		case hwmon_curr_input:
> +			err = regmap_read(data->regmap,
> +					  ISL28022_REG_CURRENT, &regval);
> +			if (err < 0)
> +				return err;
> +			*val = ((long)regval * 1250L * (long)data->gain) /
> +				(long)data->shunt;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case hwmon_power:
> +		switch (attr) {
> +		case hwmon_power_input:
> +			err = regmap_read(data->regmap,
> +					  ISL28022_REG_POWER, &regval);
> +			if (err < 0)
> +				return err;
> +			*val = ((51200000L * ((long)data->gain)) /
> +				(long)data->shunt) * (long)regval;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static umode_t isl28022_is_visible(const void *data, enum hwmon_sensor_types type,
> +				   u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		switch (attr) {
> +		case hwmon_in_input:
> +			return 0444;
> +		default:
> +			break;
> +		}
> +		break;
> +	case hwmon_curr:
> +		switch (attr) {
> +		case hwmon_curr_input:
> +			return 0444;
> +		default:
> +			break;
> +		}
> +		break;
> +	case hwmon_power:
> +		switch (attr) {
> +		case hwmon_power_input:
> +			return 0444;
> +		default:
> +			break;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +	return 0;
> +}
> +
> +static const struct hwmon_channel_info *isl28022_info[] = {
> +	HWMON_CHANNEL_INFO(in,
> +			   HWMON_I_INPUT),	/* channel 0: bus voltage (mV) */
> +	HWMON_CHANNEL_INFO(curr,
> +			   HWMON_C_INPUT),	/* channel 1: current (mA) */
> +	HWMON_CHANNEL_INFO(power,
> +			   HWMON_P_INPUT),	/* channel 1: power (µW) */
> +	NULL
> +};
> +
> +static const struct hwmon_ops isl28022_hwmon_ops = {
> +	.is_visible = isl28022_is_visible,
> +	.read = isl28022_read,
> +};
> +
> +static const struct hwmon_chip_info isl28022_chip_info = {
> +	.ops = &isl28022_hwmon_ops,
> +	.info = isl28022_info,
> +};
> +
> +static bool isl28022_is_writeable_reg(struct device *dev, unsigned int
> +reg) {
> +	switch (reg) {
> +	case ISL28022_REG_CONFIG:
> +	case ISL28022_REG_CALIB:
> +	case ISL28022_REG_SHUNT_THR:
> +	case ISL28022_REG_BUS_THR:
> +	case ISL28022_REG_INT:
> +	case ISL28022_REG_AUX:
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool isl28022_is_volatile_reg(struct device *dev, unsigned int
> +reg) {
> +	switch (reg) {
> +	case ISL28022_REG_CONFIG:
> +	case ISL28022_REG_SHUNT:
> +	case ISL28022_REG_BUS:
> +	case ISL28022_REG_POWER:
> +	case ISL28022_REG_CURRENT:
> +	case ISL28022_REG_INT:
> +	case ISL28022_REG_AUX:
> +		return true;
> +	}
> +	return true;
> +}
> +
> +static const struct regmap_config isl28022_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = ISL28022_REG_MAX,
> +	.writeable_reg = isl28022_is_writeable_reg,
> +	.volatile_reg = isl28022_is_volatile_reg,
> +	.val_format_endian = REGMAP_ENDIAN_BIG,
> +	.cache_type = REGCACHE_RBTREE,
> +	.use_single_read = true,
> +	.use_single_write = true,
> +};
> +
> +static const struct i2c_device_id isl28022_ids[] = {
> +	{ "isl28022", 0},
> +	{ /* LIST END */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, isl28022_ids);

Move this table near to the user.

> +
> +static int shunt_voltage_show(struct seq_file *seqf, void *unused) {
> +	struct isl28022_data *data = seqf->private;
> +	unsigned int regval;
> +	int err;
> +
> +	err = regmap_read(data->regmap,
> +			  ISL28022_REG_SHUNT, &regval);
> +	if (err)
> +		return err;
> +
> +	/* print shunt voltage in micro volt  */
> +	seq_printf(seqf, "%d\n", regval * 10);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(shunt_voltage);
> +
> +static struct dentry *isl28022_debugfs_root;
> +
> +static void isl28022_debugfs_remove(void *res) {
> +	debugfs_remove_recursive(res);
> +}
> +
> +static void isl28022_debugfs_init(struct i2c_client *client, struct
> +isl28022_data *data) {
> +	char name[16];
> +	struct dentry *debugfs;
> +
> +	scnprintf(name, sizeof(name), "%d-%04hx", client->adapter->nr,
> +client->addr);
> +
> +	debugfs = debugfs_create_dir(name, isl28022_debugfs_root);
> +	debugfs_create_file("shunt_voltage", 0444, debugfs, data,
> +&shunt_voltage_fops);
> +
> +	devm_add_action_or_reset(&client->dev, isl28022_debugfs_remove,
> +debugfs); }
> +
> +/*
> + * read property values and make consistency checks.
> + *
> + * following values for shunt range and resistor are allowed:
> + *   40 mV -> gain 1, shunt min.  800 micro ohms
> + *   80 mV -> gain 2, shunt min. 1600 micro ohms
> + *  160 mV -> gain 4, shunt min. 3200 micro ohms
> + *  320 mV -> gain 8, shunt min. 6400 micro ohms  */ static int
> +isl28022_read_properties(struct device *dev, struct isl28022_data
> +*data) {
> +	u32 val;
> +	int err;
> +
> +	err = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &val);
> +	if (err == -EINVAL)
> +		val = 10000;
> +	else if (err < 0)
> +		return err;
> +	data->shunt = val;
> +
> +	err = device_property_read_u32(dev, "renesas,shunt-range-microvolt", &val);
> +	if (err == -EINVAL)
> +		val = 320000;
> +	else if (err < 0)
> +		return err;
> +
> +	switch (val) {
> +	case 40000:
> +		data->gain = 1;
> +		if (data->shunt < 800)
> +			goto shunt_invalid;
> +		break;
> +	case 80000:
> +		data->gain = 2;
> +		if (data->shunt < 1600)
> +			goto shunt_invalid;
> +		break;
> +	case 160000:
> +		data->gain = 4;
> +		if (data->shunt < 3200)
> +			goto shunt_invalid;
> +		break;
> +	case 320000:
> +		data->gain = 8;
> +		if (data->shunt < 6400)
> +			goto shunt_invalid;
> +		break;
> +	default:
> +		dev_err(dev, "renesas,shunt-range-microvolt invalid value %d\n", val);
Use dev_err_probe()
> +		return -EINVAL;
> +	}
> +
> +	err = device_property_read_u32(dev, "renesas,average-samples", &val);
> +	if (err == -EINVAL)
> +		val = 1;
> +	else if (err < 0)
> +		return err;
> +	if (val > 128 || hweight32(val) != 1) {
> +		dev_err(dev, "renesas,average-samples invalid value %d\n", val);
Use dev_err_probe()
> +		return -EINVAL;
> +	}
> +	data->average = val;
> +
> +	return 0;
> +
> +shunt_invalid:
> +	dev_err(dev, "renesas,shunt-resistor-microvolt invalid value %d\n", data->shunt);


Use dev_err_probe()

Cheers,
Biju

> +	return -EINVAL;
> +}
> +
> +/*
> + * write configuration and calibration registers
> + *
> + * The driver supports only shunt and bus continuous ADC mode at 15bit
> +resolution
> + * with averaging from 1 to 128 samples (pow of 2) on both channels.
> + * Shunt voltage gain 1,2,4 or 8 is allowed.
> + * The bus voltage range is 60V fixed.
> + */
> +static int isl28022_config(struct isl28022_data *data) {
> +	int err;
> +
> +	data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
> +			(ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT) |
> +			(__ffs(data->gain) << ISL28022_PG_SHIFT) |
> +			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_SADC_SHIFT) |
> +			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_BADC_SHIFT);
> +
> +	data->calib = data->shunt ? 0x8000 / data->gain : 0;
> +
> +	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config);
> +	if (err < 0)
> +		return err;
> +
> +	err = regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int isl28022_probe(struct i2c_client *client) {
> +	struct device *dev = &client->dev;
> +	struct device *hwmon_dev;
> +	struct isl28022_data *data;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA))
> +		return -ENODEV;
> +
> +	data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	err = isl28022_read_properties(dev, data);
> +	if (err)
> +		return err;
> +
> +	data->regmap = devm_regmap_init_i2c(client, &isl28022_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return PTR_ERR(data->regmap);
> +
> +	err = isl28022_config(data);
> +	if (err)
> +		return err;
> +
> +	isl28022_debugfs_init(client, data);
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon",
> +							 data, &isl28022_chip_info, NULL);
> +	if (IS_ERR(hwmon_dev))
> +		return PTR_ERR(hwmon_dev);
> +
> +	dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), client->name);
> +	return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused isl28022_of_match[] = {
> +	{ .compatible = "renesas,isl28022"},
> +	{ /* LIST END */ }
> +};
> +MODULE_DEVICE_TABLE(of, isl28022_of_match);
> +
> +static struct i2c_driver isl28022_driver = {
> +	.class		= I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "isl28022",
> +	},
> +	.probe_new	= isl28022_probe,
> +	.id_table	= isl28022_ids,
> +};
> +
> +static int __init
> +isl28022_init(void)
> +{
> +	int err;
> +
> +	isl28022_debugfs_root = debugfs_create_dir("isl28022", NULL);
> +	err = i2c_add_driver(&isl28022_driver);
> +	if (!err)
> +		return 0;
> +
> +	debugfs_remove_recursive(isl28022_debugfs_root);
> +	return err;
> +}
> +
> +static void __exit
> +isl28022_exit(void)
> +{
> +	i2c_del_driver(&isl28022_driver);
> +	debugfs_remove_recursive(isl28022_debugfs_root);
> +}
> +
> +module_init(isl28022_init);
> +module_exit(isl28022_exit);
> +
> +MODULE_AUTHOR("Carsten Spieß <mail@carsten-spiess.de>");
> +MODULE_DESCRIPTION("ISL28022 driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
> 

Re: [PATCH v6 2/3] dt-bindings: hwmon: add renesas,isl28022

[Krzysztof Kozlowski]

On 06/09/2024 08:14, Delphine CC Chiu wrote:
> From: Carsten Spieß <mail@carsten-spiess.de>
> 
> Add dt-bindings for Renesas ISL28022 power monitor.
> 
> Signed-off-by: Carsten Spieß <mail@carsten-spiess.de>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Your SoB is missing.

This must be fixed.

Best regards,
Krzysztof

Re: [PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

[Krzysztof Kozlowski]

On 06/09/2024 08:14, Delphine CC Chiu wrote:
> Added support reading shunt voltage in mV and revise code
> for Renesas ISL28022.
> 
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> ---
>  drivers/hwmon/isl28022.c | 93 ++++++++++++++++++++++++++++------------
>  1 file changed, 66 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
> index f0494c3bd483..01220fad813d 100644
> --- a/drivers/hwmon/isl28022.c
> +++ b/drivers/hwmon/isl28022.c
> @@ -85,8 +85,6 @@ struct isl28022_data {
>  	u32			shunt;
>  	u32			gain;
>  	u32			average;
> -	u16			config;
> -	u16			calib;
>  };
>  
>  static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
> @@ -95,20 +93,61 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
>  	struct isl28022_data *data = dev_get_drvdata(dev);
>  	unsigned int regval;
>  	int err;
> +	u16 sign_bit;
>  
>  	switch (type) {
>  	case hwmon_in:
> -		switch (attr) {
> -		case hwmon_in_input:
> -			err = regmap_read(data->regmap,
> -					  ISL28022_REG_BUS, &regval);
> -			if (err < 0)
> -				return err;
> -			/* driver supports only 60V mode (BRNG 11) */
> -			*val = (long)(((u16)regval) & 0xFFFC);
> +		switch (channel) {
> +		case 0:
> +			switch (attr) {
> +			case hwmon_in_input:
> +				err = regmap_read(data->regmap,
> +						  ISL28022_REG_BUS, &regval);
> +				if (err < 0)
> +					return err;
> +				/* driver supports only 60V mode (BRNG 11) */
> +				*val = (long)(((u16)regval) & 0xFFFC);
> +				break;
> +			default:
> +				return -EOPNOTSUPP;
> +			}
> +			break;
> +		case 1:
> +			switch (attr) {
> +			case hwmon_in_input:
> +				err = regmap_read(data->regmap,
> +						  ISL28022_REG_SHUNT, &regval);
> +				if (err < 0)
> +					return err;
> +			switch (data->gain) {
> +			case 8:
> +				sign_bit = (regval >> 15) & 0x01;
> +				*val = (long)((((u16)regval) & 0x7FFF) -
> +					   (sign_bit * 32768)) / 100;
> +				break;
> +			case 4:
> +				sign_bit = (regval >> 14) & 0x01;
> +				*val = (long)((((u16)regval) & 0x3FFF) -
> +					   (sign_bit * 16384)) / 100;
> +				break;
> +			case 2:
> +				sign_bit = (regval >> 13) & 0x01;
> +				*val = (long)((((u16)regval) & 0x1FFF) -
> +					   (sign_bit * 8192)) / 100;
> +				break;
> +			case 1:
> +				sign_bit = (regval >> 12) & 0x01;
> +				*val = (long)((((u16)regval) & 0x0FFF) -
> +					   (sign_bit * 4096)) / 100;
> +				break;
> +			}
> +			break;
> +			default:
> +				return -EOPNOTSUPP;
> +			}
>  			break;
>  		default:
> -			return -EINVAL;
> +			return -EOPNOTSUPP;
>  		}
>  		break;
>  	case hwmon_curr:
> @@ -122,7 +161,7 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
>  				(long)data->shunt;
>  			break;
>  		default:
> -			return -EINVAL;
> +			return -EOPNOTSUPP;
>  		}
>  		break;
>  	case hwmon_power:
> @@ -136,11 +175,11 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
>  				(long)data->shunt) * (long)regval;
>  			break;
>  		default:
> -			return -EINVAL;
> +			return -EOPNOTSUPP;
>  		}
>  		break;
>  	default:
> -		return -EINVAL;
> +		return -EOPNOTSUPP;
>  	}
>  
>  	return 0;
> @@ -182,7 +221,8 @@ static umode_t isl28022_is_visible(const void *data, enum hwmon_sensor_types typ
>  
>  static const struct hwmon_channel_info *isl28022_info[] = {
>  	HWMON_CHANNEL_INFO(in,
> -			   HWMON_I_INPUT),	/* channel 0: bus voltage (mV) */
> +			   HWMON_I_INPUT,	/* channel 0: bus voltage (mV) */
> +			   HWMON_I_INPUT),	/* channel 1: shunt voltage (mV) */
>  	HWMON_CHANNEL_INFO(curr,
>  			   HWMON_C_INPUT),	/* channel 1: current (mA) */
>  	HWMON_CHANNEL_INFO(power,
> @@ -368,24 +408,22 @@ static int isl28022_read_properties(struct device *dev, struct isl28022_data *da
>  static int isl28022_config(struct isl28022_data *data)
>  {
>  	int err;
> +	u16 config;
> +	u16 calib;
>  
> -	data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
> +	config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) |
>  			(ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT) |
>  			(__ffs(data->gain) << ISL28022_PG_SHIFT) |
>  			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_SADC_SHIFT) |
>  			((ISL28022_ADC_15_1 + __ffs(data->average)) << ISL28022_BADC_SHIFT);
>  
> -	data->calib = data->shunt ? 0x8000 / data->gain : 0;
> -
> -	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config);
> -	if (err < 0)
> -		return err;
> +	calib = data->shunt ? 0x8000 / data->gain : 0;
>  
> -	err = regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib);
> +	err = regmap_write(data->regmap, ISL28022_REG_CONFIG, config);
>  	if (err < 0)
>  		return err;
>  
> -	return 0;
> +	return regmap_write(data->regmap, ISL28022_REG_CALIB, calib);
>  }
>  
>  static int isl28022_probe(struct i2c_client *client)
> @@ -396,8 +434,8 @@ static int isl28022_probe(struct i2c_client *client)
>  	int err;
>  
>  	if (!i2c_check_functionality(client->adapter,
> -				     I2C_FUNC_SMBUS_BYTE_DATA |
> -				     I2C_FUNC_SMBUS_WORD_DATA))
> +					 I2C_FUNC_SMBUS_BYTE_DATA |
> +					 I2C_FUNC_SMBUS_WORD_DATA))
>  		return -ENODEV;
>  
>  	data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL);
> @@ -418,7 +456,7 @@ static int isl28022_probe(struct i2c_client *client)
>  
>  	isl28022_debugfs_init(client, data);
>  
> -	hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon",
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
>  							 data, &isl28022_chip_info, NULL);
>  	if (IS_ERR(hwmon_dev))
>  		return PTR_ERR(hwmon_dev);
> @@ -437,8 +475,9 @@ static struct i2c_driver isl28022_driver = {
>  	.class		= I2C_CLASS_HWMON,
>  	.driver = {
>  		.name	= "isl28022",
> +		.of_match_table = of_match_ptr(isl28022_of_match),

This is both unrelated and wrong. Your of_match_ptr causes here warnings.

Organize your patchset in logical chunks and be sure EACH has no
warnings. Your patch #1 now has such warnings. Please run standard
kernel tools for static analysis, like coccinelle, smatch and sparse,
and fix reported warnings. Also please check for warnings when building
with W=1. Most of these commands (checks or W=1 build) can build
specific targets, like some directory, to narrow the scope to only your
code. The code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

Best regards,
Krzysztof

Re: [PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

[Guenter Roeck]

On 9/5/24 23:14, Delphine CC Chiu wrote:
> Added support reading shunt voltage in mV and revise code
> for Renesas ISL28022.
> 
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> ---
>   drivers/hwmon/isl28022.c | 93 ++++++++++++++++++++++++++++------------
>   1 file changed, 66 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
> index f0494c3bd483..01220fad813d 100644
> --- a/drivers/hwmon/isl28022.c
> +++ b/drivers/hwmon/isl28022.c
> @@ -85,8 +85,6 @@ struct isl28022_data {
>   	u32			shunt;
>   	u32			gain;
>   	u32			average;
> -	u16			config;
> -	u16			calib;

I don't see the point of separating this part of the driver from the first patch
in the series. It makes me review code only to be replaced later. I am not going
to do that.

>   };
>   
>   static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
> @@ -95,20 +93,61 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
>   	struct isl28022_data *data = dev_get_drvdata(dev);
>   	unsigned int regval;
>   	int err;
> +	u16 sign_bit;
>   
>   	switch (type) {
>   	case hwmon_in:
> -		switch (attr) {
> -		case hwmon_in_input:
> -			err = regmap_read(data->regmap,
> -					  ISL28022_REG_BUS, &regval);
> -			if (err < 0)
> -				return err;
> -			/* driver supports only 60V mode (BRNG 11) */
> -			*val = (long)(((u16)regval) & 0xFFFC);
> +		switch (channel) {
> +		case 0:
> +			switch (attr) {
> +			case hwmon_in_input:
> +				err = regmap_read(data->regmap,
> +						  ISL28022_REG_BUS, &regval);
> +				if (err < 0)
> +					return err;
> +				/* driver supports only 60V mode (BRNG 11) */
> +				*val = (long)(((u16)regval) & 0xFFFC);
> +				break;
> +			default:
> +				return -EOPNOTSUPP;
> +			}
> +			break;
> +		case 1:
> +			switch (attr) {
> +			case hwmon_in_input:
> +				err = regmap_read(data->regmap,
> +						  ISL28022_REG_SHUNT, &regval);
> +				if (err < 0)
> +					return err;
> +			switch (data->gain) {
> +			case 8:
> +				sign_bit = (regval >> 15) & 0x01;
> +				*val = (long)((((u16)regval) & 0x7FFF) -
> +					   (sign_bit * 32768)) / 100;
> +				break;
> +			case 4:
> +				sign_bit = (regval >> 14) & 0x01;
> +				*val = (long)((((u16)regval) & 0x3FFF) -
> +					   (sign_bit * 16384)) / 100;
> +				break;
> +			case 2:
> +				sign_bit = (regval >> 13) & 0x01;
> +				*val = (long)((((u16)regval) & 0x1FFF) -
> +					   (sign_bit * 8192)) / 100;
> +				break;
> +			case 1:
> +				sign_bit = (regval >> 12) & 0x01;
> +				*val = (long)((((u16)regval) & 0x0FFF) -
> +					   (sign_bit * 4096)) / 100;
> +				break;
> +			}
> +			break;
> +			default:
> +				return -EOPNOTSUPP;
> +			}

Separate into its own voltage read function, and also provide separate functions
for current and power.

Guenter

Re: [PATCH v6 1/3] hwmon: (isl28022) new driver for ISL28022 power monitor

[kernel test robot]

Hi Delphine,

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.11-rc6 next-20240906]
[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/Delphine-CC-Chiu/hwmon-isl28022-new-driver-for-ISL28022-power-monitor/20240906-141717
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20240906061421.9392-2-Delphine_CC_Chiu%40wiwynn.com
patch subject: [PATCH v6 1/3] hwmon: (isl28022) new driver for ISL28022 power monitor
reproduce: (https://download.01.org/0day-ci/archive/20240907/202409070217.1YB4gnfd-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/202409070217.1YB4gnfd-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
   Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml
   Warning: Documentation/hwmon/g762.rst references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/g762.txt
>> Warning: Documentation/hwmon/isl28022.rst references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/reserved-memory/qcom
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/display/exynos/
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
   Using alabaster theme

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

Re: [PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

[kernel test robot]

Hi Delphine,

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.11-rc7 next-20240909]
[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/Delphine-CC-Chiu/hwmon-isl28022-new-driver-for-ISL28022-power-monitor/20240906-141717
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20240906061421.9392-4-Delphine_CC_Chiu%40wiwynn.com
patch subject: [PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor
config: microblaze-randconfig-r073-20240909 (https://download.01.org/0day-ci/archive/20240910/202409101229.6mTYs5Rf-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 14.1.0

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/202409101229.6mTYs5Rf-lkp@intel.com/

smatch warnings:
drivers/hwmon/isl28022.c:122 isl28022_read() warn: inconsistent indenting

vim +122 drivers/hwmon/isl28022.c

    89	
    90	static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
    91				 u32 attr, int channel, long *val)
    92	{
    93		struct isl28022_data *data = dev_get_drvdata(dev);
    94		unsigned int regval;
    95		int err;
    96		u16 sign_bit;
    97	
    98		switch (type) {
    99		case hwmon_in:
   100			switch (channel) {
   101			case 0:
   102				switch (attr) {
   103				case hwmon_in_input:
   104					err = regmap_read(data->regmap,
   105							  ISL28022_REG_BUS, &regval);
   106					if (err < 0)
   107						return err;
   108					/* driver supports only 60V mode (BRNG 11) */
   109					*val = (long)(((u16)regval) & 0xFFFC);
   110					break;
   111				default:
   112					return -EOPNOTSUPP;
   113				}
   114				break;
   115			case 1:
   116				switch (attr) {
   117				case hwmon_in_input:
   118					err = regmap_read(data->regmap,
   119							  ISL28022_REG_SHUNT, &regval);
   120					if (err < 0)
   121						return err;
 > 122				switch (data->gain) {
   123				case 8:
   124					sign_bit = (regval >> 15) & 0x01;
   125					*val = (long)((((u16)regval) & 0x7FFF) -
   126						   (sign_bit * 32768)) / 100;
   127					break;
   128				case 4:
   129					sign_bit = (regval >> 14) & 0x01;
   130					*val = (long)((((u16)regval) & 0x3FFF) -
   131						   (sign_bit * 16384)) / 100;
   132					break;
   133				case 2:
   134					sign_bit = (regval >> 13) & 0x01;
   135					*val = (long)((((u16)regval) & 0x1FFF) -
   136						   (sign_bit * 8192)) / 100;
   137					break;
   138				case 1:
   139					sign_bit = (regval >> 12) & 0x01;
   140					*val = (long)((((u16)regval) & 0x0FFF) -
   141						   (sign_bit * 4096)) / 100;
   142					break;
   143				}
   144				break;
   145				default:
   146					return -EOPNOTSUPP;
   147				}
   148				break;
   149			default:
   150				return -EOPNOTSUPP;
   151			}
   152			break;
   153		case hwmon_curr:
   154			switch (attr) {
   155			case hwmon_curr_input:
   156				err = regmap_read(data->regmap,
   157						  ISL28022_REG_CURRENT, &regval);
   158				if (err < 0)
   159					return err;
   160				*val = ((long)regval * 1250L * (long)data->gain) /
   161					(long)data->shunt;
   162				break;
   163			default:
   164				return -EOPNOTSUPP;
   165			}
   166			break;
   167		case hwmon_power:
   168			switch (attr) {
   169			case hwmon_power_input:
   170				err = regmap_read(data->regmap,
   171						  ISL28022_REG_POWER, &regval);
   172				if (err < 0)
   173					return err;
   174				*val = ((51200000L * ((long)data->gain)) /
   175					(long)data->shunt) * (long)regval;
   176				break;
   177			default:
   178				return -EOPNOTSUPP;
   179			}
   180			break;
   181		default:
   182			return -EOPNOTSUPP;
   183		}
   184	
   185		return 0;
   186	}
   187	

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