[PATCH v4 0/2] hwmon: Add support for INA233

[PATCH v4 0/2] hwmon: Add support for INA233

[Leo Yang]

Support ina233 driver with binding documents.
---
Change in v4:   
  1. Change ti,current-lsb-microamp to
     ti,maximum-expected-current-microamp.
  - Link to v3: https://lore.kernel.org/linux-hwmon/20250115015519.950795-1-leo.yang.sy0@gmail.com/
---
Change in v3:   
  1. Modify the content of MAINTAINER file.
  2. Modify the content of binding.
  3. Current_LSB is an unsigned integer, so coefficient m cannot be
     negative after calculation, simplifying the calculation step. 
  4. Current_LSB uses microamp unit suffix, change code to read_u32.
  5. Some typo fixes.
  - Link to v2: https://lore.kernel.org/linux-hwmon/20250110081546.61667-1-Leo-Yang@quantatw.com/
---
Change in v2:   
  1. Merge the binding files into ti,ina2xx.yaml. 
  2. Fix typo and non-sense dev_err.
  3. Improve program logic.
  4. pmbus_driver_info changed to be allocated in probe.
  - Link to v1: https://lore.kernel.org/linux-hwmon/20250106071337.3017926-1-Leo-Yang@quantatw.com/
---
Change in v1:     
  1. Change to pmbus standard practice of getting MFR_READ_VSHUNT
     through vmon.
  2. Change the way MFR_CALIBRATION is hard-coded to be obtained by
     calculation.
  - Link to origin: https://lore.kernel.org/linux-hwmon/20230920054739.1561080-1-Delphine_CC_Chiu@wiwynn.com/
---

Leo Yang (2):
  dt-bindings: hwmon: ti,ina2xx: Add INA233 device
  hwmon: Add driver for TI INA233 Current and Power Monitor

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml  |  30 +++
 Documentation/hwmon/ina233.rst                |  75 +++++++
 MAINTAINERS                                   |   7 +
 drivers/hwmon/pmbus/Kconfig                   |   9 +
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/ina233.c                  | 191 ++++++++++++++++++
 6 files changed, 313 insertions(+)
 create mode 100644 Documentation/hwmon/ina233.rst
 create mode 100644 drivers/hwmon/pmbus/ina233.c

-- 
2.39.2

[PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA233 device

[Leo Yang]

Add TI INA233 Current and Power Monitor bindings.

Signed-off-by: Leo Yang <leo.yang.sy0@gmail.com>
---
 .../devicetree/bindings/hwmon/ti,ina2xx.yaml  | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index 05a9cb36cd82..7372e282765b 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -27,6 +27,7 @@ properties:
       - ti,ina226
       - ti,ina230
       - ti,ina231
+      - ti,ina233
       - ti,ina237
       - ti,ina238
       - ti,ina260
@@ -75,12 +76,41 @@ properties:
       the alert polarity to active-high.
     $ref: /schemas/types.yaml#/definitions/flag
 
+  ti,maximum-expected-current-microamp:
+    description: |
+      This value indicates the maximum current in microamps that you can
+      expect to measure with ina233 in your circuit.
+
+      This value will be used to calculate the Current_LSB and current/power
+      coefficient for the pmbus and to calibrate the IC.
+    minimum: 32768
+    maximum: 4294967295
+    default: 32768000
+
 required:
   - compatible
   - reg
 
 allOf:
   - $ref: hwmon-common.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - silergy,sy24655
+              - ti,ina209
+              - ti,ina219
+              - ti,ina220
+              - ti,ina226
+              - ti,ina230
+              - ti,ina231
+              - ti,ina237
+              - ti,ina238
+              - ti,ina260
+    then:
+      properties:
+        ti,maximum-expected-current-microamp: false
 
 unevaluatedProperties: false
 
-- 
2.39.2

[PATCH v4 2/2] hwmon: Add driver for TI INA233 Current and Power Monitor

[Leo Yang]

Driver for Texas Instruments INA233 Current and Power Monitor
With I2C-, SMBus-, and PMBus-Compatible Interface

Signed-off-by: Leo Yang <leo.yang.sy0@gmail.com>
---
 Documentation/hwmon/ina233.rst |  75 +++++++++++++
 MAINTAINERS                    |   7 ++
 drivers/hwmon/pmbus/Kconfig    |   9 ++
 drivers/hwmon/pmbus/Makefile   |   1 +
 drivers/hwmon/pmbus/ina233.c   | 191 +++++++++++++++++++++++++++++++++
 5 files changed, 283 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..42323162e6db
--- /dev/null
+++ b/Documentation/hwmon/ina233.rst
@@ -0,0 +1,75 @@
+.. 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.sy0@gmail.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**
diff --git a/MAINTAINERS b/MAINTAINERS
index c575de4903db..ae67eeebd80e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11226,6 +11226,13 @@ L:	linux-fbdev@vger.kernel.org
 S:	Orphan
 F:	drivers/video/fbdev/imsttfb.c
 
+INA233 HARDWARE MONITOR DRIVERS
+M:	Leo Yang <leo.yang.sy0@gmail.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+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..dde1e1678394
--- /dev/null
+++ b/drivers/hwmon/pmbus/ina233.c
@@ -0,0 +1,191 @@
+// 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_MAX_CURRENT_DEFAULT	32768000 /* uA */
+#define INA233_RSHUNT_DEFAULT		2000 /* uOhm */
+
+#define MAX_M_VAL 32767
+
+static void calculate_coef(int *m, int *R, u32 current_lsb, int power_coef)
+{
+	u64 scaled_m;
+	int scale_factor = 0;
+	int scale_coef = 1;
+
+	/*
+	 * 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 % current_lsb) {
+		/* Scaling to keep integer precision */
+		scale_factor = -3;
+		scale_coef = 1000;
+	}
+
+	/*
+	 * Unit Conversion (Current_LSB A->uA) and use scaling(scale_factor)
+	 * to keep integer precision.
+	 * Formulae referenced from spec.
+	 */
+	scaled_m = div64_u64(1000000 * scale_coef, (u64)current_lsb * power_coef);
+
+	/* Maximize while keeping it bounded.*/
+	while (scaled_m > MAX_M_VAL) {
+		scaled_m = div_u64(scaled_m, 10);
+		scale_factor++;
+	}
+	/* Scale up only if fractional part exists. */
+	while (scaled_m * 10 < MAX_M_VAL && scale_coef != 1) {
+		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)
+{
+	struct device *dev = &client->dev;
+	int ret, m, R;
+	u32 rshunt;
+	u32 max_current;
+	u32 current_lsb;
+	u16 calibration;
+	struct pmbus_driver_info *info;
+
+	info = devm_kzalloc(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) */
+	ret = device_property_read_u32(dev, "shunt-resistor", &rshunt);
+	if (ret) {
+		if (ret != -EINVAL)
+			return dev_err_probe(dev, ret, "Shunt resistor property read fail.\n");
+		rshunt = INA233_RSHUNT_DEFAULT;
+	}
+	if (!rshunt)
+		return dev_err_probe(dev, -EINVAL,
+				     "Shunt resistor cannot be zero.\n");
+
+	/* read Maximum expected current value (uA) */
+	ret = device_property_read_u32(dev, "ti,maximum-expected-current-microamp", &max_current);
+	if (ret) {
+		if (ret != -EINVAL)
+			return dev_err_probe(dev, ret,
+					     "Maximum expected current property read fail.\n");
+		max_current = INA233_MAX_CURRENT_DEFAULT;
+	}
+	if (max_current < 32768)
+		return dev_err_probe(dev, -EINVAL,
+				     "Maximum expected current cannot less then 32768.\n");
+
+	/* Calculate Current_LSB according to the spec formula */
+	current_lsb = max_current / 32768;
+
+	/* calculate current coefficient */
+	calculate_coef(&m, &R, current_lsb, 1);
+	info->m[PSC_CURRENT_OUT] = m;
+	info->R[PSC_CURRENT_OUT] = R;
+
+	/* calculate power coefficient */
+	calculate_coef(&m, &R, current_lsb, 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)
+		return dev_err_probe(dev, -EINVAL,
+				     "Product of Current_LSB %u and shunt resistor %u too small, MFR_CALIBRATION reg exceeds 0x7FFF.\n",
+				     current_lsb, rshunt);
+	ret = i2c_smbus_write_word_data(client, MFR_CALIBRATION, calibration);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Unable to write calibration.\n");
+
+	dev_dbg(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.sy0@gmail.com>");
+MODULE_DESCRIPTION("PMBus driver for INA233 and compatible chips");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");
-- 
2.39.2

Re: [PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA233 device

[Krzysztof Kozlowski]

On Thu, Jan 16, 2025 at 04:59:40PM +0800, Leo Yang wrote:
> Add TI INA233 Current and Power Monitor bindings.
> 
> Signed-off-by: Leo Yang <leo.yang.sy0@gmail.com>
> ---
>  .../devicetree/bindings/hwmon/ti,ina2xx.yaml  | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> index 05a9cb36cd82..7372e282765b 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
> @@ -27,6 +27,7 @@ properties:
>        - ti,ina226
>        - ti,ina230
>        - ti,ina231
> +      - ti,ina233
>        - ti,ina237
>        - ti,ina238
>        - ti,ina260
> @@ -75,12 +76,41 @@ properties:
>        the alert polarity to active-high.
>      $ref: /schemas/types.yaml#/definitions/flag
>  
> +  ti,maximum-expected-current-microamp:
> +    description: |
> +      This value indicates the maximum current in microamps that you can
> +      expect to measure with ina233 in your circuit.
> +
> +      This value will be used to calculate the Current_LSB and current/power
> +      coefficient for the pmbus and to calibrate the IC.
> +    minimum: 32768
> +    maximum: 4294967295

Uh, are these real values measurable by the device? The last one looks
like UINT_MAX.

> +    default: 32768000

Default is 32 A? For what applications is this sensor used?

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA233 device

[Leo Yang]

Hi Krzysztof,

On Thu, Jan 16, 2025 at 6:47 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > +      This value will be used to calculate the Current_LSB and current/power
> > +      coefficient for the pmbus and to calibrate the IC.
> > +    minimum: 32768
> > +    maximum: 4294967295
>
> Uh, are these real values measurable by the device? The last one looks
> like UINT_MAX.

According to the spec I don't see a definition of the upper limit of the
current measurement, it all depends on how low the shunt resistance can
be, so I'll use the upper limit of the u32 as the maximum for now, even
though it's unlikely that this number will be present in the actual circuit.

>
> > +    default: 32768000
>
> Default is 32 A? For what applications is this sensor used?
>

According to spec 8.2.2.1 Programming the Calibration Register example,
a Current_LSB with a maximum expected current of 15A is approximately
457.7uA.
The example shows that a Current_LSB of 500 or 1000uA/bit can be used.
So I choose 1000uA as the default value here, this value corresponds to
the expected maximum current which is 32A (with some loss of accuracy to
 have a larger measurement range), and yes maybe the user doesn't
need such a large current, so the accuracy-sensitive use of the scene
can be adjusted according to the actual measurement range of the
expected maximum current, I'm trying to retain some flexibility for the
user.


Thank you for all your suggestions, they are very useful and
I have gained a lot from them.


Best Regards,

Leo Yang

Re: [PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA233 device

[Krzysztof Kozlowski]

On Thu, Jan 16, 2025 at 09:52:08PM +0800, Leo Yang wrote:
> Hi Krzysztof,
> 
> On Thu, Jan 16, 2025 at 6:47 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > > +      This value will be used to calculate the Current_LSB and current/power
> > > +      coefficient for the pmbus and to calibrate the IC.
> > > +    minimum: 32768
> > > +    maximum: 4294967295
> >
> > Uh, are these real values measurable by the device? The last one looks
> > like UINT_MAX.
> 
> According to the spec I don't see a definition of the upper limit of the
> current measurement, it all depends on how low the shunt resistance can
> be, so I'll use the upper limit of the u32 as the maximum for now, even
> though it's unlikely that this number will be present in the actual circuit.
> 
> >
> > > +    default: 32768000
> >
> > Default is 32 A? For what applications is this sensor used?
> >
> 
> According to spec 8.2.2.1 Programming the Calibration Register example,
> a Current_LSB with a maximum expected current of 15A is approximately
> 457.7uA.
> The example shows that a Current_LSB of 500 or 1000uA/bit can be used.
> So I choose 1000uA as the default value here, this value corresponds to
> the expected maximum current which is 32A (with some loss of accuracy to
>  have a larger measurement range), and yes maybe the user doesn't
> need such a large current, so the accuracy-sensitive use of the scene
> can be adjusted according to the actual measurement range of the
> expected maximum current, I'm trying to retain some flexibility for the
> user.
>

Datasheet indeed does not describe actual limits, expect 5 mA on the
pin, but that depends on shunt resistor, so fine with me:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add INA233 device

[Guenter Roeck]

On Thu, Jan 16, 2025 at 04:59:40PM +0800, Leo Yang wrote:
> Add TI INA233 Current and Power Monitor bindings.
> 
> Signed-off-by: Leo Yang <leo.yang.sy0@gmail.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied.

Thanks,
Guenter

Re: [PATCH v4 2/2] hwmon: Add driver for TI INA233 Current and Power Monitor

[Guenter Roeck]

On Thu, Jan 16, 2025 at 04:59:42PM +0800, Leo Yang wrote:
> Driver for Texas Instruments INA233 Current and Power Monitor
> With I2C-, SMBus-, and PMBus-Compatible Interface
> 
> Signed-off-by: Leo Yang <leo.yang.sy0@gmail.com>

Applied.

Thanks,
Guenter