Support ROHM BD127x0 hot-swap controllers

Support ROHM BD12780(A) and BD12790

The BD12780 and BD12780A hot-swap controllers are very similar to Analog Devices ADM1278. There are only some minor differences in the registers.

The BD12790 is largely similar to the ADM1272, with slightly different coefficients and minor register changes.

This series adds basic support for these ROHM ICs.

Additionally, this series contains couple of fixes, which can be applied independently from the bd127x0 support patches. Fixes are:

Patch 2/9: Prevent read from uninitialized stack (found by Sashiko) Patch 3/9: Prevent coefficient overflow with larget shunt resistor (found
by Sashiko)
Patch 4/9: Support module auto-loading when DT is used.

Revision history:
v1 => v2:
dt-bindings:
- Fix compatible list as suggested by Krzysztof
adm1275:
- Add patches to fix issues pointed by Sashiko.
- Add of_device_ids for all supported devices.
- Add own switch 'case's for the new ICs instead of overloading existing ones
- Drop i2c_device_id for bd12780a
- Don't use IC-wildcard in macro name.

---

Matti Vaittinen (9):
dt-bindings: adm1275: ROHM BD12780 hot-swap controller
hwmon: adm1275: Prevent reading uninitialized stack
hwmon: adm1275: Detect coefficient overflow
hwmon: adm1275: Support module auto-loading
doc: Add ROHM BD12780 and BD12780A
hwmon: adm1275: Support ROHM BD12780
dt-bindings: adm1275: ROHM BD12790 hot-swap controller
doc: adm1275: Add ROHM BD12790
hwmon: adm1275: Support ROHM BD12790

 .../bindings/hwmon/adi,adm1275.yaml           |  43 +++--
 Documentation/hwmon/adm1275.rst               |  24 +++
 drivers/hwmon/pmbus/Kconfig                   |   4 +-
 drivers/hwmon/pmbus/adm1275.c                 | 162 ++++++++++++++++--
 4 files changed, 208 insertions(+), 25 deletions(-)

base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731

2.54.0

From: Matti Vaittinen <[email protected]>

Support ROHM BD12780 and BD12780A hot-swap controllers, which are largely compatible with the Analog Devices adm1278. Main difference between the BD12780 and the BD12780A is, that the BD12780 has one I2C address configuration pin more (ADDR3) than the BD12780A.

Introduce own compatibles for both variants but require the BD12780A to always have the BD12780 as a fall-back.

---
Revision history:
v1 => v2:
- Drop extra -items from the compatible list as suggested by Krzysztof

---
 .../bindings/hwmon/adi,adm1275.yaml           | 38 +++++++++++++------
 1 file changed, 27 insertions(+), 11 deletions(-)

index d6a7517f2a50..503e93756584 100644
@@ -25,19 +25,34 @@ description: |
     https://www.silergy.com/
     download/downloadFile?id=5669&type=product&ftype=note
 
+  The BD12780 and BD12780A are hot-swap controllers from ROHM. They are
+  functionally compatible with the ADM1278. The main difference between
+  the BD12780A and the BD12780 is amount of configurable I2C addresses.
+
+  Datasheets:
+    https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf
+    https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf
+
 properties:
   compatible:
-    enum:
-      - adi,adm1075
-      - adi,adm1272
-      - adi,adm1273
-      - adi,adm1275
-      - adi,adm1276
-      - adi,adm1278
-      - adi,adm1281
-      - adi,adm1293
-      - adi,adm1294
-      - silergy,mc09c
+    oneOf:
+      - enum:
+          - adi,adm1075
+          - adi,adm1272
+          - adi,adm1273
+          - adi,adm1275
+          - adi,adm1276
+          - adi,adm1278
+          - adi,adm1281
+          - adi,adm1293
+          - adi,adm1294
+          - rohm,bd12780
+          - silergy,mc09c
+
+      # Require BD12780 as a fall-back for BD12780A.
+      - items:
+          - const: rohm,bd12780a
+          - const: rohm,bd12780
 
   reg:
     maxItems: 1

@@ -104,6 +119,7 @@ allOf:
               - adi,adm1281
               - adi,adm1293
               - adi,adm1294
+              - rohm,bd12780
               - silergy,mc09c
     then:
       properties:

2.54.0

pw-bot: not-applicable

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

While adding support for the ROHM BD127X0 hot-swap controllers, sashiko reported an error in device-name comparison, which can lead to reading uninitialized stack memory.

Quoting Sashiko:

This is a pre-existing issue, but I noticed that just before this block in adm1275_probe(), there might be an out-of-bounds stack read:

    ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
    if (ret < 0) { ... }
    for (mid = adm1275_id; mid->name[0]; mid++) {
            if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
                    break;
    }

Since i2c_smbus_read_block_data() reads up to 32 bytes into the uninitialized stack array block_buffer without appending a null terminator, strncasecmp() could read past the valid bytes returned in ret.

For example, if the device returns a shorter string like "adm12", checking it against "adm1275" up to the length of "adm1275" will continue reading into uninitialized stack bounds.

Prevent reading uninitialized memory by zeroing the stack array.

---
Revision history:
v1 => v2:
- New patch

---
 drivers/hwmon/pmbus/adm1275.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index bc2a6a07dc3e..43baa5ded35e 100644
@@ -512,7 +512,7 @@ static int adm1275_enable_vout_temp(struct adm1275_data *data,
 static int adm1275_probe(struct i2c_client *client)
 {
 	s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
-	u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
+	u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1] = {0};
 	int config, device_config;
 	int ret;
 	struct pmbus_driver_info *info;

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Sashiko detected potential coefficient overflow if large shunt resistor is used. When going unnoticed it can cause "drastically incorrect telemetry scaling factors" as Sashiko put it.

I am not convinced such "drastically incorrect telemetry scaling factors" could have gone unnoticed, so I suspect such large shunt resistors aren't really used. Well, it shouldn't hurt to detect the error and abort the probe before Really Wrong current / power -values are reported to user by the hwmon.

---
Revision history:
v1 => v2:
- New patch

This patch returns -EOVERFLOW with an error print if overflow is detected. IF there really are systems where the overflow truly occurs, then this change will cause the probe to fail - which might hurt the boot process. It might be safer to only print the warning. One could also try changing the order of the shunt resistor value division (/1000) and the multiplication and see if overflow goes away - but it'll be somewhat more complex then. Hence, I just decided to error-out if this happens, and leave this for the people facing the real overflow to fix (if needed)... It's still fair to mention this might cause issues.

---
 drivers/hwmon/pmbus/adm1275.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

index 43baa5ded35e..ccc3ad21e38e 100644
@@ -839,15 +839,25 @@ static int adm1275_probe(struct i2c_client *client)
 		info->R[PSC_VOLTAGE_OUT] = coefficients[voindex].R;
 	}
 	if (cindex >= 0) {
+		u32 m;
+
 		/* Scale current with sense resistor value */
-		info->m[PSC_CURRENT_OUT] =
-			coefficients[cindex].m * shunt / 1000;
+		if (unlikely(check_mul_overflow(coefficients[cindex].m, shunt, &m))) {
+			dev_err(&client->dev, "Current coefficient overflow\n");
+			return -EOVERFLOW;
+		}
+		info->m[PSC_CURRENT_OUT] = m / 1000;
 		info->b[PSC_CURRENT_OUT] = coefficients[cindex].b;
 		info->R[PSC_CURRENT_OUT] = coefficients[cindex].R;
 	}
 	if (pindex >= 0) {
-		info->m[PSC_POWER] =
-			coefficients[pindex].m * shunt / 1000;
+		u32 m;
+
+		if (unlikely(check_mul_overflow(coefficients[pindex].m, shunt, &m))) {
+			dev_err(&client->dev, "Power coefficient overflow\n");
+			return -EOVERFLOW;
+		}
+		info->m[PSC_POWER] = m / 1000;
 		info->b[PSC_POWER] = coefficients[pindex].b;
 		info->R[PSC_POWER] = coefficients[pindex].R;
 	}

2.54.0

Let's see if it happens.

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Populating the i2c_device_id -table is not enough to make the driver module automatically load when device-tree node for the power-monitor is parsed at boot.

Adding the of_device_id tables causes the driver module to be automatically load at boot. Testing has been done with rather old Debian system.

When inspecting the generated module-aliases with the insmod, following entries seem to be the difference:

alias:          of:N*T*Cadi,adm1075C*
alias:          of:N*T*Cadi,adm1075

I suspect these are required for the module loading to work.

---
Revision history:
v1 => v2:
- New patch as discussed with Guenter here:
https://lore.kernel.org/all/[email protected]/

---
 drivers/hwmon/pmbus/adm1275.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index ccc3ad21e38e..1ea2037711e1 100644
@@ -870,9 +870,25 @@ static int adm1275_probe(struct i2c_client *client)
 	return pmbus_do_probe(client, info);
 }
 
+static const struct of_device_id adm1275_of_match[] = {
+	{ .compatible = "adi,adm1075", },
+	{ .compatible = "adi,adm1272", },
+	{ .compatible = "adi,adm1273", },
+	{ .compatible = "adi,adm1275", },
+	{ .compatible = "adi,adm1276", },
+	{ .compatible = "adi,adm1278", },
+	{ .compatible = "adi,adm1281", },
+	{ .compatible = "adi,adm1293", },
+	{ .compatible = "adi,adm1294", },
+	{ .compatible = "silergy,mc09c", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adm1275_of_match);
+
 static struct i2c_driver adm1275_driver = {
 	.driver = {
 		   .name = "adm1275",
+		   .of_match_table = adm1275_of_match,
 		   },
 	.probe = adm1275_probe,
 	.id_table = adm1275_id,

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Add the ROHM BD12780 and the BD12780A to the list of the ICs supported by the adm1275 driver.

---

Revision history:
v1 => :
- No changes

---
 Documentation/hwmon/adm1275.rst | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index cf923f20fa52..8a793dd2b412 100644
@@ -67,6 +67,22 @@ Supported chips:
 
     Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1293_1294.pdf
 
+  * ROHM Semiconductor BD12780
+
+    Prefix: 'bd12780'
+
+    Addresses scanned: -
+
+    Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf
+
+  * ROHM Semiconductor BD12780A
+
+    Prefix: 'bd12780'
+
+    Addresses scanned: -
+
+    Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf
+
   * Silergy SQ24905C
 
     Prefix: 'mc09c'

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark PMON_CONFIG bits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.

The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".

Support ROHM BD12780 and BD12780A controllers.

---

Revision history:
v1 => v2: Changes based on the review by Guenter:
- Drop i2c_device_id for bd12780a
- Add own 'case' for the bd12780 instead of overloading the existing one and still having an 'if (id == bd12780)' inside the case.

---
 drivers/hwmon/pmbus/Kconfig   |  2 +-
 drivers/hwmon/pmbus/adm1275.c | 55 ++++++++++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 9 deletions(-)

index 8f4bff375ecb..b3c27f3b2712 100644
@@ -52,7 +52,7 @@ config SENSORS_ADM1275
 	help
 	  If you say yes here you get hardware monitoring support for Analog
 	  Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281,
-	  ADM1293, ADM1294 and SQ24905C Hot-Swap Controller and
+	  ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and
 	  Digital Power Monitors.
 
 	  This driver can also be built as a module. If so, the module will

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Support ROHM BD12790 hot-swap controller which is largely compatible with the Analog Devices adm1272.

---

Revision history:
v1 => :
- Fixed double space from description

---
 Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml | 5 +++++
 1 file changed, 5 insertions(+)

index 503e93756584..283cd1662689 100644
@@ -33,6 +33,9 @@ description: |
     https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf
     https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf
 
+  The BD12790 is a ROHM hot-swap controller, functionally similar to the
+  ADM1272.
+
 properties:
   compatible:
     oneOf:

@@ -47,6 +50,7 @@ properties:
           - adi,adm1293
           - adi,adm1294
           - rohm,bd12780
+          - rohm,bd12790
           - silergy,mc09c
 
       # Require BD12780 as a fall-back for BD12780A.

@@ -103,6 +107,7 @@ allOf:
             enum:
               - adi,adm1272
               - adi,adm1273
+              - rohm,bd12790
     then:
       properties:
         adi,volt-curr-sample-average:

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Add the ROHM BD12790 to the list of the ICs supported by the adm1275 driver.

---
Revision history:
v1 => :
- No changes

---
 Documentation/hwmon/adm1275.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

index 8a793dd2b412..d8495be313b8 100644
@@ -83,6 +83,14 @@ Supported chips:
 
     Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf
 
+  * ROHM Semiconductor BD12790
+
+    Prefix: 'bd12790'
+
+    Addresses scanned: -
+
+    Datasheet: -
+
   * Silergy SQ24905C
 
     Prefix: 'mc09c'

2.54.0

Applied.

Thanks,
Guenter

From: Matti Vaittinen <[email protected]>

Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.

The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrors adm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)

---
Revision history:
v1 => v2: Changes mostly based on the review by Guenter:
- Add own 'case' for the bd12790.
- Don't use wildcard in the macro name.
- Fix the coefficient computation comment for power to take the shunt-resistor scale into account.

---
 drivers/hwmon/pmbus/Kconfig   |  4 +-
 drivers/hwmon/pmbus/adm1275.c | 79 +++++++++++++++++++++++++++++++++--
 2 files changed, 78 insertions(+), 5 deletions(-)

index b3c27f3b2712..6ebc01e26db3 100644
@@ -52,8 +52,8 @@ config SENSORS_ADM1275
 	help
 	  If you say yes here you get hardware monitoring support for Analog
 	  Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281,
-	  ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and
-	  Digital Power Monitors.
+	  ADM1293, ADM1294, ROHM BD12780, ROHM BD12790, and SQ24905C
+	  Hot-Swap Controller and Digital Power Monitors.
 
 	  This driver can also be built as a module. If so, the module will
 	  be called adm1275.

2.54.0

Gah. I think Sashiko is right. The fixed comment should say:

m = round(k * 10^(-3-R))

-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Applied, after fixing the comment about BD12790 coefficients as suggested.

Thanks,
Guenter

Thanks for going the extra mile and fixing while applying. I am glad I didn't need to re-spin for this! :)

And it seems gmail is getting more and more useless :( This (and couple of other replies) never emerged in my gmail inbox. So, just as a heads-up - gmail may silently drop some messages when reveiving. :(

Yours,
-- Matti

That is Google / gmail deciding that my e-mails are spam. Happens a lot lately. They actually send me a note about it when they do it, but AFAICS there is nothing I can do to prevent it.

Guenter