| Message ID | 20250110081546.61667-3-Leo-Yang@quantatw.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: Add support for INA233 | expand |
On 10/01/2025 09:15, Leo Yang wrote: > Support ina233 driver for Meta Yosemite V4. > > Driver for Texas Instruments INA233 Current and Power Monitor > With I2C-, SMBus-, and PMBus-Compatible Interface > > Reported-by: kernel test robot <lkp@intel.com> No, what did the robot report? Drop. > Closes: https://lore.kernel.org/oe-kbuild-all/202501092213.X9mbPW5Q-lkp@intel.com/ Drop > Closes: https://lore.kernel.org/oe-kbuild-all/202501061734.nPNdRKqO-lkp@intel.com/ Drop > Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> > --- > Documentation/hwmon/ina233.rst | 77 ++++++++++++++ > MAINTAINERS | 8 ++ > drivers/hwmon/pmbus/Kconfig | 9 ++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/ina233.c | 184 +++++++++++++++++++++++++++++++++ > 5 files changed, 279 insertions(+) > create mode 100644 Documentation/hwmon/ina233.rst > create mode 100644 drivers/hwmon/pmbus/ina233.c > > diff --git a/Documentation/hwmon/ina233.rst b/Documentation/hwmon/ina233.rst > new file mode 100644 > index 000000000000..41537f89bed5 > --- /dev/null > +++ b/Documentation/hwmon/ina233.rst > @@ -0,0 +1,77 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +Kernel driver ina233 > +==================== > + > +Supported chips: > + > + * TI INA233 > + > + Prefix: 'ina233' > + > + * Datasheet > + > + Publicly available at the TI website : https://www.ti.com/lit/ds/symlink/ina233.pdf > + > +Author: > + > + Leo Yang <Leo-Yang@quantatw.com> > + > +Usage Notes > +----------- > + > +The shunt resistor value can be configured by a device tree property; > +see Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for details. > + > + > +Description > +----------- > + > +This driver supports hardware monitoring for TI INA233. > + > +The driver is a client driver to the core PMBus driver. Please see > +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. > + > +The driver provides the following attributes for input voltage: > + > +**in1_input** > + > +**in1_label** > + > +**in1_max** > + > +**in1_max_alarm** > + > +**in1_min** > + > +**in1_min_alarm** > + > +The driver provides the following attributes for shunt voltage: > + > +**in2_input** > + > +**in2_label** > + > +The driver provides the following attributes for output voltage: > + > +**in3_input** > + > +**in3_label** > + > +**in3_alarm** > + > +The driver provides the following attributes for output current: > + > +**curr1_input** > + > +**curr1_label** > + > +**curr1_max** > + > +**curr1_max_alarm** > + > +The driver provides the following attributes for input power: > + > +**power1_input** > + > +**power1_label** > \ No newline at end of file You still have patch warnings. I already commented on this, so you have to fix it everywhere. > diff --git a/MAINTAINERS b/MAINTAINERS > index c575de4903db..fde1713dff9d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11226,6 +11226,14 @@ L: linux-fbdev@vger.kernel.org > S: Orphan > F: drivers/video/fbdev/imsttfb.c > > +INA233 HARDWARE MONITOR DRIVER > +M: Leo Yang <Leo-Yang@quantatw.com> > +M: Leo Yang <leo.yang.sy0@gmail.com> One email. > +L: linux-hwmon@vger.kernel.org > +S: Odd Fixes Why would we like to have unmaintained driver? Odd fixes is candidate to removal, so shall we accept it and remove immediately? > +F: Documentation/hwmon/ina233.rst > +F: drivers/hwmon/pmbus/ina233.c > + ... > + > + /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */ > + /* read rshunt value (uOhm) */ > + if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0) > + rshunt = INA233_RSHUNT_DEFAULT; > + > + /* read current_lsb value (uA) */ > + if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", ¤t_lsb) < 0) > + current_lsb = INA233_CURRENT_LSB_DEFAULT; > + > + if (!rshunt || !current_lsb) { > + dev_err(&client->dev, "shunt-resistor and current-lsb cannot be zero.\n"); Then properties must have constraints in your schema. > + return -EINVAL; Best regards, Krzysztof
On 1/10/25 00:15, Leo Yang wrote: > Support ina233 driver for Meta Yosemite V4. > Irrelevant. Please drop. The subject wrongly refers to INA232. > Driver for Texas Instruments INA233 Current and Power Monitor > With I2C-, SMBus-, and PMBus-Compatible Interface > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202501092213.X9mbPW5Q-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202501061734.nPNdRKqO-lkp@intel.com/ > Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> Other feedback is on top of feedback from Krzysztof. > --- > Documentation/hwmon/ina233.rst | 77 ++++++++++++++ > MAINTAINERS | 8 ++ > drivers/hwmon/pmbus/Kconfig | 9 ++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/ina233.c | 184 +++++++++++++++++++++++++++++++++ > 5 files changed, 279 insertions(+) > create mode 100644 Documentation/hwmon/ina233.rst > create mode 100644 drivers/hwmon/pmbus/ina233.c > > diff --git a/Documentation/hwmon/ina233.rst b/Documentation/hwmon/ina233.rst > new file mode 100644 > index 000000000000..41537f89bed5 > --- /dev/null > +++ b/Documentation/hwmon/ina233.rst > @@ -0,0 +1,77 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +Kernel driver ina233 > +==================== > + > +Supported chips: > + > + * TI INA233 > + > + Prefix: 'ina233' > + > + * Datasheet > + > + Publicly available at the TI website : https://www.ti.com/lit/ds/symlink/ina233.pdf > + > +Author: > + > + Leo Yang <Leo-Yang@quantatw.com> > + > +Usage Notes > +----------- > + > +The shunt resistor value can be configured by a device tree property; > +see Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for details. > + > + > +Description > +----------- > + > +This driver supports hardware monitoring for TI INA233. > + > +The driver is a client driver to the core PMBus driver. Please see > +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. > + > +The driver provides the following attributes for input voltage: > + > +**in1_input** > + > +**in1_label** > + > +**in1_max** > + > +**in1_max_alarm** > + > +**in1_min** > + > +**in1_min_alarm** > + > +The driver provides the following attributes for shunt voltage: > + > +**in2_input** > + > +**in2_label** > + > +The driver provides the following attributes for output voltage: > + > +**in3_input** > + > +**in3_label** > + > +**in3_alarm** > + > +The driver provides the following attributes for output current: > + > +**curr1_input** > + > +**curr1_label** > + > +**curr1_max** > + > +**curr1_max_alarm** > + > +The driver provides the following attributes for input power: > + > +**power1_input** > + > +**power1_label** > \ No newline at end of file > diff --git a/MAINTAINERS b/MAINTAINERS > index c575de4903db..fde1713dff9d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11226,6 +11226,14 @@ L: linux-fbdev@vger.kernel.org > S: Orphan > F: drivers/video/fbdev/imsttfb.c > > +INA233 HARDWARE MONITOR DRIVER > +M: Leo Yang <Leo-Yang@quantatw.com> > +M: Leo Yang <leo.yang.sy0@gmail.com> > +L: linux-hwmon@vger.kernel.org > +S: Odd Fixes > +F: Documentation/hwmon/ina233.rst > +F: drivers/hwmon/pmbus/ina233.c > + In addition to other comments: No "Odd Fixes" for hwmon drivers, please. Just drop this entry. > INDEX OF FURTHER KERNEL DOCUMENTATION > M: Carlos Bilbao <carlos.bilbao.osdev@gmail.com> > S: Maintained > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > index f6d352841953..55db0ddc94ed 100644 > --- a/drivers/hwmon/pmbus/Kconfig > +++ b/drivers/hwmon/pmbus/Kconfig > @@ -124,6 +124,15 @@ config SENSORS_DPS920AB > This driver can also be built as a module. If so, the module will > be called dps920ab. > > +config SENSORS_INA233 > + tristate "Texas Instruments INA233 and compatibles" > + help > + If you say yes here you get hardware monitoring support for Texas > + Instruments INA233. > + > + This driver can also be built as a module. If so, the module will > + be called ina233. > + > config SENSORS_INSPUR_IPSPS > tristate "INSPUR Power System Power Supply" > help > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > index d00bcc758b97..3c4b06fb939e 100644 > --- a/drivers/hwmon/pmbus/Makefile > +++ b/drivers/hwmon/pmbus/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o > obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o > obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o > obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o > +obj-$(CONFIG_SENSORS_INA233) += ina233.o > obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o > obj-$(CONFIG_SENSORS_IR35221) += ir35221.o > obj-$(CONFIG_SENSORS_IR36021) += ir36021.o > diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c > new file mode 100644 > index 000000000000..7899c3e99eeb > --- /dev/null > +++ b/drivers/hwmon/pmbus/ina233.c > @@ -0,0 +1,184 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Hardware monitoring driver for ina233 > + * > + * Copyright (c) 2025 Leo Yang > + */ > + > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include "pmbus.h" > + > +#define MFR_READ_VSHUNT 0xd1 > +#define MFR_CALIBRATION 0xd4 > + > +#define INA233_CURRENT_LSB_DEFAULT 1000 /* uA */ > +#define INA233_RSHUNT_DEFAULT 2000 /* uOhm */ > + > +#define MAX_M_VAL 32767 > +#define MIN_M_VAL -32768 > + > +static void calculate_coef(int *m, int *R, int power_coef) > +{ > + s64 scaled_m; > + int scale_factor = 0; > + int scale_coef = 1; > + bool is_integer = false; > + > + /* > + * 1000000 from Current_LSB A->uA . > + * scale_coef is for scaling up to minimize rounding errors, > + * If there is no decimal information, No need to scale. s/No/no/ > + */ > + if (1000000 % *m) { > + /* Scaling to keep integer precision */ > + scale_factor = -3; > + scale_coef = 1000; > + } else { > + is_integer = true; > + } > + > + /* > + * Unit Conversion (Current_LSB A->uA) and use scaling(scale_factor) > + * to keep integer precision. > + * Formulae referenced from spec. > + */ > + scaled_m = div_s64(1000000 * scale_coef, *m * power_coef); > + > + /* Maximize while keeping it bounded.*/ > + while (scaled_m > MAX_M_VAL || scaled_m < MIN_M_VAL) { > + scaled_m = div_s64(scaled_m, 10); > + scale_factor++; > + } > + /* Scale up only if fractional part exists. */ > + while (scaled_m * 10 < MAX_M_VAL && scaled_m * 10 > MIN_M_VAL && !is_integer) { > + scaled_m *= 10; > + scale_factor--; > + } > + > + *m = scaled_m; > + *R = scale_factor; > +} > + > +static int ina233_read_word_data(struct i2c_client *client, int page, > + int phase, int reg) > +{ > + int ret; > + > + switch (reg) { > + case PMBUS_VIRT_READ_VMON: > + ret = pmbus_read_word_data(client, 0, 0xff, MFR_READ_VSHUNT); > + > + /* Adjust returned value to match VIN coefficients */ > + /* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */ > + ret = DIV_ROUND_CLOSEST(ret * 25, 12500); > + break; > + default: > + ret = -ENODATA; > + break; > + } > + return ret; > +} > + > +static int ina233_probe(struct i2c_client *client) > +{ > + int ret, m, R; > + u32 rshunt; > + u16 current_lsb; > + u16 calibration; > + struct pmbus_driver_info *info; > + > + info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info), > + GFP_KERNEL); > + if (!info) > + return -ENOMEM; > + > + info->pages = 1; > + info->format[PSC_VOLTAGE_IN] = direct; > + info->format[PSC_VOLTAGE_OUT] = direct; > + info->format[PSC_CURRENT_OUT] = direct; > + info->format[PSC_POWER] = direct; > + info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_INPUT > + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT > + | PMBUS_HAVE_POUT > + | PMBUS_HAVE_VMON | PMBUS_HAVE_STATUS_VMON; > + info->m[PSC_VOLTAGE_IN] = 8; > + info->R[PSC_VOLTAGE_IN] = 2; > + info->m[PSC_VOLTAGE_OUT] = 8; > + info->R[PSC_VOLTAGE_OUT] = 2; > + info->read_word_data = ina233_read_word_data; > + > + /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */ > + /* read rshunt value (uOhm) */ > + if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0) > + rshunt = INA233_RSHUNT_DEFAULT; > + > + /* read current_lsb value (uA) */ > + if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", ¤t_lsb) < 0) > + current_lsb = INA233_CURRENT_LSB_DEFAULT; > + of_property_read_u16() returns -EOVERFLOW if the value provided was too large. This should be checked to avoid situations where the value provided in devicetree is too large. A more higher level question: why not use device_property functions ? > + if (!rshunt || !current_lsb) { > + dev_err(&client->dev, "shunt-resistor and current-lsb cannot be zero.\n"); > + return -EINVAL; return dev_err_probe(...); Please use a consistent term for current LSB in error messages. > + } > + > + /* calculate current coefficient */ > + m = current_lsb; > + calculate_coef(&m, &R, 1); > + info->m[PSC_CURRENT_OUT] = m; > + info->R[PSC_CURRENT_OUT] = R; > + > + /* calculate power coefficient */ > + m = current_lsb; > + calculate_coef(&m, &R, 25); > + info->m[PSC_POWER] = m; > + info->R[PSC_POWER] = R; > + > + /* write MFR_CALIBRATION register, Apply formula from spec with unit scaling. */ > + calibration = div64_u64(5120000000ULL, (u64)rshunt * current_lsb); > + if (calibration > 0x7FFF) { > + dev_err(&client->dev, "Exceeds MFR_CALIBRATION limit, Use a reasonable Current_LSB with Shunt resistor value.\n"); Use dev_err_probe(). The error message is not actionable. Provide the values. Something like "Current_LSB %u too small for shunt resistor value %u" or similar would be more helpful. > + return -EINVAL; > + } > + ret = i2c_smbus_write_word_data(client, MFR_CALIBRATION, calibration); > + if (ret < 0) { > + dev_err(&client->dev, "Unable to write calibration\n"); > + return ret; return dev_err_probe(...); > + } > + > + dev_dbg(&client->dev, "power monitor %s (Rshunt = %u uOhm, Current_LSB = %u uA/bit)\n", > + client->name, rshunt, current_lsb); > + > + return pmbus_do_probe(client, info); > +} > + > +static const struct i2c_device_id ina233_id[] = { > + {"ina233", 0}, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ina233_id); > + > +static const struct of_device_id __maybe_unused ina233_of_match[] = { > + { .compatible = "ti,ina233" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, ina233_of_match); > + > +static struct i2c_driver ina233_driver = { > + .driver = { > + .name = "ina233", > + .of_match_table = of_match_ptr(ina233_of_match), > + }, > + .probe = ina233_probe, > + .id_table = ina233_id, > +}; > + > +module_i2c_driver(ina233_driver); > + > +MODULE_AUTHOR("Leo Yang <Leo-Yang@quantatw.com>"); > +MODULE_DESCRIPTION("PMBus driver for INA233 and compatible chips"); > +MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS("PMBUS");
> Re: [PATCH v2 2/2] hwmon: Add driver for TI INA232 Current and Power Monitor Should the subject be INA233? Le 10/01/2025 à 09:15, Leo Yang a écrit : > Support ina233 driver for Meta Yosemite V4. > > Driver for Texas Instruments INA233 Current and Power Monitor > With I2C-, SMBus-, and PMBus-Compatible Interface > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202501092213.X9mbPW5Q-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202501061734.nPNdRKqO-lkp@intel.com/ > Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> ... > +static void calculate_coef(int *m, int *R, int power_coef) > +{ > + s64 scaled_m; > + int scale_factor = 0; > + int scale_coef = 1; > + bool is_integer = false; Is is_integer really needed? See below. > + > + /* > + * 1000000 from Current_LSB A->uA . > + * scale_coef is for scaling up to minimize rounding errors, > + * If there is no decimal information, No need to scale. > + */ > + if (1000000 % *m) { > + /* Scaling to keep integer precision */ > + scale_factor = -3; > + scale_coef = 1000; > + } else { > + is_integer = true; > + } > + > + /* > + * Unit Conversion (Current_LSB A->uA) and use scaling(scale_factor) > + * to keep integer precision. > + * Formulae referenced from spec. > + */ > + scaled_m = div_s64(1000000 * scale_coef, *m * power_coef); > + > + /* Maximize while keeping it bounded.*/ > + while (scaled_m > MAX_M_VAL || scaled_m < MIN_M_VAL) { > + scaled_m = div_s64(scaled_m, 10); > + scale_factor++; > + } > + /* Scale up only if fractional part exists. */ > + while (scaled_m * 10 < MAX_M_VAL && scaled_m * 10 > MIN_M_VAL && !is_integer) { s/!is_integer/scale_coef != 1/ ? > + scaled_m *= 10; > + scale_factor--; > + } > + > + *m = scaled_m; > + *R = scale_factor; > +} ... > +static const struct i2c_device_id ina233_id[] = { > + {"ina233", 0}, Extra spaces to be consistant with ina233_of_match below? > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ina233_id); > + > +static const struct of_device_id __maybe_unused ina233_of_match[] = { > + { .compatible = "ti,ina233" }, > + { }, No need for an extra comma after a terminator. > +}; > +MODULE_DEVICE_TABLE(of, ina233_of_match); ... CJ
Hi Guenter, On Sat, Jan 11, 2025 at 12:22 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > + > > + /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */ > > + /* read rshunt value (uOhm) */ > > + if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0) > > + rshunt = INA233_RSHUNT_DEFAULT; > > + > > + /* read current_lsb value (uA) */ > > + if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", ¤t_lsb) < 0) > > + current_lsb = INA233_CURRENT_LSB_DEFAULT; > > + > of_property_read_u16() returns -EOVERFLOW if the value provided was too large. > This should be checked to avoid situations where the value provided in devicetree > is too large. > Sorry I have a question, I can't get it to return -EOVERFLOW when I test it I am using the following properties: test16 = /bits/ 16 <0xfffd>; of_property_read_u16 reads 0xfffd test16o = /bits/ 16 <0xfffdd>; of_property_read_u16 reads 0xffdd test16o = <0xfffdd>; of_property_read_u16 reads 0xf test16array = /bits/ 16 <0xfffd 0xfffe>; of_property_read_u16 reads 0xfffd The same result for device_property_read_u16, it seems that a data truncation occurs and none of them return -EOVERFLOW. So maybe there is no need to check EOVERFLOW? Or maybe we could use the minimum and maximum of the binding to indicate the range. Best Regards, Leo Yang
On 1/14/25 00:02, Leo Yang wrote: > Hi Guenter, > > On Sat, Jan 11, 2025 at 12:22 AM Guenter Roeck <linux@roeck-us.net> wrote: >> >>> + >>> + /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */ >>> + /* read rshunt value (uOhm) */ >>> + if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0) >>> + rshunt = INA233_RSHUNT_DEFAULT; >>> + >>> + /* read current_lsb value (uA) */ >>> + if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", ¤t_lsb) < 0) >>> + current_lsb = INA233_CURRENT_LSB_DEFAULT; >>> + >> of_property_read_u16() returns -EOVERFLOW if the value provided was too large. >> This should be checked to avoid situations where the value provided in devicetree >> is too large. >> > Sorry I have a question, I can't get it to return -EOVERFLOW when I test it > I am using the following properties: > test16 = /bits/ 16 <0xfffd>; > of_property_read_u16 reads 0xfffd > > test16o = /bits/ 16 <0xfffdd>; > of_property_read_u16 reads 0xffdd > > test16o = <0xfffdd>; > of_property_read_u16 reads 0xf > > test16array = /bits/ 16 <0xfffd 0xfffe>; > of_property_read_u16 reads 0xfffd > > The same result for device_property_read_u16, it seems that a data > truncation occurs and none of them return -EOVERFLOW. > > So maybe there is no need to check EOVERFLOW? > Or maybe we could use the minimum and maximum of the binding to > indicate the range. > of_property_read_u16() calls of_property_read_u16_array(). The API documentation of of_property_read_u16_array() lists -ENODATA and -EOVERFLOW as possible error return values, in addition to -EINVAL. Ignoring those errors is not a good idea, even if it is not immediately obvious how to reproduce them. Guenter
diff --git a/Documentation/hwmon/ina233.rst b/Documentation/hwmon/ina233.rst new file mode 100644 index 000000000000..41537f89bed5 --- /dev/null +++ b/Documentation/hwmon/ina233.rst @@ -0,0 +1,77 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Kernel driver ina233 +==================== + +Supported chips: + + * TI INA233 + + Prefix: 'ina233' + + * Datasheet + + Publicly available at the TI website : https://www.ti.com/lit/ds/symlink/ina233.pdf + +Author: + + Leo Yang <Leo-Yang@quantatw.com> + +Usage Notes +----------- + +The shunt resistor value can be configured by a device tree property; +see Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml for details. + + +Description +----------- + +This driver supports hardware monitoring for TI INA233. + +The driver is a client driver to the core PMBus driver. Please see +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. + +The driver provides the following attributes for input voltage: + +**in1_input** + +**in1_label** + +**in1_max** + +**in1_max_alarm** + +**in1_min** + +**in1_min_alarm** + +The driver provides the following attributes for shunt voltage: + +**in2_input** + +**in2_label** + +The driver provides the following attributes for output voltage: + +**in3_input** + +**in3_label** + +**in3_alarm** + +The driver provides the following attributes for output current: + +**curr1_input** + +**curr1_label** + +**curr1_max** + +**curr1_max_alarm** + +The driver provides the following attributes for input power: + +**power1_input** + +**power1_label** \ No newline at end of file diff --git a/MAINTAINERS b/MAINTAINERS index c575de4903db..fde1713dff9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11226,6 +11226,14 @@ L: linux-fbdev@vger.kernel.org S: Orphan F: drivers/video/fbdev/imsttfb.c +INA233 HARDWARE MONITOR DRIVER +M: Leo Yang <Leo-Yang@quantatw.com> +M: Leo Yang <leo.yang.sy0@gmail.com> +L: linux-hwmon@vger.kernel.org +S: Odd Fixes +F: Documentation/hwmon/ina233.rst +F: drivers/hwmon/pmbus/ina233.c + INDEX OF FURTHER KERNEL DOCUMENTATION M: Carlos Bilbao <carlos.bilbao.osdev@gmail.com> S: Maintained diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index f6d352841953..55db0ddc94ed 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -124,6 +124,15 @@ config SENSORS_DPS920AB This driver can also be built as a module. If so, the module will be called dps920ab. +config SENSORS_INA233 + tristate "Texas Instruments INA233 and compatibles" + help + If you say yes here you get hardware monitoring support for Texas + Instruments INA233. + + This driver can also be built as a module. If so, the module will + be called ina233. + config SENSORS_INSPUR_IPSPS tristate "INSPUR Power System Power Supply" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index d00bcc758b97..3c4b06fb939e 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o +obj-$(CONFIG_SENSORS_INA233) += ina233.o obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o obj-$(CONFIG_SENSORS_IR35221) += ir35221.o obj-$(CONFIG_SENSORS_IR36021) += ir36021.o diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c new file mode 100644 index 000000000000..7899c3e99eeb --- /dev/null +++ b/drivers/hwmon/pmbus/ina233.c @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Hardware monitoring driver for ina233 + * + * Copyright (c) 2025 Leo Yang + */ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include "pmbus.h" + +#define MFR_READ_VSHUNT 0xd1 +#define MFR_CALIBRATION 0xd4 + +#define INA233_CURRENT_LSB_DEFAULT 1000 /* uA */ +#define INA233_RSHUNT_DEFAULT 2000 /* uOhm */ + +#define MAX_M_VAL 32767 +#define MIN_M_VAL -32768 + +static void calculate_coef(int *m, int *R, int power_coef) +{ + s64 scaled_m; + int scale_factor = 0; + int scale_coef = 1; + bool is_integer = false; + + /* + * 1000000 from Current_LSB A->uA . + * scale_coef is for scaling up to minimize rounding errors, + * If there is no decimal information, No need to scale. + */ + if (1000000 % *m) { + /* Scaling to keep integer precision */ + scale_factor = -3; + scale_coef = 1000; + } else { + is_integer = true; + } + + /* + * Unit Conversion (Current_LSB A->uA) and use scaling(scale_factor) + * to keep integer precision. + * Formulae referenced from spec. + */ + scaled_m = div_s64(1000000 * scale_coef, *m * power_coef); + + /* Maximize while keeping it bounded.*/ + while (scaled_m > MAX_M_VAL || scaled_m < MIN_M_VAL) { + scaled_m = div_s64(scaled_m, 10); + scale_factor++; + } + /* Scale up only if fractional part exists. */ + while (scaled_m * 10 < MAX_M_VAL && scaled_m * 10 > MIN_M_VAL && !is_integer) { + scaled_m *= 10; + scale_factor--; + } + + *m = scaled_m; + *R = scale_factor; +} + +static int ina233_read_word_data(struct i2c_client *client, int page, + int phase, int reg) +{ + int ret; + + switch (reg) { + case PMBUS_VIRT_READ_VMON: + ret = pmbus_read_word_data(client, 0, 0xff, MFR_READ_VSHUNT); + + /* Adjust returned value to match VIN coefficients */ + /* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */ + ret = DIV_ROUND_CLOSEST(ret * 25, 12500); + break; + default: + ret = -ENODATA; + break; + } + return ret; +} + +static int ina233_probe(struct i2c_client *client) +{ + int ret, m, R; + u32 rshunt; + u16 current_lsb; + u16 calibration; + struct pmbus_driver_info *info; + + info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info), + GFP_KERNEL); + if (!info) + return -ENOMEM; + + info->pages = 1; + info->format[PSC_VOLTAGE_IN] = direct; + info->format[PSC_VOLTAGE_OUT] = direct; + info->format[PSC_CURRENT_OUT] = direct; + info->format[PSC_POWER] = direct; + info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_INPUT + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT + | PMBUS_HAVE_POUT + | PMBUS_HAVE_VMON | PMBUS_HAVE_STATUS_VMON; + info->m[PSC_VOLTAGE_IN] = 8; + info->R[PSC_VOLTAGE_IN] = 2; + info->m[PSC_VOLTAGE_OUT] = 8; + info->R[PSC_VOLTAGE_OUT] = 2; + info->read_word_data = ina233_read_word_data; + + /* If INA233 skips current/power, shunt-resistor and current-lsb aren't needed. */ + /* read rshunt value (uOhm) */ + if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &rshunt) < 0) + rshunt = INA233_RSHUNT_DEFAULT; + + /* read current_lsb value (uA) */ + if (of_property_read_u16(client->dev.of_node, "ti,current-lsb", ¤t_lsb) < 0) + current_lsb = INA233_CURRENT_LSB_DEFAULT; + + if (!rshunt || !current_lsb) { + dev_err(&client->dev, "shunt-resistor and current-lsb cannot be zero.\n"); + return -EINVAL; + } + + /* calculate current coefficient */ + m = current_lsb; + calculate_coef(&m, &R, 1); + info->m[PSC_CURRENT_OUT] = m; + info->R[PSC_CURRENT_OUT] = R; + + /* calculate power coefficient */ + m = current_lsb; + calculate_coef(&m, &R, 25); + info->m[PSC_POWER] = m; + info->R[PSC_POWER] = R; + + /* write MFR_CALIBRATION register, Apply formula from spec with unit scaling. */ + calibration = div64_u64(5120000000ULL, (u64)rshunt * current_lsb); + if (calibration > 0x7FFF) { + dev_err(&client->dev, "Exceeds MFR_CALIBRATION limit, Use a reasonable Current_LSB with Shunt resistor value.\n"); + return -EINVAL; + } + ret = i2c_smbus_write_word_data(client, MFR_CALIBRATION, calibration); + if (ret < 0) { + dev_err(&client->dev, "Unable to write calibration\n"); + return ret; + } + + dev_dbg(&client->dev, "power monitor %s (Rshunt = %u uOhm, Current_LSB = %u uA/bit)\n", + client->name, rshunt, current_lsb); + + return pmbus_do_probe(client, info); +} + +static const struct i2c_device_id ina233_id[] = { + {"ina233", 0}, + { } +}; +MODULE_DEVICE_TABLE(i2c, ina233_id); + +static const struct of_device_id __maybe_unused ina233_of_match[] = { + { .compatible = "ti,ina233" }, + { }, +}; +MODULE_DEVICE_TABLE(of, ina233_of_match); + +static struct i2c_driver ina233_driver = { + .driver = { + .name = "ina233", + .of_match_table = of_match_ptr(ina233_of_match), + }, + .probe = ina233_probe, + .id_table = ina233_id, +}; + +module_i2c_driver(ina233_driver); + +MODULE_AUTHOR("Leo Yang <Leo-Yang@quantatw.com>"); +MODULE_DESCRIPTION("PMBus driver for INA233 and compatible chips"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("PMBUS");
Support ina233 driver for Meta Yosemite V4. Driver for Texas Instruments INA233 Current and Power Monitor With I2C-, SMBus-, and PMBus-Compatible Interface Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202501092213.X9mbPW5Q-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202501061734.nPNdRKqO-lkp@intel.com/ Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> --- Documentation/hwmon/ina233.rst | 77 ++++++++++++++ MAINTAINERS | 8 ++ drivers/hwmon/pmbus/Kconfig | 9 ++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/ina233.c | 184 +++++++++++++++++++++++++++++++++ 5 files changed, 279 insertions(+) create mode 100644 Documentation/hwmon/ina233.rst create mode 100644 drivers/hwmon/pmbus/ina233.c