RE: [PATCH 1/2] Revert "xhci 1.0: Limit arbitrarily-aligned scatter gather."
From: Alan Stern > On Fri, 7 Mar 2014, David Laight wrote: > > > From: Mathias Nyman > > > This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. > > > > You need to revert further. > > Just don?t set hcd->self.no_sg_constraint ever - since it just > > doesn't work. > > No; it does work most of the time. > > If hcd->self.no_sg_constraint wasn't sent, then certain commands would > fail always, instead of failing occasionally. The point is that the no_sg_constraint was added in order to allow the ax88179_178a driver to send arbitrarily aligned transfers. Nothing else looks at it (well didn't when I scanned the tree a while back). In effect all the other transfer requests were assumed to be suitably aligned. The check that caused things to fail was the one added to xhci relatively recently that verified the alignment of the fragments. David
RE: [PATCH 1/2] Revert "xhci 1.0: Limit arbitrarily-aligned scatter gather."
On Fri, 7 Mar 2014, David Laight wrote: > From: Mathias Nyman > > This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. > > You need to revert further. > Just don�t set hcd->self.no_sg_constraint ever - since it just > doesn't work. No; it does work most of the time. If hcd->self.no_sg_constraint wasn't sent, then certain commands would fail always, instead of failing occasionally. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] Revert "xhci 1.0: Limit arbitrarily-aligned scatter gather."
From: Mathias Nyman > This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. You need to revert further. Just don’t set hcd->self.no_sg_constraint ever - since it just doesn't work. That will stop the ax88179_178a driver sending fragmented packets. With the check for aligned non-terminal fragments removed the code will be as bad as the earlier releases. > This commit, together with commit 3804fad45411b48233b48003e33a78f290d227c8 > "USBNET: ax88179_178a: enable tso if usb host supports sg dma" were > origially added to get xHCI 1.0 hosts and usb ethernet ax88179_178a devices > working together with scatter gather. xHCI 1.0 hosts pose some requirement on > how transfer > buffers are aligned, setting this requirement for 1.0 hosts caused USB 3.0 > mass > storage devices to fail more frequently. > > USB 3.0 mass storage devices used to work before 3.14-rc1. Theoretically, > the TD fragment rules could have caused an occasional disk glitch. > Now the devices *will* fail, instead of theoretically failing. > From a user perspective, this looks like a regression; the USB device > obviously > fails on 3.14-rc1, and may sometimes silently fail on prior kernels. > > The proper soluition is to implement the TD fragment rules required, but for > now > this patch needs to be reverted to get USB 3.0 mass storage devices working > at the > level they used to. > > Signed-off-by: Mathias Nyman > Cc: stable > --- > drivers/usb/host/xhci.c | 14 +++--- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6fe577d..924a6cc 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4733,6 +4733,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, > xhci_get_quirks_t get_quirks) > /* Accept arbitrarily long scatter-gather lists */ > hcd->self.sg_tablesize = ~0; > > + /* support to build packet from discontinuous buffers */ > + hcd->self.no_sg_constraint = 1; > + Don't add the above - it only looked at by the usbnet code and in particular the ax88179_178a driver > /* XHCI controllers don't stop the ep queue on short packets :| */ > hcd->self.no_stop_on_short = 1; > > @@ -4757,14 +4760,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, > xhci_get_quirks_t get_quirks) > /* xHCI private pointer was set in xhci_pci_probe for the second >* registered roothub. >*/ > - xhci = hcd_to_xhci(hcd); > - /* > - * Support arbitrarily aligned sg-list entries on hosts without > - * TD fragment rules (which are currently unsupported). > - */ > - if (xhci->hci_version < 0x100) > - hcd->self.no_sg_constraint = 1; > - > return 0; > } > > @@ -4793,9 +4788,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, > xhci_get_quirks_t get_quirks) > if (xhci->hci_version > 0x96) > xhci->quirks |= XHCI_SPURIOUS_SUCCESS; > > - if (xhci->hci_version < 0x100) > - hcd->self.no_sg_constraint = 1; > - > /* Make sure the HC is halted. */ > retval = xhci_halt(xhci); > if (retval) > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] Revert xhci 1.0: Limit arbitrarily-aligned scatter gather.
From: Mathias Nyman This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. You need to revert further. Just don’t set hcd-self.no_sg_constraint ever - since it just doesn't work. That will stop the ax88179_178a driver sending fragmented packets. With the check for aligned non-terminal fragments removed the code will be as bad as the earlier releases. This commit, together with commit 3804fad45411b48233b48003e33a78f290d227c8 USBNET: ax88179_178a: enable tso if usb host supports sg dma were origially added to get xHCI 1.0 hosts and usb ethernet ax88179_178a devices working together with scatter gather. xHCI 1.0 hosts pose some requirement on how transfer buffers are aligned, setting this requirement for 1.0 hosts caused USB 3.0 mass storage devices to fail more frequently. USB 3.0 mass storage devices used to work before 3.14-rc1. Theoretically, the TD fragment rules could have caused an occasional disk glitch. Now the devices *will* fail, instead of theoretically failing. From a user perspective, this looks like a regression; the USB device obviously fails on 3.14-rc1, and may sometimes silently fail on prior kernels. The proper soluition is to implement the TD fragment rules required, but for now this patch needs to be reverted to get USB 3.0 mass storage devices working at the level they used to. Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com Cc: stable sta...@vger.kernel.org --- drivers/usb/host/xhci.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6fe577d..924a6cc 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4733,6 +4733,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) /* Accept arbitrarily long scatter-gather lists */ hcd-self.sg_tablesize = ~0; + /* support to build packet from discontinuous buffers */ + hcd-self.no_sg_constraint = 1; + Don't add the above - it only looked at by the usbnet code and in particular the ax88179_178a driver /* XHCI controllers don't stop the ep queue on short packets :| */ hcd-self.no_stop_on_short = 1; @@ -4757,14 +4760,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) /* xHCI private pointer was set in xhci_pci_probe for the second * registered roothub. */ - xhci = hcd_to_xhci(hcd); - /* - * Support arbitrarily aligned sg-list entries on hosts without - * TD fragment rules (which are currently unsupported). - */ - if (xhci-hci_version 0x100) - hcd-self.no_sg_constraint = 1; - return 0; } @@ -4793,9 +4788,6 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) if (xhci-hci_version 0x96) xhci-quirks |= XHCI_SPURIOUS_SUCCESS; - if (xhci-hci_version 0x100) - hcd-self.no_sg_constraint = 1; - /* Make sure the HC is halted. */ retval = xhci_halt(xhci); if (retval) -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] Revert xhci 1.0: Limit arbitrarily-aligned scatter gather.
On Fri, 7 Mar 2014, David Laight wrote: From: Mathias Nyman This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. You need to revert further. Just don�t set hcd-self.no_sg_constraint ever - since it just doesn't work. No; it does work most of the time. If hcd-self.no_sg_constraint wasn't sent, then certain commands would fail always, instead of failing occasionally. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] Revert xhci 1.0: Limit arbitrarily-aligned scatter gather.
From: Alan Stern On Fri, 7 Mar 2014, David Laight wrote: From: Mathias Nyman This reverts commit 247bf557273dd775505fb9240d2d152f4f20d304. You need to revert further. Just don?t set hcd-self.no_sg_constraint ever - since it just doesn't work. No; it does work most of the time. If hcd-self.no_sg_constraint wasn't sent, then certain commands would fail always, instead of failing occasionally. The point is that the no_sg_constraint was added in order to allow the ax88179_178a driver to send arbitrarily aligned transfers. Nothing else looks at it (well didn't when I scanned the tree a while back). In effect all the other transfer requests were assumed to be suitably aligned. The check that caused things to fail was the one added to xhci relatively recently that verified the alignment of the fragments. David
