| Message ID | 20200618094300.1887727-2-gregkh@linuxfoundation.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 91c7eaa686c3b7ae2d5b2aed22a45a02c8baa30e |
| Headers | show |
| Series | USB: fix up some old and obsolete terminology, we can do better | expand |
On Thu, 2020-06-18 at 11:42 +0200, Greg Kroah-Hartman wrote: > The USB core has a quirk flag to ignore specific endpoints, so rename > it > to be more obvious what this quirk does. > > Cc: Johan Hovold <johan@kernel.org> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Richard Dodd <richard.o.dodd@gmail.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Jonathan Cox <jonathan@jdcox.net> > Cc: Bastien Nocera <hadess@hadess.net> > Cc: "Thiébaud Weksteen" <tweek@google.com> > Cc: Nishad Kamdar <nishadkamdar@gmail.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> If the driver API change below is agreeable, you can add my: Reviewed-by: Bastien Nocera <hadess@hadess.net> Good job. <snip> > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h > index 22c1f579afe3..5e4c497f54d6 100644 > --- a/include/linux/usb/quirks.h > +++ b/include/linux/usb/quirks.h > @@ -69,7 +69,7 @@ > /* Hub needs extra delay after resetting its port. */ > #define USB_QUIRK_HUB_SLOW_RESET BIT(14) > > -/* device has blacklisted endpoints */ > -#define USB_QUIRK_ENDPOINT_BLACKLIST BIT(15) > +/* device has endpoints that should be ignored */ > +#define USB_QUIRK_ENDPOINT_IGNORE BIT(15) > > #endif /* __LINUX_USB_QUIRKS_H */
Hi Bastien, On 6/19/20 12:50 PM, Bastien Nocera wrote: > On Thu, 2020-06-18 at 11:42 +0200, Greg Kroah-Hartman wrote: >> The USB core has a quirk flag to ignore specific endpoints, so rename >> it >> to be more obvious what this quirk does. >> >> Cc: Johan Hovold <johan@kernel.org> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Richard Dodd <richard.o.dodd@gmail.com> >> Cc: Hans de Goede <hdegoede@redhat.com> >> Cc: Jonathan Cox <jonathan@jdcox.net> >> Cc: Bastien Nocera <hadess@hadess.net> >> Cc: "Thiébaud Weksteen" <tweek@google.com> >> Cc: Nishad Kamdar <nishadkamdar@gmail.com> >> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > If the driver API change below is agreeable, you can add my: > Reviewed-by: Bastien Nocera <hadess@hadess.net> A note for future reference, not sure what you mean with driver API here. If you mean the in kernel API, the kernel rules are that we are always free to change that (Linux does not have a stable driver API). So if a header does not sit under include/uapi (indicating that it is an userspace API) then a change like this is fine. Regards, Hans > Good job. > > <snip> >> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h >> index 22c1f579afe3..5e4c497f54d6 100644 >> --- a/include/linux/usb/quirks.h >> +++ b/include/linux/usb/quirks.h >> @@ -69,7 +69,7 @@ >> /* Hub needs extra delay after resetting its port. */ >> #define USB_QUIRK_HUB_SLOW_RESET BIT(14) >> >> -/* device has blacklisted endpoints */ >> -#define USB_QUIRK_ENDPOINT_BLACKLIST BIT(15) >> +/* device has endpoints that should be ignored */ >> +#define USB_QUIRK_ENDPOINT_IGNORE BIT(15) >> >> #endif /* __LINUX_USB_QUIRKS_H */ > > >
On Fri, 2020-06-19 at 12:53 +0200, Hans de Goede wrote: > A note for future reference, not sure what you mean with driver > > API here. If you mean the in kernel API, the kernel rules are > > that we are always free to change that (Linux does not have a > > stable driver API). > > > > So if a header does not sit under include/uapi (indicating that > > it is an userspace API) then a change like this is fine. I meant the internal driver API, which might break out-of-tree drivers. I know that this API is fluid, and that there are no stability guarantees, but I'd still expect at least a note in the commit message to that effect.
On Fri, Jun 19, 2020 at 01:08:53PM +0200, Bastien Nocera wrote: > On Fri, 2020-06-19 at 12:53 +0200, Hans de Goede wrote: > > A note for future reference, not sure what you mean with driver > > > > API here. If you mean the in kernel API, the kernel rules are > > > > that we are always free to change that (Linux does not have a > > > > stable driver API). > > > > > > > > So if a header does not sit under include/uapi (indicating that > > > > it is an userspace API) then a change like this is fine. > > I meant the internal driver API, which might break out-of-tree drivers. There is no such thing. Well, there might be, but we don't care and have to act as if there are no such thing otherwise we would never be able to change anything :) > I know that this API is fluid, and that there are no stability > guarantees, but I'd still expect at least a note in the commit message > to that effect. Why? Who cares? Anyone who lives outside of the kernel already knows they have to dig in the kernel if their code breaks, that's the cost of living outside of the kernel. thanks, greg k-h
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index b7918f695434..37442f423a41 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -298,10 +298,10 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno, goto skip_to_next_endpoint_or_interface_descriptor; } - /* Ignore blacklisted endpoints */ - if (udev->quirks & USB_QUIRK_ENDPOINT_BLACKLIST) { - if (usb_endpoint_is_blacklisted(udev, ifp, d)) { - dev_warn(ddev, "config %d interface %d altsetting %d has a blacklisted endpoint with address 0x%X, skipping\n", + /* Ignore some endpoints */ + if (udev->quirks & USB_QUIRK_ENDPOINT_IGNORE) { + if (usb_endpoint_is_ignored(udev, ifp, d)) { + dev_warn(ddev, "config %d interface %d altsetting %d has an ignored endpoint with address 0x%X, skipping\n", cfgno, inum, asnum, d->bEndpointAddress); goto skip_to_next_endpoint_or_interface_descriptor; diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 3e8efe759c3e..20dccf34182d 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -359,7 +359,7 @@ static const struct usb_device_id usb_quirk_list[] = { /* Sound Devices USBPre2 */ { USB_DEVICE(0x0926, 0x0202), .driver_info = - USB_QUIRK_ENDPOINT_BLACKLIST }, + USB_QUIRK_ENDPOINT_IGNORE }, /* Keytouch QWERTY Panel keyboard */ { USB_DEVICE(0x0926, 0x3333), .driver_info = @@ -493,24 +493,24 @@ static const struct usb_device_id usb_amd_resume_quirk_list[] = { }; /* - * Entries for blacklisted endpoints that should be ignored when parsing - * configuration descriptors. + * Entries for endpoints that should be ignored when parsing configuration + * descriptors. * - * Matched for devices with USB_QUIRK_ENDPOINT_BLACKLIST. + * Matched for devices with USB_QUIRK_ENDPOINT_IGNORE. */ -static const struct usb_device_id usb_endpoint_blacklist[] = { +static const struct usb_device_id usb_endpoint_ignore[] = { { USB_DEVICE_INTERFACE_NUMBER(0x0926, 0x0202, 1), .driver_info = 0x85 }, { } }; -bool usb_endpoint_is_blacklisted(struct usb_device *udev, - struct usb_host_interface *intf, - struct usb_endpoint_descriptor *epd) +bool usb_endpoint_is_ignored(struct usb_device *udev, + struct usb_host_interface *intf, + struct usb_endpoint_descriptor *epd) { const struct usb_device_id *id; unsigned int address; - for (id = usb_endpoint_blacklist; id->match_flags; ++id) { + for (id = usb_endpoint_ignore; id->match_flags; ++id) { if (!usb_match_device(udev, id)) continue; diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 19e4c550bc73..98e7d1ee63dc 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -37,7 +37,7 @@ extern void usb_authorize_interface(struct usb_interface *); extern void usb_detect_quirks(struct usb_device *udev); extern void usb_detect_interface_quirks(struct usb_device *udev); extern void usb_release_quirk_list(void); -extern bool usb_endpoint_is_blacklisted(struct usb_device *udev, +extern bool usb_endpoint_is_ignored(struct usb_device *udev, struct usb_host_interface *intf, struct usb_endpoint_descriptor *epd); extern int usb_remove_device(struct usb_device *udev); diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h index 22c1f579afe3..5e4c497f54d6 100644 --- a/include/linux/usb/quirks.h +++ b/include/linux/usb/quirks.h @@ -69,7 +69,7 @@ /* Hub needs extra delay after resetting its port. */ #define USB_QUIRK_HUB_SLOW_RESET BIT(14) -/* device has blacklisted endpoints */ -#define USB_QUIRK_ENDPOINT_BLACKLIST BIT(15) +/* device has endpoints that should be ignored */ +#define USB_QUIRK_ENDPOINT_IGNORE BIT(15) #endif /* __LINUX_USB_QUIRKS_H */
The USB core has a quirk flag to ignore specific endpoints, so rename it to be more obvious what this quirk does. Cc: Johan Hovold <johan@kernel.org> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Richard Dodd <richard.o.dodd@gmail.com> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Jonathan Cox <jonathan@jdcox.net> Cc: Bastien Nocera <hadess@hadess.net> Cc: "Thiébaud Weksteen" <tweek@google.com> Cc: Nishad Kamdar <nishadkamdar@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/usb/core/config.c | 8 ++++---- drivers/usb/core/quirks.c | 18 +++++++++--------- drivers/usb/core/usb.h | 2 +- include/linux/usb/quirks.h | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-)