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

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.

> Signed-off-by: Alexis Czezar Torreno <[email protected]>
> ---
>   Documentation/hwmon/max20830.rst | 27 ++++++++++++++++++++++-----
>   drivers/hwmon/pmbus/max20830.c   | 27 ++++++++++++++-------------
>   2 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/hwmon/max20830.rst b/Documentation/hwmon/max20830.rst
> index 936e409dcc5c0898dde27d782308d4a7e1357e73..b850f3b6e40d1f1d0cec944be40af02265aced59 100644
> --- a/Documentation/hwmon/max20830.rst
> +++ b/Documentation/hwmon/max20830.rst
> @@ -13,6 +13,22 @@ Supported chips:
>   
>       Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max20830.pdf
>   
> +  * Analog Devices MAX20830C
> +
> +    Prefix: 'max20830c'
> +
> +    Addresses scanned: -
> +
> +    Datasheet:
> +
> +  * Analog Devices MAX20840C
> +
> +    Prefix: 'max20840c'
> +
> +    Addresses scanned: -
> +
> +    Datasheet:
> +
>   Author:
>   
>     - Alexis Czezar Torreno <[email protected]>
> @@ -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..029733a377123ae6a861841db4a40dd540f1149e 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.

Thanks,
Guenter

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