* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally [not found] <CAAd214WBzcsFamzqkdTp0AYDRo1=Uq1=Z5fJhzkOZ6znVFo3MA@mail.gmail.com> @ 2018-12-04 21:36 ` Alan Stern 2018-12-07 22:18 ` Kyle Williams 2018-12-05 9:39 ` Greg Kroah-Hartman 1 sibling, 1 reply; 14+ messages in thread From: Alan Stern @ 2018-12-04 21:36 UTC (permalink / raw) To: Kyle Williams Cc: linux-usb, Greg Kroah-Hartman, Kai-Heng Feng, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On Tue, 4 Dec 2018, Kyle Williams wrote: > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of certain Logitech devices and other powered media > devices such as the Atrus device causes 'not enough bandwidth for > new device state'error. > > This patch creates quirk entries for the tested Logitech device > indicating LPM should remain disabled for the device. > > Signed-off-by: Kyle Williams <kdgwill@chromium.org> > --- > drivers/usb/core/quirks.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 0690fcff0ea2..9403edee4797 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -246,6 +246,22 @@ static const struct usb_device_id usb_quirk_list[] = { > /* Logitech Harmony 700-series */ > { USB_DEVICE(0x046d, 0xc122), .driver_info = USB_QUIRK_DELAY_INIT }, > > + /* Logitech Flare */ > + { USB_DEVICE(0x046d, 0x0876), .driver_info = USB_QUIRK_NO_LPM }, This entry is out of order with the preceding entry. And some of the new entries below are out of order with each other (entries are supposed to be sorted by Vendor ID, then Product ID). Also, perhaps instead of adding all these new entries, we should set the NO_LPM quirk flag for all Logitech devices? Alan Stern > + > + /* Logitech Rally Camera */ > + { USB_DEVICE(0x046d, 0x0881), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x0888), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x0889), .driver_info = USB_QUIRK_NO_LPM }, > + > + /* Logitech Meetup */ > + { USB_DEVICE(0x046d, 0x0867), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x0866), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x086a), .driver_info = USB_QUIRK_NO_LPM }, > + > + /* Logitech Brio */ > + { USB_DEVICE(0x046d, 0x085e), .driver_info = USB_QUIRK_NO_LPM }, > + > /* Philips PSC805 audio device */ > { USB_DEVICE(0x0471, 0x0155), .driver_info = USB_QUIRK_RESET_RESUME > }, > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally 2018-12-04 21:36 ` [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally Alan Stern @ 2018-12-07 22:18 ` Kyle Williams 2018-12-10 13:50 ` Mathias Nyman 0 siblings, 1 reply; 14+ messages in thread From: Kyle Williams @ 2018-12-07 22:18 UTC (permalink / raw) To: Alan Stern Cc: linux-usb, Greg Kroah-Hartman, Kai-Heng Feng, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On Tue, Dec 04, 2018 at 04:36:18PM -0500, Alan Stern wrote: > On Tue, 4 Dec 2018, Kyle Williams wrote: > > > Description: Some USB device / host controller combinations seem to have > > problems with Link Power management. In particular it is described that > > the combination of certain Logitech devices and other powered media > > devices such as the Atrus device causes 'not enough bandwidth for > > new device state'error. > > > > This patch creates quirk entries for the tested Logitech device > > indicating LPM should remain disabled for the device. > > > > Signed-off-by: Kyle Williams <kdgwill@chromium.org> > > --- > > drivers/usb/core/quirks.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > > index 0690fcff0ea2..9403edee4797 100644 > > --- a/drivers/usb/core/quirks.c > > +++ b/drivers/usb/core/quirks.c > > @@ -246,6 +246,22 @@ static const struct usb_device_id usb_quirk_list[] = { > > /* Logitech Harmony 700-series */ > > { USB_DEVICE(0x046d, 0xc122), .driver_info = USB_QUIRK_DELAY_INIT }, > > > > + /* Logitech Flare */ > > + { USB_DEVICE(0x046d, 0x0876), .driver_info = USB_QUIRK_NO_LPM }, > > This entry is out of order with the preceding entry. And some of the > new entries below are out of order with each other (entries are > supposed to be sorted by Vendor ID, then Product ID). > > Also, perhaps instead of adding all these new entries, we should set > the NO_LPM quirk flag for all Logitech devices? > > Alan Stern Setting USB_QUIRK_NO_LPM for all Logitech devices instead of specific ones seem to be a better solution as there are a lot of other devices that have the issue as well Kyle Williams > > > + > > + /* Logitech Rally Camera */ > > + { USB_DEVICE(0x046d, 0x0881), .driver_info = USB_QUIRK_NO_LPM }, > > + { USB_DEVICE(0x046d, 0x0888), .driver_info = USB_QUIRK_NO_LPM }, > > + { USB_DEVICE(0x046d, 0x0889), .driver_info = USB_QUIRK_NO_LPM }, > > + > > + /* Logitech Meetup */ > > + { USB_DEVICE(0x046d, 0x0867), .driver_info = USB_QUIRK_NO_LPM }, > > + { USB_DEVICE(0x046d, 0x0866), .driver_info = USB_QUIRK_NO_LPM }, > > + { USB_DEVICE(0x046d, 0x086a), .driver_info = USB_QUIRK_NO_LPM }, > > + > > + /* Logitech Brio */ > > + { USB_DEVICE(0x046d, 0x085e), .driver_info = USB_QUIRK_NO_LPM }, > > + > > /* Philips PSC805 audio device */ > > { USB_DEVICE(0x0471, 0x0155), .driver_info = USB_QUIRK_RESET_RESUME > > }, > > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally 2018-12-07 22:18 ` Kyle Williams @ 2018-12-10 13:50 ` Mathias Nyman 2019-01-10 14:36 ` Kyle Williams 0 siblings, 1 reply; 14+ messages in thread From: Mathias Nyman @ 2018-12-10 13:50 UTC (permalink / raw) To: Kyle Williams, Alan Stern Cc: linux-usb, Greg Kroah-Hartman, Kai-Heng Feng, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On 08.12.2018 00:18, Kyle Williams wrote: > On Tue, Dec 04, 2018 at 04:36:18PM -0500, Alan Stern wrote: >> On Tue, 4 Dec 2018, Kyle Williams wrote: >> >>> Description: Some USB device / host controller combinations seem to have >>> problems with Link Power management. In particular it is described that >>> the combination of certain Logitech devices and other powered media >>> devices such as the Atrus device causes 'not enough bandwidth for >>> new device state'error. >>> >>> This patch creates quirk entries for the tested Logitech device >>> indicating LPM should remain disabled for the device. >>> >>> Signed-off-by: Kyle Williams <kdgwill@chromium.org> >>> --- >>> drivers/usb/core/quirks.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c >>> index 0690fcff0ea2..9403edee4797 100644 >>> --- a/drivers/usb/core/quirks.c >>> +++ b/drivers/usb/core/quirks.c >>> @@ -246,6 +246,22 @@ static const struct usb_device_id usb_quirk_list[] = { >>> /* Logitech Harmony 700-series */ >>> { USB_DEVICE(0x046d, 0xc122), .driver_info = USB_QUIRK_DELAY_INIT }, >>> >>> + /* Logitech Flare */ >>> + { USB_DEVICE(0x046d, 0x0876), .driver_info = USB_QUIRK_NO_LPM }, >> >> This entry is out of order with the preceding entry. And some of the >> new entries below are out of order with each other (entries are >> supposed to be sorted by Vendor ID, then Product ID). >> >> Also, perhaps instead of adding all these new entries, we should set >> the NO_LPM quirk flag for all Logitech devices? >> >> Alan Stern > Setting USB_QUIRK_NO_LPM for all Logitech devices instead of specific > ones seem to be a better solution as there are a lot of other devices > that have the issue as well > > Kyle Williams I recently found a cause for the "not enough bandwidth for new device state" error. Patch just got applied to v4.20-rc6 0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long Does it work for your Logitech devices? -Mathias ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally 2018-12-10 13:50 ` Mathias Nyman @ 2019-01-10 14:36 ` Kyle Williams 0 siblings, 0 replies; 14+ messages in thread From: Kyle Williams @ 2019-01-10 14:36 UTC (permalink / raw) To: Mathias Nyman Cc: Alan Stern, linux-usb, Greg Kroah-Hartman, Kai-Heng Feng, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On Mon, Dec 10, 2018 at 03:50:17PM +0200, Mathias Nyman wrote: > On 08.12.2018 00:18, Kyle Williams wrote: > > On Tue, Dec 04, 2018 at 04:36:18PM -0500, Alan Stern wrote: > > > On Tue, 4 Dec 2018, Kyle Williams wrote: > > > > > > > Description: Some USB device / host controller combinations seem to have > > > > problems with Link Power management. In particular it is described that > > > > the combination of certain Logitech devices and other powered media > > > > devices such as the Atrus device causes 'not enough bandwidth for > > > > new device state'error. > > > > > > > > This patch creates quirk entries for the tested Logitech device > > > > indicating LPM should remain disabled for the device. > > > > > > > > Signed-off-by: Kyle Williams <kdgwill@chromium.org> > > > > --- > > > > drivers/usb/core/quirks.c | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > > > > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > > > > index 0690fcff0ea2..9403edee4797 100644 > > > > --- a/drivers/usb/core/quirks.c > > > > +++ b/drivers/usb/core/quirks.c > > > > @@ -246,6 +246,22 @@ static const struct usb_device_id usb_quirk_list[] = { > > > > /* Logitech Harmony 700-series */ > > > > { USB_DEVICE(0x046d, 0xc122), .driver_info = USB_QUIRK_DELAY_INIT }, > > > > > > > > + /* Logitech Flare */ > > > > + { USB_DEVICE(0x046d, 0x0876), .driver_info = USB_QUIRK_NO_LPM }, > > > > > > This entry is out of order with the preceding entry. And some of the > > > new entries below are out of order with each other (entries are > > > supposed to be sorted by Vendor ID, then Product ID). > > > > > > Also, perhaps instead of adding all these new entries, we should set > > > the NO_LPM quirk flag for all Logitech devices? > > > > > > Alan Stern > > Setting USB_QUIRK_NO_LPM for all Logitech devices instead of specific > > ones seem to be a better solution as there are a lot of other devices > > that have the issue as well > > > > Kyle Williams > > I recently found a cause for the "not enough bandwidth for new device state" error. > Patch just got applied to v4.20-rc6 > 0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long > > Does it work for your Logitech devices? > > -Mathias > Hey Mathias, The fix specified solved the issues reported for all of the listed products. Cheers, ~Kyle ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally [not found] <CAAd214WBzcsFamzqkdTp0AYDRo1=Uq1=Z5fJhzkOZ6znVFo3MA@mail.gmail.com> 2018-12-04 21:36 ` [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally Alan Stern @ 2018-12-05 9:39 ` Greg Kroah-Hartman 2018-12-07 22:21 ` Kyle Williams 2018-12-08 1:47 ` [PATCH v3 0/2] Disable LPM by matching interface Kyle Williams 1 sibling, 2 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2018-12-05 9:39 UTC (permalink / raw) To: Kyle Williams Cc: linux-usb, Kai-Heng Feng, Alan Stern, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On Tue, Dec 04, 2018 at 03:38:25PM -0500, Kyle Williams wrote: > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of certain Logitech devices and other powered media > devices such as the Atrus device causes 'not enough bandwidth for > new device state'error. > > This patch creates quirk entries for the tested Logitech device > indicating LPM should remain disabled for the device. > > Signed-off-by: Kyle Williams <kdgwill@chromium.org> > --- > drivers/usb/core/quirks.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) Along with what Alan said, this patch was sent in html mode, making it impossible to apply, and it was rejected by the mailing lists. Please fix up your email client before resending patches. Or just use 'git send-email'. thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally 2018-12-05 9:39 ` Greg Kroah-Hartman @ 2018-12-07 22:21 ` Kyle Williams 2018-12-08 0:46 ` [PATCH v2] USB: quirks: disable LPM for Logitech UVC devices Kyle Williams 2018-12-08 1:47 ` [PATCH v3 0/2] Disable LPM by matching interface Kyle Williams 1 sibling, 1 reply; 14+ messages in thread From: Kyle Williams @ 2018-12-07 22:21 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Kai-Heng Feng, Alan Stern, Jack Stocker, Michael Niewöhner, Nico Sneck, Tim Anderson, Danilo Krummrich, linux-kernel On Wed, Dec 05, 2018 at 10:39:02AM +0100, Greg Kroah-Hartman wrote: > On Tue, Dec 04, 2018 at 03:38:25PM -0500, Kyle Williams wrote: > > Description: Some USB device / host controller combinations seem to have > > problems with Link Power management. In particular it is described that > > the combination of certain Logitech devices and other powered media > > devices such as the Atrus device causes 'not enough bandwidth for > > new device state'error. > > > > This patch creates quirk entries for the tested Logitech device > > indicating LPM should remain disabled for the device. > > > > Signed-off-by: Kyle Williams <kdgwill@chromium.org> > > --- > > drivers/usb/core/quirks.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > Along with what Alan said, this patch was sent in html mode, making it > impossible to apply, and it was rejected by the mailing lists. > > Please fix up your email client before resending patches. Or just use > 'git send-email'. > > thanks, > > greg k-h Thanks for explaining why the patches were being rejected by the mailing lists. I will move to using git send-email for sending patches and mutt for sending replies in the future. Kyle Williams ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] USB: quirks: disable LPM for Logitech UVC devices 2018-12-07 22:21 ` Kyle Williams @ 2018-12-08 0:46 ` Kyle Williams 0 siblings, 0 replies; 14+ messages in thread From: Kyle Williams @ 2018-12-08 0:46 UTC (permalink / raw) To: linux-kernel Cc: kdgwill, Greg Kroah-Hartman, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, Kai-Heng Feng, Danilo Krummrich, Kuppuswamy Sathyanarayanan, Maxence Duprès, Kamil Lulko, Emmanuel Pescosta, linux-usb From: Kyle Williams <kdgwill@chromium.org> Description: Some USB device / host controller combinations seem to have problems with Link Power management. In particular it is described that the combination of certain Logitech uvc devices and other powered media devices such causes 'not enough bandwidth for new device state' error. This patch enables the USB_QUIRK_NO_LPM quirk entries for all connected Logitech UVC devices indicating LPM should remain disabled for the device. Signed-off-by: Kyle Williams <kdgwill@chromium.org> --- Changes in v2: - changed commit message - disble lpm for all logitech uvc devices instead of listing manually - changes to allow the lpm to correctly be disabled after enumeration drivers/usb/core/hub.c | 87 +++++++++++++++++++++------------------ drivers/usb/core/quirks.c | 2 +- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 0f9381b69a3b..8f366ec4d21b 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -365,6 +365,51 @@ static void usb_set_lpm_parameters(struct usb_device *udev) usb_set_lpm_sel(udev, &udev->u2_params); } +/* + * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM + * when they're plugged into a USB 2.0 port, but they don't work when LPM is + * enabled. + * + * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the + * device says it supports the new USB 2.0 Link PM errata by setting the BESL + * support bit in the BOS descriptor. + */ +static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev) +{ + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); + int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN; + + if (!udev->usb2_hw_lpm_capable || !udev->bos) + return; + + if (hub) + connect_type = hub->ports[udev->portnum - 1]->connect_type; + + if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) + || connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { + udev->usb2_hw_lpm_allowed = 1; + usb_set_usb2_hardware_lpm(udev, 1); + } +} + +void usb_update_device_lpm(struct usb_hcd *hcd, struct usb_device *udev) +{ + int retval; + + if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) { + retval = usb_get_bos_descriptor(udev); + if (!retval) { + udev->lpm_capable = usb_device_supports_lpm(udev); + usb_set_lpm_parameters(udev); + } + } + + /* notify HCD that we have a device connected and addressed */ + if (hcd->driver->update_device) + hcd->driver->update_device(hcd, udev); + hub_set_initial_usb2_lpm_policy(udev); +} + /* USB 2.0 spec Section 11.24.4.5 */ static int get_hub_descriptor(struct usb_device *hdev, struct usb_hub_descriptor *desc) @@ -2295,7 +2340,6 @@ static int usb_enumerate_device_otg(struct usb_device *udev) return err; } - /** * usb_enumerate_device - Read device configs/intfs/otg (usbcore-internal) * @udev: newly addressed device (in ADDRESS state) @@ -2351,6 +2395,8 @@ static int usb_enumerate_device(struct usb_device *udev) usb_detect_interface_quirks(udev); + usb_update_device_lpm(hcd, udev); + return 0; } @@ -4402,33 +4448,6 @@ static int hub_set_address(struct usb_device *udev, int devnum) return retval; } -/* - * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM - * when they're plugged into a USB 2.0 port, but they don't work when LPM is - * enabled. - * - * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the - * device says it supports the new USB 2.0 Link PM errata by setting the BESL - * support bit in the BOS descriptor. - */ -static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev) -{ - struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); - int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN; - - if (!udev->usb2_hw_lpm_capable || !udev->bos) - return; - - if (hub) - connect_type = hub->ports[udev->portnum - 1]->connect_type; - - if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) || - connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { - udev->usb2_hw_lpm_allowed = 1; - usb_set_usb2_hardware_lpm(udev, 1); - } -} - static int hub_enable_device(struct usb_device *udev) { struct usb_hcd *hcd = bus_to_hcd(udev->bus); @@ -4779,19 +4798,9 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, usb_detect_quirks(udev); - if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) { - retval = usb_get_bos_descriptor(udev); - if (!retval) { - udev->lpm_capable = usb_device_supports_lpm(udev); - usb_set_lpm_parameters(udev); - } - } + usb_update_device_lpm(hcd, udev); retval = 0; - /* notify HCD that we have a device connected and addressed */ - if (hcd->driver->update_device) - hcd->driver->update_device(hcd, udev); - hub_set_initial_usb2_lpm_policy(udev); fail: if (retval) { hub_port_disable(hub, port1, 0); diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 0690fcff0ea2..bbbb7e6df958 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -440,7 +440,7 @@ static const struct usb_device_id usb_quirk_list[] = { static const struct usb_device_id usb_interface_quirk_list[] = { /* Logitech UVC Cameras */ { USB_VENDOR_AND_INTERFACE_INFO(0x046d, USB_CLASS_VIDEO, 1, 0), - .driver_info = USB_QUIRK_RESET_RESUME }, + .driver_info = USB_QUIRK_RESET_RESUME | USB_QUIRK_NO_LPM }, { } /* terminating entry must be last */ }; -- 2.20.0.rc2.403.gdbc3b29805-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 0/2] Disable LPM by matching interface 2018-12-05 9:39 ` Greg Kroah-Hartman 2018-12-07 22:21 ` Kyle Williams @ 2018-12-08 1:47 ` Kyle Williams 2018-12-08 1:47 ` [PATCH v3 1/2] USB: quirks: Check device interface LPM capability Kyle Williams 2018-12-08 1:47 ` [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices Kyle Williams 1 sibling, 2 replies; 14+ messages in thread From: Kyle Williams @ 2018-12-08 1:47 UTC (permalink / raw) To: linux-kernel Cc: kdgwill, Kyle Williams, Greg Kroah-Hartman, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, linux-usb Changes in v3: - seperated work to allow matched interfaces to use the USB_QUIRK_NO_LPM quirk to affect a broader range of devices instead of setting each device individually Kyle Williams (2): USB: quirks: Check device interface LPM capability USB: quirks: Disable LPM for Logitech UVC devices drivers/usb/core/hub.c | 87 +++++++++++++++++++++------------------ drivers/usb/core/quirks.c | 2 +- 2 files changed, 49 insertions(+), 40 deletions(-) -- 2.20.0.rc2.403.gdbc3b29805-goog ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] USB: quirks: Check device interface LPM capability 2018-12-08 1:47 ` [PATCH v3 0/2] Disable LPM by matching interface Kyle Williams @ 2018-12-08 1:47 ` Kyle Williams 2018-12-12 11:37 ` Greg Kroah-Hartman 2018-12-08 1:47 ` [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices Kyle Williams 1 sibling, 1 reply; 14+ messages in thread From: Kyle Williams @ 2018-12-08 1:47 UTC (permalink / raw) To: linux-kernel Cc: kdgwill, Greg Kroah-Hartman, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, linux-usb From: Kyle Williams <kdgwill@chromium.org> Description: enable the ability to disable LPM for all devices matched by interface information Signed-off-by: Kyle Williams <kdgwill@chromium.org> --- drivers/usb/core/hub.c | 87 +++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 0f9381b69a3b..8f366ec4d21b 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -365,6 +365,51 @@ static void usb_set_lpm_parameters(struct usb_device *udev) usb_set_lpm_sel(udev, &udev->u2_params); } +/* + * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM + * when they're plugged into a USB 2.0 port, but they don't work when LPM is + * enabled. + * + * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the + * device says it supports the new USB 2.0 Link PM errata by setting the BESL + * support bit in the BOS descriptor. + */ +static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev) +{ + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); + int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN; + + if (!udev->usb2_hw_lpm_capable || !udev->bos) + return; + + if (hub) + connect_type = hub->ports[udev->portnum - 1]->connect_type; + + if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) + || connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { + udev->usb2_hw_lpm_allowed = 1; + usb_set_usb2_hardware_lpm(udev, 1); + } +} + +void usb_update_device_lpm(struct usb_hcd *hcd, struct usb_device *udev) +{ + int retval; + + if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) { + retval = usb_get_bos_descriptor(udev); + if (!retval) { + udev->lpm_capable = usb_device_supports_lpm(udev); + usb_set_lpm_parameters(udev); + } + } + + /* notify HCD that we have a device connected and addressed */ + if (hcd->driver->update_device) + hcd->driver->update_device(hcd, udev); + hub_set_initial_usb2_lpm_policy(udev); +} + /* USB 2.0 spec Section 11.24.4.5 */ static int get_hub_descriptor(struct usb_device *hdev, struct usb_hub_descriptor *desc) @@ -2295,7 +2340,6 @@ static int usb_enumerate_device_otg(struct usb_device *udev) return err; } - /** * usb_enumerate_device - Read device configs/intfs/otg (usbcore-internal) * @udev: newly addressed device (in ADDRESS state) @@ -2351,6 +2395,8 @@ static int usb_enumerate_device(struct usb_device *udev) usb_detect_interface_quirks(udev); + usb_update_device_lpm(hcd, udev); + return 0; } @@ -4402,33 +4448,6 @@ static int hub_set_address(struct usb_device *udev, int devnum) return retval; } -/* - * There are reports of USB 3.0 devices that say they support USB 2.0 Link PM - * when they're plugged into a USB 2.0 port, but they don't work when LPM is - * enabled. - * - * Only enable USB 2.0 Link PM if the port is internal (hardwired), or the - * device says it supports the new USB 2.0 Link PM errata by setting the BESL - * support bit in the BOS descriptor. - */ -static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev) -{ - struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); - int connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN; - - if (!udev->usb2_hw_lpm_capable || !udev->bos) - return; - - if (hub) - connect_type = hub->ports[udev->portnum - 1]->connect_type; - - if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) || - connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { - udev->usb2_hw_lpm_allowed = 1; - usb_set_usb2_hardware_lpm(udev, 1); - } -} - static int hub_enable_device(struct usb_device *udev) { struct usb_hcd *hcd = bus_to_hcd(udev->bus); @@ -4779,19 +4798,9 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, usb_detect_quirks(udev); - if (udev->wusb == 0 && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201) { - retval = usb_get_bos_descriptor(udev); - if (!retval) { - udev->lpm_capable = usb_device_supports_lpm(udev); - usb_set_lpm_parameters(udev); - } - } + usb_update_device_lpm(hcd, udev); retval = 0; - /* notify HCD that we have a device connected and addressed */ - if (hcd->driver->update_device) - hcd->driver->update_device(hcd, udev); - hub_set_initial_usb2_lpm_policy(udev); fail: if (retval) { hub_port_disable(hub, port1, 0); -- 2.20.0.rc2.403.gdbc3b29805-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] USB: quirks: Check device interface LPM capability 2018-12-08 1:47 ` [PATCH v3 1/2] USB: quirks: Check device interface LPM capability Kyle Williams @ 2018-12-12 11:37 ` Greg Kroah-Hartman 2019-01-10 15:49 ` Kyle Williams 0 siblings, 1 reply; 14+ messages in thread From: Greg Kroah-Hartman @ 2018-12-12 11:37 UTC (permalink / raw) To: Kyle Williams Cc: linux-kernel, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, linux-usb On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote: > From: Kyle Williams <kdgwill@chromium.org> > > Description: enable the ability to disable LPM for all devices matched > by interface information Why have "Description:" in here? Of course this is the description :) And the subject doesn't make much sense, what does this have to do with quirks? Also, you need more information here in the description about why you are doing all of this, as it is, I have no idea why this patch is needed at all. thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] USB: quirks: Check device interface LPM capability 2018-12-12 11:37 ` Greg Kroah-Hartman @ 2019-01-10 15:49 ` Kyle Williams 2019-01-11 9:09 ` Greg Kroah-Hartman 0 siblings, 1 reply; 14+ messages in thread From: Kyle Williams @ 2019-01-10 15:49 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, linux-usb On Wed, Dec 12, 2018 at 12:37:09PM +0100, Greg Kroah-Hartman wrote: > On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote: > > From: Kyle Williams <kdgwill@chromium.org> > > > > Description: enable the ability to disable LPM for all devices matched > > by interface information > > Why have "Description:" in here? Of course this is the description :) > > And the subject doesn't make much sense, what does this have to do with > quirks? > > Also, you need more information here in the description about why you > are doing all of this, as it is, I have no idea why this patch is needed > at all. > > thanks, > > greg k-h It seems the underlying issue has been solved in 0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long However, the patch to fix the issue when matching the USB_QUIRK_NO_LPM quirk by iterface could be useful in the future. Currently nothing will happen that quirk is matched by interface instead of by vid/pid pair. Is their anything that would need to be changed besides removing the Description tag? -Kyle ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] USB: quirks: Check device interface LPM capability 2019-01-10 15:49 ` Kyle Williams @ 2019-01-11 9:09 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-01-11 9:09 UTC (permalink / raw) To: Kyle Williams Cc: linux-kernel, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, linux-usb On Thu, Jan 10, 2019 at 10:49:09AM -0500, Kyle Williams wrote: > On Wed, Dec 12, 2018 at 12:37:09PM +0100, Greg Kroah-Hartman wrote: > > On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote: > > > From: Kyle Williams <kdgwill@chromium.org> > > > > > > Description: enable the ability to disable LPM for all devices matched > > > by interface information > > > > Why have "Description:" in here? Of course this is the description :) > > > > And the subject doesn't make much sense, what does this have to do with > > quirks? > > > > Also, you need more information here in the description about why you > > are doing all of this, as it is, I have no idea why this patch is needed > > at all. > > > > thanks, > > > > greg k-h > > It seems the underlying issue has been solved in > 0472bf0 xhci: Prevent U1/U2 link pm states if exit latency is too long > > However, the patch to fix the issue when matching the USB_QUIRK_NO_LPM > quirk by iterface could be useful in the future. Currently nothing will > happen that quirk is matched by interface instead of by vid/pid pair. > Is their anything that would need to be changed besides removing the > Description tag? Don't add code to the kernel that is not used now. If in the future someone needs to add this type of functionality, worry about it then... thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices 2018-12-08 1:47 ` [PATCH v3 0/2] Disable LPM by matching interface Kyle Williams 2018-12-08 1:47 ` [PATCH v3 1/2] USB: quirks: Check device interface LPM capability Kyle Williams @ 2018-12-08 1:47 ` Kyle Williams 2018-12-12 11:37 ` Greg Kroah-Hartman 1 sibling, 1 reply; 14+ messages in thread From: Kyle Williams @ 2018-12-08 1:47 UTC (permalink / raw) To: linux-kernel Cc: kdgwill, Greg Kroah-Hartman, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, Kai-Heng Feng, Kuppuswamy Sathyanarayanan, Harry Pan, Emmanuel Pescosta, Danilo Krummrich, Dmitry Fleytman Dmitry Fleytman, linux-usb From: Kyle Williams <kdgwill@chromium.org> Description: Some USB device / host controller combinations seem to have problems with Link Power management. In particular it is described that the combination of certain Logitech uvc devices and other powered media devices such causes 'not enough bandwidth for new device state' error. This patch enables the USB_QUIRK_NO_LPM quirk entries for all connected Logitech UVC devices indicating LPM should remain disabled for the device. Signed-off-by: Kyle Williams <kdgwill@chromium.org> --- drivers/usb/core/quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 0690fcff0ea2..bbbb7e6df958 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -440,7 +440,7 @@ static const struct usb_device_id usb_quirk_list[] = { static const struct usb_device_id usb_interface_quirk_list[] = { /* Logitech UVC Cameras */ { USB_VENDOR_AND_INTERFACE_INFO(0x046d, USB_CLASS_VIDEO, 1, 0), - .driver_info = USB_QUIRK_RESET_RESUME }, + .driver_info = USB_QUIRK_RESET_RESUME | USB_QUIRK_NO_LPM }, { } /* terminating entry must be last */ }; -- 2.20.0.rc2.403.gdbc3b29805-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices 2018-12-08 1:47 ` [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices Kyle Williams @ 2018-12-12 11:37 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2018-12-12 11:37 UTC (permalink / raw) To: Kyle Williams Cc: linux-kernel, Alan Stern, Mathias Nyman, Nicolas Boichat, Jon Flatley, Felipe Balbi, Kai-Heng Feng, Kuppuswamy Sathyanarayanan, Harry Pan, Emmanuel Pescosta, Danilo Krummrich, Dmitry Fleytman Dmitry Fleytman, linux-usb On Fri, Dec 07, 2018 at 08:47:22PM -0500, Kyle Williams wrote: > From: Kyle Williams <kdgwill@chromium.org> > > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of certain Logitech uvc devices and other powered media > devices such causes 'not enough bandwidth for new device state' error. Same thing with "Description:" :( ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-01-11 9:10 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CAAd214WBzcsFamzqkdTp0AYDRo1=Uq1=Z5fJhzkOZ6znVFo3MA@mail.gmail.com> 2018-12-04 21:36 ` [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally Alan Stern 2018-12-07 22:18 ` Kyle Williams 2018-12-10 13:50 ` Mathias Nyman 2019-01-10 14:36 ` Kyle Williams 2018-12-05 9:39 ` Greg Kroah-Hartman 2018-12-07 22:21 ` Kyle Williams 2018-12-08 0:46 ` [PATCH v2] USB: quirks: disable LPM for Logitech UVC devices Kyle Williams 2018-12-08 1:47 ` [PATCH v3 0/2] Disable LPM by matching interface Kyle Williams 2018-12-08 1:47 ` [PATCH v3 1/2] USB: quirks: Check device interface LPM capability Kyle Williams 2018-12-12 11:37 ` Greg Kroah-Hartman 2019-01-10 15:49 ` Kyle Williams 2019-01-11 9:09 ` Greg Kroah-Hartman 2018-12-08 1:47 ` [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices Kyle Williams 2018-12-12 11:37 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).