[PATCH v2 0/2] Add support for xhci-sg-trb-cache-size-quirk

[PATCH v2 0/2] Add support for xhci-sg-trb-cache-size-quirk

[Prashanth K]

XHCI_SG_TRB_CACHE_SIZE_QUIRK was introduced in XHCI to resolve
XHC timeout while using SG buffers, which was seen Synopsys XHCs.
The support for this isn't present in DWC3 layer, this series
enables XHCI_SG_TRB_CACHE_SIZE_QUIRK since this is needed for
DWC3 controller.

Changes in v2:
Included xhci-plat.c to pass the quirks via priv data.
Added Fixes tag in device tree binding patch.

Prashanth K (2):
  usb: dwc3: core: Add support for xhci-sg-trb-cache-size-quirk
  dt-bindings: usb: snps,dwc3: Add 'xhci-sg-trb-cache-size-quirk'

 Documentation/devicetree/bindings/usb/snps,dwc3.yaml |  7 +++++++
 drivers/usb/dwc3/core.c                              |  2 ++
 drivers/usb/dwc3/core.h                              |  3 +++
 drivers/usb/dwc3/host.c                              | 10 ++++++++++
 4 files changed, 22 insertions(+)

-- 
2.25.1

[PATCH v2 1/2] usb: dwc3: core: Add support for xhci-sg-trb-cache-size-quirk

[Prashanth K]

Upstream commit bac1ec551434 ("usb: xhci: Set quirk for
XHCI_SG_TRB_CACHE_SIZE_QUIRK") introduced a new quirk in XHCI
which fixes XHC timeout while using SG buffers. Add a new quirk
in DWC3 layer to enable this XHCI quirk since this is needed
for DWC3 controller. Added xhci private data structure to pass
the quirk to XHCI.

In Synopsys DWC3 databook,
Table 9-3: xHCI Debug Capability Limitations
Chained TRBs greater than TRB cache size: The debug capability
driver must not create a multi-TRB TD that describes smaller
than a 1K packet that spreads across 8 or more TRBs on either
the IN TR or the OUT TR

Cc: <stable@vger.kernel.org> # 5.11
Fixes: bac1ec551434 ("usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK")
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
---
 drivers/usb/dwc3/core.c |  2 ++
 drivers/usb/dwc3/core.h |  3 +++
 drivers/usb/dwc3/host.c | 10 ++++++++++
 3 files changed, 15 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0328c86ef806..c6b0ae941678 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1633,6 +1633,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 				"snps,parkmode-disable-hs-quirk");
 	dwc->gfladj_refclk_lpm_sel = device_property_read_bool(dev,
 				"snps,gfladj-refclk-lpm-sel-quirk");
+	dwc->xhci_sg_trb_cache_size_quirk = device_property_read_bool(dev,
+				"xhci-sg-trb-cache-size-quirk");
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index efe6caf4d0e8..dde144734119 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1119,6 +1119,8 @@ struct dwc3_scratchpad_array {
  *			instances in park mode.
  * @gfladj_refclk_lpm_sel: set if we need to enable SOF/ITP counter
  *                          running based on ref_clk
+ * @xhci_sg_trb_cache_size_quirk: set to prevent XHC timeout when scatter-gather
+ *			is enabled due to TRB_CACHE_SIZE.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  *	0	- -6dB de-emphasis
@@ -1342,6 +1344,7 @@ struct dwc3 {
 	unsigned		parkmode_disable_ss_quirk:1;
 	unsigned		parkmode_disable_hs_quirk:1;
 	unsigned		gfladj_refclk_lpm_sel:1;
+	unsigned		xhci_sg_trb_cache_size_quirk:1;
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 61f57fe5bb78..6424942fd673 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
+#include "../host/xhci-plat.h"
 #include "core.h"
 
 static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc,
@@ -63,6 +64,7 @@ int dwc3_host_init(struct dwc3 *dwc)
 {
 	struct property_entry	props[4];
 	struct platform_device	*xhci;
+	struct xhci_plat_priv   dwc3_xhci_plat_priv = {0};
 	int			ret, irq;
 	int			prop_idx = 0;
 
@@ -87,6 +89,14 @@ int dwc3_host_init(struct dwc3 *dwc)
 		goto err;
 	}
 
+	if (dwc->xhci_sg_trb_cache_size_quirk)
+		dwc3_xhci_plat_priv.quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
+
+	ret = platform_device_add_data(xhci, &dwc3_xhci_plat_priv,
+					sizeof(dwc3_xhci_plat_priv));
+	if (ret)
+		goto err;
+
 	memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props));
 
 	if (dwc->usb3_lpm_capable)
-- 
2.25.1

[PATCH v2 2/2] dt-bindings: usb: snps,dwc3: Add 'xhci-sg-trb-cache-size-quirk'

[Prashanth K]

Add a new 'xhci-sg-trb-cache-size-quirk' DT quirk to dwc3 core
for preventing xhci hang issue while using SG buffers.

Cc: <stable@vger.kernel.org> # 5.11
Fixes: bac1ec551434 ("usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK")
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index ee5af4b381b1..768fdb5b1f05 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -459,6 +459,13 @@ properties:
     description:
       Enable USB remote wakeup.
 
+  xhci-sg-trb-cache-size-quirk:
+    description:
+      When set, fixes the SNPS xHC hang issue when the data is scattered across
+      small buffers which does not make at least MPS size for given controller
+      TRB cache size.
+    type: boolean
+
 unevaluatedProperties: false
 
 required:
-- 
2.25.1

Re: [PATCH v2 2/2] dt-bindings: usb: snps,dwc3: Add 'xhci-sg-trb-cache-size-quirk'

[Krzysztof Kozlowski]

On 20/11/2023 06:58, Prashanth K wrote:
> Add a new 'xhci-sg-trb-cache-size-quirk' DT quirk to dwc3 core
> for preventing xhci hang issue while using SG buffers.

Neither commit msg nor property describes the hardware feature. Please
describe the hardware, not OS behavior, in the property. Both in the
property name and in its description.

> 
> Cc: <stable@vger.kernel.org> # 5.11
> Fixes: bac1ec551434 ("usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK")
> Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
> ---

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

Also, use proper order of patches - first bindings, then their user.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: usb: snps,dwc3: Add 'xhci-sg-trb-cache-size-quirk'

[Krzysztof Kozlowski]

On 20/11/2023 06:58, Prashanth K wrote:
> Add a new 'xhci-sg-trb-cache-size-quirk' DT quirk to dwc3 core
> for preventing xhci hang issue while using SG buffers.
> 
> Cc: <stable@vger.kernel.org> # 5.11
> Fixes: bac1ec551434 ("usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK")

Also, you need to describe the bug to justify backporting. Don't add
useless stable-cc tags just because kernel robot asked you...


Best regards,
Krzysztof

Re: [PATCH v2 0/2] Add support for xhci-sg-trb-cache-size-quirk

[Krzysztof Kozlowski]

On 20/11/2023 06:58, Prashanth K wrote:
> XHCI_SG_TRB_CACHE_SIZE_QUIRK was introduced in XHCI to resolve
> XHC timeout while using SG buffers, which was seen Synopsys XHCs.
> The support for this isn't present in DWC3 layer, this series
> enables XHCI_SG_TRB_CACHE_SIZE_QUIRK since this is needed for
> DWC3 controller.

You keep Cc-ing incorrect mailing lists (bogus addresses). Just use
get_maintainers.pl  --no-git-fallback without changing its output.

I repeated this comment multiple times to Qualcomm. It's awesome that
Qualcomm participates so much in upstream development, I really
appreciate this. However repeating the same comment over-and-over again,
makes me quite tired. Can you instruct your colleagues to use b4 which
solves this problem? If not, use script like:
https://github.com/krzk/tools/blob/master/linux/.bash_aliases_linux#L91
(or one of many other variants posted by multiple people on the mailing
lists)

Best regards,
Krzysztof

Re: [PATCH v2 0/2] Add support for xhci-sg-trb-cache-size-quirk

[Prashanth K]

On 20-11-23 03:00 pm, Krzysztof Kozlowski wrote:
> On 20/11/2023 06:58, Prashanth K wrote:
>> XHCI_SG_TRB_CACHE_SIZE_QUIRK was introduced in XHCI to resolve
>> XHC timeout while using SG buffers, which was seen Synopsys XHCs.
>> The support for this isn't present in DWC3 layer, this series
>> enables XHCI_SG_TRB_CACHE_SIZE_QUIRK since this is needed for
>> DWC3 controller.
> 
> You keep Cc-ing incorrect mailing lists (bogus addresses). Just use
> get_maintainers.pl  --no-git-fallback without changing its output.
> 
> I repeated this comment multiple times to Qualcomm. It's awesome that
> Qualcomm participates so much in upstream development, I really
> appreciate this. However repeating the same comment over-and-over again,
> makes me quite tired. Can you instruct your colleagues to use b4 which
> solves this problem? If not, use script like:
> https://github.com/krzk/tools/blob/master/linux/.bash_aliases_linux#L91
> (or one of many other variants posted by multiple people on the mailing
> lists)
> 
> Best regards,
> Krzysztof
> 



Thanks for your comments! I accidentally added 'yy' in the USB mailing 
list while configuring it. A careless mistake indeed :)

I will resend the the patch without adding the quirk (only driver 
change) since this should be applicable for all the dwc3 devices.

Thanks again,
Prashanth K