* [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC @ 2020-03-27 8:52 Tejas Joglekar 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar 2020-03-27 9:28 ` [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Greg KH 0 siblings, 2 replies; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 8:52 UTC (permalink / raw) To: Felipe Balbi, Tejas Joglekar, linux-usb, devicetree, Rob Herring, Chunfeng Yun, Fredrik Noring, Mathias Nyman, Mark Rutland, Sebastian Andrzej Siewior, Raul E Rangel, Laurentiu Tudor, Marek Szyprowski Cc: John Youn The Synopsys xHC has an internal TRB cache of size TRB_CACHE_SIZE for each endpoint. The default value for TRB_CACHE_SIZE is 16 for SS and 8 for HS. The controller loads and updates the TRB cache from the transfer ring in system memory whenever the driver issues a start transfer or update transfer command. For chained TRBs, the Synopsys xHC requires that the total amount of bytes for all TRBs loaded in the TRB cache be greater than or equal to 1 MPS. Or the chain ends within the TRB cache (with a last TRB). If this requirement is not met, the controller will not be able to send or receive a packet and it will hang causing a driver timeout and error. This patch set adds logic to the XHCI driver to detect and prevent this from happening along with the quirk to enable this logic for Synopsys HAPS platform. Based on Mathias's feedback on previous implementation where consolidation was done in TRB cache, with this patch series the implementation is done during mapping of the URB by consolidating the SG list into a temporary buffer if the SG list buffer sizes within TRB_CACHE_SIZE is less than MPS. Tejas Joglekar (4): dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl usb: xhci: Set quirk for XHCI_CONSOLIDATE_SG_LIST usb: dwc3: Add device property consolidate-sgl usb: xhci: Use temporary buffer to consolidate SG Resending as 'umlaut' is not accepted by some servers in email. Documentation/devicetree/bindings/usb/dwc3.txt | 3 + Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 + drivers/usb/core/hcd.c | 8 ++ drivers/usb/dwc3/core.c | 2 + drivers/usb/dwc3/core.h | 2 + drivers/usb/dwc3/dwc3-haps.c | 1 + drivers/usb/dwc3/host.c | 5 +- drivers/usb/host/xhci-pci.c | 3 + drivers/usb/host/xhci-plat.c | 3 + drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.c | 128 +++++++++++++++++++++ drivers/usb/host/xhci.h | 5 + 12 files changed, 163 insertions(+), 2 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 8:52 [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Tejas Joglekar @ 2020-03-27 8:34 ` Tejas Joglekar 2020-03-27 9:41 ` [RESENDING RFC " Tejas Joglekar ` (2 more replies) 2020-03-27 9:28 ` [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Greg KH 1 sibling, 3 replies; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 8:34 UTC (permalink / raw) To: Tejas Joglekar, linux-usb, devicetree, linux-kernel, Rob Herring, Mark Rutland Cc: John Youn This commit adds the documentation for consolidate-sgl, and snps,consolidate-sgl property. These when set enables the quirk for XHCI driver for consolidation of sg list into a temporary buffer when small buffer sizes are scattered over the sg list not making up to MPS or total transfer size within TRB cache size with Synopsys xHC. Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> --- Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 9946ff9ba735..292d1f7969e4 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -104,6 +104,9 @@ Optional properties: this and tx-thr-num-pkt-prd to a valid, non-zero value 1-16 (DWC_usb31 programming guide section 1.2.3) to enable periodic ESS TX threshold. + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use + SG buffers of at least MPS size by consolidating smaller SG + buffers list into a single buffer. - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt index 3f378951d624..a90d853557ee 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt @@ -43,6 +43,9 @@ Optional properties: - quirk-broken-port-ped: set if the controller has broken port disable mechanism - imod-interval-ns: default interrupt moderation interval is 5000ns - phys : see usb-hcd.yaml in the current directory + - consolidate-sgl: indicate if you need to consolidate sg list into a + temporary buffer when small SG buffer sizes does not make upto MPS + size or total transfer size across the TRB cache size. additionally the properties from usb-hcd.yaml (in the current directory) are supported. -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar @ 2020-03-27 9:41 ` Tejas Joglekar 2020-03-27 9:54 ` Greg KH 2020-04-05 1:45 ` Rob Herring 2 siblings, 0 replies; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 9:41 UTC (permalink / raw) To: Tejas Joglekar, linux-usb, devicetree, linux-kernel, Rob Herring, Mark Rutland Cc: John Youn This commit adds the documentation for consolidate-sgl, and snps,consolidate-sgl property. These when set enables the quirk for XHCI driver for consolidation of sg list into a temporary buffer when small buffer sizes are scattered over the sg list not making up to MPS or total transfer size within TRB cache size with Synopsys xHC. Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> --- Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 9946ff9ba735..292d1f7969e4 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -104,6 +104,9 @@ Optional properties: this and tx-thr-num-pkt-prd to a valid, non-zero value 1-16 (DWC_usb31 programming guide section 1.2.3) to enable periodic ESS TX threshold. + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use + SG buffers of at least MPS size by consolidating smaller SG + buffers list into a single buffer. - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt index 3f378951d624..a90d853557ee 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt @@ -43,6 +43,9 @@ Optional properties: - quirk-broken-port-ped: set if the controller has broken port disable mechanism - imod-interval-ns: default interrupt moderation interval is 5000ns - phys : see usb-hcd.yaml in the current directory + - consolidate-sgl: indicate if you need to consolidate sg list into a + temporary buffer when small SG buffer sizes does not make upto MPS + size or total transfer size across the TRB cache size. additionally the properties from usb-hcd.yaml (in the current directory) are supported. -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar 2020-03-27 9:41 ` [RESENDING RFC " Tejas Joglekar @ 2020-03-27 9:54 ` Greg KH 2020-03-27 10:11 ` Tejas Joglekar 2020-04-05 1:45 ` Rob Herring 2 siblings, 1 reply; 14+ messages in thread From: Greg KH @ 2020-03-27 9:54 UTC (permalink / raw) To: Tejas Joglekar Cc: linux-usb, devicetree, linux-kernel, Rob Herring, Mark Rutland, John Youn On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: > This commit adds the documentation for consolidate-sgl, and > snps,consolidate-sgl property. These when set enables the quirk for > XHCI driver for consolidation of sg list into a temporary buffer when small > buffer sizes are scattered over the sg list not making up to MPS or total > transfer size within TRB cache size with Synopsys xHC. > > Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ > Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt > index 9946ff9ba735..292d1f7969e4 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > @@ -104,6 +104,9 @@ Optional properties: > this and tx-thr-num-pkt-prd to a valid, non-zero value > 1-16 (DWC_usb31 programming guide section 1.2.3) to > enable periodic ESS TX threshold. > + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use > + SG buffers of at least MPS size by consolidating smaller SG > + buffers list into a single buffer. > > - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. > - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 > diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt > index 3f378951d624..a90d853557ee 100644 > --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt > @@ -43,6 +43,9 @@ Optional properties: > - quirk-broken-port-ped: set if the controller has broken port disable mechanism > - imod-interval-ns: default interrupt moderation interval is 5000ns > - phys : see usb-hcd.yaml in the current directory > + - consolidate-sgl: indicate if you need to consolidate sg list into a > + temporary buffer when small SG buffer sizes does not make upto MPS > + size or total transfer size across the TRB cache size. Shouldn't this refer to the fact that the hardware is broken? Otherwise why would anyone know if they should, or should not, enable this option? thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 9:54 ` Greg KH @ 2020-03-27 10:11 ` Tejas Joglekar 2020-03-27 10:24 ` Greg KH 0 siblings, 1 reply; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 10:11 UTC (permalink / raw) To: Greg KH, Tejas Joglekar Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland, John Youn Hi, On 3/27/2020 3:24 PM, Greg KH wrote: > On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >> This commit adds the documentation for consolidate-sgl, and >> snps,consolidate-sgl property. These when set enables the quirk for >> XHCI driver for consolidation of sg list into a temporary buffer when small >> buffer sizes are scattered over the sg list not making up to MPS or total >> transfer size within TRB cache size with Synopsys xHC. >> >> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >> --- >> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >> index 9946ff9ba735..292d1f7969e4 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >> @@ -104,6 +104,9 @@ Optional properties: >> this and tx-thr-num-pkt-prd to a valid, non-zero value >> 1-16 (DWC_usb31 programming guide section 1.2.3) to >> enable periodic ESS TX threshold. >> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >> + SG buffers of at least MPS size by consolidating smaller SG >> + buffers list into a single buffer. >> >> - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. >> - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> index 3f378951d624..a90d853557ee 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> @@ -43,6 +43,9 @@ Optional properties: >> - quirk-broken-port-ped: set if the controller has broken port disable mechanism >> - imod-interval-ns: default interrupt moderation interval is 5000ns >> - phys : see usb-hcd.yaml in the current directory >> + - consolidate-sgl: indicate if you need to consolidate sg list into a >> + temporary buffer when small SG buffer sizes does not make upto MPS >> + size or total transfer size across the TRB cache size. > > Shouldn't this refer to the fact that the hardware is broken? Otherwise > why would anyone know if they should, or should not, enable this option? > We have not seen issue with Linux environment for now. But with other OS with SG list with very small buffer sizes the xHC controller hang was seen. So currently introducing the binding as optional one. One could enable this option when xHC halt happens due to small SG list sizes. > thanks, > > greg k-h > Thanks & Regards, Tejas Joglekar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 10:11 ` Tejas Joglekar @ 2020-03-27 10:24 ` Greg KH 2020-03-27 10:28 ` Tejas Joglekar 0 siblings, 1 reply; 14+ messages in thread From: Greg KH @ 2020-03-27 10:24 UTC (permalink / raw) To: Tejas Joglekar Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland, John Youn On Fri, Mar 27, 2020 at 10:11:59AM +0000, Tejas Joglekar wrote: > Hi, > On 3/27/2020 3:24 PM, Greg KH wrote: > > On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: > >> This commit adds the documentation for consolidate-sgl, and > >> snps,consolidate-sgl property. These when set enables the quirk for > >> XHCI driver for consolidation of sg list into a temporary buffer when small > >> buffer sizes are scattered over the sg list not making up to MPS or total > >> transfer size within TRB cache size with Synopsys xHC. > >> > >> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> > >> --- > >> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ > >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ > >> 2 files changed, 6 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt > >> index 9946ff9ba735..292d1f7969e4 100644 > >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt > >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > >> @@ -104,6 +104,9 @@ Optional properties: > >> this and tx-thr-num-pkt-prd to a valid, non-zero value > >> 1-16 (DWC_usb31 programming guide section 1.2.3) to > >> enable periodic ESS TX threshold. > >> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use > >> + SG buffers of at least MPS size by consolidating smaller SG > >> + buffers list into a single buffer. > >> > >> - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. > >> - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 > >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt > >> index 3f378951d624..a90d853557ee 100644 > >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt > >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt > >> @@ -43,6 +43,9 @@ Optional properties: > >> - quirk-broken-port-ped: set if the controller has broken port disable mechanism > >> - imod-interval-ns: default interrupt moderation interval is 5000ns > >> - phys : see usb-hcd.yaml in the current directory > >> + - consolidate-sgl: indicate if you need to consolidate sg list into a > >> + temporary buffer when small SG buffer sizes does not make upto MPS > >> + size or total transfer size across the TRB cache size. > > > > Shouldn't this refer to the fact that the hardware is broken? Otherwise > > why would anyone know if they should, or should not, enable this option? > > > We have not seen issue with Linux environment for now. But with other OS with > SG list with very small buffer sizes the xHC controller hang was seen. So > currently introducing the binding as optional one. One could enable this > option when xHC halt happens due to small SG list sizes. What I mean is this should be something like, "quirk-broken-sg-list-handler" or something like that. Otherwise how does anyone know if this really is needed or not. Reading this would seem like everyone would like to do this, as consolidating links sounds like a good optimization, when instead this really cause more memory allocations, making this possibly worse performance. thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 10:24 ` Greg KH @ 2020-03-27 10:28 ` Tejas Joglekar 0 siblings, 0 replies; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 10:28 UTC (permalink / raw) To: Greg KH, Tejas Joglekar Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland, John Youn On 3/27/2020 3:54 PM, Greg KH wrote: > On Fri, Mar 27, 2020 at 10:11:59AM +0000, Tejas Joglekar wrote: >> Hi, >> On 3/27/2020 3:24 PM, Greg KH wrote: >>> On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >>>> This commit adds the documentation for consolidate-sgl, and >>>> snps,consolidate-sgl property. These when set enables the quirk for >>>> XHCI driver for consolidation of sg list into a temporary buffer when small >>>> buffer sizes are scattered over the sg list not making up to MPS or total >>>> transfer size within TRB cache size with Synopsys xHC. >>>> >>>> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >>>> --- >>>> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >>>> 2 files changed, 6 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>>> index 9946ff9ba735..292d1f7969e4 100644 >>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>>> @@ -104,6 +104,9 @@ Optional properties: >>>> this and tx-thr-num-pkt-prd to a valid, non-zero value >>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to >>>> enable periodic ESS TX threshold. >>>> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >>>> + SG buffers of at least MPS size by consolidating smaller SG >>>> + buffers list into a single buffer. >>>> >>>> - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated. >>>> - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0 >>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt >>>> index 3f378951d624..a90d853557ee 100644 >>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >>>> @@ -43,6 +43,9 @@ Optional properties: >>>> - quirk-broken-port-ped: set if the controller has broken port disable mechanism >>>> - imod-interval-ns: default interrupt moderation interval is 5000ns >>>> - phys : see usb-hcd.yaml in the current directory >>>> + - consolidate-sgl: indicate if you need to consolidate sg list into a >>>> + temporary buffer when small SG buffer sizes does not make upto MPS >>>> + size or total transfer size across the TRB cache size. >>> >>> Shouldn't this refer to the fact that the hardware is broken? Otherwise >>> why would anyone know if they should, or should not, enable this option? >>> >> We have not seen issue with Linux environment for now. But with other OS with >> SG list with very small buffer sizes the xHC controller hang was seen. So >> currently introducing the binding as optional one. One could enable this >> option when xHC halt happens due to small SG list sizes. > > What I mean is this should be something like, > "quirk-broken-sg-list-handler" or something like that. Otherwise how > does anyone know if this really is needed or not. Reading this would > seem like everyone would like to do this, as consolidating links > sounds like a good optimization, when instead this really cause more > memory allocations, making this possibly worse performance. > Sure, understood. I will rename this entry. > thanks, > > greg k-h > Thanks & Regards, Tejas Joglekar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar 2020-03-27 9:41 ` [RESENDING RFC " Tejas Joglekar 2020-03-27 9:54 ` Greg KH @ 2020-04-05 1:45 ` Rob Herring 2020-04-14 10:09 ` Tejas Joglekar 2 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2020-04-05 1:45 UTC (permalink / raw) To: Tejas Joglekar Cc: linux-usb, devicetree, linux-kernel, Mark Rutland, John Youn On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: > This commit adds the documentation for consolidate-sgl, and > snps,consolidate-sgl property. These when set enables the quirk for > XHCI driver for consolidation of sg list into a temporary buffer when small > buffer sizes are scattered over the sg list not making up to MPS or total > transfer size within TRB cache size with Synopsys xHC. > > Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ > Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt > index 9946ff9ba735..292d1f7969e4 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > @@ -104,6 +104,9 @@ Optional properties: > this and tx-thr-num-pkt-prd to a valid, non-zero value > 1-16 (DWC_usb31 programming guide section 1.2.3) to > enable periodic ESS TX threshold. > + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use > + SG buffers of at least MPS size by consolidating smaller SG > + buffers list into a single buffer. The preference is not to keep adding properties for every single quirk or feature. These should be implied by specific compatibles. As Synopsys knows what quirks/errata/features are in each version of IP, the compatible strings should reflect those versions. (And yes, I'm sure there's customer ECO fixes that aren't reflected in the version, but that's why we have SoC specific compatibles too.) This is the only way we can fix quirks in the OS without doing DT updates. For comparison, do you want to have to update your PC BIOS so an OS can work-around issues? Rob ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-04-05 1:45 ` Rob Herring @ 2020-04-14 10:09 ` Tejas Joglekar 2020-04-16 8:22 ` Felipe Balbi 0 siblings, 1 reply; 14+ messages in thread From: Tejas Joglekar @ 2020-04-14 10:09 UTC (permalink / raw) To: Rob Herring, Tejas Joglekar, balbi@kernel.org Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Rutland, John Youn Hi, On 4/5/2020 7:15 AM, Rob Herring wrote: > On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >> This commit adds the documentation for consolidate-sgl, and >> snps,consolidate-sgl property. These when set enables the quirk for >> XHCI driver for consolidation of sg list into a temporary buffer when small >> buffer sizes are scattered over the sg list not making up to MPS or total >> transfer size within TRB cache size with Synopsys xHC. >> >> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >> --- >> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >> index 9946ff9ba735..292d1f7969e4 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >> @@ -104,6 +104,9 @@ Optional properties: >> this and tx-thr-num-pkt-prd to a valid, non-zero value >> 1-16 (DWC_usb31 programming guide section 1.2.3) to >> enable periodic ESS TX threshold. >> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >> + SG buffers of at least MPS size by consolidating smaller SG >> + buffers list into a single buffer. > > The preference is not to keep adding properties for every single quirk > or feature. These should be implied by specific compatibles. As Synopsys > knows what quirks/errata/features are in each version of IP, the > compatible strings should reflect those versions. (And yes, I'm sure > there's customer ECO fixes that aren't reflected in the version, but > that's why we have SoC specific compatibles too.) This is the only way > we can fix quirks in the OS without doing DT updates. For comparison, do > you want to have to update your PC BIOS so an OS can work-around issues? > Yes, I understand what you want to say here. But I think this compatible string does not work with the platform drivers with PCI based systems. So based on the vendor id and device id then I need to set the quirk required. @Felipe: What do you suggest for setting up quirk without the DT update for dwc3? > Rob > Thanks & Regards, Tejas Joglekar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-04-14 10:09 ` Tejas Joglekar @ 2020-04-16 8:22 ` Felipe Balbi 2020-04-16 16:37 ` Tejas Joglekar 0 siblings, 1 reply; 14+ messages in thread From: Felipe Balbi @ 2020-04-16 8:22 UTC (permalink / raw) To: Tejas Joglekar, Rob Herring, Tejas Joglekar Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Rutland, John Youn [-- Attachment #1: Type: text/plain, Size: 2694 bytes --] Hi, Tejas Joglekar <Tejas.Joglekar@synopsys.com> writes: > Hi, > On 4/5/2020 7:15 AM, Rob Herring wrote: >> On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >>> This commit adds the documentation for consolidate-sgl, and >>> snps,consolidate-sgl property. These when set enables the quirk for >>> XHCI driver for consolidation of sg list into a temporary buffer when small >>> buffer sizes are scattered over the sg list not making up to MPS or total >>> transfer size within TRB cache size with Synopsys xHC. >>> >>> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >>> --- >>> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >>> 2 files changed, 6 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>> index 9946ff9ba735..292d1f7969e4 100644 >>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>> @@ -104,6 +104,9 @@ Optional properties: >>> this and tx-thr-num-pkt-prd to a valid, non-zero value >>> 1-16 (DWC_usb31 programming guide section 1.2.3) to >>> enable periodic ESS TX threshold. >>> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >>> + SG buffers of at least MPS size by consolidating smaller SG >>> + buffers list into a single buffer. >> >> The preference is not to keep adding properties for every single quirk >> or feature. These should be implied by specific compatibles. As Synopsys >> knows what quirks/errata/features are in each version of IP, the >> compatible strings should reflect those versions. (And yes, I'm sure >> there's customer ECO fixes that aren't reflected in the version, but >> that's why we have SoC specific compatibles too.) This is the only way >> we can fix quirks in the OS without doing DT updates. For comparison, do >> you want to have to update your PC BIOS so an OS can work-around issues? >> > Yes, I understand what you want to say here. But I think this compatible string > does not work with the platform drivers with PCI based systems. So based on the vendor > id and device id then I need to set the quirk required. > > @Felipe: What do you suggest for setting up quirk without the DT update for dwc3? We have been using Synopsys controller's revision register, but that's not visible to xhci driver and we don't have a separate compatible for each synopsys version on the xhci driver side. One option would be to add "snps,xhci-foo-bar" to xhci-plat and use that. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-04-16 8:22 ` Felipe Balbi @ 2020-04-16 16:37 ` Tejas Joglekar 2020-04-17 11:44 ` Mathias Nyman 0 siblings, 1 reply; 14+ messages in thread From: Tejas Joglekar @ 2020-04-16 16:37 UTC (permalink / raw) To: Felipe Balbi, Tejas Joglekar, Rob Herring, Mathias Nyman Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Rutland, John Youn Hi, On 4/16/2020 1:52 PM, Felipe Balbi wrote: > > Hi, > > Tejas Joglekar <Tejas.Joglekar@synopsys.com> writes: >> Hi, >> On 4/5/2020 7:15 AM, Rob Herring wrote: >>> On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >>>> This commit adds the documentation for consolidate-sgl, and >>>> snps,consolidate-sgl property. These when set enables the quirk for >>>> XHCI driver for consolidation of sg list into a temporary buffer when small >>>> buffer sizes are scattered over the sg list not making up to MPS or total >>>> transfer size within TRB cache size with Synopsys xHC. >>>> >>>> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >>>> --- >>>> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >>>> 2 files changed, 6 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>>> index 9946ff9ba735..292d1f7969e4 100644 >>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>>> @@ -104,6 +104,9 @@ Optional properties: >>>> this and tx-thr-num-pkt-prd to a valid, non-zero value >>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to >>>> enable periodic ESS TX threshold. >>>> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >>>> + SG buffers of at least MPS size by consolidating smaller SG >>>> + buffers list into a single buffer. >>> >>> The preference is not to keep adding properties for every single quirk >>> or feature. These should be implied by specific compatibles. As Synopsys >>> knows what quirks/errata/features are in each version of IP, the >>> compatible strings should reflect those versions. (And yes, I'm sure >>> there's customer ECO fixes that aren't reflected in the version, but >>> that's why we have SoC specific compatibles too.) This is the only way >>> we can fix quirks in the OS without doing DT updates. For comparison, do >>> you want to have to update your PC BIOS so an OS can work-around issues? >>> >> Yes, I understand what you want to say here. But I think this compatible string >> does not work with the platform drivers with PCI based systems. So based on the vendor >> id and device id then I need to set the quirk required. >> >> @Felipe: What do you suggest for setting up quirk without the DT update for dwc3? > > We have been using Synopsys controller's revision register, but that's > not visible to xhci driver and we don't have a separate compatible for > each synopsys version on the xhci driver side. One option would be to > add "snps,xhci-foo-bar" to xhci-plat and use that. > @Felipe: Thanks for suggestion @Mathias: Do you prefer parameter passed this way? I can send the v2 patch set, after your input. Thanks & Regards, Tejas Joglekar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl 2020-04-16 16:37 ` Tejas Joglekar @ 2020-04-17 11:44 ` Mathias Nyman 0 siblings, 0 replies; 14+ messages in thread From: Mathias Nyman @ 2020-04-17 11:44 UTC (permalink / raw) To: Tejas Joglekar, Felipe Balbi, Rob Herring, Mathias Nyman Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Rutland, John Youn On 16.4.2020 19.37, Tejas Joglekar wrote: > Hi, > On 4/16/2020 1:52 PM, Felipe Balbi wrote: >> >> Hi, >> >> Tejas Joglekar <Tejas.Joglekar@synopsys.com> writes: >>> Hi, >>> On 4/5/2020 7:15 AM, Rob Herring wrote: >>>> On Fri, Mar 27, 2020 at 03:11:56PM +0530, Tejas Joglekar wrote: >>>>> This commit adds the documentation for consolidate-sgl, and >>>>> snps,consolidate-sgl property. These when set enables the quirk for >>>>> XHCI driver for consolidation of sg list into a temporary buffer when small >>>>> buffer sizes are scattered over the sg list not making up to MPS or total >>>>> transfer size within TRB cache size with Synopsys xHC. >>>>> >>>>> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com> >>>>> --- >>>>> Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ >>>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++ >>>>> 2 files changed, 6 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>> index 9946ff9ba735..292d1f7969e4 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>> @@ -104,6 +104,9 @@ Optional properties: >>>>> this and tx-thr-num-pkt-prd to a valid, non-zero value >>>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to >>>>> enable periodic ESS TX threshold. >>>>> + - snps,consolidate-sgl: enable sg list consolidation - host mode only. Set to use >>>>> + SG buffers of at least MPS size by consolidating smaller SG >>>>> + buffers list into a single buffer. >>>> >>>> The preference is not to keep adding properties for every single quirk >>>> or feature. These should be implied by specific compatibles. As Synopsys >>>> knows what quirks/errata/features are in each version of IP, the >>>> compatible strings should reflect those versions. (And yes, I'm sure >>>> there's customer ECO fixes that aren't reflected in the version, but >>>> that's why we have SoC specific compatibles too.) This is the only way >>>> we can fix quirks in the OS without doing DT updates. For comparison, do >>>> you want to have to update your PC BIOS so an OS can work-around issues? >>>> >>> Yes, I understand what you want to say here. But I think this compatible string >>> does not work with the platform drivers with PCI based systems. So based on the vendor >>> id and device id then I need to set the quirk required. >>> >>> @Felipe: What do you suggest for setting up quirk without the DT update for dwc3? >> >> We have been using Synopsys controller's revision register, but that's >> not visible to xhci driver and we don't have a separate compatible for >> each synopsys version on the xhci driver side. One option would be to >> add "snps,xhci-foo-bar" to xhci-plat and use that. >> > @Felipe: Thanks for suggestion > > @Mathias: Do you prefer parameter passed this way? I can send the v2 patch set, > after your input. should work, I don't see any issues with that. -Mathias ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC 2020-03-27 8:52 [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Tejas Joglekar 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar @ 2020-03-27 9:28 ` Greg KH 2020-03-27 9:43 ` Tejas Joglekar 1 sibling, 1 reply; 14+ messages in thread From: Greg KH @ 2020-03-27 9:28 UTC (permalink / raw) To: Tejas Joglekar Cc: Felipe Balbi, linux-usb, devicetree, Rob Herring, Chunfeng Yun, Fredrik Noring, Mathias Nyman, Mark Rutland, Sebastian Andrzej Siewior, Raul E Rangel, Laurentiu Tudor, Marek Szyprowski, John Youn On Fri, Mar 27, 2020 at 02:22:23PM +0530, Tejas Joglekar wrote: > The Synopsys xHC has an internal TRB cache of size TRB_CACHE_SIZE for > each endpoint. The default value for TRB_CACHE_SIZE is 16 for SS and 8 > for HS. The controller loads and updates the TRB cache from the > transfer ring in system memory whenever the driver issues a start > transfer or update transfer command. > > For chained TRBs, the Synopsys xHC requires that the total amount of > bytes for all TRBs loaded in the TRB cache be greater than or equal to > 1 MPS. Or the chain ends within the TRB cache (with a last TRB). > > If this requirement is not met, the controller will not be able to > send or receive a packet and it will hang causing a driver timeout and > error. > > This patch set adds logic to the XHCI driver to detect and prevent this > from happening along with the quirk to enable this logic for Synopsys > HAPS platform. > > Based on Mathias's feedback on previous implementation where consolidation > was done in TRB cache, with this patch series the implementation is done > during mapping of the URB by consolidating the SG list into a temporary > buffer if the SG list buffer sizes within TRB_CACHE_SIZE is less than MPS. > > > Tejas Joglekar (4): > dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl > usb: xhci: Set quirk for XHCI_CONSOLIDATE_SG_LIST > usb: dwc3: Add device property consolidate-sgl > usb: xhci: Use temporary buffer to consolidate SG > > Resending as 'umlaut' is not accepted by some servers in email. Only patch 4/4 seems to have gone through :( ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC 2020-03-27 9:28 ` [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Greg KH @ 2020-03-27 9:43 ` Tejas Joglekar 0 siblings, 0 replies; 14+ messages in thread From: Tejas Joglekar @ 2020-03-27 9:43 UTC (permalink / raw) To: Greg KH, Tejas Joglekar Cc: Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Rob Herring, Chunfeng Yun, Fredrik Noring, Mathias Nyman, Mark Rutland, Sebastian Andrzej Siewior, Raul E Rangel, Laurentiu Tudor, Marek Szyprowski, John Youn On 3/27/2020 2:58 PM, Greg KH wrote: > On Fri, Mar 27, 2020 at 02:22:23PM +0530, Tejas Joglekar wrote: >> The Synopsys xHC has an internal TRB cache of size TRB_CACHE_SIZE for >> each endpoint. The default value for TRB_CACHE_SIZE is 16 for SS and 8 >> for HS. The controller loads and updates the TRB cache from the >> transfer ring in system memory whenever the driver issues a start >> transfer or update transfer command. >> >> For chained TRBs, the Synopsys xHC requires that the total amount of >> bytes for all TRBs loaded in the TRB cache be greater than or equal to >> 1 MPS. Or the chain ends within the TRB cache (with a last TRB). >> >> If this requirement is not met, the controller will not be able to >> send or receive a packet and it will hang causing a driver timeout and >> error. >> >> This patch set adds logic to the XHCI driver to detect and prevent this >> from happening along with the quirk to enable this logic for Synopsys >> HAPS platform. >> >> Based on Mathias's feedback on previous implementation where consolidation >> was done in TRB cache, with this patch series the implementation is done >> during mapping of the URB by consolidating the SG list into a temporary >> buffer if the SG list buffer sizes within TRB_CACHE_SIZE is less than MPS. >> >> >> Tejas Joglekar (4): >> dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl >> usb: xhci: Set quirk for XHCI_CONSOLIDATE_SG_LIST >> usb: dwc3: Add device property consolidate-sgl >> usb: xhci: Use temporary buffer to consolidate SG >> >> Resending as 'umlaut' is not accepted by some servers in email. > > Only patch 4/4 seems to have gone through :( > Resent the missing patches ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-04-17 11:42 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-27 8:52 [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Tejas Joglekar 2020-03-27 8:34 ` [RFC PATCH 1/4] dt-bindings: usb: Add snps,consolidate-sgl & consolidate-sgl Tejas Joglekar 2020-03-27 9:41 ` [RESENDING RFC " Tejas Joglekar 2020-03-27 9:54 ` Greg KH 2020-03-27 10:11 ` Tejas Joglekar 2020-03-27 10:24 ` Greg KH 2020-03-27 10:28 ` Tejas Joglekar 2020-04-05 1:45 ` Rob Herring 2020-04-14 10:09 ` Tejas Joglekar 2020-04-16 8:22 ` Felipe Balbi 2020-04-16 16:37 ` Tejas Joglekar 2020-04-17 11:44 ` Mathias Nyman 2020-03-27 9:28 ` [RESENDING RFC PATCH 0/4] Add logic to consolidate TRBs for Synopsys xHC Greg KH 2020-03-27 9:43 ` Tejas Joglekar
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).