diff mbox series

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

Message ID 20240220-onboard_xvf3500-v4-6-dc1617cc5dd4@wolfvision.net (mailing list archive)
State Superseded
Headers show
Series usb: misc: onboard_hub: add support for XMOS XVF3500 | expand

Commit Message

Javier Carrasco Feb. 20, 2024, 2:05 p.m. UTC
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@wolfvision.net>
---
 drivers/usb/misc/onboard_usb_dev.c | 54 ++++++++++++++++++++------------------
 drivers/usb/misc/onboard_usb_dev.h | 23 ++++++++++++++++
 2 files changed, 52 insertions(+), 25 deletions(-)

Comments

Matthias Kaehlcke Feb. 21, 2024, 8:25 p.m. UTC | #1
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@wolfvision.net>
> ---
>  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,
>  };
>
> ...
Javier Carrasco Feb. 21, 2024, 8:40 p.m. UTC | #2
On 21.02.24 21:25, Matthias Kaehlcke wrote:
> 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@wolfvision.net>
>> ---
>>  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.
> 

That looks much cleaner, I will apply it to the next version.

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

I am not completely sure about this. Someone could increase MAX_SUPPLIES
without adding a generic name. 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.

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.

>> +
>>  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,
>>  };
>>
>> ...

Thanks again for your feedback.

Best regards,
Javier Carrasco
Matthias Kaehlcke Feb. 21, 2024, 9:18 p.m. UTC | #3
Hi Javier,

On Wed, Feb 21, 2024 at 09:40:38PM +0100, Javier Carrasco wrote:
> On 21.02.24 21:25, Matthias Kaehlcke wrote:
> > 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@wolfvision.net>
> >> ---
> >>  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.
> > 
> 
> That looks much cleaner, I will apply it to the next version.
> 
> >> 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.
> > 
> 
> 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.
Javier Carrasco Feb. 21, 2024, 9:33 p.m. UTC | #4
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
Matthias Kaehlcke Feb. 21, 2024, 9:46 p.m. UTC | #5
On Wed, Feb 21, 2024 at 10:33:53PM +0100, Javier Carrasco wrote:
> 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.

Sounds good to me. Another similar option would be to assign
'generic_supply_names' to '.supply_names'. I don't have a strong
preference.
Matthias Kaehlcke Feb. 27, 2024, 6:12 p.m. UTC | #6
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@wolfvision.net>

For v5 you could consider making this [1/8] (i.e. before the renaming
of the driver). That way support for new hubs doesn't necessarily have
to wait for the entire series to land. Since this series is underway
I think new bindings shouldn't use 'vdd-supply' but the device
specific name of the supply.
diff mbox series

Patch

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;
+
+	for (i = 0; i < onboard_dev->pdata->num_supplies; i++)
+		onboard_dev->supplies[i].supply = supply_names[i];
+
+	err = devm_regulator_bulk_get(dev, onboard_dev->pdata->num_supplies,
+				      onboard_dev->supplies);
+	if (err)
+		dev_err(dev, "Failed to get regulator supplies: %pe\n",
+			ERR_PTR(err));
+
+	return err;
+}
+
 static int onboard_dev_power_on(struct onboard_dev *onboard_dev)
 {
 	int err;
@@ -266,7 +281,6 @@  static int onboard_dev_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct onboard_dev *onboard_dev;
-	unsigned int i;
 	int err;
 
 	onboard_dev = devm_kzalloc(dev, sizeof(*onboard_dev), GFP_KERNEL);
@@ -277,19 +291,9 @@  static int onboard_dev_probe(struct platform_device *pdev)
 	if (!onboard_dev->pdata)
 		return -EINVAL;
 
-	if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES)
-		return dev_err_probe(dev, -EINVAL, "max %zu supplies supported!\n",
-				     MAX_SUPPLIES);
-
-	for (i = 0; i < onboard_dev->pdata->num_supplies; i++)
-		onboard_dev->supplies[i].supply = supply_names[i];
-
-	err = devm_regulator_bulk_get(dev, onboard_dev->pdata->num_supplies,
-				      onboard_dev->supplies);
-	if (err) {
-		dev_err(dev, "Failed to get regulator supplies: %pe\n", ERR_PTR(err));
+	err = onboard_dev_get_regulator_bulk(dev, onboard_dev);
+	if (err)
 		return err;
-	}
 
 	onboard_dev->clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(onboard_dev->clk))
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)
+
 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,
 };
 
 static const struct onboard_dev_pdata microchip_usb5744_data = {
 	.reset_us = 0,
 	.num_supplies = 2,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata realtek_rts5411_data = {
 	.reset_us = 0,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata ti_tusb8041_data = {
 	.reset_us = 3000,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata cypress_hx3_data = {
 	.reset_us = 10000,
 	.num_supplies = 2,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata cypress_hx2vl_data = {
 	.reset_us = 1,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata genesys_gl850g_data = {
 	.reset_us = 3,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata genesys_gl852g_data = {
 	.reset_us = 50,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };
 
 static const struct onboard_dev_pdata vialab_vl817_data = {
 	.reset_us = 10,
 	.num_supplies = 1,
+	.supply_names = { NULL },
 	.is_hub = true,
 };