Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

[Javier Carrasco]

On 21.02.24 22:18, Matthias Kaehlcke wrote:
>>>> +/*
>>>> + * Fallback supply names for backwards compatibility. If the device requires
>>>> + * more than the currently supported supplies, add a new one here, and if
>>>> + * possible, the real name supplies to the device-specific data.
>>>> + */
>>>> +static const char * const generic_supply_names[] = {
>>>> +	"vdd",
>>>> +	"vdd2",
>>>> +};
>>>> +
>>>> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names)
>>>
>>> This will have to change when support for a device with more than 2 non-generic
>>> supply names gets added. Please use a literal value for MAX_SUPPLIES instead of
>>> ARRAY_SIZE. If the literal is 2 it would still need to change for future devices
>>> with more supplies, but that change would be more straighforward.
>>>
>>
>> I am not completely sure about this. Someone could increase MAX_SUPPLIES
>> without adding a generic name.
> 
> That's perfectly fine and intended. MAX_SUPPLIES is a max, any list
> shorther than that is valid. Any longer list will result in probe()
> being aborted with a clear error message.
> 
>> Actually two modifications will be necessary for every addition (name
>> and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required,
>> and MAX_SUPPLIES is automatically increased.
> 
> As per above it's not necessary to add a new name when MAX_SUPPLIES is
> increased to support more non-generic names. It would only be necessary
> if more generic names were added, my understanding is that this
> should not happen because any newly supported onboard devices are
> supposed to use device specific supply names. I don't like to idea of
> adding unused pseudo supply names to the list, just for the sake of
> using ARRAY_SIZE.
> 
>> I understand that the whole point of this is getting rid of the generic
>> names, but we still have to provide generic names for every extra
>> supply, at least for code consistency and to avoid size mismatches
>> between real an generic supply names.
> 
> Please let me know if you still think the extra names are needed.

Not really, the only case I could come up is if an existing device that
uses generic names might end up requiring a third supply, which would
also be generic. But even such an unlikely event would be cover without
ARRAY_SIZE.

Actually one could argue that every existing device could have "vdd" and
"vdd2" as their supply names and remove checks and the generic array.

Best regards,
Javier Carrasco