Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator
@ 2026-07-20  3:14 Alexis Czezar Torreno
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Alexis Czezar Torreno @ 2026-07-20  3:14 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc,
	Alexis Czezar Torreno, Krzysztof Kozlowski

This series adds a few features before adding support to MAX20830C and
MAX20840C.

Patches 1 and 2 adds new resistor feedback properties that is needed to
properly scale VOUT

Patches 3 and 4 adds support for devices MAX20830C and MAX20840C.

MAX20830C and MAX20840 are step-down DC-DC switching regulator with PMBus
interface. MAX20830C is a different packaging for MAX20830, and MAX20840C
supports 40A regulation compared to MAX20830 that is only 30A.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Changes in v3:
- Dropped patch that add EN-GPIO property in DTS
    - discussed to be skippable until an actual use-case arise
- Patch 1/4: 
    - Carried over the completed examples from previously dropped patch
    - Kept the review tag as the carried info from the dropped patched
      was also reviewed.
- Patch 2/4: 
    - added clamping to read_vout during scaling
    - improved comments regarding future regulator support 
- Link to v2: https://lore.kernel.org/r/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com

Changes in v2:
- Patch 1/5: completing example
- Dropped patch adding GPIO enable in driver (no use case)
- Patch 3/5: added notes on READ_VOUT and related VOUT commands
- Patch 4/5: removed comment on fallback
- Patch 5/5: simplified device ID check handling
- Link to v1: https://lore.kernel.org/r/20260630-dev-max20830c-v1-0-a02786bde470@analog.com

---
Alexis Czezar Torreno (4):
      dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
      hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
      dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
      hwmon: (pmbus/max20830): add support for max20830c and max20840c

 .../bindings/hwmon/pmbus/adi,max20830.yaml         | 28 ++++++-
 Documentation/hwmon/max20830.rst                   | 27 +++++--
 drivers/hwmon/pmbus/max20830.c                     | 86 ++++++++++++++++++----
 3 files changed, 121 insertions(+), 20 deletions(-)
---
base-commit: 1a42625604046790ea8ec61c2a749bbf76b28943
change-id: 20260630-dev-max20830c-9d38d8b0a765

Best regards,
-- 
Alexis Czezar Torreno <alexisczezar.torreno@analog.com>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
  2026-07-20  3:14 [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator Alexis Czezar Torreno
@ 2026-07-20  3:14 ` Alexis Czezar Torreno
  2026-07-20  3:22   ` sashiko-bot
                     ` (2 more replies)
  2026-07-20  3:14 ` [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support Alexis Czezar Torreno
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 18+ messages in thread
From: Alexis Czezar Torreno @ 2026-07-20  3:14 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc,
	Alexis Czezar Torreno, Krzysztof Kozlowski

Add adi,vout-rfb1-ohms and adi,vout-rfb2-ohms properties to support
external voltage divider configuration for VOUT sensing. When the
desired output voltage is higher than VREF, a resistor divider (RFB1
and RFB2) is required to reach the intended value.

The properties use a dependency constraint to ensure both resistors
are specified together, or neither. This prevents misconfiguration
where only one resistor value is provided.

This patch also added missing entries in the examples.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 .../bindings/hwmon/pmbus/adi,max20830.yaml           | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
index 1625dd59417f1b3ca689a9c86ca266da913d1217..214adf1a3b9e9efdeca7db3bd7679c70656e9a7e 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
@@ -44,11 +44,26 @@ properties:
       GPIO connected to the power-good status output pin.
     maxItems: 1
 
+  adi,vout-rfb1-ohms:
+    description:
+      Top feedback resistor (RFB1) value in ohms for VOUT sensing divider.
+      When the desired output voltage is higher than VREF, a resistor divider
+      is required. VOUT = VREF × (1 + RFB1/RFB2)
+
+  adi,vout-rfb2-ohms:
+    description:
+      Bottom feedback resistor (RFB2) value in ohms for VOUT sensing divider.
+      Datasheet recommends that RFB2 does not exceed 2.5kΩ.
+
 required:
   - compatible
   - reg
   - vddh-supply
 
+dependencies:
+  adi,vout-rfb1-ohms: ['adi,vout-rfb2-ohms']
+  adi,vout-rfb2-ohms: ['adi,vout-rfb1-ohms']
+
 unevaluatedProperties: false
 
 examples:
@@ -61,6 +76,11 @@ examples:
             compatible = "adi,max20830";
             reg = <0x30>;
             vddh-supply = <&vddh>;
+            avdd-supply = <&avdd>;
+            ldoin-supply = <&ldoin>;
+            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+            adi,vout-rfb1-ohms = <10000>;
+            adi,vout-rfb2-ohms = <2000>;
         };
     };
 ...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
  2026-07-20  3:14 [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator Alexis Czezar Torreno
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
@ 2026-07-20  3:14 ` Alexis Czezar Torreno
  2026-07-20  3:25   ` sashiko-bot
  2026-07-20  3:14 ` [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support Alexis Czezar Torreno
  2026-07-20  3:14 ` [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c Alexis Czezar Torreno
  3 siblings, 1 reply; 18+ messages in thread
From: Alexis Czezar Torreno @ 2026-07-20  3:14 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc,
	Alexis Czezar Torreno

Implement support for external voltage divider scaling using the
adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree properties.

When the desired output voltage exceeds VREF, a resistor divider
(RFB1 and RFB2) is used to scale down the feedback voltage. The
driver reads these resistor values from device tree and applies
the scaling formula: VOUT_actual = VOUT_measured × (1 + RFB1/RFB2)

The properties are optional. If not specified, the driver assumes
no voltage divider is present and reports the raw VOUT reading.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 drivers/hwmon/pmbus/max20830.c | 59 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
index cb2c23672166d641852199ca07eb716924f4f286..7a5e67bd33c0a8b84511d138ac5f5f48334a0a45 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -7,6 +7,7 @@
 
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/math64.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/string.h>
@@ -14,6 +15,50 @@
 
 #define MAX20830_IC_DEVICE_ID_LENGTH	9
 
+struct max20830_data {
+	struct pmbus_driver_info info;
+	u32 vout_rfb1;
+	u32 vout_rfb2;
+};
+
+/*
+ * MAX20830 only supports READ_VOUT for VOUT monitoring.
+ *
+ * MAX20830 uses an external resistor divider for voltage sensing:
+ * - VOUT_COMMAND and VOUT_MAX set the reference voltage at the feedback pin
+ * - READ_VOUT reports the feedback voltage, which needs to be scaled for actual
+ *   output voltage
+ *
+ * Scaling formula: vout_actual = vout_fb × (1 + RFB1 / RFB2)
+ *
+ * If regulator support is added in the future, some adjustments are needed to
+ * ensure correct feedback voltages are set.
+ */
+static int max20830_read_word_data(struct i2c_client *client, int page,
+				   int phase, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	const struct max20830_data *data = container_of(info, struct max20830_data, info);
+	int ret;
+
+	switch (reg) {
+	case PMBUS_READ_VOUT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+
+		/* Apply voltage divider scaling if resistors are non-zero */
+		if (data->vout_rfb1 && data->vout_rfb2) {
+			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data->vout_rfb1 +
+						    data->vout_rfb2), data->vout_rfb2);
+			ret = clamp_val(ret, 0, 0xFFFF);
+		}
+		return ret;
+	default:
+		return -ENODATA;
+	}
+}
+
 static struct pmbus_driver_info max20830_info = {
 	.pages = 1,
 	.format[PSC_VOLTAGE_IN] = linear,
@@ -24,13 +69,25 @@ static struct pmbus_driver_info max20830_info = {
 		PMBUS_HAVE_TEMP |
 		PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
 		PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
+	.read_word_data = max20830_read_word_data,
 };
 
 static int max20830_probe(struct i2c_client *client)
 {
 	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
+	struct max20830_data *data;
 	int ret;
 
+	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->info = max20830_info;
+
+	/* Read optional voltage divider resistor values */
+	device_property_read_u32(&client->dev, "adi,vout-rfb1-ohms", &data->vout_rfb1);
+	device_property_read_u32(&client->dev, "adi,vout-rfb2-ohms", &data->vout_rfb2);
+
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_BLOCK_DATA) &&
 	    !i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK))
 		return -ENODEV;
@@ -78,7 +135,7 @@ static int max20830_probe(struct i2c_client *client)
 		return dev_err_probe(&client->dev, -ENODEV,
 				     "Unsupported device: '%s'\n", buf);
 
-	return pmbus_do_probe(client, &max20830_info);
+	return pmbus_do_probe(client, &data->info);
 }
 
 static const struct i2c_device_id max20830_id[] = {

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
  2026-07-20  3:14 [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator Alexis Czezar Torreno
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
  2026-07-20  3:14 ` [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support Alexis Czezar Torreno
@ 2026-07-20  3:14 ` Alexis Czezar Torreno
  2026-07-20  3:21   ` sashiko-bot
  2026-07-20  4:38   ` Rob Herring (Arm)
  2026-07-20  3:14 ` [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c Alexis Czezar Torreno
  3 siblings, 2 replies; 18+ messages in thread
From: Alexis Czezar Torreno @ 2026-07-20  3:14 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc,
	Alexis Czezar Torreno, Krzysztof Kozlowski

Add compatible strings for variants of MAX20830 which are MAX20830C
and MAX20840C. These devices have the same register functionality with
MAX20830 but with a longer IC_DEVICE_ID.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
index 214adf1a3b9e9efdeca7db3bd7679c70656e9a7e..b7d3b6152b58273a4c35581bbd66ec293fb2e925 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
@@ -22,7 +22,13 @@ allOf:
 
 properties:
   compatible:
-    const: adi,max20830
+    oneOf:
+      - const: adi,max20830
+      - items:
+          - enum:
+              - adi,max20830c
+              - adi,max20840c
+          - const: adi,max20830
 
   reg:
     maxItems: 1

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c
  2026-07-20  3:14 [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator Alexis Czezar Torreno
                   ` (2 preceding siblings ...)
  2026-07-20  3:14 ` [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support Alexis Czezar Torreno
@ 2026-07-20  3:14 ` Alexis Czezar Torreno
  2026-07-20  3:29   ` sashiko-bot
  3 siblings, 1 reply; 18+ messages in thread
From: Alexis Czezar Torreno @ 2026-07-20  3:14 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc,
	Alexis Czezar Torreno

Add support for MAX20830C and MAX20840 step-down DC-DC switching
regulator with PMBus interface. MAX20830C is a different packaging
for MAX20830, and MAX20840C supports 40A regulation compared to
MAX20830 that is only 30A.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 Documentation/hwmon/max20830.rst | 27 ++++++++++++++++++++++-----
 drivers/hwmon/pmbus/max20830.c   | 27 ++++++++++++++-------------
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/Documentation/hwmon/max20830.rst b/Documentation/hwmon/max20830.rst
index 936e409dcc5c0898dde27d782308d4a7e1357e73..b850f3b6e40d1f1d0cec944be40af02265aced59 100644
--- a/Documentation/hwmon/max20830.rst
+++ b/Documentation/hwmon/max20830.rst
@@ -13,6 +13,22 @@ Supported chips:
 
     Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max20830.pdf
 
+  * Analog Devices MAX20830C
+
+    Prefix: 'max20830c'
+
+    Addresses scanned: -
+
+    Datasheet:
+
+  * Analog Devices MAX20840C
+
+    Prefix: 'max20840c'
+
+    Addresses scanned: -
+
+    Datasheet:
+
 Author:
 
   - Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
@@ -21,12 +37,13 @@ Author:
 Description
 -----------
 
-This driver supports hardware monitoring for Analog Devices MAX20830
-Step-Down Switching Regulator with PMBus Interface.
+This driver supports hardware monitoring for Analog Devices MAX20830, MAX20830C
+and MAX20840C. These are Step-Down Switching Regulator with PMBus Interface.
 
-The MAX20830 is a 2.7V to 16V, 30A fully integrated step-down DC-DC switching
-regulator. Through the PMBus interface, the device can monitor input/output
-voltages, output current and temperature.
+MAX20830, and MAX20830C are 2.7V to 16V, 30A fully integrated step-down DC-DC
+switching regulators. MAX20840C is similar but can reach 40A. Through the PMBus
+interface, these devices can monitor input/output voltages, output current and
+temperature.
 
 The driver is a client driver to the core PMBus driver. Please see
 Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
index 7a5e67bd33c0a8b84511d138ac5f5f48334a0a45..73fe59685cf157cc27c1011ad9cc045486bbeab0 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -98,13 +98,12 @@ static int max20830_probe(struct i2c_client *client)
 	 * which do not support SMBus block reads.
 	 */
 	if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
-		/* Reads 9 Data bytes from MAX20830 */
 		ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
 		if (ret < 0)
 			return dev_err_probe(&client->dev, ret,
 					     "Failed to read IC_DEVICE_ID\n");
 	} else {
-		/* Reads 1 length byte + 9 Data bytes from MAX20830 */
+		/* Reads 1 length byte + data bytes */
 		ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
 						    MAX20830_IC_DEVICE_ID_LENGTH + 1,
 						    buf);
@@ -120,26 +119,28 @@ static int max20830_probe(struct i2c_client *client)
 		ret = ret - 1;
 	}
 
-	/*
-	 * MAX20830 IC_DEVICE_ID sends string data "MAX20830\0".
-	 * Return value should at least be 9 bytes of data.
-	 */
+	/* Verify we read the expected number of bytes */
 	if (ret < MAX20830_IC_DEVICE_ID_LENGTH)
 		return dev_err_probe(&client->dev, -ENODEV,
-				     "IC_DEVICE_ID too short: expected at least 9 bytes, got %d\n",
-				     ret);
+				     "IC_DEVICE_ID too short: expected %d bytes, got %d\n",
+				     MAX20830_IC_DEVICE_ID_LENGTH, ret);
+
+	/* Null-terminate the string */
+	buf[ret] = '\0';
 
-	/* 9 bytes of data, buf[0]-buf[7] = "MAX20830", buf[8] = '\0' */
-	buf[MAX20830_IC_DEVICE_ID_LENGTH - 1] = '\0';
-	if (strncmp(buf, "MAX20830", MAX20830_IC_DEVICE_ID_LENGTH - 1))
+	/* Verify the device ID matches what we expect */
+	if ((strcmp(buf, "MAX20830") && strcmp(buf, "MAX20830C") &&
+	    strcmp(buf, "MAX20840C")))
 		return dev_err_probe(&client->dev, -ENODEV,
-				     "Unsupported device: '%s'\n", buf);
+				     "Unsupported device: '%*pE'\n", ret, buf);
 
 	return pmbus_do_probe(client, &data->info);
 }
 
 static const struct i2c_device_id max20830_id[] = {
-	{"max20830"},
+	{ "max20830" },
+	{ "max20830c" },
+	{ "max20840c" },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max20830_id);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
  2026-07-20  3:14 ` [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support Alexis Czezar Torreno
@ 2026-07-20  3:21   ` sashiko-bot
  2026-07-21  0:44     ` Torreno, Alexis Czezar
  2026-07-20  4:38   ` Rob Herring (Arm)
  1 sibling, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-20  3:21 UTC (permalink / raw)
  To: Alexis Czezar Torreno; +Cc: linux-hwmon, devicetree, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The max20830 driver will reject the newly added MAX20840C device during probe due to a hardcoded IC_DEVICE_ID check.
--

--- Patch [3]: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support ---
commit b032b17e50351faefc688ad31e1428a3c641197a
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support

This commit adds compatible strings for the MAX20830C and MAX20840C variants
to the max20830 device tree binding, using a fallback to adi,max20830.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> index 214adf1a3b9e9..b7d3b6152b582 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> @@ -22,7 +22,13 @@ allOf:
>  
>  properties:
>    compatible:
> -    const: adi,max20830
> +    oneOf:
> +      - const: adi,max20830
> +      - items:
> +          - enum:
> +              - adi,max20830c
> +              - adi,max20840c

[Severity: Medium]
Will the driver correctly probe for the 'adi,max20840c' variant?

Looking at max20830_probe() in drivers/hwmon/pmbus/max20830.c, there is a
hardcoded string comparison for the device ID:

drivers/hwmon/pmbus/max20830.c:max20830_probe() {
    ...
    if (strncmp(buf, "MAX20830", MAX20830_IC_DEVICE_ID_LENGTH - 1))
        return dev_err_probe(&client->dev, -ENODEV,
                     "Unsupported device: '%s'\n", buf);
    ...
}

If a MAX20840C device is probed via the 'adi,max20830' fallback, it seems the
buffer will start with "MAX20840", causing this check to fail and return
-ENODEV.

Does the driver need to be updated to recognize this new device ID so it does
not reject the device?

> +          - const: adi,max20830

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dev-max20830c-v3-0-9d06d27d6da3@analog.com?part=3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
@ 2026-07-20  3:22   ` sashiko-bot
  2026-07-20  3:28     ` Torreno, Alexis Czezar
  2026-07-20  3:41   ` Torreno, Alexis Czezar
  2026-07-20  4:38   ` Rob Herring (Arm)
  2 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-20  3:22 UTC (permalink / raw)
  To: Alexis Czezar Torreno; +Cc: devicetree, linux-hwmon, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#include <dt-bindings/gpio/gpio.h>` in the examples section causes `make dt_binding_check` to fail.
--

--- Patch [1]: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples ---
commit f18524325452b02a7accd64c49a8b82f9362d94c
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples

This commit adds properties for external voltage divider configuration for
VOUT sensing (adi,vout-rfb1-ohms and adi,vout-rfb2-ohms) to the MAX20830
binding. It also adds missing entries to the example device tree node.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml

[ ... ]

> @@ -61,6 +76,11 @@ examples:
>              compatible = "adi,max20830";
>              reg = <0x30>;
>              vddh-supply = <&vddh>;
> +            avdd-supply = <&avdd>;
> +            ldoin-supply = <&ldoin>;
> +            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Does the missing #include <dt-bindings/gpio/gpio.h> here cause make
dt_binding_check to fail due to the unexpanded GPIO_ACTIVE_HIGH macro?

> +            adi,vout-rfb1-ohms = <10000>;
> +            adi,vout-rfb2-ohms = <2000>;
>          };
>      };
>  ...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dev-max20830c-v3-0-9d06d27d6da3@analog.com?part=1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
  2026-07-20  3:14 ` [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support Alexis Czezar Torreno
@ 2026-07-20  3:25   ` sashiko-bot
  2026-07-21  0:44     ` Torreno, Alexis Czezar
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-20  3:25 UTC (permalink / raw)
  To: Alexis Czezar Torreno; +Cc: linux-hwmon, conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The voltage divider scaling calculation suffers from a 32-bit addition overflow and an unsafe 64-bit to 32-bit signed integer truncation before clamping.
--

--- Patch [2]: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support ---
commit ac88893286286c633f117369db91c2cffd163afd
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

This commit implements support for external voltage divider scaling using
the adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree properties. The
driver reads these values and applies the scaling formula to the raw VOUT
reading.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d..7a5e67bd33c0a 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c

[ ... ]

> +static int max20830_read_word_data(struct i2c_client *client, int page,
> +				   int phase, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	const struct max20830_data *data = container_of(info, struct max20830_data, info);
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_READ_VOUT:
> +		ret = pmbus_read_word_data(client, page, phase, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Apply voltage divider scaling if resistors are non-zero */
> +		if (data->vout_rfb1 && data->vout_rfb2) {
> +			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data->vout_rfb1 +
> +						    data->vout_rfb2), data->vout_rfb2);
> +			ret = clamp_val(ret, 0, 0xFFFF);

[Severity: Medium]
Can this calculation suffer from a 32-bit addition overflow and an unsafe
64-bit to 32-bit signed integer truncation?

If data->vout_rfb1 and data->vout_rfb2 sum to more than 2^32 - 1, the 32-bit
addition will wrap around before the 64-bit multiplication.

Additionally, since ret is a signed 32-bit integer, if the 64-bit result
returned from DIV_ROUND_CLOSEST_ULL exceeds 2^31 - 1, the assignment to ret
will overflow into a negative integer. The clamp_val() macro will then
evaluate this negative number and incorrectly clamp it to 0 instead of
0xFFFF.

> +		}
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dev-max20830c-v3-0-9d06d27d6da3@analog.com?part=2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
  2026-07-20  3:22   ` sashiko-bot
@ 2026-07-20  3:28     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-20  3:28 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org

> 
> > @@ -61,6 +76,11 @@ examples:
> >              compatible = "adi,max20830";
> >              reg = <0x30>;
> >              vddh-supply = <&vddh>;
> > +            avdd-supply = <&avdd>;
> > +            ldoin-supply = <&ldoin>;
> > +            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> 
> [Severity: Low]
> Does the missing #include <dt-bindings/gpio/gpio.h> here cause make
> dt_binding_check to fail due to the unexpanded GPIO_ACTIVE_HIGH macro?
> 

Missed something so basic...
Apologies for even carrying the review tag, I shouldn't do that.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c
  2026-07-20  3:14 ` [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c Alexis Czezar Torreno
@ 2026-07-20  3:29   ` sashiko-bot
  2026-07-21  0:44     ` Torreno, Alexis Czezar
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-20  3:29 UTC (permalink / raw)
  To: Alexis Czezar Torreno; +Cc: robh, linux-hwmon, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing compatible strings for MAX20830C and MAX20840C in the of_match_table.
- [Medium] I2C block read path truncates the device ID for 9-character names, leading to potential false positive matches.
- [Low] struct i2c_device_id array uses positional instead of named initializers.
--

--- Patch [4]: [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c ---
commit dca1bbdb54e3f769bf22d26e97ead67754787a95
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max20830): add support for max20830c and max20840c
    
Add support for MAX20830C and MAX20840 step-down DC-DC switching
regulator with PMBus interface. MAX20830C is a different packaging
for MAX20830, and MAX20840C supports 40A regulation compared to
MAX20830 that is only 30A.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index 7a5e67bd33c0a..73fe59685cf15 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
> @@ -98,13 +98,12 @@ static int max20830_probe(struct i2c_client *client)

[ ... ]

>  	} else {
> -		/* Reads 1 length byte + 9 Data bytes from MAX20830 */
> +		/* Reads 1 length byte + data bytes */
>  		ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
>  						    MAX20830_IC_DEVICE_ID_LENGTH + 1,
>  						    buf);

[Severity: Medium]
Since MAX20830C and MAX20840C have 9-character names, does this hardcoded
MAX20830_IC_DEVICE_ID_LENGTH + 1 truncate the newly added device IDs?

In max20830_probe(), if the I2C controller lacks SMBUS_READ_BLOCK_DATA support,
the fallback i2c_smbus_read_i2c_block_data() call uses
MAX20830_IC_DEVICE_ID_LENGTH. Because this length was initially sized for the
8-character "MAX20830" identifier, could it under-read the new longer device
IDs and cause false positive matches in the I2C fallback path?

[ ... ]

> @@ -120,26 +119,28 @@ static int max20830_probe(struct i2c_client *client)

[ ... ]

>  static const struct i2c_device_id max20830_id[] = {
> -	{"max20830"},
> +	{ "max20830" },
> +	{ "max20830c" },
> +	{ "max20840c" },

[Severity: High]
Are the compatible strings for max20830c and max20840c missing from the
max20830_of_match[] table?

Without adding them alongside the newly added i2c_device_id entries, could
this break module autoloading on systems using Device Tree when the OS attempts
to autoload the driver via the udev modalias?

[Severity: Low]
This isn't a bug, but do these entries violate the I2C subsystem coding style
by using positional initializers?

The I2C client guidelines require initialized arrays of type
struct i2c_device_id to use named initializers (e.g., { .name = "max20830c" }).

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, max20830_id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dev-max20830c-v3-0-9d06d27d6da3@analog.com?part=4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
  2026-07-20  3:22   ` sashiko-bot
@ 2026-07-20  3:41   ` Torreno, Alexis Czezar
  2026-07-20  4:38   ` Rob Herring (Arm)
  2 siblings, 0 replies; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-20  3:41 UTC (permalink / raw)
  To: Torreno, Alexis Czezar, Guenter Roeck, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	Krzysztof Kozlowski



> -----Original Message-----
> From: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Sent: Monday, July 20, 2026 11:15 AM
> To: Guenter Roeck <linux@roeck-us.net>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Shuah Khan
> <skhan@linuxfoundation.org>
> Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-doc@vger.kernel.org; Torreno, Alexis Czezar
> <AlexisCzezar.Torreno@analog.com>; Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com>
> Subject: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT
> feedback resistor properties and complete examples
> 
> Add adi,vout-rfb1-ohms and adi,vout-rfb2-ohms properties to support external
> voltage divider configuration for VOUT sensing. When the desired output
> voltage is higher than VREF, a resistor divider (RFB1 and RFB2) is required to
> reach the intended value.
> 
> The properties use a dependency constraint to ensure both resistors are
> specified together, or neither. This prevents misconfiguration where only one
> resistor value is provided.
> 
> This patch also added missing entries in the examples.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../bindings/hwmon/pmbus/adi,max20830.yaml           | 20
> ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> index
> 1625dd59417f1b3ca689a9c86ca266da913d1217..214adf1a3b9e9efdeca7db3bd
> 7679c70656e9a7e 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> @@ -44,11 +44,26 @@ properties:
>        GPIO connected to the power-good status output pin.
>      maxItems: 1
> 
> +  adi,vout-rfb1-ohms:
> +    description:
> +      Top feedback resistor (RFB1) value in ohms for VOUT sensing divider.
> +      When the desired output voltage is higher than VREF, a resistor divider
> +      is required. VOUT = VREF × (1 + RFB1/RFB2)
> +
> +  adi,vout-rfb2-ohms:
> +    description:
> +      Bottom feedback resistor (RFB2) value in ohms for VOUT sensing divider.
> +      Datasheet recommends that RFB2 does not exceed 2.5kΩ.
> +
>  required:
>    - compatible
>    - reg
>    - vddh-supply
> 
> +dependencies:
> +  adi,vout-rfb1-ohms: ['adi,vout-rfb2-ohms']
> +  adi,vout-rfb2-ohms: ['adi,vout-rfb1-ohms']
> +
>  unevaluatedProperties: false
> 
>  examples:
> @@ -61,6 +76,11 @@ examples:
>              compatible = "adi,max20830";
>              reg = <0x30>;
>              vddh-supply = <&vddh>;
> +            avdd-supply = <&avdd>;
> +            ldoin-supply = <&ldoin>;
> +            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> +            adi,vout-rfb1-ohms = <10000>;
> +            adi,vout-rfb2-ohms = <2000>;
>          };
>      };

Disregard please, I carried over a few info from a dropped patch to combine
with this patch and missed the include for gpio.

Apologies on carrying the review tag, should've removed it even if it was a
small edit.

Regards,
Alexis


>  ...
> 
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples
  2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
  2026-07-20  3:22   ` sashiko-bot
  2026-07-20  3:41   ` Torreno, Alexis Czezar
@ 2026-07-20  4:38   ` Rob Herring (Arm)
  2 siblings, 0 replies; 18+ messages in thread
From: Rob Herring (Arm) @ 2026-07-20  4:38 UTC (permalink / raw)
  To: Alexis Czezar Torreno
  Cc: Shuah Khan, Guenter Roeck, linux-kernel, Krzysztof Kozlowski,
	Krzysztof Kozlowski, linux-hwmon, Conor Dooley, devicetree,
	Jonathan Corbet, linux-doc


On Mon, 20 Jul 2026 11:14:42 +0800, Alexis Czezar Torreno wrote:
> Add adi,vout-rfb1-ohms and adi,vout-rfb2-ohms properties to support
> external voltage divider configuration for VOUT sensing. When the
> desired output voltage is higher than VREF, a resistor divider (RFB1
> and RFB2) is required to reach the intended value.
> 
> The properties use a dependency constraint to ensure both resistors
> are specified together, or neither. This prevents misconfiguration
> where only one resistor value is provided.
> 
> This patch also added missing entries in the examples.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../bindings/hwmon/pmbus/adi,max20830.yaml           | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Lexical error: Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.example.dts:28.43-59 Unexpected 'GPIO_ACTIVE_HIGH'
FATAL ERROR: Syntax error parsing input tree
make[2]: *** [scripts/Makefile.dtbs:140: Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1669: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260720-dev-max20830c-v3-1-9d06d27d6da3@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
  2026-07-20  3:14 ` [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support Alexis Czezar Torreno
  2026-07-20  3:21   ` sashiko-bot
@ 2026-07-20  4:38   ` Rob Herring (Arm)
  1 sibling, 0 replies; 18+ messages in thread
From: Rob Herring (Arm) @ 2026-07-20  4:38 UTC (permalink / raw)
  To: Alexis Czezar Torreno
  Cc: Guenter Roeck, Shuah Khan, linux-hwmon, Krzysztof Kozlowski,
	linux-kernel, Jonathan Corbet, devicetree, Krzysztof Kozlowski,
	linux-doc, Conor Dooley


On Mon, 20 Jul 2026 11:14:44 +0800, Alexis Czezar Torreno wrote:
> Add compatible strings for variants of MAX20830 which are MAX20830C
> and MAX20840C. These devices have the same register functionality with
> MAX20830 but with a longer IC_DEVICE_ID.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:


doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260720-dev-max20830c-v3-3-9d06d27d6da3@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
  2026-07-20  3:21   ` sashiko-bot
@ 2026-07-21  0:44     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-21  0:44 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	conor+dt@kernel.org, robh@kernel.org



> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, July 20, 2026 11:22 AM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org;
> conor+dt@kernel.org; robh@kernel.org
> Subject: Re: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add
> max20830c and max20840c support
> 
> [External]
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
> consider:
> - [Medium] The max20830 driver will reject the newly added MAX20840C
> device during probe due to a hardcoded IC_DEVICE_ID check.
> --
> 
> --- Patch [3]: [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add
> max20830c and max20840c support --- commit
> b032b17e50351faefc688ad31e1428a3c641197a
> Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> 
> dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c
> support
> 
> This commit adds compatible strings for the MAX20830C and MAX20840C
> variants to the max20830 device tree binding, using a fallback to adi,max20830.
> 
> > diff --git
> > a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> > b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> > index 214adf1a3b9e9..b7d3b6152b582 100644
> > --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> > +++
> b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> > @@ -22,7 +22,13 @@ allOf:
> >
> >  properties:
> >    compatible:
> > -    const: adi,max20830
> > +    oneOf:
> > +      - const: adi,max20830
> > +      - items:
> > +          - enum:
> > +              - adi,max20830c
> > +              - adi,max20840c
> 
> [Severity: Medium]
> Will the driver correctly probe for the 'adi,max20840c' variant?
> 
> Looking at max20830_probe() in drivers/hwmon/pmbus/max20830.c, there is a
> hardcoded string comparison for the device ID:
> 

Addressed in patch 4/4. Driver support for new variants is added in the next patch
in this series.

> drivers/hwmon/pmbus/max20830.c:max20830_probe() {
>     ...
>     if (strncmp(buf, "MAX20830", MAX20830_IC_DEVICE_ID_LENGTH - 1))
>         return dev_err_probe(&client->dev, -ENODEV,
>                      "Unsupported device: '%s'\n", buf);
>     ...
> }
> 
> If a MAX20840C device is probed via the 'adi,max20830' fallback, it seems the
> buffer will start with "MAX20840", causing this check to fail and return -
> ENODEV.
> 
> Does the driver need to be updated to recognize this new device ID so it does
> not reject the device?
> 
> > +          - const: adi,max20830
> 
> --
> Sashiko AI review *
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260720-dev-
> max20830c-v3-0-9d06d27d6da3@analog.com?part=3__;Iw!!A3Ni8CS0y2Y!9Y1-
> 8bG621IAS4vRsBACPsWZTRxOMjvl6GwpDV_aXAdZxqf91aj0gx9M_D5G-
> LpeIpdqqGP6DniI1ScWRRNswxG98cP105I$

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
  2026-07-20  3:25   ` sashiko-bot
@ 2026-07-21  0:44     ` Torreno, Alexis Czezar
  2026-07-21  1:05       ` Guenter Roeck
  0 siblings, 1 reply; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-21  0:44 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, robh@kernel.org



> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, July 20, 2026 11:26 AM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: linux-hwmon@vger.kernel.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; robh@kernel.org
> Subject: Re: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback
> resistor scaling support
> 
> [External]
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
> consider:
> - [Medium] The voltage divider scaling calculation suffers from a 32-bit addition
> overflow and an unsafe 64-bit to 32-bit signed integer truncation before
> clamping.
> --
> 
> --- Patch [2]: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback
> resistor scaling support --- commit
> ac88893286286c633f117369db91c2cffd163afd
> Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> 
> hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
> 
> This commit implements support for external voltage divider scaling using the
> adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree properties. The driver
> reads these values and applies the scaling formula to the raw VOUT reading.
> 
> > diff --git a/drivers/hwmon/pmbus/max20830.c
> > b/drivers/hwmon/pmbus/max20830.c index cb2c23672166d..7a5e67bd33c0a
> > 100644
> > --- a/drivers/hwmon/pmbus/max20830.c
> > +++ b/drivers/hwmon/pmbus/max20830.c
> 
> [ ... ]
> 
> > +static int max20830_read_word_data(struct i2c_client *client, int page,
> > +				   int phase, int reg)
> > +{
> > +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> > +	const struct max20830_data *data = container_of(info, struct
> max20830_data, info);
> > +	int ret;
> > +
> > +	switch (reg) {
> > +	case PMBUS_READ_VOUT:
> > +		ret = pmbus_read_word_data(client, page, phase, reg);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		/* Apply voltage divider scaling if resistors are non-zero */
> > +		if (data->vout_rfb1 && data->vout_rfb2) {
> > +			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data-
> >vout_rfb1 +
> > +						    data->vout_rfb2), data-
> >vout_rfb2);
> > +			ret = clamp_val(ret, 0, 0xFFFF);
> 
> [Severity: Medium]
> Can this calculation suffer from a 32-bit addition overflow and an unsafe 64-bit
> to 32-bit signed integer truncation?
> 
> If data->vout_rfb1 and data->vout_rfb2 sum to more than 2^32 - 1, the 32-bit
> addition will wrap around before the 64-bit multiplication.
> 
> Additionally, since ret is a signed 32-bit integer, if the 64-bit result returned
> from DIV_ROUND_CLOSEST_ULL exceeds 2^31 - 1, the assignment to ret will
> overflow into a negative integer. The clamp_val() macro will then evaluate this
> negative number and incorrectly clamp it to 0 instead of 0xFFFF.
> 

Although technically correct, is sashiko a bit paranoid?
Feedback resistors won't typically reach those values.

> > +		}
> > +		return ret;
> 
> --
> Sashiko AI review *
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260720-dev-
> max20830c-v3-0-9d06d27d6da3@analog.com?part=2__;Iw!!A3Ni8CS0y2Y!-
> Nb04qCcfItlq4xUQf1LOznoGLxKoSEsno7xJWN0eYkdUVeuxpbLDHOwPO7BxUuG
> Ay8TA8LSvrg2p2pTJxd3gvxozfIGEIg$

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c
  2026-07-20  3:29   ` sashiko-bot
@ 2026-07-21  0:44     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-21  0:44 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: robh@kernel.org, linux-hwmon@vger.kernel.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org



> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, July 20, 2026 11:30 AM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: robh@kernel.org; linux-hwmon@vger.kernel.org;
> devicetree@vger.kernel.org; conor+dt@kernel.org
> Subject: Re: [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for
> max20830c and max20840c
> 
> [External]
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to
> consider:
> - [High] Missing compatible strings for MAX20830C and MAX20840C in the
> of_match_table.
> - [Medium] I2C block read path truncates the device ID for 9-character names,
> leading to potential false positive matches.
> - [Low] struct i2c_device_id array uses positional instead of named initializers.
> --
> 
> --- Patch [4]: [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for
> max20830c and max20840c --- commit
> dca1bbdb54e3f769bf22d26e97ead67754787a95
> Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> 
> hwmon: (pmbus/max20830): add support for max20830c and max20840c
> 
> Add support for MAX20830C and MAX20840 step-down DC-DC switching
> regulator with PMBus interface. MAX20830C is a different packaging for
> MAX20830, and MAX20840C supports 40A regulation compared to
> MAX20830 that is only 30A.
> 
> > diff --git a/drivers/hwmon/pmbus/max20830.c
> > b/drivers/hwmon/pmbus/max20830.c index 7a5e67bd33c0a..73fe59685cf15
> > 100644
> > --- a/drivers/hwmon/pmbus/max20830.c
> > +++ b/drivers/hwmon/pmbus/max20830.c
> > @@ -98,13 +98,12 @@ static int max20830_probe(struct i2c_client
> > *client)
> 
> [ ... ]
> 
> >  	} else {
> > -		/* Reads 1 length byte + 9 Data bytes from MAX20830 */
> > +		/* Reads 1 length byte + data bytes */
> >  		ret = i2c_smbus_read_i2c_block_data(client,
> PMBUS_IC_DEVICE_ID,
> >
> MAX20830_IC_DEVICE_ID_LENGTH + 1,
> >  						    buf);
> 
> [Severity: Medium]
> Since MAX20830C and MAX20840C have 9-character names, does this
> hardcoded MAX20830_IC_DEVICE_ID_LENGTH + 1 truncate the newly added
> device IDs?
> 
> In max20830_probe(), if the I2C controller lacks SMBUS_READ_BLOCK_DATA
> support, the fallback i2c_smbus_read_i2c_block_data() call uses
> MAX20830_IC_DEVICE_ID_LENGTH. Because this length was initially sized for
> the 8-character "MAX20830" identifier, could it under-read the new longer
> device IDs and cause false positive matches in the I2C fallback path?
> 

Non issue. MAX20830_IC_DEVICE_ID_LENGTH is already declared as 9.
The quirk is MAX20830 replies a 9th character of Null. Declaring them all as
9 length poses no issue to the string checks.

> [ ... ]
> 
> > @@ -120,26 +119,28 @@ static int max20830_probe(struct i2c_client
> > *client)
> 
> [ ... ]
> 
> >  static const struct i2c_device_id max20830_id[] = {
> > -	{"max20830"},
> > +	{ "max20830" },
> > +	{ "max20830c" },
> > +	{ "max20840c" },
> 
> [Severity: High]
> Are the compatible strings for max20830c and max20840c missing from the
> max20830_of_match[] table?
> 
> Without adding them alongside the newly added i2c_device_id entries, could
> this break module autoloading on systems using Device Tree when the OS
> attempts to autoload the driver via the udev modalias?
> 

max20830c and max20840c are functionally identical to max20830 and uses
the same configuration. DTS has adi,max20830 as a fallback compatible for the
new variants. Driver only needs to match the fallback compatible string.

> [Severity: Low]
> This isn't a bug, but do these entries violate the I2C subsystem coding style by
> using positional initializers?
> 
> The I2C client guidelines require initialized arrays of type struct i2c_device_id to
> use named initializers (e.g., { .name = "max20830c" }).
> 

max20830.c was using positional initializers, so for consistency the others were added
as positional initializers as well.

If requested, it is possible to change them all, including the original max20830, to named
initializers


> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, max20830_id);
> 
> --
> Sashiko AI review *
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260720-dev-
> max20830c-v3-0-
> 9d06d27d6da3@analog.com?part=4__;Iw!!A3Ni8CS0y2Y!5gKYVqJULNay7eEZv_
> L_8_gucJb1N86NTpoIzkSsewioUyVMMHaaxOqZzCyePUogn-
> 8pmaMWp4WmbDN1rInYx6qi_lW_gTM$

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
  2026-07-21  0:44     ` Torreno, Alexis Czezar
@ 2026-07-21  1:05       ` Guenter Roeck
  2026-07-21  1:13         ` Torreno, Alexis Czezar
  0 siblings, 1 reply; 18+ messages in thread
From: Guenter Roeck @ 2026-07-21  1:05 UTC (permalink / raw)
  To: Torreno, Alexis Czezar, sashiko-reviews@lists.linux.dev
  Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, robh@kernel.org

On 7/20/26 17:44, Torreno, Alexis Czezar wrote:
> 
> 
>> -----Original Message-----
>> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
>> Sent: Monday, July 20, 2026 11:26 AM
>> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
>> Cc: linux-hwmon@vger.kernel.org; conor+dt@kernel.org;
>> devicetree@vger.kernel.org; robh@kernel.org
>> Subject: Re: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback
>> resistor scaling support
>>
>> [External]
>>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
>> consider:
>> - [Medium] The voltage divider scaling calculation suffers from a 32-bit addition
>> overflow and an unsafe 64-bit to 32-bit signed integer truncation before
>> clamping.
>> --
>>
>> --- Patch [2]: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback
>> resistor scaling support --- commit
>> ac88893286286c633f117369db91c2cffd163afd
>> Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
>>
>> hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
>>
>> This commit implements support for external voltage divider scaling using the
>> adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree properties. The driver
>> reads these values and applies the scaling formula to the raw VOUT reading.
>>
>>> diff --git a/drivers/hwmon/pmbus/max20830.c
>>> b/drivers/hwmon/pmbus/max20830.c index cb2c23672166d..7a5e67bd33c0a
>>> 100644
>>> --- a/drivers/hwmon/pmbus/max20830.c
>>> +++ b/drivers/hwmon/pmbus/max20830.c
>>
>> [ ... ]
>>
>>> +static int max20830_read_word_data(struct i2c_client *client, int page,
>>> +				   int phase, int reg)
>>> +{
>>> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
>>> +	const struct max20830_data *data = container_of(info, struct
>> max20830_data, info);
>>> +	int ret;
>>> +
>>> +	switch (reg) {
>>> +	case PMBUS_READ_VOUT:
>>> +		ret = pmbus_read_word_data(client, page, phase, reg);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +
>>> +		/* Apply voltage divider scaling if resistors are non-zero */
>>> +		if (data->vout_rfb1 && data->vout_rfb2) {
>>> +			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data-
>>> vout_rfb1 +
>>> +						    data->vout_rfb2), data-
>>> vout_rfb2);
>>> +			ret = clamp_val(ret, 0, 0xFFFF);
>>
>> [Severity: Medium]
>> Can this calculation suffer from a 32-bit addition overflow and an unsafe 64-bit
>> to 32-bit signed integer truncation?
>>
>> If data->vout_rfb1 and data->vout_rfb2 sum to more than 2^32 - 1, the 32-bit
>> addition will wrap around before the 64-bit multiplication.
>>
>> Additionally, since ret is a signed 32-bit integer, if the 64-bit result returned
>> from DIV_ROUND_CLOSEST_ULL exceeds 2^31 - 1, the assignment to ret will
>> overflow into a negative integer. The clamp_val() macro will then evaluate this
>> negative number and incorrectly clamp it to 0 instead of 0xFFFF.
>>
> 
> Although technically correct, is sashiko a bit paranoid?
> Feedback resistors won't typically reach those values.
> 

It does what it is asked to do, which for hwmon drivers is to explicitly check
for over- and underflows because those happen surprisingly often in hwmon
drivers. There is a reason why more than 50 patches in drivers/hwmon
have either "overflow" or "underflow" in the subject line.

Guenter


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
  2026-07-21  1:05       ` Guenter Roeck
@ 2026-07-21  1:13         ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 18+ messages in thread
From: Torreno, Alexis Czezar @ 2026-07-21  1:13 UTC (permalink / raw)
  To: Guenter Roeck, sashiko-reviews@lists.linux.dev
  Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, robh@kernel.org

> >> [External]
> >>
> >> Thank you for your contribution! Sashiko AI review found 1 potential
> >> issue(s) to
> >> consider:
> >> - [Medium] The voltage divider scaling calculation suffers from a
> >> 32-bit addition overflow and an unsafe 64-bit to 32-bit signed
> >> integer truncation before clamping.
> >> --
> >>
> >> --- Patch [2]: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT
> >> feedback resistor scaling support --- commit
> >> ac88893286286c633f117369db91c2cffd163afd
> >> Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> >>
> >> hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
> >>
> >> This commit implements support for external voltage divider scaling
> >> using the adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree
> >> properties. The driver reads these values and applies the scaling formula to
> the raw VOUT reading.
> >>
> >>> diff --git a/drivers/hwmon/pmbus/max20830.c
> >>> b/drivers/hwmon/pmbus/max20830.c index
> cb2c23672166d..7a5e67bd33c0a
> >>> 100644
> >>> --- a/drivers/hwmon/pmbus/max20830.c
> >>> +++ b/drivers/hwmon/pmbus/max20830.c
> >>
> >> [ ... ]
> >>
> >>> +static int max20830_read_word_data(struct i2c_client *client, int page,
> >>> +				   int phase, int reg)
> >>> +{
> >>> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> >>> +	const struct max20830_data *data = container_of(info, struct
> >> max20830_data, info);
> >>> +	int ret;
> >>> +
> >>> +	switch (reg) {
> >>> +	case PMBUS_READ_VOUT:
> >>> +		ret = pmbus_read_word_data(client, page, phase, reg);
> >>> +		if (ret < 0)
> >>> +			return ret;
> >>> +
> >>> +		/* Apply voltage divider scaling if resistors are non-zero */
> >>> +		if (data->vout_rfb1 && data->vout_rfb2) {
> >>> +			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data-
> >>> vout_rfb1 +
> >>> +						    data->vout_rfb2), data-
> >>> vout_rfb2);
> >>> +			ret = clamp_val(ret, 0, 0xFFFF);
> >>
> >> [Severity: Medium]
> >> Can this calculation suffer from a 32-bit addition overflow and an
> >> unsafe 64-bit to 32-bit signed integer truncation?
> >>
> >> If data->vout_rfb1 and data->vout_rfb2 sum to more than 2^32 - 1, the
> >> 32-bit addition will wrap around before the 64-bit multiplication.
> >>
> >> Additionally, since ret is a signed 32-bit integer, if the 64-bit
> >> result returned from DIV_ROUND_CLOSEST_ULL exceeds 2^31 - 1, the
> >> assignment to ret will overflow into a negative integer. The
> >> clamp_val() macro will then evaluate this negative number and incorrectly
> clamp it to 0 instead of 0xFFFF.
> >>
> >
> > Although technically correct, is sashiko a bit paranoid?
> > Feedback resistors won't typically reach those values.
> >
> 
> It does what it is asked to do, which for hwmon drivers is to explicitly check for
> over- and underflows because those happen surprisingly often in hwmon
> drivers. There is a reason why more than 50 patches in drivers/hwmon have
> either "overflow" or "underflow" in the subject line.
> 

I see, will improve this part. I'll be more mindful of this.

Thanks,
Alexis

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-07-21  1:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20  3:14 [PATCH v3 0/4] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator Alexis Czezar Torreno
2026-07-20  3:14 ` [PATCH v3 1/4] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties and complete examples Alexis Czezar Torreno
2026-07-20  3:22   ` sashiko-bot
2026-07-20  3:28     ` Torreno, Alexis Czezar
2026-07-20  3:41   ` Torreno, Alexis Czezar
2026-07-20  4:38   ` Rob Herring (Arm)
2026-07-20  3:14 ` [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support Alexis Czezar Torreno
2026-07-20  3:25   ` sashiko-bot
2026-07-21  0:44     ` Torreno, Alexis Czezar
2026-07-21  1:05       ` Guenter Roeck
2026-07-21  1:13         ` Torreno, Alexis Czezar
2026-07-20  3:14 ` [PATCH v3 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support Alexis Czezar Torreno
2026-07-20  3:21   ` sashiko-bot
2026-07-21  0:44     ` Torreno, Alexis Czezar
2026-07-20  4:38   ` Rob Herring (Arm)
2026-07-20  3:14 ` [PATCH v3 4/4] hwmon: (pmbus/max20830): add support for max20830c and max20840c Alexis Czezar Torreno
2026-07-20  3:29   ` sashiko-bot
2026-07-21  0:44     ` Torreno, Alexis Czezar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox