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

Guenter Roeck <[email protected]> Tue, 28 Jul 2026 07:25:22 -0700
Newsgroups org.kernel.vger.linux-hwmon,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 7/27/26 23:58, Torreno, Alexis Czezar wrote:
>>
>>>>
>>>> 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.
>>
> 
> Yeah the T is weird, but I did test the C variants and they do reply the 'c' char.
> 
>>> 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 ?
>>
> 
> Will think about how to make to scale the code when adding newer variants.
> 

Something like the following should do.

const char *supported_chips[] = {
	"MAX20830",
	"MAX20830C",
	"MAX20840",
	"MAX20840C"
};

...
	for (i = 0, supported = false; i < ARRAY_SIZE(supported_chips) && !supported; i++)
		supported = !strcmp(buf, supported_chips[i]);

	if (!supported)
		return dev_err_probe(&client->dev, -ENODEV,
				     "Unsupported device: '%*pE'\n", ret, buf);

Then all you'd need to do to add more chips would be to add the strings to
the supported_chips[] array.

Thanks,
Guenter