| Message ID | 20230726152235.249569-2-mail@carsten-spiess.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] hwmon: (isl28022) new driver for ISL28022 power monitor | expand |
On 7/26/23 08:22, Carsten Spieß wrote: > 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 | 56 +++++ > MAINTAINERS | 7 + > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/isl28022.c | 405 +++++++++++++++++++++++++++++++ > 6 files changed, 481 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 d11924667f76..c9548fc5c40e 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -90,6 +90,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..84c27ddcd33e > --- /dev/null > +++ b/Documentation/hwmon/isl28022.rst > @@ -0,0 +1,56 @@ > +.. 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 gain and averaging can be set > +via device tree at compile-time. At _compile-time_ ? > +Please refer to the Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml > +for bindings if the device tree is used. > + > +Sysfs entries > +------------- > + > +The following attributes are supported. All attributes are read-only. > + > +======================= ======================================================= > +in0_input shunt voltage (micro Volt) No. You must not change the ABI like that. > +in1_input bus voltage (milli Volt) > + > +curr1_input current (milli Ampere) > +power1_input power (micro Watt) > + > +note current and power attributes are supported only when > + shunt value is configured via device tree No. Use a reasonable default if there are no devicetree properties. > + > + shunt voltage is in micro Volt, not milli Volt, > + to get useful values I'd argue that shunt voltage is all but useless, but if you want to have it supported it _has_ to be in mV. Why not support limit attributes ? > +======================= ======================================================= > diff --git a/MAINTAINERS b/MAINTAINERS > index 7abb5710e1bb..c61aa688cd11 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11072,6 +11072,13 @@ S: Maintained > F: Documentation/filesystems/isofs.rst > F: fs/isofs/ > > +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 > + > IT87 HARDWARE MONITORING DRIVER > M: Jean Delvare <jdelvare@suse.com> > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 2913299c2c9e..3049c519e6ac 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -800,6 +800,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 !PPC > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index ff6bfd109c72..4046fed7f48d 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -98,6 +98,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..67cf47c31600 > --- /dev/null > +++ b/drivers/hwmon/isl28022.c > @@ -0,0 +1,405 @@ > +// 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/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/err.h> > +#include <linux/of_device.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > +#include <linux/util_macros.h> > +#include <linux/regulator/consumer.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 i2c_client *client; > + 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_SHUNT + channel, ®val); That never reads REG_BUS. > + if (err < 0) > + return err; > + *val = (channel == 0) ? > + (long)((s16)((u16)regval)) * 10 : > + (long)(((u16)regval) & 0xFFFC); I don't think the sign extensions are correct based on the datasheet. This will have to use sign_extend. > + break; > + default: > + return -EINVAL; > + } > + break; > + case hwmon_curr: > + switch (attr) { > + case hwmon_curr_input: > + err = regmap_read(data->regmap, > + ISL28022_REG_CURRENT, ®val); > + if (err < 0) > + return err; > + if (!data->shunt) > + return -EINVAL; Getting an error message each time the "sensors" command is executed ? Unacceptable. > + *val = ((long)regval * 10000L * (long)data->gain) / > + (long)(8 * data->shunt); > + break; > + default: > + return -EINVAL; > + } > + break; > + case hwmon_power: > + switch (attr) { > + case hwmon_power_input: > + err = regmap_read(data->regmap, > + ISL28022_REG_POWER, ®val); > + if (err < 0) > + return err; > + if (!data->shunt) > + return -EINVAL; Unacceptable. > + *val = ((long)regval * 409600000L * (long)data->gain) / > + (long)(8 * data->shunt); I don't think this was checked for overflows. > + 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; > + } > + break; > + case hwmon_curr: > + switch (attr) { > + case hwmon_curr_input: > + return 0444; > + } > + break; > + case hwmon_power: > + switch (attr) { > + case hwmon_power_input: > + return 0444; > + } > + break; > + default: > + break; > + } > + return 0; > +} > + > +static const struct hwmon_channel_info *isl28022_info[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ > + HWMON_I_INPUT), /* channel 1: bus voltage (mV) */ > + NULL > +}; > + > +static const struct hwmon_channel_info *isl28022_info_shunt[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ > + HWMON_I_INPUT), /* channel 1: 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 const struct hwmon_chip_info isl28022_chip_info_shunt = { > + .ops = &isl28022_hwmon_ops, > + .info = isl28022_info_shunt, > +}; > + > +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 isl28022_config(struct device *dev) > +{ > + struct isl28022_data *data = dev_get_drvdata(dev); > + > + if (!dev || !data) > + return -EINVAL; How would this ever happen ? > + ; > + data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) | > + (ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT); > + > + switch (data->gain) { > + case 1: > + data->config |= (ISL28022_PG_40 << ISL28022_PG_SHIFT); > + break; > + case 2: > + data->config |= (ISL28022_PG_80 << ISL28022_PG_SHIFT); > + break; > + case 4: > + data->config |= (ISL28022_PG_160 << ISL28022_PG_SHIFT); > + break; > + default: > + data->config |= (ISL28022_PG_320 << ISL28022_PG_SHIFT); > + data->gain = 8; > + break; > + } > + > + data->calib = data->shunt ? 0x8000 / data->gain : 0; > + > + switch (data->average) { > + case 128: > + data->config |= (ISL28022_ADC_15_128 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_128 << ISL28022_BADC_SHIFT); > + break; > + case 64: > + data->config |= (ISL28022_ADC_15_64 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_64 << ISL28022_BADC_SHIFT); > + break; > + case 32: > + data->config |= (ISL28022_ADC_15_32 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_32 << ISL28022_BADC_SHIFT); > + break; > + case 16: > + data->config |= (ISL28022_ADC_15_16 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_16 << ISL28022_BADC_SHIFT); > + break; > + case 8: > + data->config |= (ISL28022_ADC_15_8 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_8 << ISL28022_BADC_SHIFT); > + break; > + case 4: > + data->config |= (ISL28022_ADC_15_4 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_4 << ISL28022_BADC_SHIFT); > + break; > + case 2: > + data->config |= (ISL28022_ADC_15_2 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_2 << ISL28022_BADC_SHIFT); > + break; > + case 1: > + data->config |= (ISL28022_ADC_15_1 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_1 << ISL28022_BADC_SHIFT); > + break; > + default: > + data->config |= (ISL28022_ADC_15 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15 << ISL28022_BADC_SHIFT); > + data->average = 0; > + break; > + } > + > + regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config); > + regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib); Error checking needed. > + > + return 0; > +} > + > +static int isl28022_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct isl28022_data *data; > + int status; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_WORD_DATA)) > + return -EIO; This is not an IO error. Return -ENODEV as most other drivers do. > + > + data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + > + of_property_read_u32(dev->of_node, "shunt-resistor-micro-ohms", &data->shunt); > + of_property_read_u32(dev->of_node, "shunt-gain", &data->gain); > + of_property_read_u32(dev->of_node, "average", &data->average); Check for errors and provide defaults if properties are not set. Also please use device_property_read_u32() to enable use from non-devicetree systems. > + > + data->regmap = devm_regmap_init_i2c(client, &isl28022_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon", > + data, data->shunt ? > + &isl28022_chip_info_shunt : > + &isl28022_chip_info, NULL); > + if (IS_ERR(hwmon_dev)) > + return PTR_ERR(hwmon_dev); > + > + status = isl28022_config(hwmon_dev); > + if (status) > + return status; That has to happen before the call to devm_hwmon_device_register_with_info() to avoid race conditions. > + > + 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", > + .of_match_table = of_match_ptr(isl28022_of_match), Drop of_match_ptr() > + }, > + .probe_new = isl28022_probe, > + .id_table = isl28022_ids, > +}; > + > +module_i2c_driver(isl28022_driver); > + > +MODULE_AUTHOR("Carsten Spiess <mail@carsten-spiess.de>"); > +MODULE_DESCRIPTION("ISL28022 driver"); > +MODULE_LICENSE("GPL");
On 7/26/23 08:22, Carsten Spieß wrote: > 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> Please provide a register dump (using i2cdump) for this chip. I strongly suspect that the conversions will result in overflows and that they are not always correct. I'll want to write unit test code before accepting the driver. Guenter > --- > Documentation/hwmon/index.rst | 1 + > Documentation/hwmon/isl28022.rst | 56 +++++ > MAINTAINERS | 7 + > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/isl28022.c | 405 +++++++++++++++++++++++++++++++ > 6 files changed, 481 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 d11924667f76..c9548fc5c40e 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -90,6 +90,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..84c27ddcd33e > --- /dev/null > +++ b/Documentation/hwmon/isl28022.rst > @@ -0,0 +1,56 @@ > +.. 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 gain and averaging can be set > +via device tree at compile-time. > +Please refer to the Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml > +for bindings if the device tree is used. > + > +Sysfs entries > +------------- > + > +The following attributes are supported. All attributes are read-only. > + > +======================= ======================================================= > +in0_input shunt voltage (micro Volt) > +in1_input bus voltage (milli Volt) > + > +curr1_input current (milli Ampere) > +power1_input power (micro Watt) > + > +note current and power attributes are supported only when > + shunt value is configured via device tree > + > + shunt voltage is in micro Volt, not milli Volt, > + to get useful values > +======================= ======================================================= > diff --git a/MAINTAINERS b/MAINTAINERS > index 7abb5710e1bb..c61aa688cd11 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11072,6 +11072,13 @@ S: Maintained > F: Documentation/filesystems/isofs.rst > F: fs/isofs/ > > +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 > + > IT87 HARDWARE MONITORING DRIVER > M: Jean Delvare <jdelvare@suse.com> > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 2913299c2c9e..3049c519e6ac 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -800,6 +800,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 !PPC > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index ff6bfd109c72..4046fed7f48d 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -98,6 +98,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..67cf47c31600 > --- /dev/null > +++ b/drivers/hwmon/isl28022.c > @@ -0,0 +1,405 @@ > +// 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/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/err.h> > +#include <linux/of_device.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > +#include <linux/util_macros.h> > +#include <linux/regulator/consumer.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 i2c_client *client; > + 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_SHUNT + channel, ®val); > + if (err < 0) > + return err; > + *val = (channel == 0) ? > + (long)((s16)((u16)regval)) * 10 : > + (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, ®val); > + if (err < 0) > + return err; > + if (!data->shunt) > + return -EINVAL; > + *val = ((long)regval * 10000L * (long)data->gain) / > + (long)(8 * data->shunt); > + break; > + default: > + return -EINVAL; > + } > + break; > + case hwmon_power: > + switch (attr) { > + case hwmon_power_input: > + err = regmap_read(data->regmap, > + ISL28022_REG_POWER, ®val); > + if (err < 0) > + return err; > + if (!data->shunt) > + return -EINVAL; > + *val = ((long)regval * 409600000L * (long)data->gain) / > + (long)(8 * data->shunt); > + 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; > + } > + break; > + case hwmon_curr: > + switch (attr) { > + case hwmon_curr_input: > + return 0444; > + } > + break; > + case hwmon_power: > + switch (attr) { > + case hwmon_power_input: > + return 0444; > + } > + break; > + default: > + break; > + } > + return 0; > +} > + > +static const struct hwmon_channel_info *isl28022_info[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ > + HWMON_I_INPUT), /* channel 1: bus voltage (mV) */ > + NULL > +}; > + > +static const struct hwmon_channel_info *isl28022_info_shunt[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ > + HWMON_I_INPUT), /* channel 1: 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 const struct hwmon_chip_info isl28022_chip_info_shunt = { > + .ops = &isl28022_hwmon_ops, > + .info = isl28022_info_shunt, > +}; > + > +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 isl28022_config(struct device *dev) > +{ > + struct isl28022_data *data = dev_get_drvdata(dev); > + > + if (!dev || !data) > + return -EINVAL; > + ; > + data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) | > + (ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT); > + > + switch (data->gain) { > + case 1: > + data->config |= (ISL28022_PG_40 << ISL28022_PG_SHIFT); > + break; > + case 2: > + data->config |= (ISL28022_PG_80 << ISL28022_PG_SHIFT); > + break; > + case 4: > + data->config |= (ISL28022_PG_160 << ISL28022_PG_SHIFT); > + break; > + default: > + data->config |= (ISL28022_PG_320 << ISL28022_PG_SHIFT); > + data->gain = 8; > + break; > + } > + > + data->calib = data->shunt ? 0x8000 / data->gain : 0; > + > + switch (data->average) { > + case 128: > + data->config |= (ISL28022_ADC_15_128 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_128 << ISL28022_BADC_SHIFT); > + break; > + case 64: > + data->config |= (ISL28022_ADC_15_64 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_64 << ISL28022_BADC_SHIFT); > + break; > + case 32: > + data->config |= (ISL28022_ADC_15_32 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_32 << ISL28022_BADC_SHIFT); > + break; > + case 16: > + data->config |= (ISL28022_ADC_15_16 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_16 << ISL28022_BADC_SHIFT); > + break; > + case 8: > + data->config |= (ISL28022_ADC_15_8 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_8 << ISL28022_BADC_SHIFT); > + break; > + case 4: > + data->config |= (ISL28022_ADC_15_4 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_4 << ISL28022_BADC_SHIFT); > + break; > + case 2: > + data->config |= (ISL28022_ADC_15_2 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_2 << ISL28022_BADC_SHIFT); > + break; > + case 1: > + data->config |= (ISL28022_ADC_15_1 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15_1 << ISL28022_BADC_SHIFT); > + break; > + default: > + data->config |= (ISL28022_ADC_15 << ISL28022_SADC_SHIFT) | > + (ISL28022_ADC_15 << ISL28022_BADC_SHIFT); > + data->average = 0; > + break; > + } > + > + regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config); > + regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib); > + > + return 0; > +} > + > +static int isl28022_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct isl28022_data *data; > + int status; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_SMBUS_BYTE_DATA | > + I2C_FUNC_SMBUS_WORD_DATA)) > + return -EIO; > + > + data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + > + of_property_read_u32(dev->of_node, "shunt-resistor-micro-ohms", &data->shunt); > + of_property_read_u32(dev->of_node, "shunt-gain", &data->gain); > + of_property_read_u32(dev->of_node, "average", &data->average); > + > + data->regmap = devm_regmap_init_i2c(client, &isl28022_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon", > + data, data->shunt ? > + &isl28022_chip_info_shunt : > + &isl28022_chip_info, NULL); > + if (IS_ERR(hwmon_dev)) > + return PTR_ERR(hwmon_dev); > + > + status = isl28022_config(hwmon_dev); > + if (status) > + return status; > + > + 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", > + .of_match_table = of_match_ptr(isl28022_of_match), > + }, > + .probe_new = isl28022_probe, > + .id_table = isl28022_ids, > +}; > + > +module_i2c_driver(isl28022_driver); > + > +MODULE_AUTHOR("Carsten Spiess <mail@carsten-spiess.de>"); > +MODULE_DESCRIPTION("ISL28022 driver"); > +MODULE_LICENSE("GPL");
On 7/26/23 08:22, Carsten Spieß wrote: > diff --git a/MAINTAINERS b/MAINTAINERS > index 7abb5710e1bb..c61aa688cd11 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11072,6 +11072,13 @@ S: Maintained > F: Documentation/filesystems/isofs.rst > F: fs/isofs/ > New entry is not quite in the correct place for alphabetical order. It should be just before the ISOFS entry, not just after it. > +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 > + > IT87 HARDWARE MONITORING DRIVER > M: Jean Delvare <jdelvare@suse.com> > L: linux-hwmon@vger.kernel.org
On 7/26/23 18:14, Gueter Roeck wrote: > On 7/26/23 08:22, Carsten Spieß wrote: > > +The shunt value in micro-ohms, shunt gain and averaging can be set > > +via device tree at compile-time. > > At _compile-time_ ? How to explain better that it isn't set at runtime? Other drivers use the same term. > I'd argue that shunt voltage is all but useless, but if you want to have it supported > it _has_ to be in mV. That's a problem. In my case the ER-6P has a 8 milli Ohm (or 8000 micro Ohm) shunt and a powersupply with max. 2.5 A. This gives a max shunt voltage of 20 mV at 2.5 A. The device normaly consumes between 200 and 500 mA. (typ ~250 mA). This results in shunt voltage of 1.6 to 4.0 mV (typ ~2mV). Having no fractions will make it useless. Unfortunately there is no possibility to give a scaling factor. Or returning float values (i know, this can't and shouldn't be changed) > Why not support limit attributes ? Due to limited time, left for later enhancement. > > +#define ISL28022_REG_SHUNT 0x01 > > +#define ISL28022_REG_BUS 0x02 > > + switch (type) { > > + case hwmon_in: > > + switch (attr) { > > + case hwmon_in_input: > > + err = regmap_read(data->regmap, > > + ISL28022_REG_SHUNT + channel, ®val); > > That never reads REG_BUS. Hmm, channel 0: ISL28022_REG_SHUNT + 0 == 0x01 channel 1: ISL28022_REG_SHUNT + 1 == 0x02 == ISL28022_REG_BUS or do i miss something? > > + if (err < 0) > > + return err; > > + *val = (channel == 0) ? > > + (long)((s16)((u16)regval)) * 10 : > > + (long)(((u16)regval) & 0xFFFC); > > I don't think the sign extensions are correct based on the datasheet. > This will have to use sign_extend. From my understading (see table 11 on page 16 of the ISL28022 datasheet) shunt value is already sign extended, (D15-D12 is sign) bus value (table 12 on page 16) is unsigned. > > + err = regmap_read(data->regmap, > > + ISL28022_REG_CURRENT, ®val); > > + if (err < 0) > > + return err; > > + if (!data->shunt) > > + return -EINVAL; > > Getting an error message each time the "sensors" command is executed ? > Unacceptable. o.k., will change to set *val = 0; > > + err = regmap_read(data->regmap, > > + ISL28022_REG_POWER, ®val); > > + if (err < 0) > > + return err; > > + if (!data->shunt) > > + return -EINVAL; > > Unacceptable. o.k., as above > > + *val = ((long)regval * 409600000L * (long)data->gain) / > > + (long)(8 * data->shunt); > > I don't think this was checked for overflows. Yes, i must first divide then multiply. I have to think about how to keep accuracy on high shunt resistor values. > > +static int isl28022_config(struct device *dev) > > +{ > > + struct isl28022_data *data = dev_get_drvdata(dev); > > + > > + if (!dev || !data) > > + return -EINVAL; > > How would this ever happen ? Shouldn't, but i'm carefully (i had it once during development due to an error (using dev instead of hwmon_dev) on calling this function > > + regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config); > > + regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib); > > Error checking needed. o.k. will add. > > +static int isl28022_probe(struct i2c_client *client) > > +{ > > + if (!i2c_check_functionality(client->adapter, > > + I2C_FUNC_SMBUS_BYTE_DATA | > > + I2C_FUNC_SMBUS_WORD_DATA)) > > + return -EIO; > > This is not an IO error. Return -ENODEV as most other drivers do. o.k. > > + of_property_read_u32(dev->of_node, "shunt-gain", &data->gain); > > + of_property_read_u32(dev->of_node, "average", &data->average); > > Check for errors and provide defaults if properties are not set. o.k. > Also please use device_property_read_u32() to enable use from non-devicetree > systems. o.k. Never used this, have to look for an example on using it. Many (old) drivers are using the of_property_* functions, is it intended to replace it. What about backporting this driver to e.g. 5.15, will it affect it? > > + status = isl28022_config(hwmon_dev); > > + if (status) > > + return status; > > That has to happen before the call to devm_hwmon_device_register_with_info() > to avoid race conditions. o.k. > > +static struct i2c_driver isl28022_driver = { > > + .class = I2C_CLASS_HWMON, > > + .driver = { > > + .name = "isl28022", > > + .of_match_table = of_match_ptr(isl28022_of_match), > > Drop of_match_ptr() Most drivers have this, why drop? Regards, Carsten
On 7/26/23 18:19, Guenter Roeck wrote:
> Please provide a register dump (using i2cdump) for this chip.
# i2cdump -y -r 0-9 1 0x40 w
0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f
00: ff67 ca00 a25d c803 5006 0080 817f 00ff
08: 0000 0000
Please note that due to big vs.- little endian bytes are swapped,
should be read as:
00: 67ff 00ca 5da2 03c8 0650 8000 7f81 ff00
08: 0000 0000
corresponding sensor values are about (not read at same time):
in0: 1.99 V
in1: 23.97 V
power1: 6.10 W
curr1: 248.00 mA
in0 should be read as mV
Regards, Carsten
On 7/27/23 00:35, Carsten Spieß wrote: > > On 7/26/23 18:14, Gueter Roeck wrote: >> On 7/26/23 08:22, Carsten Spieß wrote: >>> +The shunt value in micro-ohms, shunt gain and averaging can be set >>> +via device tree at compile-time. >> >> At _compile-time_ ? > How to explain better that it isn't set at runtime? > Other drivers use the same term. You mean it is ok to exceed the speed limit because others do it as well ? [ Sorry, I have heard the "Other drivers do it" pseudo-argument too many times. That doesn't mean it is the right thing to do. ] Why not just leave it off ? What value does it add ? Besides, even "via devicetree" isn't really correct because it can also be set via ACPI or by a platform driver when using device_ properties. I would suggest "can be set with device properties". > >> I'd argue that shunt voltage is all but useless, but if you want to have it supported >> it _has_ to be in mV. > That's a problem. > > In my case the ER-6P has a 8 milli Ohm (or 8000 micro Ohm) shunt and a powersupply with > max. 2.5 A. This gives a max shunt voltage of 20 mV at 2.5 A. > The device normaly consumes between 200 and 500 mA. (typ ~250 mA). > This results in shunt voltage of 1.6 to 4.0 mV (typ ~2mV). > Having no fractions will make it useless. > > Unfortunately there is no possibility to give a scaling factor. > Or returning float values (i know, this can't and shouldn't be changed) > Just like the ABI must not be changed. The sensors command would display your 4mV shunt voltage as 4V, which is just as useless. In practice, the shunt voltage _is_ useless for hardware monitoring purpose because it can be calculated from current and shunt resistor value. I'd say if you really want it, provide it as debugfs attribute. As hwmon attribute it has to be in mV. >> Why not support limit attributes ? > Due to limited time, left for later enhancement. > > >>> +#define ISL28022_REG_SHUNT 0x01 >>> +#define ISL28022_REG_BUS 0x02 > > >>> + switch (type) { >>> + case hwmon_in: >>> + switch (attr) { >>> + case hwmon_in_input: >>> + err = regmap_read(data->regmap, >>> + ISL28022_REG_SHUNT + channel, ®val); >> >> That never reads REG_BUS. > Hmm, > channel 0: ISL28022_REG_SHUNT + 0 == 0x01 > channel 1: ISL28022_REG_SHUNT + 1 == 0x02 == ISL28022_REG_BUS > or do i miss something? > No, I missed the "+ channel". >>> + if (err < 0) >>> + return err; >>> + *val = (channel == 0) ? >>> + (long)((s16)((u16)regval)) * 10 : >>> + (long)(((u16)regval) & 0xFFFC); >> >> I don't think the sign extensions are correct based on the datasheet. >> This will have to use sign_extend. > From my understading (see table 11 on page 16 of the ISL28022 datasheet) > shunt value is already sign extended, (D15-D12 is sign) > bus value (table 12 on page 16) is unsigned. > Not really. For the shunt voltage, 0xf000 has different meanings depending on scale and range settings. LSB for bus voltage is 4 mV and starts at bit 2 or 3 depending on BRNG. The above just happens to be correct if BRNG = 10 OR 11 per datasheet. If that is intentional, it needs to get a comment. >>> + err = regmap_read(data->regmap, >>> + ISL28022_REG_CURRENT, ®val); >>> + if (err < 0) >>> + return err; >>> + if (!data->shunt) >>> + return -EINVAL; >> >> Getting an error message each time the "sensors" command is executed ? >> Unacceptable. > o.k., will change to set *val = 0; > Still unacceptable. >>> + err = regmap_read(data->regmap, >>> + ISL28022_REG_POWER, ®val); >>> + if (err < 0) >>> + return err; >>> + if (!data->shunt) >>> + return -EINVAL; >> >> Unacceptable. > o.k., as above > >>> + *val = ((long)regval * 409600000L * (long)data->gain) / >>> + (long)(8 * data->shunt); >> >> I don't think this was checked for overflows. > Yes, i must first divide then multiply. > I have to think about how to keep accuracy on high shunt resistor values. > >>> +static int isl28022_config(struct device *dev) >>> +{ >>> + struct isl28022_data *data = dev_get_drvdata(dev); >>> + >>> + if (!dev || !data) >>> + return -EINVAL; >> >> How would this ever happen ? > Shouldn't, but i'm carefully (i had it once during development due to an error > (using dev instead of hwmon_dev) on calling this function > Parameter checks are only acceptable on API functions. This is not an API function. Local functions are expected to be consistent. If this function is called with a bad argument, that needs to be fixed during development. >>> + regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config); >>> + regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib); >> >> Error checking needed. > o.k. will add. > >>> +static int isl28022_probe(struct i2c_client *client) >>> +{ > >>> + if (!i2c_check_functionality(client->adapter, >>> + I2C_FUNC_SMBUS_BYTE_DATA | >>> + I2C_FUNC_SMBUS_WORD_DATA)) >>> + return -EIO; >> >> This is not an IO error. Return -ENODEV as most other drivers do. > o.k. > >>> + of_property_read_u32(dev->of_node, "shunt-gain", &data->gain); >>> + of_property_read_u32(dev->of_node, "average", &data->average); >> >> Check for errors and provide defaults if properties are not set. > o.k. > >> Also please use device_property_read_u32() to enable use from non-devicetree >> systems. > o.k. Never used this, have to look for an example on using it. > Many (old) drivers are using the of_property_* functions, is it intended to replace it. This is not an old driver. It is more generic than of_ functions and should be used in new drivers. > What about backporting this driver to e.g. 5.15, will it affect it? > Device property callback functions existed for a long time. The function exists in v4.14.y, which is the oldest supported kernel. Either case, lack of support in an older kernel version is not an argument for avoiding a new API. Anyone who backports a driver to an older kernel is responsible for handling the backport, which may include new API functions. Specific example: Your driver will have to use the .probe callback. That has one argument in the latest kernel. In v5.15.y, it has two arguments. You'll have to use the .probe_new callback there. Yes, technically you could try sneaking in the use of .probe_new in your driver, but that callback will be removed in v6.6. So you'll _have_ to do some backport, if you want it or not. >>> + status = isl28022_config(hwmon_dev); >>> + if (status) >>> + return status; >> >> That has to happen before the call to devm_hwmon_device_register_with_info() >> to avoid race conditions. > o.k. > >>> +static struct i2c_driver isl28022_driver = { >>> + .class = I2C_CLASS_HWMON, >>> + .driver = { >>> + .name = "isl28022", >>> + .of_match_table = of_match_ptr(isl28022_of_match), >> >> Drop of_match_ptr() > Most drivers have this, why drop? > It is needed for device_property_read_u32() to work. And, again, "other drivers do it" is not a valid argument. Guenter
On 7/27/23 16:30, Gueter Roeck wrote: > >> On 7/26/23 08:22, Carsten Spieß wrote: > >> At _compile-time_ ? > > How to explain better that it isn't set at runtime? > I would suggest "can be set with device properties". Yeah, i like that. > >> I'd argue that shunt voltage is all but useless, but if you want to have it supported > >> it _has_ to be in mV. > > That's a problem. > > > > In my case the ER-6P has a 8 milli Ohm (or 8000 micro Ohm) shunt and a powersupply with > > max. 2.5 A. This gives a max shunt voltage of 20 mV at 2.5 A. > > The device normaly consumes between 200 and 500 mA. (typ ~250 mA). > > This results in shunt voltage of 1.6 to 4.0 mV (typ ~2mV). > > Having no fractions will make it useless. > > > > Unfortunately there is no possibility to give a scaling factor. > > Or returning float values (i know, this can't and shouldn't be changed) > > > > Just like the ABI must not be changed. The sensors command would display your > 4mV shunt voltage as 4V, which is just as useless. > > In practice, the shunt voltage _is_ useless for hardware monitoring purpose > because it can be calculated from current and shunt resistor value. > I'd say if you really want it, provide it as debugfs attribute. As hwmon > attribute it has to be in mV. O.k. will move to debugfs. > >> I don't think the sign extensions are correct based on the datasheet. > >> This will have to use sign_extend. > > From my understading (see table 11 on page 16 of the ISL28022 datasheet) > > shunt value is already sign extended, (D15-D12 is sign) > > bus value (table 12 on page 16) is unsigned. > > > > Not really. For the shunt voltage, 0xf000 has different meanings depending on scale > and range settings. Sorry, i don't agree, 0xf000 is -40.96 mV on all scale settings. > LSB for bus voltage is 4 mV and starts at bit 2 or 3 depending > on BRNG. The above just happens to be correct if BRNG = 10 OR 11 per datasheet. > If that is intentional, it needs to get a comment. Yes, will add comment. > >> Getting an error message each time the "sensors" command is executed ? > >> Unacceptable. > > o.k., will change to set *val = 0; > > > Still unacceptable. O.k. i will limit shunt-resistor-milli-ohms to a minimal value > 0 and drop check here. > >>> + if (!dev || !data) > >>> + return -EINVAL; > >> > >> How would this ever happen ? > > Shouldn't, but i'm carefully (i had it once during development due to an error > > (using dev instead of hwmon_dev) on calling this function > > > > Parameter checks are only acceptable on API functions. This is not an API function. > Local functions are expected to be consistent. If this function is called with > a bad argument, that needs to be fixed during development. O.k., removed. > >>> +static struct i2c_driver isl28022_driver = { > >>> + .class = I2C_CLASS_HWMON, > >>> + .driver = { > >>> + .name = "isl28022", > >>> + .of_match_table = of_match_ptr(isl28022_of_match), > >> > >> Drop of_match_ptr() > > Most drivers have this, why drop? > > > > It is needed for device_property_read_u32() to work. O.k. dropped, i wasn't familiar with device_property_read functions. Regards Carsten
> On 7/26/23 18:19, Guenter Roeck wrote: > I strongly suspect that the conversions will result in overflows and that they are not always correct. > I'll want to write unit test code before accepting the driver. I changed the conversion for current to *val = ((long)regval * 1250L * (long)data->gain) / (long)data->shunt; The term (1250 * gain) will be 10000 max, fits to 14 bit. So no risk for 32bit overflow when multiply with 16bit value. And changed the conversion for power to *val = ((51200000L * ((long)data->gain)) / (long)data->shunt) * (long)regval; The first term (51200000 * gain / shunt) will be larger than 16bit when (shunt/gain) is less than ~800. So min values for shunt are - 6400 µOhm for 320 mV range - 3200 µOhm for 160 mV range - 1600 µOhm for 80 mV range - 800 µOhm for 40 mV range Can i set this conditionally in the .yaml file? Regards Carsten
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index d11924667f76..c9548fc5c40e 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -90,6 +90,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..84c27ddcd33e --- /dev/null +++ b/Documentation/hwmon/isl28022.rst @@ -0,0 +1,56 @@ +.. 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 gain and averaging can be set +via device tree at compile-time. +Please refer to the Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml +for bindings if the device tree is used. + +Sysfs entries +------------- + +The following attributes are supported. All attributes are read-only. + +======================= ======================================================= +in0_input shunt voltage (micro Volt) +in1_input bus voltage (milli Volt) + +curr1_input current (milli Ampere) +power1_input power (micro Watt) + +note current and power attributes are supported only when + shunt value is configured via device tree + + shunt voltage is in micro Volt, not milli Volt, + to get useful values +======================= ======================================================= diff --git a/MAINTAINERS b/MAINTAINERS index 7abb5710e1bb..c61aa688cd11 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11072,6 +11072,13 @@ S: Maintained F: Documentation/filesystems/isofs.rst F: fs/isofs/ +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 + IT87 HARDWARE MONITORING DRIVER M: Jean Delvare <jdelvare@suse.com> L: linux-hwmon@vger.kernel.org diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 2913299c2c9e..3049c519e6ac 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -800,6 +800,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 !PPC diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index ff6bfd109c72..4046fed7f48d 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -98,6 +98,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..67cf47c31600 --- /dev/null +++ b/drivers/hwmon/isl28022.c @@ -0,0 +1,405 @@ +// 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/module.h> +#include <linux/init.h> +#include <linux/slab.h> +#include <linux/i2c.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/err.h> +#include <linux/of_device.h> +#include <linux/of.h> +#include <linux/regmap.h> +#include <linux/util_macros.h> +#include <linux/regulator/consumer.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 i2c_client *client; + 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_SHUNT + channel, ®val); + if (err < 0) + return err; + *val = (channel == 0) ? + (long)((s16)((u16)regval)) * 10 : + (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, ®val); + if (err < 0) + return err; + if (!data->shunt) + return -EINVAL; + *val = ((long)regval * 10000L * (long)data->gain) / + (long)(8 * data->shunt); + break; + default: + return -EINVAL; + } + break; + case hwmon_power: + switch (attr) { + case hwmon_power_input: + err = regmap_read(data->regmap, + ISL28022_REG_POWER, ®val); + if (err < 0) + return err; + if (!data->shunt) + return -EINVAL; + *val = ((long)regval * 409600000L * (long)data->gain) / + (long)(8 * data->shunt); + 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; + } + break; + case hwmon_curr: + switch (attr) { + case hwmon_curr_input: + return 0444; + } + break; + case hwmon_power: + switch (attr) { + case hwmon_power_input: + return 0444; + } + break; + default: + break; + } + return 0; +} + +static const struct hwmon_channel_info *isl28022_info[] = { + HWMON_CHANNEL_INFO(in, + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ + HWMON_I_INPUT), /* channel 1: bus voltage (mV) */ + NULL +}; + +static const struct hwmon_channel_info *isl28022_info_shunt[] = { + HWMON_CHANNEL_INFO(in, + HWMON_I_INPUT, /* channel 0: shunt voltage (µV) */ + HWMON_I_INPUT), /* channel 1: 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 const struct hwmon_chip_info isl28022_chip_info_shunt = { + .ops = &isl28022_hwmon_ops, + .info = isl28022_info_shunt, +}; + +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 isl28022_config(struct device *dev) +{ + struct isl28022_data *data = dev_get_drvdata(dev); + + if (!dev || !data) + return -EINVAL; + ; + data->config = (ISL28022_MODE_CONT_SB << ISL28022_MODE_SHIFT) | + (ISL28022_BRNG_60 << ISL28022_BRNG_SHIFT); + + switch (data->gain) { + case 1: + data->config |= (ISL28022_PG_40 << ISL28022_PG_SHIFT); + break; + case 2: + data->config |= (ISL28022_PG_80 << ISL28022_PG_SHIFT); + break; + case 4: + data->config |= (ISL28022_PG_160 << ISL28022_PG_SHIFT); + break; + default: + data->config |= (ISL28022_PG_320 << ISL28022_PG_SHIFT); + data->gain = 8; + break; + } + + data->calib = data->shunt ? 0x8000 / data->gain : 0; + + switch (data->average) { + case 128: + data->config |= (ISL28022_ADC_15_128 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_128 << ISL28022_BADC_SHIFT); + break; + case 64: + data->config |= (ISL28022_ADC_15_64 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_64 << ISL28022_BADC_SHIFT); + break; + case 32: + data->config |= (ISL28022_ADC_15_32 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_32 << ISL28022_BADC_SHIFT); + break; + case 16: + data->config |= (ISL28022_ADC_15_16 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_16 << ISL28022_BADC_SHIFT); + break; + case 8: + data->config |= (ISL28022_ADC_15_8 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_8 << ISL28022_BADC_SHIFT); + break; + case 4: + data->config |= (ISL28022_ADC_15_4 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_4 << ISL28022_BADC_SHIFT); + break; + case 2: + data->config |= (ISL28022_ADC_15_2 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_2 << ISL28022_BADC_SHIFT); + break; + case 1: + data->config |= (ISL28022_ADC_15_1 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15_1 << ISL28022_BADC_SHIFT); + break; + default: + data->config |= (ISL28022_ADC_15 << ISL28022_SADC_SHIFT) | + (ISL28022_ADC_15 << ISL28022_BADC_SHIFT); + data->average = 0; + break; + } + + regmap_write(data->regmap, ISL28022_REG_CONFIG, data->config); + regmap_write(data->regmap, ISL28022_REG_CALIB, data->calib); + + return 0; +} + +static int isl28022_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct device *hwmon_dev; + struct isl28022_data *data; + int status; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_BYTE_DATA | + I2C_FUNC_SMBUS_WORD_DATA)) + return -EIO; + + data = devm_kzalloc(dev, sizeof(struct isl28022_data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->client = client; + + of_property_read_u32(dev->of_node, "shunt-resistor-micro-ohms", &data->shunt); + of_property_read_u32(dev->of_node, "shunt-gain", &data->gain); + of_property_read_u32(dev->of_node, "average", &data->average); + + data->regmap = devm_regmap_init_i2c(client, &isl28022_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + hwmon_dev = devm_hwmon_device_register_with_info(dev, "isl28022_hwmon", + data, data->shunt ? + &isl28022_chip_info_shunt : + &isl28022_chip_info, NULL); + if (IS_ERR(hwmon_dev)) + return PTR_ERR(hwmon_dev); + + status = isl28022_config(hwmon_dev); + if (status) + return status; + + 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", + .of_match_table = of_match_ptr(isl28022_of_match), + }, + .probe_new = isl28022_probe, + .id_table = isl28022_ids, +}; + +module_i2c_driver(isl28022_driver); + +MODULE_AUTHOR("Carsten Spiess <mail@carsten-spiess.de>"); +MODULE_DESCRIPTION("ISL28022 driver"); +MODULE_LICENSE("GPL");
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 | 56 +++++ MAINTAINERS | 7 + drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/isl28022.c | 405 +++++++++++++++++++++++++++++++ 6 files changed, 481 insertions(+) create mode 100644 Documentation/hwmon/isl28022.rst create mode 100644 drivers/hwmon/isl28022.c