[PATCH v2 0/5] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator

[PATCH v2 0/5] Add support for MAX20830C and MAX20840C step-down DC-DC switching regulator

[Alexis Czezar Torreno]

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

Patch 1 adds an enable gpio property that should have been included before.

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

Patches 4 and 5 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 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 (5):
      dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples
      dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties
      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         | 36 +++++++++-
 Documentation/hwmon/max20830.rst                   | 27 ++++++--
 drivers/hwmon/pmbus/max20830.c                     | 81 ++++++++++++++++++----
 3 files changed, 124 insertions(+), 20 deletions(-)
---
base-commit: 1a42625604046790ea8ec61c2a749bbf76b28943
change-id: 20260630-dev-max20830c-9d38d8b0a765

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

[PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[Alexis Czezar Torreno]

Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
was not included before. Also edited examples entry to be more complete.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml         | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
index 1625dd59417f1b3ca689a9c86ca266da913d1217..f3ba6351aa5d657590d92a538910fd6b787e7a3a 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
@@ -39,6 +39,11 @@ properties:
     description:
       Optional 2.5V to 5.5V LDO input supply.
 
+  enable-gpios:
+    description:
+      GPIO connected to the EN (enable) pin.
+    maxItems: 1
+
   pwr-good-gpios:
     description:
       GPIO connected to the power-good status output pin.
@@ -53,6 +58,8 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
+
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -61,6 +68,10 @@ examples:
             compatible = "adi,max20830";
             reg = <0x30>;
             vddh-supply = <&vddh>;
+            avdd-supply = <&avdd>;
+            ldoin-supply = <&ldoin>;
+            enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
         };
     };
 ...

-- 
2.34.1

[PATCH v2 2/5] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties

[Alexis Czezar Torreno]

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.

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

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
index f3ba6351aa5d657590d92a538910fd6b787e7a3a..3d62c03abdc30c8aada303bad263e972219b33a4 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
@@ -49,11 +49,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:
@@ -72,6 +87,8 @@ examples:
             ldoin-supply = <&ldoin>;
             enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
             pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+            adi,vout-rfb1-ohms = <10000>;
+            adi,vout-rfb2-ohms = <2000>;
         };
     };
 ...

-- 
2.34.1

[PATCH v2 3/5] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[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 | 54 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
index cb2c23672166d641852199ca07eb716924f4f286..5aad4448054fc55bb0a87920d89f96bc28fda072 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,45 @@
 
 #define MAX20830_IC_DEVICE_ID_LENGTH	9
 
+struct max20830_data {
+	struct pmbus_driver_info info;
+	u32 vout_rfb1;
+	u32 vout_rfb2;
+};
+
+/*
+ * Driver currently only supports READ_VOUT for monitoring that needs scaling to
+ * convert from feedback voltage to actual output voltage.
+ *
+ * vout_actual = vout_fb × (1 + RFB1/RFB2)
+ *
+ * VOUT_COMMAND and VOUT_MAX are currently not in use, but work with feedback
+ * voltage. If regulator support is added, VOUT_COMMAND values would need
+ * to be adjusted accordingly.
+ */
+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);
+		return ret;
+	default:
+		return -ENODATA;
+	}
+}
+
 static struct pmbus_driver_info max20830_info = {
 	.pages = 1,
 	.format[PSC_VOLTAGE_IN] = linear,
@@ -24,13 +64,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 +130,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

[PATCH v2 4/5] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support

[Alexis Czezar Torreno]

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.

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 3d62c03abdc30c8aada303bad263e972219b33a4..826be4d818908becc1e185e640a3b684c6df004c 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

[PATCH v2 5/5] hwmon: (pmbus/max20830): add support for max20830c and max20840c

[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 5aad4448054fc55bb0a87920d89f96bc28fda072..88fc45447792b1fea7691dee9fb385238c5d82c1 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -93,13 +93,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);
@@ -115,26 +114,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

Re: [PATCH v2 2/5] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties

[sashiko-bot]

> 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.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com?part=2

Re: [PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[sashiko-bot]

> Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
> was not included before. Also edited examples entry to be more complete.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com?part=1

Re: [PATCH v2 4/5] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support

[sashiko-bot]

> 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.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com?part=4

Re: [PATCH v2 5/5] hwmon: (pmbus/max20830): add support for max20830c and max20840c

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com?part=5

Re: [PATCH v2 3/5] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Scaled `PMBUS_READ_VOUT` value can exceed the 16-bit capacity of the PMBus `LINEAR16` format, causing silent truncation and incorrect voltage reporting.
- [High] Missing software scaling for VOUT limit attributes and missing `.write_word_data` callback causes hardware limits to be programmed completely out of bounds.
--

commit 19e6041d100f0e80c9e49784d17900b141e3ea61
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
    
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)

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

[ ... ]

> @@ -14,6 +15,45 @@
>  
>  #define MAX20830_IC_DEVICE_ID_LENGTH	9
>  
> +struct max20830_data {
> +	struct pmbus_driver_info info;
> +	u32 vout_rfb1;
> +	u32 vout_rfb2;
> +};
> +
> +/*
> + * Driver currently only supports READ_VOUT for monitoring that needs scaling to
> + * convert from feedback voltage to actual output voltage.
> + *
> + * vout_actual = vout_fb × (1 + RFB1/RFB2)
> + *
> + * VOUT_COMMAND and VOUT_MAX are currently not in use, but work with feedback
> + * voltage. If regulator support is added, VOUT_COMMAND values would need
> + * to be adjusted accordingly.
> + */
> +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);
> +		return ret;

[Severity: High]
Does this returned value need bounds checking?

If a large resistor divider is used, the scaled PMBus word calculated here
can exceed 0xFFFF. The generic PMBus core function pmbus_reg2data_linear()
will cast this returned value to a u16, silently wrapping the value modulo
65536 and resulting in wildly incorrect telemetry (e.g., reporting 2V instead
of 12V).

Should this be clamped using clamp_val(ret, 0, 0xffff)?

> +	default:
> +		return -ENODATA;
> +	}
> +}

[Severity: High]
Will returning -ENODATA bypass software scaling for VOUT limit attributes?

The driver sets PMBUS_HAVE_VOUT below, which exposes hardware VOUT limits
(like inX_max or inX_lcrit). Because only PMBUS_READ_VOUT is intercepted
here, the core will read those limits unscaled from the hardware.

> +
>  static struct pmbus_driver_info max20830_info = {
>  	.pages = 1,
>  	.format[PSC_VOLTAGE_IN] = linear,
> @@ -24,13 +64,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,

[Severity: High]
Does this struct also need a .write_word_data callback?

Since PMBUS_HAVE_VOUT is enabled, user writes to limit attributes will fall
through to the PMBus core, which writes the scaled PMBus word directly into
the hardware's limit register. Because the hardware expects limits in the
unscaled feedback voltage range, the written value will be completely out
of bounds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dev-max20830c-v2-0-37761e89bb5f@analog.com?part=3

Re: [PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[Krzysztof Kozlowski]

On Mon, Jul 06, 2026 at 10:08:41AM +0800, Alexis Czezar Torreno wrote:
> Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
> was not included before. Also edited examples entry to be more complete.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml         | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>

How did you address previous feedback?

Best regards,
Krzysztof

Re: [PATCH v2 4/5] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support

[Krzysztof Kozlowski]

On Mon, Jul 06, 2026 at 10:08:44AM +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>

Best regards,
Krzysztof

RE: [PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[Torreno, Alexis Czezar]

> On Mon, Jul 06, 2026 at 10:08:41AM +0800, Alexis Czezar Torreno wrote:
> > Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
> > was not included before. Also edited examples entry to be more complete.
> >
> > Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> > ---
> >  .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml         | 11
> +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> 
> How did you address previous feedback?
> 

Regarding the enable pin, I added this since I know bindings like being complete
and saw that I didn't add it the first time I submitted max20830.
I added driver code for the gpio but learned that it wasn't really a use case so
I simply dropped the patch for it. 

I only added a few words in the commit description and added more entries
in the example for v2 this patch. 

Regards,
Alexis

Re: [PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[Guenter Roeck]

On Mon, Jul 06, 2026 at 10:08:41AM +0800, Alexis Czezar Torreno wrote:
> Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
> was not included before. Also edited examples entry to be more complete.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

I still do not see the point of these properties. The driver can extract
the power good information from its registers, and it can set the enable
value from there as well. It does not need either.
In practice the power good output is likely to be connected to an LED or
to the enable pin of another power controller. Neither would need
devicetree properties.

Again, please provide a use case. The mere presence of these pins (which
exist on pretty much all PMBus chips) does not warrant devicetree
properties for them.

Thanks,
Guenter

> ---
>  .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml         | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> index 1625dd59417f1b3ca689a9c86ca266da913d1217..f3ba6351aa5d657590d92a538910fd6b787e7a3a 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> @@ -39,6 +39,11 @@ properties:
>      description:
>        Optional 2.5V to 5.5V LDO input supply.
>  
> +  enable-gpios:
> +    description:
> +      GPIO connected to the EN (enable) pin.
> +    maxItems: 1
> +
>    pwr-good-gpios:
>      description:
>        GPIO connected to the power-good status output pin.
> @@ -53,6 +58,8 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -61,6 +68,10 @@ examples:
>              compatible = "adi,max20830";
>              reg = <0x30>;
>              vddh-supply = <&vddh>;
> +            avdd-supply = <&avdd>;
> +            ldoin-supply = <&ldoin>;
> +            enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> +            pwr-good-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
>          };
>      };
>  ...

Re: [PATCH v2 1/5] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property and complete examples

[Guenter Roeck]

On 7/6/26 00:13, Torreno, Alexis Czezar wrote:
> 
>> On Mon, Jul 06, 2026 at 10:08:41AM +0800, Alexis Czezar Torreno wrote:
>>> Adding an entry for the MAX20830 EN (enable) pin. This pin exist but
>>> was not included before. Also edited examples entry to be more complete.
>>>
>>> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
>>> ---
>>>   .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml         | 11
>> +++++++++++
>>>   1 file changed, 11 insertions(+)
>>>
>>
>> How did you address previous feedback?
>>
> 
> Regarding the enable pin, I added this since I know bindings like being complete
> and saw that I didn't add it the first time I submitted max20830.
> I added driver code for the gpio but learned that it wasn't really a use case so
> I simply dropped the patch for it.
> 

I guess I am completely missing the point here. I can not imagine a situation
where one would want to connect the enable pin to a driver-controlled GPIO pin,
or why would one connect the chip's PGOOD output pin to a GPIO input pin
and connect that back to the driver.

I think we will need guidance from devicetree maintainers explaining what
"complete" means in such a context to avoid having to repeat this discussion
for every driver going forward.

Thanks,
Guenter