[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

[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

[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

[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

[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

[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

[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