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

[PATCH 0/6] 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.

Patches 1 and 2 adds an enable gpio that should have been included before.

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

Patches 5 and 6 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>
---
Alexis Czezar Torreno (6):
      dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property
      hwmon: (pmbus/max20830): add support for enable GPIO
      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         |  30 +++++-
 Documentation/hwmon/max20830.rst                   |  27 ++++-
 drivers/hwmon/pmbus/max20830.c                     | 117 +++++++++++++++++----
 3 files changed, 148 insertions(+), 26 deletions(-)
---
base-commit: 1a42625604046790ea8ec61c2a749bbf76b28943
change-id: 20260630-dev-max20830c-9d38d8b0a765

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

[PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[Alexis Czezar Torreno]

Adding a missing entry for the MAX20830 EN (enable) pin.

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

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
index 1625dd59417f1b3ca689a9c86ca266da913d1217..ab8f6324866f29de8c66c3c63300845b2e02207e 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.

-- 
2.34.1

Re: [PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[sashiko-bot]

> Adding a missing entry for the MAX20830 EN (enable) pin.
> 
> 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/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=1

Re: [PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[Krzysztof Kozlowski]

On Tue, Jun 30, 2026 at 10:46:43AM +0800, Alexis Czezar Torreno wrote:
> Adding a missing entry for the MAX20830 EN (enable) pin.

... because? Device has it? Was missing?

> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> index 1625dd59417f1b3ca689a9c86ca266da913d1217..ab8f6324866f29de8c66c3c63300845b2e02207e 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

Make also the example complete.

Best regards,
Krzysztof

RE: [PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[Torreno, Alexis Czezar]

> [External]
> 
> On Tue, Jun 30, 2026 at 10:46:43AM +0800, Alexis Czezar Torreno wrote:
> > Adding a missing entry for the MAX20830 EN (enable) pin.
> 
> ... because? Device has it? Was missing?

Yes, device has it and I forgot to include it in the past.

> 
> Make also the example complete.
> 

Will do

Regards,
Alexis

Re: [PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[Guenter Roeck]

On 6/30/26 01:07, Torreno, Alexis Czezar wrote:
>> [External]
>>
>> On Tue, Jun 30, 2026 at 10:46:43AM +0800, Alexis Czezar Torreno wrote:
>>> Adding a missing entry for the MAX20830 EN (enable) pin.
>>
>> ... because? Device has it? Was missing?
> 
> Yes, device has it and I forgot to include it in the past.
> 

Pretty much every PMBus chip has it. Many non-PMBus hardware
monitoring chips have it. Problem is that there is no practical
use case that I am aware of to connect it to a GPIO pin,
especially for PMBus chips because PMBus (and the chip) has
a command to override it. Connecting the Enable pin to a GPIO
output would mean another extra and unnecessary wire on a board
plus the need for additional software to support - in a more
complex way - what is already supported by the chip itself.

This will even more complicated if anyone ever adds regulator
support to the driver. In order to fully support this, the driver
would have to add support for enabling the chip through the
GPIO pin (or to override it permanently, making it pointless
to have it) even though it is highly unlikely that this code
can ever be tested on real hardware.

That is why I was asking for a use case. "The chip has the
pin" is not a use case.

Thanks,
Guenter

RE: [PATCH 1/6] dt-bindings: hwmon: (pmbus/max20830): add enable-gpios property

[Torreno, Alexis Czezar]

> >> [External]
> >>
> >> On Tue, Jun 30, 2026 at 10:46:43AM +0800, Alexis Czezar Torreno wrote:
> >>> Adding a missing entry for the MAX20830 EN (enable) pin.
> >>
> >> ... because? Device has it? Was missing?
> >
> > Yes, device has it and I forgot to include it in the past.
> >
> 
> Pretty much every PMBus chip has it. Many non-PMBus hardware monitoring
> chips have it. Problem is that there is no practical use case that I am aware of to
> connect it to a GPIO pin, especially for PMBus chips because PMBus (and the
> chip) has a command to override it. Connecting the Enable pin to a GPIO output
> would mean another extra and unnecessary wire on a board plus the need for
> additional software to support - in a more complex way - what is already
> supported by the chip itself.
> 
> This will even more complicated if anyone ever adds regulator support to the
> driver. In order to fully support this, the driver would have to add support for
> enabling the chip through the GPIO pin (or to override it permanently, making it
> pointless to have it) even though it is highly unlikely that this code can ever be
> tested on real hardware.
> 
> That is why I was asking for a use case. "The chip has the pin" is not a use case.
> 

Got it, especially the part that PMBUS does have commands to override it, and adding
this is unnecessarily complex.
I don't have a use case. Will remove the en-gpio code from the driver, but keep the
binding entry.

Thanks,
Alexis

[PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[Alexis Czezar Torreno]

Add support for the GPIO controlled EN pin. The EN pin is asserted high
for device to operate.

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

diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
index cb2c23672166d641852199ca07eb716924f4f286..cb3a39d747edee3aefb0fb4051ef957436b3c15b 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/errno.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
 static int max20830_probe(struct i2c_client *client)
 {
 	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
+	struct gpio_desc *enable_gpio;
 	int ret;
 
+	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(enable_gpio))
+		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
+				     "Failed to get enable GPIO\n");
+
 	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;

-- 
2.34.1

Re: [PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing delay between asserting the enable GPIO and initiating I2C communication.
--

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

hwmon: (pmbus/max20830): add support for enable GPIO

This commit adds support for a GPIO-controlled EN pin. The EN pin is
asserted high to allow the device to operate.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d..cb3a39d747ede 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
[ ... ]
> @@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
>  static int max20830_probe(struct i2c_client *client)
>  {
>  	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
> +	struct gpio_desc *enable_gpio;
>  	int ret;
>  
> +	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(enable_gpio))
> +		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
> +				     "Failed to get enable GPIO\n");
> +

[Severity: High]
Will this cause the probe to fail if the device was previously unpowered?

When the enable GPIO is asserted high using devm_gpiod_get_optional() with
GPIOD_OUT_HIGH, the device initiates its power-on reset and internal 
initialization sequence. 

Because there is no explicit delay between asserting the enable pin and the 
subsequent I2C communication in max20830_probe(), is it possible the first
I2C transaction will fail with a NACK because the hardware hasn't finished
initializing?

Should there be a short delay here after pulling the enable pin high to
give the device time to be ready for I2C communication?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=2

Re: [PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[Guenter Roeck]

On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> Add support for the GPIO controlled EN pin. The EN pin is asserted high
> for device to operate.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>   drivers/hwmon/pmbus/max20830.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d641852199ca07eb716924f4f286..cb3a39d747edee3aefb0fb4051ef957436b3c15b 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
> @@ -6,6 +6,7 @@
>    */
>   
>   #include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/i2c.h>
>   #include <linux/mod_devicetable.h>
>   #include <linux/module.h>
> @@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
>   static int max20830_probe(struct i2c_client *client)
>   {
>   	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
> +	struct gpio_desc *enable_gpio;
>   	int ret;
>   
> +	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(enable_gpio))
> +		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
> +				     "Failed to get enable GPIO\n");
> +

The above code gets the gpio reference, and then it doesn't do anything
with it. What exactly is the point of this exercise ? Where is the
chip actually enabled ?

Do you have an actual customer with such a set-up or is this
"just in case" ? Have you tested this code to ensure that the chip
is actually enabled in this setup ?

If there is indeed a use case where a customer indeed connects the
enable pin to a gpio output, wouldn't that same customer also want
to connect the "pgood" output to a gpio pin ? And what about
the LDOIN pin ? Shouldn't that be connected to a power supply ?

Guenter

Re: [PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[Guenter Roeck]

On 6/29/26 20:50, Guenter Roeck wrote:
> On 6/29/26 19:46, Alexis Czezar Torreno wrote:
>> Add support for the GPIO controlled EN pin. The EN pin is asserted high
>> for device to operate.
>>
>> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
>> ---
>>   drivers/hwmon/pmbus/max20830.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
>> index cb2c23672166d641852199ca07eb716924f4f286..cb3a39d747edee3aefb0fb4051ef957436b3c15b 100644
>> --- a/drivers/hwmon/pmbus/max20830.c
>> +++ b/drivers/hwmon/pmbus/max20830.c
>> @@ -6,6 +6,7 @@
>>    */
>>   #include <linux/errno.h>
>> +#include <linux/gpio/consumer.h>
>>   #include <linux/i2c.h>
>>   #include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>> @@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
>>   static int max20830_probe(struct i2c_client *client)
>>   {
>>       u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
>> +    struct gpio_desc *enable_gpio;
>>       int ret;
>> +    enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
>> +    if (IS_ERR(enable_gpio))
>> +        return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
>> +                     "Failed to get enable GPIO\n");
>> +
> 
> The above code gets the gpio reference, and then it doesn't do anything
> with it. What exactly is the point of this exercise ? Where is the
> chip actually enabled ?
> 
> Do you have an actual customer with such a set-up or is this
> "just in case" ? Have you tested this code to ensure that the chip
> is actually enabled in this setup ?
> 

Also, please explain the need in detail, especially in the context of
PMBus command 0x02 (ON_OFF_CONFIG) which can be used to configure
the pin functionality. Specifically, what would be the point of
trying to force-enable the chip if on-off-config happens to be set
to 0x1b (ignore EN pin and require OPERATION = 0x80) ?
And if ON_OFF_CONFIG happens to be set to 0x17 (Ignore OPERATION
command), why not just set it to 0x1b and override EN ?

In other words, I expect the use case to be explained in the context
of the ON_OFF_CONFIG and OPERATION commands. "The EN pin is asserted
high for device to operate" is misleading and only half-true since that
is only the case if the chip is configured to actually use it (which,
I notice, you are not doing here).

Thanks,
Guenter

> If there is indeed a use case where a customer indeed connects the
> enable pin to a gpio output, wouldn't that same customer also want
> to connect the "pgood" output to a gpio pin ? And what about
> the LDOIN pin ? Shouldn't that be connected to a power supply ?
> 
> Guenter
> 
> 

RE: [PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[Torreno, Alexis Czezar]

> > +	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable",
> GPIOD_OUT_HIGH);
> > +	if (IS_ERR(enable_gpio))
> > +		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
> > +				     "Failed to get enable GPIO\n");
> > +
> 
> The above code gets the gpio reference, and then it doesn't do anything with it.
> What exactly is the point of this exercise ? Where is the chip actually enabled ?
> 
> Do you have an actual customer with such a set-up or is this "just in case" ?
> Have you tested this code to ensure that the chip is actually enabled in this
> setup ?
> 
> If there is indeed a use case where a customer indeed connects the enable pin
> to a gpio output, wouldn't that same customer also want to connect the
> "pgood" output to a gpio pin ? And what about the LDOIN pin ? Shouldn't that
> be connected to a power supply ?
> 

You're right. I initially just wanted to add the enable pin in the bindings, I might've
thought I need to do something to the driver code too. Which is unnecessary...
I will be removing this patch 2/6.

Regards,
Alexis

[PATCH 3/6] 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.

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 ab8f6324866f29de8c66c3c63300845b2e02207e..caedad40bc592c8489df235f02c6ff051070cb1e 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:
@@ -66,6 +81,8 @@ examples:
             compatible = "adi,max20830";
             reg = <0x30>;
             vddh-supply = <&vddh>;
+            adi,vout-rfb1-ohms = <10000>;
+            adi,vout-rfb2-ohms = <2000>;
         };
     };
 ...

-- 
2.34.1

Re: [PATCH 3/6] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing initialization delay after asserting the enable GPIO causes probe failure if the device was previously powered off.
--

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

hwmon: (pmbus/max20830): add support for enable GPIO

Add support for the GPIO controlled EN pin. The EN pin is asserted high
for device to operate.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d..cb3a39d747ede 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
[ ... ]
> @@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
>  static int max20830_probe(struct i2c_client *client)
>  {
>  	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
> +	struct gpio_desc *enable_gpio;
>  	int ret;
>  
> +	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(enable_gpio))
> +		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
> +				     "Failed to get enable GPIO\n");
> +

[Severity: High]
Could this cause a regression where the driver fails to probe due to a missing
hardware boot-up delay?

When the enable GPIO is configured in the device tree and the device was
previously powered off, asserting it with GPIOD_OUT_HIGH will power up the
device. However, the driver proceeds to call i2c_smbus_read_block_data()
shortly after without any settling time.

If the regulator requires a delay to boot up before it can acknowledge i2c
packets, this immediate read might receive a nack and cause the driver
probe to abort.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=3

Re: [PATCH 3/6] dt-bindings: hwmon: (pmbus/max20830): add VOUT feedback resistor properties

[Krzysztof Kozlowski]

On Tue, Jun 30, 2026 at 10:46:45AM +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.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../devicetree/bindings/hwmon/pmbus/adi,max20830.yaml   | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 4/6] 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 | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
index cb3a39d747edee3aefb0fb4051ef957436b3c15b..a3abd24437e8e7560264aad55fc4f456d30ae235 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -8,6 +8,7 @@
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/math64.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/string.h>
@@ -15,6 +16,35 @@
 
 #define MAX20830_IC_DEVICE_ID_LENGTH	9
 
+struct max20830_data {
+	struct pmbus_driver_info info;
+	u32 vout_rfb1;
+	u32 vout_rfb2;
+};
+
+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,
@@ -25,14 +55,26 @@ 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;
 	struct gpio_desc *enable_gpio;
 	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);
+
 	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
 	if (IS_ERR(enable_gpio))
 		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),

-- 
2.34.1

Re: [PATCH 4/6] 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] Applying a voltage scaling multiplier directly to the PMBus word data for VOUT will result in severe silent truncation by the PMBus core framework if the scaled mantissa exceeds 16 bits.
- [High] Voltage divider scaling is applied only to PMBUS_READ_VOUT, leaving other VOUT-related PMBus commands and fault limits unscaled and ignoring write operations.
--

commit 690eab506d6910945bf8fa7dc51390239ffa2898
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.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb3a39d747ede..a3abd24437e8e 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c

[ ... ]

> @@ -15,6 +16,35 @@
>  
>  #define MAX20830_IC_DEVICE_ID_LENGTH	9
>  
> +struct max20830_data {
> +	struct pmbus_driver_info info;
> +	u32 vout_rfb1;
> +	u32 vout_rfb2;
> +};
> +
> +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:
[Severity: High]
Does this code apply voltage divider scaling only to PMBUS_READ_VOUT, leaving
other VOUT-related PMBus commands and fault limits unscaled?

If userspace reads limit registers (e.g., in1_max), the switch statement
misses them and returns raw values.

> +		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);
[Severity: High]
Can this scaling cause silent truncation?

When userspace reads the output voltage via sysfs, this code multiplies the
raw 16-bit register value by the resistor ratio. The PMBus core reads this
scaled word, but in pmbus_reg2data_linear(), it casts the value to a 16-bit
unsigned integer:

    mantissa = (u16)sensor->data;

This will silently truncate any value that exceeds 65535, corrupting the
voltage telemetry.

> +		return ret;
> +	default:
> +		return -ENODATA;
> +	}
> +}
> +
>  static struct pmbus_driver_info max20830_info = {
>  	.pages = 1,
>  	.format[PSC_VOLTAGE_IN] = linear,
> @@ -25,14 +55,26 @@ 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]
Should this also provide a write_word_data callback?

Without a write_word_data override, any user writes to voltage limits are
passed unscaled to the device. The writes go directly to the PMBus core
without inverse scaling, causing hardware protection limits (over/under
voltage) to be misconfigured.

>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=4

Re: [PATCH 4/6] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[Guenter Roeck]

On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> 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.
> 

This will require a detailed explanation why only PMBUS_READ_VOUT
would require scaling but not any of the other vout related commands.

Guenter

RE: [PATCH 4/6] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[Torreno, Alexis Czezar]

> [External]
> 
> On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> > 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.
> >
> 
> This will require a detailed explanation why only PMBUS_READ_VOUT would
> require scaling but not any of the other vout related commands.
> 
> Guenter

Will add it.
For reference, the device only has 3 vout related commands. The other 2
is referred to a feedback/reference voltage rather than a scaled output value.
Hence only read_vout has scaling.

Regards,
Alexis

Re: [PATCH 4/6] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[Guenter Roeck]

On 6/30/26 01:07, Torreno, Alexis Czezar wrote:
> 
>> [External]
>>
>> On 6/29/26 19:46, Alexis Czezar Torreno wrote:
>>> 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.
>>>
>>
>> This will require a detailed explanation why only PMBUS_READ_VOUT would
>> require scaling but not any of the other vout related commands.
>>
>> Guenter
> 
> Will add it.
> For reference, the device only has 3 vout related commands. The other 2
> is referred to a feedback/reference voltage rather than a scaled output value.
> Hence only read_vout has scaling.

The other two commands are VOUT_COMMAND and VOUT_MAX. While they are currently
not used, VOUT_COMMAND _would_ be used if someone adds regulator support to the
driver in the future. Thus, even though those two commands are currently not
used, you have to at least add a note into the driver that the register values
will have to be adjusted if regulator support is added to the driver in the
future.

Thanks,
Guenter

RE: [PATCH 4/6] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[Torreno, Alexis Czezar]

> >>
> >> On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> >>> 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.
> >>>
> >>
> >> This will require a detailed explanation why only PMBUS_READ_VOUT
> >> would require scaling but not any of the other vout related commands.
> >>
> >> Guenter
> >
> > Will add it.
> > For reference, the device only has 3 vout related commands. The other
> > 2 is referred to a feedback/reference voltage rather than a scaled output
> value.
> > Hence only read_vout has scaling.
> 
> The other two commands are VOUT_COMMAND and VOUT_MAX. While they
> are currently not used, VOUT_COMMAND _would_ be used if someone adds
> regulator support to the driver in the future. Thus, even though those two
> commands are currently not used, you have to at least add a note into the
> driver that the register values will have to be adjusted if regulator support is
> added to the driver in the future.
> 

Thanks for the added info, will add this as a comment in the driver.

Regards,
Alexis

[PATCH 5/6] 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 caedad40bc592c8489df235f02c6ff051070cb1e..b8ca8ec0446fae2a16484e5ff8f1bb563cdb2405 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  # Fallback compatible
 
   reg:
     maxItems: 1

-- 
2.34.1

Re: [PATCH 5/6] 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/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=5

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

[Krzysztof Kozlowski]

On Tue, Jun 30, 2026 at 10:46:47AM +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.
> 
> 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 caedad40bc592c8489df235f02c6ff051070cb1e..b8ca8ec0446fae2a16484e5ff8f1bb563cdb2405 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  # Fallback compatible

Drop the comment, it is obvious. Can this be not a fallback compatible?


Best regards,
Krzysztof

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

[Torreno, Alexis Czezar]

> >
> >  properties:
> >    compatible:
> > -    const: adi,max20830
> > +    oneOf:
> > +      - const: adi,max20830
> > +      - items:
> > +          - enum:
> > +              - adi,max20830c
> > +              - adi,max20840c
> > +          - const: adi,max20830  # Fallback compatible
> 
> Drop the comment, it is obvious. Can this be not a fallback compatible?
> 

After considering the feedback from patch 6/6 regarding this fallback, I think I
would not make this fallback compatible. Will edit out.

Regards,
Alexis

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

[Guenter Roeck]

On 6/30/26 01:07, Torreno, Alexis Czezar wrote:
> 
>>>
>>>   properties:
>>>     compatible:
>>> -    const: adi,max20830
>>> +    oneOf:
>>> +      - const: adi,max20830
>>> +      - items:
>>> +          - enum:
>>> +              - adi,max20830c
>>> +              - adi,max20840c
>>> +          - const: adi,max20830  # Fallback compatible
>>
>> Drop the comment, it is obvious. Can this be not a fallback compatible?
>>
> 
> After considering the feedback from patch 6/6 regarding this fallback, I think I
> would not make this fallback compatible. Will edit out.
> 

The chips are technically identical (register compatible) except for
the device ID string. That can be evaluated in the driver without
requiring a separate compatible entry in the devicetree description.

Guenter

[PATCH 6/6] 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   | 68 ++++++++++++++++++++++++++++------------
 2 files changed, 70 insertions(+), 25 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 a3abd24437e8e7560264aad55fc4f456d30ae235..252c77beb243c5a2d90fcf96941605ff31439383 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -14,7 +14,30 @@
 #include <linux/string.h>
 #include "pmbus.h"
 
-#define MAX20830_IC_DEVICE_ID_LENGTH	9
+struct max20830_chip_info {
+	const char *id_str;
+	u8 id_length;
+};
+
+static const struct max20830_chip_info max20830_chip = {
+	/*
+	 * MAX20830 IC_DEVICE_ID has a byte length of 9 despite being an 8
+	 * character string, as it includes a null terminator. The other
+	 * devices do not include null.
+	 */
+	.id_str = "MAX20830\0",
+	.id_length = 9,
+};
+
+static const struct max20830_chip_info max20830c_chip = {
+	.id_str = "MAX20830C",
+	.id_length = 9,
+};
+
+static const struct max20830_chip_info max20840c_chip = {
+	.id_str = "MAX20840C",
+	.id_length = 9,
+};
 
 struct max20830_data {
 	struct pmbus_driver_info info;
@@ -60,11 +83,14 @@ static struct pmbus_driver_info max20830_info = {
 
 static int max20830_probe(struct i2c_client *client)
 {
+	const struct max20830_chip_info *chip;
 	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
 	struct max20830_data *data;
 	struct gpio_desc *enable_gpio;
 	int ret;
 
+	chip = i2c_get_match_data(client);
+
 	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -90,16 +116,14 @@ 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);
+						    chip->id_length + 1, buf);
 		if (ret < 0)
 			return dev_err_probe(&client->dev, ret,
 					     "Failed to read IC_DEVICE_ID\n");
@@ -108,36 +132,40 @@ static int max20830_probe(struct i2c_client *client)
 		 * match the format of i2c_smbus_read_block_data().
 		 * Also adjust return value to reflect length byte removal.
 		 */
-		memmove(buf, buf + 1, MAX20830_IC_DEVICE_ID_LENGTH);
+		memmove(buf, buf + 1, chip->id_length);
 		ret = ret - 1;
 	}
 
-	/*
-	 * MAX20830 IC_DEVICE_ID sends string data "MAX20830\0".
-	 * Return value should at least be 9 bytes of data.
-	 */
-	if (ret < MAX20830_IC_DEVICE_ID_LENGTH)
+	/* Verify we read the expected number of bytes */
+	if (ret < chip->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",
+				     chip->id_length, ret);
+
+	/* Null-terminate the string */
+	buf[chip->id_length] = '\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 (strncmp(buf, chip->id_str, chip->id_length))
 		return dev_err_probe(&client->dev, -ENODEV,
-				     "Unsupported device: '%s'\n", buf);
+				     "Device mismatch: expected '%s', got '%s'\n",
+				     chip->id_str, buf);
 
-	return pmbus_do_probe(client, &max20830_info);
+	return pmbus_do_probe(client, &data->info);
 }
 
 static const struct i2c_device_id max20830_id[] = {
-	{"max20830"},
+	{ "max20830", (kernel_ulong_t)&max20830_chip },
+	{ "max20830c", (kernel_ulong_t)&max20830c_chip },
+	{ "max20840c", (kernel_ulong_t)&max20840c_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max20830_id);
 
 static const struct of_device_id max20830_of_match[] = {
-	{ .compatible = "adi,max20830" },
+	{ .compatible = "adi,max20830", .data = &max20830_chip },
+	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
+	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, max20830_of_match);

-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing NULL check for the return value of i2c_get_match_data() leads to a potential NULL pointer dereference.
--

commit 44325aff0c44d84d978feed33af57c60c31e89f3
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max20830): add support for max20830c and max20840c

This commit adds support for the MAX20830C and MAX20840C step-down DC-DC
switching regulators with PMBus interfaces. It updates the driver to handle
multiple device types by introducing chip-specific information structures
and matching data.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index a3abd24437e8e..252c77beb243c 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
[ ... ]
> @@ -60,11 +83,14 @@ static struct pmbus_driver_info max20830_info = {
>  
>  static int max20830_probe(struct i2c_client *client)
>  {
> +	const struct max20830_chip_info *chip;
>  	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
>  	struct max20830_data *data;
>  	struct gpio_desc *enable_gpio;
>  	int ret;
>  
> +	chip = i2c_get_match_data(client);
> +

[Severity: High]
Can this result in a NULL pointer dereference if i2c_get_match_data() returns
NULL?

If an I2C device is instantiated via sysfs using driver_override with a
device name that doesn't exist in the driver's ID tables, table matching is
bypassed and i2c_get_match_data() will return NULL.

>  	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
[ ... ]
>  	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);
> +						    chip->id_length + 1, buf);

The chip pointer is then dereferenced here when reading chip->id_length without
first checking if it is valid. Should there be a validation check on chip after
it is obtained from i2c_get_match_data()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-dev-max20830c-v1-0-a02786bde470@analog.com?part=6

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

[Guenter Roeck]

On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> 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   | 68 ++++++++++++++++++++++++++++------------
>   2 files changed, 70 insertions(+), 25 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 a3abd24437e8e7560264aad55fc4f456d30ae235..252c77beb243c5a2d90fcf96941605ff31439383 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
> @@ -14,7 +14,30 @@
>   #include <linux/string.h>
>   #include "pmbus.h"
>   
> -#define MAX20830_IC_DEVICE_ID_LENGTH	9
> +struct max20830_chip_info {
> +	const char *id_str;
> +	u8 id_length;
> +};
> +
> +static const struct max20830_chip_info max20830_chip = {
> +	/*
> +	 * MAX20830 IC_DEVICE_ID has a byte length of 9 despite being an 8
> +	 * character string, as it includes a null terminator. The other
> +	 * devices do not include null.
> +	 */
> +	.id_str = "MAX20830\0",
> +	.id_length = 9,
> +};
> +
> +static const struct max20830_chip_info max20830c_chip = {
> +	.id_str = "MAX20830C",
> +	.id_length = 9,
> +};
> +
> +static const struct max20830_chip_info max20840c_chip = {
> +	.id_str = "MAX20840C",
> +	.id_length = 9,
> +};
>   
>   struct max20830_data {
>   	struct pmbus_driver_info info;
> @@ -60,11 +83,14 @@ static struct pmbus_driver_info max20830_info = {
>   
>   static int max20830_probe(struct i2c_client *client)
>   {
> +	const struct max20830_chip_info *chip;
>   	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
>   	struct max20830_data *data;
>   	struct gpio_desc *enable_gpio;
>   	int ret;
>   
> +	chip = i2c_get_match_data(client);
> +
>   	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
>   		return -ENOMEM;
> @@ -90,16 +116,14 @@ 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);
> +						    chip->id_length + 1, buf);
>   		if (ret < 0)
>   			return dev_err_probe(&client->dev, ret,
>   					     "Failed to read IC_DEVICE_ID\n");
> @@ -108,36 +132,40 @@ static int max20830_probe(struct i2c_client *client)
>   		 * match the format of i2c_smbus_read_block_data().
>   		 * Also adjust return value to reflect length byte removal.
>   		 */
> -		memmove(buf, buf + 1, MAX20830_IC_DEVICE_ID_LENGTH);
> +		memmove(buf, buf + 1, chip->id_length);
>   		ret = ret - 1;
>   	}
>   
> -	/*
> -	 * MAX20830 IC_DEVICE_ID sends string data "MAX20830\0".
> -	 * Return value should at least be 9 bytes of data.
> -	 */
> -	if (ret < MAX20830_IC_DEVICE_ID_LENGTH)
> +	/* Verify we read the expected number of bytes */
> +	if (ret < chip->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",
> +				     chip->id_length, ret);
> +
> +	/* Null-terminate the string */
> +	buf[chip->id_length] = '\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 (strncmp(buf, chip->id_str, chip->id_length))
>   		return dev_err_probe(&client->dev, -ENODEV,
> -				     "Unsupported device: '%s'\n", buf);
> +				     "Device mismatch: expected '%s', got '%s'\n",
> +				     chip->id_str, buf);
>   
> -	return pmbus_do_probe(client, &max20830_info);
> +	return pmbus_do_probe(client, &data->info);
>   }
>   
>   static const struct i2c_device_id max20830_id[] = {
> -	{"max20830"},
> +	{ "max20830", (kernel_ulong_t)&max20830_chip },
> +	{ "max20830c", (kernel_ulong_t)&max20830c_chip },
> +	{ "max20840c", (kernel_ulong_t)&max20840c_chip },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, max20830_id);
>   
>   static const struct of_device_id max20830_of_match[] = {
> -	{ .compatible = "adi,max20830" },
> +	{ .compatible = "adi,max20830", .data = &max20830_chip },
> +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
> +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },

"adi,max20830" is a fallback for the other two chips, but that
is not how the code is implemented.

Guenter

>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, max20830_of_match);
> 

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

[Torreno, Alexis Czezar]

> >
> >   static const struct of_device_id max20830_of_match[] = {
> > -	{ .compatible = "adi,max20830" },
> > +	{ .compatible = "adi,max20830", .data = &max20830_chip },
> > +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
> > +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
> 
> "adi,max20830" is a fallback for the other two chips, but that is not how the
> code is implemented.
> 

I may be inclined to just not use fallback as it seems to be more complicated
and a bit unnecessary. There's also other devices that may be added on top
of this so it lessens the complexity. Will edit the bindings regarding this.

Regards,
Alexis

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

[Guenter Roeck]

On 6/30/26 01:07, Torreno, Alexis Czezar wrote:
> 
>>>
>>>    static const struct of_device_id max20830_of_match[] = {
>>> -	{ .compatible = "adi,max20830" },
>>> +	{ .compatible = "adi,max20830", .data = &max20830_chip },
>>> +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
>>> +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
>>
>> "adi,max20830" is a fallback for the other two chips, but that is not how the
>> code is implemented.
>>
> 
> I may be inclined to just not use fallback as it seems to be more complicated
> and a bit unnecessary. There's also other devices that may be added on top
> of this so it lessens the complexity. Will edit the bindings regarding this.
> 

You are missing the point. The fallback is perfectly valid. Technically
the driver does not even need to support adi,max20830c and adi,max20840c.
That is only used to validate that the chip is supported. That does not
need a separate devicetree entry.

Guenter

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

[Torreno, Alexis Czezar]

> >>>
> >>>    static const struct of_device_id max20830_of_match[] = {
> >>> -	{ .compatible = "adi,max20830" },
> >>> +	{ .compatible = "adi,max20830", .data = &max20830_chip },
> >>> +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
> >>> +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
> >>
> >> "adi,max20830" is a fallback for the other two chips, but that is not
> >> how the code is implemented.
> >>
> >
> > I may be inclined to just not use fallback as it seems to be more
> > complicated and a bit unnecessary. There's also other devices that may
> > be added on top of this so it lessens the complexity. Will edit the bindings
> regarding this.
> >
> 
> You are missing the point. The fallback is perfectly valid. Technically the driver
> does not even need to support adi,max20830c and adi,max20840c.
> That is only used to validate that the chip is supported. That does not need a
> separate devicetree entry.
> 

Checking if I understand this. I need to add a check that if DT chip->id = max20830,
and if the HW returns string of either nmax20830c or max20840c, device still probes

if ((buf == "max20830c" || buf  == "max20840c) && chip->id != "max20830""
        // then it's an error...
}



and

"That does not need a separate devicetree entry."

does this mean I shouldn't add to the bindings and max20830_id[]/max20830_of_match[]
entries, and keep it as is that it only takes max20830? 

To add support for the 830c and 840c variants, the code discussed above is enough?

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

[Guenter Roeck]

On 7/1/26 19:50, Torreno, Alexis Czezar wrote:
> 
>>>>>
>>>>>     static const struct of_device_id max20830_of_match[] = {
>>>>> -	{ .compatible = "adi,max20830" },
>>>>> +	{ .compatible = "adi,max20830", .data = &max20830_chip },
>>>>> +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
>>>>> +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
>>>>
>>>> "adi,max20830" is a fallback for the other two chips, but that is not
>>>> how the code is implemented.
>>>>
>>>
>>> I may be inclined to just not use fallback as it seems to be more
>>> complicated and a bit unnecessary. There's also other devices that may
>>> be added on top of this so it lessens the complexity. Will edit the bindings
>> regarding this.
>>>
>>
>> You are missing the point. The fallback is perfectly valid. Technically the driver
>> does not even need to support adi,max20830c and adi,max20840c.
>> That is only used to validate that the chip is supported. That does not need a
>> separate devicetree entry.
>>
> 
> Checking if I understand this. I need to add a check that if DT chip->id = max20830,
> and if the HW returns string of either nmax20830c or max20840c, device still probes
> 
> if ((buf == "max20830c" || buf  == "max20840c) && chip->id != "max20830""
>          // then it's an error...
> }
> 

Technically, you don't even have to check the device ID in the first place.
You could drop all the device ID checking code.

Checking the device ID is just a safety feature that ensure that the
physical chip is supported by the driver. You could do something similar
to the max31785 driver, i.e., read the device ID and do a length check and
hard-coded string comparison. Separate compatible entries are not needed
at all.

Something like

	buf[ret] = '\0';
	if (ret != MAX20830_IC_DEVICE_ID_LENGTH ||
	    (strcmp(buf, "MAX20830") &&
	     strcmp(buf, "MAX20830C") &&
	     strcmp(buf, "MAX20840C"))
		return dev_err_probe(&client->dev, -ENODEV,
                                      "Unsupported device: '%*pE'\n", buf);

should do (using %*pE because the unknown device might not necessarily
have printable characters).

Guenter

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

[Torreno, Alexis Czezar]

> >>>>>
> >>>>>     static const struct of_device_id max20830_of_match[] = {
> >>>>> -	{ .compatible = "adi,max20830" },
> >>>>> +	{ .compatible = "adi,max20830", .data = &max20830_chip },
> >>>>> +	{ .compatible = "adi,max20830c", .data = &max20830c_chip },
> >>>>> +	{ .compatible = "adi,max20840c", .data = &max20840c_chip },
> >>>>
> >>>> "adi,max20830" is a fallback for the other two chips, but that is
> >>>> not how the code is implemented.
> >>>>
> >>>
> >>> I may be inclined to just not use fallback as it seems to be more
> >>> complicated and a bit unnecessary. There's also other devices that
> >>> may be added on top of this so it lessens the complexity. Will edit
> >>> the bindings
> >> regarding this.
> >>>
> >>
> >> You are missing the point. The fallback is perfectly valid.
> >> Technically the driver does not even need to support adi,max20830c and
> adi,max20840c.
> >> That is only used to validate that the chip is supported. That does
> >> not need a separate devicetree entry.
> >>
> >
> > Checking if I understand this. I need to add a check that if DT
> > chip->id = max20830, and if the HW returns string of either nmax20830c
> > or max20840c, device still probes
> >
> > if ((buf == "max20830c" || buf  == "max20840c) && chip->id != "max20830""
> >          // then it's an error...
> > }
> >
> 
> Technically, you don't even have to check the device ID in the first place.
> You could drop all the device ID checking code.
> 
> Checking the device ID is just a safety feature that ensure that the physical chip
> is supported by the driver. You could do something similar to the max31785
> driver, i.e., read the device ID and do a length check and hard-coded string
> comparison. Separate compatible entries are not needed at all.
> 
> Something like
> 
> 	buf[ret] = '\0';
> 	if (ret != MAX20830_IC_DEVICE_ID_LENGTH ||
> 	    (strcmp(buf, "MAX20830") &&
> 	     strcmp(buf, "MAX20830C") &&
> 	     strcmp(buf, "MAX20840C"))
> 		return dev_err_probe(&client->dev, -ENODEV,
>                                       "Unsupported device: '%*pE'\n", buf);
> 
> should do (using %*pE because the unknown device might not necessarily have
> printable characters).
> 


I see, Ill base it on the max31785 and simplify it similar to code given. Thanks!

Regards,
Alexis