On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote:
> The current mechanism uses generic names for the power supplies, which
> conflicts with proper name definitions in the device bindings.
>
> Add a per-device property to include real supply names and keep generic
> names as a fallback mechanism for backward compatibility.
>
> Signed-off-by: Javier Carrasco <javier.carrasco@xxxxxxxxxxxxxx>
> ---
> drivers/usb/misc/onboard_usb_dev.c | 54 ++++++++++++++++++++------------------
> drivers/usb/misc/onboard_usb_dev.h | 23 ++++++++++++++++
> 2 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
> index f43130a6786f..e66fcac93006 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
> @@ -29,18 +29,6 @@
>
> #include "onboard_usb_dev.h"
>
> -/*
> - * Use generic names, as the actual names might differ between devices. If a new
> - * device requires more than the currently supported supplies, add a new one
> - * here.
> - */
> -static const char * const supply_names[] = {
> - "vdd",
> - "vdd2",
> -};
> -
> -#define MAX_SUPPLIES ARRAY_SIZE(supply_names)
> -
> static void onboard_dev_attach_usb_driver(struct work_struct *work);
>
> static struct usb_device_driver onboard_dev_usbdev_driver;
> @@ -66,6 +54,33 @@ struct onboard_dev {
> struct clk *clk;
> };
>
> +static int onboard_dev_get_regulator_bulk(struct device *dev,
> + struct onboard_dev *onboard_dev)
> +{
> + unsigned int i;
> + int err;
> +
> + const char * const *supply_names = onboard_dev->pdata->supply_names;
> +
> + if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES)
> + return dev_err_probe(dev, -EINVAL, "max %zu supplies supported!\n",
> + MAX_SUPPLIES);
> +
> + if (!supply_names[0])
> + supply_names = generic_supply_names;
Please change to 'if (!supply_names)' and omit the initialization of
.supply_names for devices that use the generic supply names.
> diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h
> index ebe83e19d818..59dced6bd339 100644
> --- a/drivers/usb/misc/onboard_usb_dev.h
> +++ b/drivers/usb/misc/onboard_usb_dev.h
> @@ -6,63 +6,86 @@
> #ifndef _USB_MISC_ONBOARD_USB_DEV_H
> #define _USB_MISC_ONBOARD_USB_DEV_H
>
> +/*
> + * 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.
> +
> struct onboard_dev_pdata {
> unsigned long reset_us; /* reset pulse width in us */
> unsigned int num_supplies; /* number of supplies */
> bool is_hub;
> + const char * const supply_names[MAX_SUPPLIES];
> +
> };
>
> static const struct onboard_dev_pdata microchip_usb424_data = {
> .reset_us = 1,
> .num_supplies = 1,
> + .supply_names = { NULL },
> .is_hub = true,
> };
>
> ...
