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

[Torreno, Alexis Czezar]

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

For some reason they didn't ask me to add MAX20840, and I did double check with them.

Code wise, I ended up with this. (I guess am trying to avoid mailing a v6)

...
static const char * const supported_chip_ids[] = {
	"MAX20830",
	"MAX20830C",
	"MAX20840C",
};
...
	for (i = 0; i < ARRAY_SIZE(supported_chip_ids); i++) {
		if (!strcmp(buf, supported_chip_ids[i]))
			goto id_ok;
	}

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

id_ok:
... (probe after)

Thank you,
Alexis