[RFC PATCH v2 0/4] Add logic to consolidate TRBs for Synopsys xHC

[RFC PATCH v2 0/4] Add logic to consolidate TRBs for Synopsys xHC

[Tejas Joglekar]

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.

Changes since v1:
 - Comments from Greg are addressed on [PATCH 4/4] and [PATCH 1/4]
 - Renamed the property and quirk as in other patches based on [PATCH 1/4]

Tejas Joglekar (4):
  dt-bindings: usb: Add documentation for SG trb cache size quirk
  usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK
  usb: dwc3: Add device property sgl-trb-cache-size-quirk
  usb: xhci: Use temporary buffer to consolidate SG

 Documentation/devicetree/bindings/usb/dwc3.txt     |   4 +
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   3 +
 drivers/usb/dwc3/core.c                            |   2 +
 drivers/usb/dwc3/core.h                            |   2 +
 drivers/usb/dwc3/dwc3-haps.c                       |   1 +
 drivers/usb/dwc3/host.c                            |   6 +-
 drivers/usb/host/xhci-pci.c                        |   3 +
 drivers/usb/host/xhci-plat.c                       |   4 +
 drivers/usb/host/xhci-ring.c                       |   2 +-
 drivers/usb/host/xhci.c                            | 125 +++++++++++++++++++++
 drivers/usb/host/xhci.h                            |   5 +
 11 files changed, 155 insertions(+), 2 deletions(-)

-- 
2.11.0

[RFC PATCH v2 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk

[Tejas Joglekar]

This commit adds the documentation for sgl-trb-cache-size-quirk, and
snps,sgl-trb-cache-size-quirk 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>
---
 Changes in v2:
 - Renamed the property

 Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 9946ff9ba735..6d0418ee4dbd 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -104,6 +104,10 @@ 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,sgl-trb-cache-size-quirk: 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..14d900474894 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
+  - sgl-trb-cache-size-quirk: set 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

Re: [RFC PATCH v2 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk

[Rob Herring]

On Tue, Apr 21, 2020 at 03:18:09PM +0530, Tejas Joglekar wrote:
> This commit adds the documentation for sgl-trb-cache-size-quirk, and
> snps,sgl-trb-cache-size-quirk 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>
> ---
>  Changes in v2:
>  - Renamed the property
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
>  Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 9946ff9ba735..6d0418ee4dbd 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -104,6 +104,10 @@ 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,sgl-trb-cache-size-quirk: 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..14d900474894 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
> +  - sgl-trb-cache-size-quirk: set 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.

Why do we have 2 different names?

>  
>  additionally the properties from usb-hcd.yaml (in the current directory) are
>  supported.
> -- 
> 2.11.0
> 

Re: [RFC PATCH v2 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk

[Rob Herring]

On Tue, Apr 21, 2020 at 03:18:09PM +0530, Tejas Joglekar wrote:
> This commit adds the documentation for sgl-trb-cache-size-quirk, and
> snps,sgl-trb-cache-size-quirk 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>

The author and S-o-b emails don't match.

> ---
>  Changes in v2:
>  - Renamed the property
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
>  Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 9946ff9ba735..6d0418ee4dbd 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -104,6 +104,10 @@ 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,sgl-trb-cache-size-quirk: 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..14d900474894 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
> +  - sgl-trb-cache-size-quirk: set 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
> 

Re: [RFC PATCH v2 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk

[Tejas Joglekar]

Hi,
On 5/7/2020 1:45 AM, Rob Herring wrote:
> On Tue, Apr 21, 2020 at 03:18:09PM +0530, Tejas Joglekar wrote:
>> This commit adds the documentation for sgl-trb-cache-size-quirk, and
>> snps,sgl-trb-cache-size-quirk 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>
>> ---
>>  Changes in v2:
>>  - Renamed the property
>>
>>  Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
>>  Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 9946ff9ba735..6d0418ee4dbd 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -104,6 +104,10 @@ 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,sgl-trb-cache-size-quirk: 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..14d900474894 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
>> +  - sgl-trb-cache-size-quirk: set 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.
> 
> Why do we have 2 different names?
> 
I tried to follow the usb2-lpm-disable property implementation where in usb-xhci
the snps is not added to the property name. Should I use snps,sgl-trb-cache-size-quirk
in usb-xhci too ?
>>  
>>  additionally the properties from usb-hcd.yaml (in the current directory) are
>>  supported.
>> -- 
>> 2.11.0
>>

Thanks & Regards,
  Tejas Joglekar

Re: [RFC PATCH v2 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk

[Tejas Joglekar]

Hi,
On 5/7/2020 1:46 AM, Rob Herring wrote:
> On Tue, Apr 21, 2020 at 03:18:09PM +0530, Tejas Joglekar wrote:
>> This commit adds the documentation for sgl-trb-cache-size-quirk, and
>> snps,sgl-trb-cache-size-quirk 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>
> 
> The author and S-o-b emails don't match.
> 
I intend to use short email, but the server mangles the full email even if we
set it manually to short email. I have been using the same in the past to send
the patches to community. Do you want me to change the sign-off to sender email address? 

>> ---
>>  Changes in v2:
>>  - Renamed the property
>>
>>  Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
>>  Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 9946ff9ba735..6d0418ee4dbd 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -104,6 +104,10 @@ 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,sgl-trb-cache-size-quirk: 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..14d900474894 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
>> +  - sgl-trb-cache-size-quirk: set 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
>>
Thanks & Regards,
  Tejas Joglekar