| Message ID | 20240228-onboard_xvf3500-v5-6-76b805fd3fe6@wolfvision.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: misc: onboard_hub: add support for XMOS XVF3500 | expand |
On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: > Most of the functionality this driver provides can be used by non-hub > devices as well. > > To account for the hub-specific code, add a flag to the device data > structure and check its value for hub-specific code. > > The 'always_powered_in_supend' attribute is only available for hub > devices, keeping the driver's default behavior for non-hub devices (keep > on in suspend). > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > --- > drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- > drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ > 2 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > index e1779bd2d126..df0ed172c7ec 100644 > --- a/drivers/usb/misc/onboard_usb_dev.c > +++ b/drivers/usb/misc/onboard_usb_dev.c > @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) > struct usbdev_node *node; > bool power_off = true; > > - if (onboard_dev->always_powered_in_suspend) > + if (onboard_dev->always_powered_in_suspend && > + !onboard_dev->pdata->is_hub) > return 0; With this non-hub devices would always be powered down, since 'always_powerd_in_suspend' is not set for them. This should be: if (!onboard_dev->pdata->is_hub || onboard_dev->always_powered_in_suspend) Checking for the (non-)hub status first is clearer IMO, also it avoids an unneccessary check of 'always_powered' for non-hub devices. Without code context: for hubs there can be multiple device tree nodes for the same physical hub chip (e.g. one for the USB2 and another for the USB3 part). I suppose this could also be the case for non-hub devices. For hubs there is the 'peer-hub' device tree property to establish a link between the two USB devices, as a result the onboard driver only creates a single platform device (which is desired, otherwise two platform devices would be in charge for power sequencing the same phyiscal device. For non-hub devices there is currently no such link. In many cases I expect there will be just one DT entry even though the device has multiple USB interfaces, but it could happen and would actually be a more accurate representation. General support is already there (the code dealing with 'peer-hub'), but we'd have to come up with a suitable name. 'peer-device' is the first thing that comes to my mind, but there might be better options. If such a generic property is added then we should deprecate 'peer-hub', but maintain backwards compatibility.
On 28.02.24 19:10, Matthias Kaehlcke wrote: > On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: >> Most of the functionality this driver provides can be used by non-hub >> devices as well. >> >> To account for the hub-specific code, add a flag to the device data >> structure and check its value for hub-specific code. >> >> The 'always_powered_in_supend' attribute is only available for hub >> devices, keeping the driver's default behavior for non-hub devices (keep >> on in suspend). >> >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> >> --- >> drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- >> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ >> 2 files changed, 33 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c >> index e1779bd2d126..df0ed172c7ec 100644 >> --- a/drivers/usb/misc/onboard_usb_dev.c >> +++ b/drivers/usb/misc/onboard_usb_dev.c >> @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) >> struct usbdev_node *node; >> bool power_off = true; >> >> - if (onboard_dev->always_powered_in_suspend) >> + if (onboard_dev->always_powered_in_suspend && >> + !onboard_dev->pdata->is_hub) >> return 0; > > With this non-hub devices would always be powered down, since > 'always_powerd_in_suspend' is not set for them. This should be: > May I ask you what you meant in v4 with this comment? > Even without the sysfs attribute the field 'always_powered_in_suspend' > could > be set to true by probe() for non-hub devices. > if (!onboard_dev->pdata->is_hub || > onboard_dev->always_powered_in_suspend) > > Checking for the (non-)hub status first is clearer IMO, also it avoids > an unneccessary check of 'always_powered' for non-hub devices. > That makes sense and will be fixed. > Without code context: for hubs there can be multiple device tree nodes > for the same physical hub chip (e.g. one for the USB2 and another for > the USB3 part). I suppose this could also be the case for non-hub > devices. For hubs there is the 'peer-hub' device tree property to > establish a link between the two USB devices, as a result the onboard > driver only creates a single platform device (which is desired, > otherwise two platform devices would be in charge for power sequencing > the same phyiscal device. For non-hub devices there is currently no such > link. In many cases I expect there will be just one DT entry even though > the device has multiple USB interfaces, but it could happen and would > actually be a more accurate representation. > > General support is already there (the code dealing with 'peer-hub'), but > we'd have to come up with a suitable name. 'peer-device' is the first > thing that comes to my mind, but there might be better options. If such > a generic property is added then we should deprecate 'peer-hub', but > maintain backwards compatibility. I have nothing against that, but the first non-hub device that will be added does not have multiple DT nodes, so I have nothing to test that extension with real hardware. That could be added in the future, though, if the need ever arises. Best regards, Javier Carrasco
On Wed, Feb 28, 2024 at 09:21:00PM +0100, Javier Carrasco wrote: > On 28.02.24 19:10, Matthias Kaehlcke wrote: > > On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: > >> Most of the functionality this driver provides can be used by non-hub > >> devices as well. > >> > >> To account for the hub-specific code, add a flag to the device data > >> structure and check its value for hub-specific code. > >> > >> The 'always_powered_in_supend' attribute is only available for hub > >> devices, keeping the driver's default behavior for non-hub devices (keep > >> on in suspend). > >> > >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > >> --- > >> drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- > >> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ > >> 2 files changed, 33 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > >> index e1779bd2d126..df0ed172c7ec 100644 > >> --- a/drivers/usb/misc/onboard_usb_dev.c > >> +++ b/drivers/usb/misc/onboard_usb_dev.c > >> @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) > >> struct usbdev_node *node; > >> bool power_off = true; > >> > >> - if (onboard_dev->always_powered_in_suspend) > >> + if (onboard_dev->always_powered_in_suspend && > >> + !onboard_dev->pdata->is_hub) > >> return 0; > > > > With this non-hub devices would always be powered down, since > > 'always_powerd_in_suspend' is not set for them. This should be: > > > > May I ask you what you meant in v4 with this comment? > > > Even without the sysfs attribute the field 'always_powered_in_suspend' > > could > > be set to true by probe() for non-hub devices. struct onboard_dev always has the field 'always_powered_in_suspend', even for non-hubs, that don't have the corresponding sysfs attribute. Currently it is left uninitialized (i.e. false) for non-hubs. Instead it could be initialized to true by probe() for non-hubs, which would be semantically correct. With that it wouldn't be necessary to check here whether a device is hub, because the field would provide the necessary information. > > if (!onboard_dev->pdata->is_hub || > > onboard_dev->always_powered_in_suspend) > > > > Checking for the (non-)hub status first is clearer IMO, also it avoids > > an unneccessary check of 'always_powered' for non-hub devices. > > > > That makes sense and will be fixed. > > > Without code context: for hubs there can be multiple device tree nodes > > for the same physical hub chip (e.g. one for the USB2 and another for > > the USB3 part). I suppose this could also be the case for non-hub > > devices. For hubs there is the 'peer-hub' device tree property to > > establish a link between the two USB devices, as a result the onboard > > driver only creates a single platform device (which is desired, > > otherwise two platform devices would be in charge for power sequencing > > the same phyiscal device. For non-hub devices there is currently no such > > link. In many cases I expect there will be just one DT entry even though > > the device has multiple USB interfaces, but it could happen and would > > actually be a more accurate representation. > > > > General support is already there (the code dealing with 'peer-hub'), but > > we'd have to come up with a suitable name. 'peer-device' is the first > > thing that comes to my mind, but there might be better options. If such > > a generic property is added then we should deprecate 'peer-hub', but > > maintain backwards compatibility. > > I have nothing against that, but the first non-hub device that will be > added does not have multiple DT nodes, so I have nothing to test that > extension with real hardware. I see, the XVF3500 is USB 2.0 only, so it isn't suitable for testing. > That could be added in the future, though, if the need ever arises. I expect it will, when a DT maintainer asks the hardware to be represented correctly for a device that is connected to more than one USB bus. IIRC that's how 'peer-hub' was born :) Ok, we can leave it out for now. I might send a dedicated patch after your series landed. If a switch to 'peer-device' or similar is anticipated then it's probably best to deprecate 'peer-hub' ASAP, to avoid it from getting added to more bindings.
On 28.02.24 21:41, Matthias Kaehlcke wrote: > On Wed, Feb 28, 2024 at 09:21:00PM +0100, Javier Carrasco wrote: >> On 28.02.24 19:10, Matthias Kaehlcke wrote: >>> On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: >>>> Most of the functionality this driver provides can be used by non-hub >>>> devices as well. >>>> >>>> To account for the hub-specific code, add a flag to the device data >>>> structure and check its value for hub-specific code. >>>> >>>> The 'always_powered_in_supend' attribute is only available for hub >>>> devices, keeping the driver's default behavior for non-hub devices (keep >>>> on in suspend). >>>> >>>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> >>>> --- >>>> drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- >>>> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ >>>> 2 files changed, 33 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c >>>> index e1779bd2d126..df0ed172c7ec 100644 >>>> --- a/drivers/usb/misc/onboard_usb_dev.c >>>> +++ b/drivers/usb/misc/onboard_usb_dev.c >>>> @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) >>>> struct usbdev_node *node; >>>> bool power_off = true; >>>> >>>> - if (onboard_dev->always_powered_in_suspend) >>>> + if (onboard_dev->always_powered_in_suspend && >>>> + !onboard_dev->pdata->is_hub) >>>> return 0; >>> >>> With this non-hub devices would always be powered down, since >>> 'always_powerd_in_suspend' is not set for them. This should be: >>> >> >> May I ask you what you meant in v4 with this comment? >> >>> Even without the sysfs attribute the field 'always_powered_in_suspend' >>> could >>> be set to true by probe() for non-hub devices. > > struct onboard_dev always has the field 'always_powered_in_suspend', > even for non-hubs, that don't have the corresponding sysfs attribute. > Currently it is left uninitialized (i.e. false) for non-hubs. Instead > it could be initialized to true by probe() for non-hubs, which would > be semantically correct. With that it wouldn't be necessary to check > here whether a device is hub, because the field would provide the > necessary information. > That is maybe what is confusing me a bit. Should it not be false for non-hub devices? That property is only meant for hubs, so why should non-hub devices be always powered in suspend? I thought it should always be false for non-hub devices, and configurable for hubs. >>> if (!onboard_dev->pdata->is_hub || >>> onboard_dev->always_powered_in_suspend) >>> >>> Checking for the (non-)hub status first is clearer IMO, also it avoids >>> an unneccessary check of 'always_powered' for non-hub devices. >>> >> >> That makes sense and will be fixed. >> >>> Without code context: for hubs there can be multiple device tree nodes >>> for the same physical hub chip (e.g. one for the USB2 and another for >>> the USB3 part). I suppose this could also be the case for non-hub >>> devices. For hubs there is the 'peer-hub' device tree property to >>> establish a link between the two USB devices, as a result the onboard >>> driver only creates a single platform device (which is desired, >>> otherwise two platform devices would be in charge for power sequencing >>> the same phyiscal device. For non-hub devices there is currently no such >>> link. In many cases I expect there will be just one DT entry even though >>> the device has multiple USB interfaces, but it could happen and would >>> actually be a more accurate representation. >>> >>> General support is already there (the code dealing with 'peer-hub'), but >>> we'd have to come up with a suitable name. 'peer-device' is the first >>> thing that comes to my mind, but there might be better options. If such >>> a generic property is added then we should deprecate 'peer-hub', but >>> maintain backwards compatibility. >> >> I have nothing against that, but the first non-hub device that will be >> added does not have multiple DT nodes, so I have nothing to test that >> extension with real hardware. > > I see, the XVF3500 is USB 2.0 only, so it isn't suitable for testing. > >> That could be added in the future, though, if the need ever arises. > > I expect it will, when a DT maintainer asks the hardware to be > represented correctly for a device that is connected to more than one USB > bus. IIRC that's how 'peer-hub' was born :) > > Ok, we can leave it out for now. I might send a dedicated patch after your > series landed. If a switch to 'peer-device' or similar is anticipated then > it's probably best to deprecate 'peer-hub' ASAP, to avoid it from getting > added to more bindings. Best regards, Javier Carrasco
On Wed, Feb 28, 2024 at 09:50:22PM +0100, Javier Carrasco wrote: > On 28.02.24 21:41, Matthias Kaehlcke wrote: > > On Wed, Feb 28, 2024 at 09:21:00PM +0100, Javier Carrasco wrote: > >> On 28.02.24 19:10, Matthias Kaehlcke wrote: > >>> On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: > >>>> Most of the functionality this driver provides can be used by non-hub > >>>> devices as well. > >>>> > >>>> To account for the hub-specific code, add a flag to the device data > >>>> structure and check its value for hub-specific code. > >>>> > >>>> The 'always_powered_in_supend' attribute is only available for hub > >>>> devices, keeping the driver's default behavior for non-hub devices (keep > >>>> on in suspend). > >>>> > >>>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > >>>> --- > >>>> drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- > >>>> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ > >>>> 2 files changed, 33 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > >>>> index e1779bd2d126..df0ed172c7ec 100644 > >>>> --- a/drivers/usb/misc/onboard_usb_dev.c > >>>> +++ b/drivers/usb/misc/onboard_usb_dev.c > >>>> @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) > >>>> struct usbdev_node *node; > >>>> bool power_off = true; > >>>> > >>>> - if (onboard_dev->always_powered_in_suspend) > >>>> + if (onboard_dev->always_powered_in_suspend && > >>>> + !onboard_dev->pdata->is_hub) > >>>> return 0; > >>> > >>> With this non-hub devices would always be powered down, since > >>> 'always_powerd_in_suspend' is not set for them. This should be: > >>> > >> > >> May I ask you what you meant in v4 with this comment? > >> > >>> Even without the sysfs attribute the field 'always_powered_in_suspend' > >>> could > >>> be set to true by probe() for non-hub devices. > > > > struct onboard_dev always has the field 'always_powered_in_suspend', > > even for non-hubs, that don't have the corresponding sysfs attribute. > > Currently it is left uninitialized (i.e. false) for non-hubs. Instead > > it could be initialized to true by probe() for non-hubs, which would > > be semantically correct. With that it wouldn't be necessary to check > > here whether a device is hub, because the field would provide the > > necessary information. > > > > That is maybe what is confusing me a bit. Should it not be false for > non-hub devices? That property is only meant for hubs, so why should > non-hub devices be always powered in suspend? I thought it should always > be false for non-hub devices, and configurable for hubs. I suspect the confusion stems from the sysfs attribute 'always_powered_...' vs. the struct field with the same name. The sysfs attribute defaults to 'false', which results in USB devices being powered down in suspend. That was the desired behavior for a device I was working on when I implemented this driver, but in hindsight I think the default should have been 'true'. We agreed that non-hub devices shouldn't be powered down in suspend. It would be unexpected for users and could have side effects like delays or losing status. Since (I think) we can't change the default of the attribute we said we'd limit it to hubs, and not create it for other types of USB devices. Other USB devices would remain powered during system suspend. Are we in agreement up to this point, in particular that non-hub devices should remain powered? struct onboard_dev has the field 'always_powered_...', which in the existing code is *always* associated with the sysfs attribute of the same name. But there is no reason to not to use the field when the sysfs attribute does not exist. For any device at any given time the field could indicate whether the device should be remain powered during suspend. For hubs the value can be changed at runtime through the sysfs attribute, for non-hubs it would be static and always indicate that the device should remain powered. Does that clarify your doubts?
On 28.02.24 22:34, Matthias Kaehlcke wrote: > On Wed, Feb 28, 2024 at 09:50:22PM +0100, Javier Carrasco wrote: >> On 28.02.24 21:41, Matthias Kaehlcke wrote: >>> On Wed, Feb 28, 2024 at 09:21:00PM +0100, Javier Carrasco wrote: >>>> On 28.02.24 19:10, Matthias Kaehlcke wrote: >>>>> On Wed, Feb 28, 2024 at 02:51:33PM +0100, Javier Carrasco wrote: >>>>>> Most of the functionality this driver provides can be used by non-hub >>>>>> devices as well. >>>>>> >>>>>> To account for the hub-specific code, add a flag to the device data >>>>>> structure and check its value for hub-specific code. >>>>>> >>>>>> The 'always_powered_in_supend' attribute is only available for hub >>>>>> devices, keeping the driver's default behavior for non-hub devices (keep >>>>>> on in suspend). >>>>>> >>>>>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> >>>>>> --- >>>>>> drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- >>>>>> drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ >>>>>> 2 files changed, 33 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c >>>>>> index e1779bd2d126..df0ed172c7ec 100644 >>>>>> --- a/drivers/usb/misc/onboard_usb_dev.c >>>>>> +++ b/drivers/usb/misc/onboard_usb_dev.c >>>>>> @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) >>>>>> struct usbdev_node *node; >>>>>> bool power_off = true; >>>>>> >>>>>> - if (onboard_dev->always_powered_in_suspend) >>>>>> + if (onboard_dev->always_powered_in_suspend && >>>>>> + !onboard_dev->pdata->is_hub) >>>>>> return 0; >>>>> >>>>> With this non-hub devices would always be powered down, since >>>>> 'always_powerd_in_suspend' is not set for them. This should be: >>>>> >>>> >>>> May I ask you what you meant in v4 with this comment? >>>> >>>>> Even without the sysfs attribute the field 'always_powered_in_suspend' >>>>> could >>>>> be set to true by probe() for non-hub devices. >>> >>> struct onboard_dev always has the field 'always_powered_in_suspend', >>> even for non-hubs, that don't have the corresponding sysfs attribute. >>> Currently it is left uninitialized (i.e. false) for non-hubs. Instead >>> it could be initialized to true by probe() for non-hubs, which would >>> be semantically correct. With that it wouldn't be necessary to check >>> here whether a device is hub, because the field would provide the >>> necessary information. >>> >> >> That is maybe what is confusing me a bit. Should it not be false for >> non-hub devices? That property is only meant for hubs, so why should >> non-hub devices be always powered in suspend? I thought it should always >> be false for non-hub devices, and configurable for hubs. > > I suspect the confusion stems from the sysfs attribute 'always_powered_...' > vs. the struct field with the same name. > > The sysfs attribute defaults to 'false', which results in USB devices > being powered down in suspend. That was the desired behavior for a device > I was working on when I implemented this driver, but in hindsight I think > the default should have been 'true'. > > We agreed that non-hub devices shouldn't be powered down in suspend. It > would be unexpected for users and could have side effects like delays > or losing status. Since (I think) we can't change the default of the > attribute we said we'd limit it to hubs, and not create it for other > types of USB devices. Other USB devices would remain powered during > system suspend. > > Are we in agreement up to this point, in particular that non-hub > devices should remain powered? > > struct onboard_dev has the field 'always_powered_...', which in the > existing code is *always* associated with the sysfs attribute of > the same name. But there is no reason to not to use the field when > the sysfs attribute does not exist. For any device at any given time > the field could indicate whether the device should be remain powered > during suspend. For hubs the value can be changed at runtime > through the sysfs attribute, for non-hubs it would be static and > always indicate that the device should remain powered. > > Does that clarify your doubts? It is crystal clear now, thank you. Best regards, Javier Carrasco
diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c index e1779bd2d126..df0ed172c7ec 100644 --- a/drivers/usb/misc/onboard_usb_dev.c +++ b/drivers/usb/misc/onboard_usb_dev.c @@ -132,7 +132,8 @@ static int __maybe_unused onboard_dev_suspend(struct device *dev) struct usbdev_node *node; bool power_off = true; - if (onboard_dev->always_powered_in_suspend) + if (onboard_dev->always_powered_in_suspend && + !onboard_dev->pdata->is_hub) return 0; mutex_lock(&onboard_dev->lock); @@ -262,7 +263,27 @@ static struct attribute *onboard_dev_attrs[] = { &dev_attr_always_powered_in_suspend.attr, NULL, }; -ATTRIBUTE_GROUPS(onboard_dev); + +static umode_t onboard_dev_attrs_are_visible(struct kobject *kobj, + struct attribute *attr, + int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct onboard_dev *onboard_dev = dev_get_drvdata(dev); + + if (attr == &dev_attr_always_powered_in_suspend.attr && + !onboard_dev->pdata->is_hub) + return 0; + + return attr->mode; +} + +static const struct attribute_group onboard_dev_group = { + .is_visible = onboard_dev_attrs_are_visible, + .attrs = onboard_dev_attrs, +}; +__ATTRIBUTE_GROUPS(onboard_dev); + static void onboard_dev_attach_usb_driver(struct work_struct *work) { diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h index 470736483cdf..106480ce72b5 100644 --- a/drivers/usb/misc/onboard_usb_dev.h +++ b/drivers/usb/misc/onboard_usb_dev.h @@ -12,60 +12,70 @@ struct onboard_dev_pdata { unsigned long reset_us; /* reset pulse width in us */ unsigned int num_supplies; /* number of supplies */ const char * const supply_names[MAX_SUPPLIES]; /* use the real names */ + bool is_hub; /* true if the device is a HUB */ }; static const struct onboard_dev_pdata microchip_usb424_data = { .reset_us = 1, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata microchip_usb5744_data = { .reset_us = 0, .num_supplies = 2, .supply_names = { "vdd", "vdd2" }, + .is_hub = true, }; static const struct onboard_dev_pdata realtek_rts5411_data = { .reset_us = 0, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata ti_tusb8041_data = { .reset_us = 3000, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata cypress_hx3_data = { .reset_us = 10000, .num_supplies = 2, .supply_names = { "vdd", "vdd2" }, + .is_hub = true, }; static const struct onboard_dev_pdata cypress_hx2vl_data = { .reset_us = 1, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata genesys_gl850g_data = { .reset_us = 3, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata genesys_gl852g_data = { .reset_us = 50, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct onboard_dev_pdata vialab_vl817_data = { .reset_us = 10, .num_supplies = 1, .supply_names = { "vdd" }, + .is_hub = true, }; static const struct of_device_id onboard_dev_match[] = {
Most of the functionality this driver provides can be used by non-hub devices as well. To account for the hub-specific code, add a flag to the device data structure and check its value for hub-specific code. The 'always_powered_in_supend' attribute is only available for hub devices, keeping the driver's default behavior for non-hub devices (keep on in suspend). Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> --- drivers/usb/misc/onboard_usb_dev.c | 25 +++++++++++++++++++++++-- drivers/usb/misc/onboard_usb_dev.h | 10 ++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-)