2019-02-17 06:30:17

by Måns Rullgård

[permalink] [raw]
Subject: [PATCH] usb: core: skip interfaces disabled in devicetree

If an interface has an associated devicetree node with status disabled,
do not register the device. This is useful for boards with a built-in
multifunction USB device where some functions are broken or otherwise
undesired.

Signed-off-by: Mans Rullgard <[email protected]>
---
drivers/usb/core/message.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index bfa5eda0cc26..6b45d4835e41 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -2007,6 +2007,10 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
for (i = 0; i < nintf; ++i) {
struct usb_interface *intf = cp->interface[i];

+ if (intf->dev.of_node &&
+ !of_device_is_available(intf->dev.of_node))
+ continue;
+
dev_dbg(&dev->dev,
"adding %s (config #%d, interface %d)\n",
dev_name(&intf->dev), configuration,
--
2.20.1



2019-02-19 13:12:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: core: skip interfaces disabled in devicetree

On Sat, Feb 16, 2019 at 05:04:52PM +0000, Mans Rullgard wrote:
> If an interface has an associated devicetree node with status disabled,
> do not register the device. This is useful for boards with a built-in
> multifunction USB device where some functions are broken or otherwise
> undesired.
>
> Signed-off-by: Mans Rullgard <[email protected]>
> ---
> drivers/usb/core/message.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index bfa5eda0cc26..6b45d4835e41 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -2007,6 +2007,10 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
> for (i = 0; i < nintf; ++i) {
> struct usb_interface *intf = cp->interface[i];
>
> + if (intf->dev.of_node &&
> + !of_device_is_available(intf->dev.of_node))
> + continue;

Shouldn't you at least print some message out saying you are skipping
this? Odds are this is going to cause regressions in devices that were
not expecting this, right? So pointing them at why their devices now no
longer work would be good :)

thanks,

greg k-h

2019-02-19 13:25:42

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH] usb: core: skip interfaces disabled in devicetree

Greg Kroah-Hartman <[email protected]> writes:

> On Sat, Feb 16, 2019 at 05:04:52PM +0000, Mans Rullgard wrote:
>> If an interface has an associated devicetree node with status disabled,
>> do not register the device. This is useful for boards with a built-in
>> multifunction USB device where some functions are broken or otherwise
>> undesired.
>>
>> Signed-off-by: Mans Rullgard <[email protected]>
>> ---
>> drivers/usb/core/message.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index bfa5eda0cc26..6b45d4835e41 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -2007,6 +2007,10 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
>> for (i = 0; i < nintf; ++i) {
>> struct usb_interface *intf = cp->interface[i];
>>
>> + if (intf->dev.of_node &&
>> + !of_device_is_available(intf->dev.of_node))
>> + continue;
>
> Shouldn't you at least print some message out saying you are skipping
> this? Odds are this is going to cause regressions in devices that were
> not expecting this, right? So pointing them at why their devices now no
> longer work would be good :)

They will only be skipped if there is a device tree node for the
interface _and_ it has and explicit status = "disabled" property.
The default is still to create devices for all interfaces.

That said, printing a message is probably a good idea anyway. Would
"info" level be appropriate for this?

--
M?ns Rullg?rd

2019-02-19 13:44:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: core: skip interfaces disabled in devicetree

On Tue, Feb 19, 2019 at 01:24:51PM +0000, M?ns Rullg?rd wrote:
> Greg Kroah-Hartman <[email protected]> writes:
>
> > On Sat, Feb 16, 2019 at 05:04:52PM +0000, Mans Rullgard wrote:
> >> If an interface has an associated devicetree node with status disabled,
> >> do not register the device. This is useful for boards with a built-in
> >> multifunction USB device where some functions are broken or otherwise
> >> undesired.
> >>
> >> Signed-off-by: Mans Rullgard <[email protected]>
> >> ---
> >> drivers/usb/core/message.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> >> index bfa5eda0cc26..6b45d4835e41 100644
> >> --- a/drivers/usb/core/message.c
> >> +++ b/drivers/usb/core/message.c
> >> @@ -2007,6 +2007,10 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
> >> for (i = 0; i < nintf; ++i) {
> >> struct usb_interface *intf = cp->interface[i];
> >>
> >> + if (intf->dev.of_node &&
> >> + !of_device_is_available(intf->dev.of_node))
> >> + continue;
> >
> > Shouldn't you at least print some message out saying you are skipping
> > this? Odds are this is going to cause regressions in devices that were
> > not expecting this, right? So pointing them at why their devices now no
> > longer work would be good :)
>
> They will only be skipped if there is a device tree node for the
> interface _and_ it has and explicit status = "disabled" property.
> The default is still to create devices for all interfaces.

Yes, but today if you have such a status field set, it will not skip
them, so who knows who has their DT messed up :)

> That said, printing a message is probably a good idea anyway. Would
> "info" level be appropriate for this?

That is a good idea.

thanks,

greg k-h

2019-02-19 14:54:29

by Måns Rullgård

[permalink] [raw]
Subject: [PATCH v2] usb: core: skip interfaces disabled in devicetree

If an interface has an associated devicetree node with status disabled,
do not register the device. This is useful for boards with a built-in
multifunction USB device where some functions are broken or otherwise
undesired.

Signed-off-by: Mans Rullgard <[email protected]>
---
Changed in v2:
- print message about skipped interfaces
---
drivers/usb/core/message.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index bfa5eda0cc26..46bf79bc03ad 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -2007,6 +2007,13 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
for (i = 0; i < nintf; ++i) {
struct usb_interface *intf = cp->interface[i];

+ if (intf->dev.of_node &&
+ !of_device_is_available(intf->dev.of_node)) {
+ dev_info(&dev->dev, "skipping disabled interface %d\n",
+ intf->cur_altsetting->desc.bInterfaceNumber);
+ continue;
+ }
+
dev_dbg(&dev->dev,
"adding %s (config #%d, interface %d)\n",
dev_name(&intf->dev), configuration,
--
2.20.1