lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 21 Feb 2024 22:33:53 +0100
From: Javier Carrasco <javier.carrasco@...fvision.net>
To: Matthias Kaehlcke <mka@...omium.org>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Helen Koike <helen.koike@...labora.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Russell King <linux@...linux.org.uk>, linux-sound@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-usb@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 6/8] usb: misc: onboard_dev: use device supply names

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ