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

From: Guenter Roeck

Date: Tue Jul 28 2026 - 01:43:02 EST

On 7/27/26 22:05, Torreno, Alexis Czezar wrote:

[External]

On 7/27/26 20:32, 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.


Are those chips still not published ? I find MAX20840T, but no "C" variants.
And MAX20840T presumably has an I2C device ID of "MAX20840", not
"MAX20840C".

I also noticed that MAX20810 and MAX20815 seem to be register compatible.


I believe so yes, they aren't yet.


I just hope they are really compatible, and that the device ID strings really
include the "C". The "T" variants seem to have no T in the device ID string,
making it a bit odd that it was (or will be) added for the C variants.

Actually MAX20810/815 and a few more are next in line after this. A different
person is handling it, but they're waiting on how this patches go.


You are making yourself more work than necessary. Knowing that there are more
chips coming, the sequence of strcmp() is not really that desirable anymore.
It might make sense to create an array with all chips supported by the driver
instead of adding up strcmp sequences. That would make it much easier to add
support for new variants.

There also seems to be a MAX20830T. Does it actually make sense to list the
variants (C/T) in the first place ?

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

diff --git a/Documentation/hwmon/max20830.rst
b/Documentation/hwmon/max20830.rst
index

936e409dcc5c0898dde27d782308d4a7e1357e73..b850f3b6e40d1f1d0cec944be
40a
f02265aced59 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/ma
x20830.pdf

+ * Analog Devices MAX20830C
+
+ Prefix: 'max20830c'
+
+ Addresses scanned: -
+
+ Datasheet:
+
+ * Analog Devices MAX20840C
+
+ Prefix: 'max20840c'
+
+ Addresses scanned: -
+
+ Datasheet:
+
Author:

- Alexis Czezar Torreno <alexisczezar.torreno@xxxxxxxxxx> @@
-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

7c1e94c43ac4e5d1b4f69dc77b02e424a674e908..029733a377123ae6a861841d
b4a4
0dd540f1149e 100644
--- a/drivers/hwmon/pmbus/max20830.c
+++ b/drivers/hwmon/pmbus/max20830.c
@@ -99,13 +99,12 @@ static int max20830_probe(struct i2c_client *client)
* which do not support SMBus block reads.
*/
if (i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
- /* Reads 9 Data bytes from MAX20830 */
ret = i2c_smbus_read_block_data(client,
PMBUS_IC_DEVICE_ID, buf);
if (ret < 0)
return dev_err_probe(&client->dev, ret,
"Failed to read IC_DEVICE_ID\n");
} else {
- /* Reads 1 length byte + 9 Data bytes from MAX20830 */
+ /* Reads 1 length byte + data bytes */
ret = i2c_smbus_read_i2c_block_data(client,
PMBUS_IC_DEVICE_ID,

MAX20830_IC_DEVICE_ID_LENGTH + 1,
buf);
@@ -121,26 +120,28 @@ static int max20830_probe(struct i2c_client
*client)
ret = ret - 1;
}

- /*
- * MAX20830 IC_DEVICE_ID sends string data "MAX20830\0".
- * Return value should at least be 9 bytes of data.
- */
+ /* Verify we read the expected number of bytes */
if (ret < MAX20830_IC_DEVICE_ID_LENGTH)
return dev_err_probe(&client->dev, -ENODEV,
- "IC_DEVICE_ID too short: expected at least
9 bytes, got %d\n",
- ret);
+ "IC_DEVICE_ID too short: expected %d
bytes, got %d\n",
+ MAX20830_IC_DEVICE_ID_LENGTH, ret);
+
+ /* Null-terminate the string */
+ buf[ret] = '\0';

- /* 9 bytes of data, buf[0]-buf[7] = "MAX20830", buf[8] = '\0' */
- buf[MAX20830_IC_DEVICE_ID_LENGTH - 1] = '\0';
- if (strncmp(buf, "MAX20830", MAX20830_IC_DEVICE_ID_LENGTH - 1))
+ /* Verify the device ID matches what we expect */
+ if ((strcmp(buf, "MAX20830") && strcmp(buf, "MAX20830C") &&
+ strcmp(buf, "MAX20840C")))
return dev_err_probe(&client->dev, -ENODEV,
- "Unsupported device: '%s'\n", buf);
+ "Unsupported device: '%*pE'\n", ret, buf);

return pmbus_do_probe(client, &data->info);
}

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

I don't think that is needed or warranted, given that there is no matching
devicetree node.


I see, will remove.

Same for the prefix string in the documentation. You might want to mention the
supported chips, though, in both the configuration and Kconfig. In Kconfig,
that could be a generic such as "MAX20830 and compatible chips", or list the
base variants, such as "MAX20830, MAX20840 and variants".

Thanks,
Guenter