All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
@ 2020-12-16 11:51 ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: Chunfeng Yun, Greg Kroah-Hartman, Matthias Brugger, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Hsin-Yi Wang, Ikjoon Jang

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, so add a new
property "mediatek,broken_streams_quirk" to fix it.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
this patch depends on:
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
  [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
---
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e5e03f902802..330f7294bf34 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -120,6 +120,11 @@ properties:
     description: The mask to disable u3ports, bit0 for u3port0,
       bit1 for u3port1, ... etc
 
+  mediatek,broken_streams_quirk:
+    description: set if the controller doesn't support bulk stream but
+      HCCPARAMS says support.
+    type: boolean
+
   "#address-cells":
     const: 1
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
@ 2020-12-16 11:51 ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, so add a new
property "mediatek,broken_streams_quirk" to fix it.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
this patch depends on:
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
  [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
---
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e5e03f902802..330f7294bf34 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -120,6 +120,11 @@ properties:
     description: The mask to disable u3ports, bit0 for u3port0,
       bit1 for u3port1, ... etc
 
+  mediatek,broken_streams_quirk:
+    description: set if the controller doesn't support bulk stream but
+      HCCPARAMS says support.
+    type: boolean
+
   "#address-cells":
     const: 1
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
@ 2020-12-16 11:51 ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, so add a new
property "mediatek,broken_streams_quirk" to fix it.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
this patch depends on:
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
  [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
---
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e5e03f902802..330f7294bf34 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -120,6 +120,11 @@ properties:
     description: The mask to disable u3ports, bit0 for u3port0,
       bit1 for u3port1, ... etc
 
+  mediatek,broken_streams_quirk:
+    description: set if the controller doesn't support bulk stream but
+      HCCPARAMS says support.
+    type: boolean
+
   "#address-cells":
     const: 1
 
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-16 11:51 ` Chunfeng Yun
  (?)
@ 2020-12-16 11:51   ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: Chunfeng Yun, Greg Kroah-Hartman, Matthias Brugger, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Hsin-Yi Wang, Ikjoon Jang

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, here use
XHCI_BROKEN_STREAMS quirk to fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 7 ++++++-
 drivers/usb/host/xhci-mtk.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8f321f39ab96..08dab974d847 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
 	xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
 	if (mtk->lpm_support)
 		xhci->quirks |= XHCI_LPM_SUPPORT;
+
+	if (mtk->broken_streams)
+		xhci->quirks |= XHCI_BROKEN_STREAMS;
 }
 
 /* called during probe() after chip reset completes */
@@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 
 	mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
+	mtk->broken_streams =
+		of_property_read_bool(node, "mediatek,broken_streams_quirk");
 	/* optional property, ignore the error if it does not exist */
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
@@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	if (ret)
 		goto put_usb3_hcd;
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe817904..86aa4978915e 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
 	struct phy **phys;
 	int num_phys;
 	bool lpm_support;
+	bool broken_streams;
 	/* usb remote wakeup */
 	bool uwk_en;
 	struct regmap *uwk;
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-16 11:51   ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, here use
XHCI_BROKEN_STREAMS quirk to fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 7 ++++++-
 drivers/usb/host/xhci-mtk.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8f321f39ab96..08dab974d847 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
 	xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
 	if (mtk->lpm_support)
 		xhci->quirks |= XHCI_LPM_SUPPORT;
+
+	if (mtk->broken_streams)
+		xhci->quirks |= XHCI_BROKEN_STREAMS;
 }
 
 /* called during probe() after chip reset completes */
@@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 
 	mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
+	mtk->broken_streams =
+		of_property_read_bool(node, "mediatek,broken_streams_quirk");
 	/* optional property, ignore the error if it does not exist */
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
@@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	if (ret)
 		goto put_usb3_hcd;
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe817904..86aa4978915e 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
 	struct phy **phys;
 	int num_phys;
 	bool lpm_support;
+	bool broken_streams;
 	/* usb remote wakeup */
 	bool uwk_en;
 	struct regmap *uwk;
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-16 11:51   ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

The 0.96 xHCI controller on some platforms does not support
bulk stream even HCCPARAMS says supporting, due to MaxPSASize
is set a non-zero default value by mistake, here use
XHCI_BROKEN_STREAMS quirk to fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 7 ++++++-
 drivers/usb/host/xhci-mtk.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8f321f39ab96..08dab974d847 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
 	xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
 	if (mtk->lpm_support)
 		xhci->quirks |= XHCI_LPM_SUPPORT;
+
+	if (mtk->broken_streams)
+		xhci->quirks |= XHCI_BROKEN_STREAMS;
 }
 
 /* called during probe() after chip reset completes */
@@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 
 	mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
+	mtk->broken_streams =
+		of_property_read_bool(node, "mediatek,broken_streams_quirk");
 	/* optional property, ignore the error if it does not exist */
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
@@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	if (ret)
 		goto put_usb3_hcd;
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe817904..86aa4978915e 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
 	struct phy **phys;
 	int num_phys;
 	bool lpm_support;
+	bool broken_streams;
 	/* usb remote wakeup */
 	bool uwk_en;
 	struct regmap *uwk;
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 3/3] arm64: dts: mt8173: add a broken streams quirk property for xHCI node
  2020-12-16 11:51 ` Chunfeng Yun
  (?)
@ 2020-12-16 11:51   ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: Chunfeng Yun, Greg Kroah-Hartman, Matthias Brugger, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Hsin-Yi Wang, Ikjoon Jang

Due to 0.96 xHCI on mt8173 doesn't support bulk stream but MaxPSASize
says supporting, add a new "mediatek,broken_streams_quirk" property to
fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index e88bbfc1fc55..f305fb033020 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -886,6 +886,7 @@
 				power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
 				clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
 				clock-names = "sys_ck", "ref_ck";
+				mediatek,broken_streams_quirk;
 				status = "disabled";
 			};
 		};
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 3/3] arm64: dts: mt8173: add a broken streams quirk property for xHCI node
@ 2020-12-16 11:51   ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

Due to 0.96 xHCI on mt8173 doesn't support bulk stream but MaxPSASize
says supporting, add a new "mediatek,broken_streams_quirk" property to
fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index e88bbfc1fc55..f305fb033020 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -886,6 +886,7 @@
 				power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
 				clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
 				clock-names = "sys_ck", "ref_ck";
+				mediatek,broken_streams_quirk;
 				status = "disabled";
 			};
 		};
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 3/3] arm64: dts: mt8173: add a broken streams quirk property for xHCI node
@ 2020-12-16 11:51   ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-16 11:51 UTC (permalink / raw)
  To: Rob Herring, Mathias Nyman
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Matthias Brugger, linux-mediatek, Hsin-Yi Wang, Chunfeng Yun,
	Ikjoon Jang, linux-arm-kernel

Due to 0.96 xHCI on mt8173 doesn't support bulk stream but MaxPSASize
says supporting, add a new "mediatek,broken_streams_quirk" property to
fix it.

Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index e88bbfc1fc55..f305fb033020 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -886,6 +886,7 @@
 				power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
 				clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
 				clock-names = "sys_ck", "ref_ck";
+				mediatek,broken_streams_quirk;
 				status = "disabled";
 			};
 		};
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-16 11:51   ` Chunfeng Yun
  (?)
@ 2020-12-16 12:28     ` Nicolas Boichat
  -1 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-16 12:28 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Rob Herring, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
	linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, here use
> XHCI_BROKEN_STREAMS quirk to fix it.
>
> Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk.c | 7 ++++++-
>  drivers/usb/host/xhci-mtk.h | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 8f321f39ab96..08dab974d847 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
>         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>         if (mtk->lpm_support)
>                 xhci->quirks |= XHCI_LPM_SUPPORT;
> +
> +       if (mtk->broken_streams)
> +               xhci->quirks |= XHCI_BROKEN_STREAMS;
>  }
>
>  /* called during probe() after chip reset completes */
> @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 return ret;
>
>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> +       mtk->broken_streams =
> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");

Would it be better to add a data field to struct of_device_id
mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?

(IMHO usb3-lpm-capable detection should also be done in the same way)

Thanks,

>         /* optional property, ignore the error if it does not exist */
>         of_property_read_u32(node, "mediatek,u3p-dis-msk",
>                              &mtk->u3p_dis_msk);
> @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto put_usb3_hcd;
>
> -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
>                 xhci->shared_hcd->can_do_streams = 1;
>
>         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index a93cfe817904..86aa4978915e 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
>         struct phy **phys;
>         int num_phys;
>         bool lpm_support;
> +       bool broken_streams;
>         /* usb remote wakeup */
>         bool uwk_en;
>         struct regmap *uwk;
> --
> 2.18.0

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-16 12:28     ` Nicolas Boichat
  0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-16 12:28 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, here use
> XHCI_BROKEN_STREAMS quirk to fix it.
>
> Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk.c | 7 ++++++-
>  drivers/usb/host/xhci-mtk.h | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 8f321f39ab96..08dab974d847 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
>         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>         if (mtk->lpm_support)
>                 xhci->quirks |= XHCI_LPM_SUPPORT;
> +
> +       if (mtk->broken_streams)
> +               xhci->quirks |= XHCI_BROKEN_STREAMS;
>  }
>
>  /* called during probe() after chip reset completes */
> @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 return ret;
>
>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> +       mtk->broken_streams =
> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");

Would it be better to add a data field to struct of_device_id
mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?

(IMHO usb3-lpm-capable detection should also be done in the same way)

Thanks,

>         /* optional property, ignore the error if it does not exist */
>         of_property_read_u32(node, "mediatek,u3p-dis-msk",
>                              &mtk->u3p_dis_msk);
> @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto put_usb3_hcd;
>
> -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
>                 xhci->shared_hcd->can_do_streams = 1;
>
>         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index a93cfe817904..86aa4978915e 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
>         struct phy **phys;
>         int num_phys;
>         bool lpm_support;
> +       bool broken_streams;
>         /* usb remote wakeup */
>         bool uwk_en;
>         struct regmap *uwk;
> --
> 2.18.0

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-16 12:28     ` Nicolas Boichat
  0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-16 12:28 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, here use
> XHCI_BROKEN_STREAMS quirk to fix it.
>
> Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk.c | 7 ++++++-
>  drivers/usb/host/xhci-mtk.h | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 8f321f39ab96..08dab974d847 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
>         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>         if (mtk->lpm_support)
>                 xhci->quirks |= XHCI_LPM_SUPPORT;
> +
> +       if (mtk->broken_streams)
> +               xhci->quirks |= XHCI_BROKEN_STREAMS;
>  }
>
>  /* called during probe() after chip reset completes */
> @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 return ret;
>
>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> +       mtk->broken_streams =
> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");

Would it be better to add a data field to struct of_device_id
mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?

(IMHO usb3-lpm-capable detection should also be done in the same way)

Thanks,

>         /* optional property, ignore the error if it does not exist */
>         of_property_read_u32(node, "mediatek,u3p-dis-msk",
>                              &mtk->u3p_dis_msk);
> @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto put_usb3_hcd;
>
> -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
>                 xhci->shared_hcd->can_do_streams = 1;
>
>         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index a93cfe817904..86aa4978915e 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
>         struct phy **phys;
>         int num_phys;
>         bool lpm_support;
> +       bool broken_streams;
>         /* usb remote wakeup */
>         bool uwk_en;
>         struct regmap *uwk;
> --
> 2.18.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-16 12:28     ` Nicolas Boichat
  (?)
@ 2020-12-17  2:19       ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  2:19 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Rob Herring, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
	linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > The 0.96 xHCI controller on some platforms does not support
> > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > is set a non-zero default value by mistake, here use
> > XHCI_BROKEN_STREAMS quirk to fix it.
> >
> > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> >  drivers/usb/host/xhci-mtk.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index 8f321f39ab96..08dab974d847 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >         if (mtk->lpm_support)
> >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > +
> > +       if (mtk->broken_streams)
> > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> >  }
> >
> >  /* called during probe() after chip reset completes */
> > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >
> >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > +       mtk->broken_streams =
> > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> 
> Would it be better to add a data field to struct of_device_id
> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
when the controller don't support bulk stream. If enable this quirk only
for mt8173, then for other SoCs, the compatible need include
"mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
e.g. a new SoC has the broken stream as mt8173, but also has another
different quirk, the way you suggested will not handle it.
And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
converting the binding to YMAL.

> 
> (IMHO usb3-lpm-capable detection should also be done in the same way)
I prefer to provide a property for common issues, and use the way you
suggested for the issue only happened at a specific SoC.

Thank you

> 
> Thanks,
> 
> >         /* optional property, ignore the error if it does not exist */
> >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> >                              &mtk->u3p_dis_msk);
> > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto put_usb3_hcd;
> >
> > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> >                 xhci->shared_hcd->can_do_streams = 1;
> >
> >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..86aa4978915e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> >         struct phy **phys;
> >         int num_phys;
> >         bool lpm_support;
> > +       bool broken_streams;
> >         /* usb remote wakeup */
> >         bool uwk_en;
> >         struct regmap *uwk;
> > --
> > 2.18.0


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  2:19       ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  2:19 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > The 0.96 xHCI controller on some platforms does not support
> > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > is set a non-zero default value by mistake, here use
> > XHCI_BROKEN_STREAMS quirk to fix it.
> >
> > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> >  drivers/usb/host/xhci-mtk.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index 8f321f39ab96..08dab974d847 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >         if (mtk->lpm_support)
> >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > +
> > +       if (mtk->broken_streams)
> > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> >  }
> >
> >  /* called during probe() after chip reset completes */
> > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >
> >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > +       mtk->broken_streams =
> > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> 
> Would it be better to add a data field to struct of_device_id
> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
when the controller don't support bulk stream. If enable this quirk only
for mt8173, then for other SoCs, the compatible need include
"mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
e.g. a new SoC has the broken stream as mt8173, but also has another
different quirk, the way you suggested will not handle it.
And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
converting the binding to YMAL.

> 
> (IMHO usb3-lpm-capable detection should also be done in the same way)
I prefer to provide a property for common issues, and use the way you
suggested for the issue only happened at a specific SoC.

Thank you

> 
> Thanks,
> 
> >         /* optional property, ignore the error if it does not exist */
> >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> >                              &mtk->u3p_dis_msk);
> > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto put_usb3_hcd;
> >
> > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> >                 xhci->shared_hcd->can_do_streams = 1;
> >
> >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..86aa4978915e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> >         struct phy **phys;
> >         int num_phys;
> >         bool lpm_support;
> > +       bool broken_streams;
> >         /* usb remote wakeup */
> >         bool uwk_en;
> >         struct regmap *uwk;
> > --
> > 2.18.0

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  2:19       ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  2:19 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > The 0.96 xHCI controller on some platforms does not support
> > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > is set a non-zero default value by mistake, here use
> > XHCI_BROKEN_STREAMS quirk to fix it.
> >
> > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> >  drivers/usb/host/xhci-mtk.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index 8f321f39ab96..08dab974d847 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >         if (mtk->lpm_support)
> >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > +
> > +       if (mtk->broken_streams)
> > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> >  }
> >
> >  /* called during probe() after chip reset completes */
> > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >
> >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > +       mtk->broken_streams =
> > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> 
> Would it be better to add a data field to struct of_device_id
> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
when the controller don't support bulk stream. If enable this quirk only
for mt8173, then for other SoCs, the compatible need include
"mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
e.g. a new SoC has the broken stream as mt8173, but also has another
different quirk, the way you suggested will not handle it.
And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
converting the binding to YMAL.

> 
> (IMHO usb3-lpm-capable detection should also be done in the same way)
I prefer to provide a property for common issues, and use the way you
suggested for the issue only happened at a specific SoC.

Thank you

> 
> Thanks,
> 
> >         /* optional property, ignore the error if it does not exist */
> >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> >                              &mtk->u3p_dis_msk);
> > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto put_usb3_hcd;
> >
> > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> >                 xhci->shared_hcd->can_do_streams = 1;
> >
> >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..86aa4978915e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> >         struct phy **phys;
> >         int num_phys;
> >         bool lpm_support;
> > +       bool broken_streams;
> >         /* usb remote wakeup */
> >         bool uwk_en;
> >         struct regmap *uwk;
> > --
> > 2.18.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-17  2:19       ` Chunfeng Yun
  (?)
@ 2020-12-17  3:32         ` Nicolas Boichat
  -1 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-17  3:32 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Rob Herring, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
	linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.

It can, we do this regularly for many other components. One example:
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402

> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.

Understand, it's just different approaches, there seems to be
precedent (at least in this driver/binding) for using properties, so
I'll let the USB maintainers speak up ,-)

>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  3:32         ` Nicolas Boichat
  0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-17  3:32 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.

It can, we do this regularly for many other components. One example:
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402

> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.

Understand, it's just different approaches, there seems to be
precedent (at least in this driver/binding) for using properties, so
I'll let the USB maintainers speak up ,-)

>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  3:32         ` Nicolas Boichat
  0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Boichat @ 2020-12-17  3:32 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.

It can, we do this regularly for many other components. One example:
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402

> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.

Understand, it's just different approaches, there seems to be
precedent (at least in this driver/binding) for using properties, so
I'll let the USB maintainers speak up ,-)

>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-17  2:19       ` Chunfeng Yun
  (?)
@ 2020-12-17  3:43         ` Rosen Penev
  -1 siblings, 0 replies; 39+ messages in thread
From: Rosen Penev @ 2020-12-17  3:43 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Nicolas Boichat, Devicetree List, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.
> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
I'm guessing this also applies to mt7621?
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.
>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  3:43         ` Rosen Penev
  0 siblings, 0 replies; 39+ messages in thread
From: Rosen Penev @ 2020-12-17  3:43 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.
> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
I'm guessing this also applies to mt7621?
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.
>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  3:43         ` Rosen Penev
  0 siblings, 0 replies; 39+ messages in thread
From: Rosen Penev @ 2020-12-17  3:43 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > The 0.96 xHCI controller on some platforms does not support
> > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > is set a non-zero default value by mistake, here use
> > > XHCI_BROKEN_STREAMS quirk to fix it.
> > >
> > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > >  drivers/usb/host/xhci-mtk.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index 8f321f39ab96..08dab974d847 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +
> > > +       if (mtk->broken_streams)
> > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 return ret;
> > >
> > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > +       mtk->broken_streams =
> > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >
> > Would it be better to add a data field to struct of_device_id
> > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> when the controller don't support bulk stream. If enable this quirk only
> for mt8173, then for other SoCs, the compatible need include
> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> e.g. a new SoC has the broken stream as mt8173, but also has another
> different quirk, the way you suggested will not handle it.
> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> converting the binding to YMAL.
I'm guessing this also applies to mt7621?
>
> >
> > (IMHO usb3-lpm-capable detection should also be done in the same way)
> I prefer to provide a property for common issues, and use the way you
> suggested for the issue only happened at a specific SoC.
>
> Thank you
>
> >
> > Thanks,
> >
> > >         /* optional property, ignore the error if it does not exist */
> > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > >                              &mtk->u3p_dis_msk);
> > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto put_usb3_hcd;
> > >
> > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > >                 xhci->shared_hcd->can_do_streams = 1;
> > >
> > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > index a93cfe817904..86aa4978915e 100644
> > > --- a/drivers/usb/host/xhci-mtk.h
> > > +++ b/drivers/usb/host/xhci-mtk.h
> > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > >         struct phy **phys;
> > >         int num_phys;
> > >         bool lpm_support;
> > > +       bool broken_streams;
> > >         /* usb remote wakeup */
> > >         bool uwk_en;
> > >         struct regmap *uwk;
> > > --
> > > 2.18.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-17  3:32         ` Nicolas Boichat
  (?)
@ 2020-12-17  6:23           ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  6:23 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Rob Herring, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
	linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> 
> It can, we do this regularly for many other components. One example:
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> 
Got it. Indeed works when add compatible private data.

Due to many SoCs supports USB and not upstream, I'd prefer to avoid
adding new compatible in driver when support new SoCs, and leave the
code as simple as possible.

> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> 
> Understand, it's just different approaches, 
Yes

> there seems to be
> precedent (at least in this driver/binding) for using properties, so
> I'll let the USB maintainers speak up ,-)
> 
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  6:23           ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  6:23 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> 
> It can, we do this regularly for many other components. One example:
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> 
Got it. Indeed works when add compatible private data.

Due to many SoCs supports USB and not upstream, I'd prefer to avoid
adding new compatible in driver when support new SoCs, and leave the
code as simple as possible.

> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> 
> Understand, it's just different approaches, 
Yes

> there seems to be
> precedent (at least in this driver/binding) for using properties, so
> I'll let the USB maintainers speak up ,-)
> 
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-17  6:23           ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-17  6:23 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Devicetree List, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	lkml, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Ikjoon Jang,
	linux-arm Mailing List

On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> 
> It can, we do this regularly for many other components. One example:
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> 
Got it. Indeed works when add compatible private data.

Due to many SoCs supports USB and not upstream, I'd prefer to avoid
adding new compatible in driver when support new SoCs, and leave the
code as simple as possible.

> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> 
> Understand, it's just different approaches, 
Yes

> there seems to be
> precedent (at least in this driver/binding) for using properties, so
> I'll let the USB maintainers speak up ,-)
> 
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
  2020-12-16 11:51 ` Chunfeng Yun
  (?)
@ 2020-12-21 19:29   ` Rob Herring
  -1 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:29 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Hsin-Yi Wang, Ikjoon Jang

On Wed, Dec 16, 2020 at 07:51:23PM +0800, Chunfeng Yun wrote:
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, so add a new
> property "mediatek,broken_streams_quirk" to fix it.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> this patch depends on:
>   https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
>   [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index e5e03f902802..330f7294bf34 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -120,6 +120,11 @@ properties:
>      description: The mask to disable u3ports, bit0 for u3port0,
>        bit1 for u3port1, ... etc
>  
> +  mediatek,broken_streams_quirk:

You should imply this from the compatible property.

(Also, don't use '_' in property names).

> +    description: set if the controller doesn't support bulk stream but
> +      HCCPARAMS says support.
> +    type: boolean
> +
>    "#address-cells":
>      const: 1
>  
> -- 
> 2.18.0

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
@ 2020-12-21 19:29   ` Rob Herring
  0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:29 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: devicetree, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	linux-kernel, linux-mediatek, Hsin-Yi Wang, Matthias Brugger,
	Ikjoon Jang, linux-arm-kernel

On Wed, Dec 16, 2020 at 07:51:23PM +0800, Chunfeng Yun wrote:
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, so add a new
> property "mediatek,broken_streams_quirk" to fix it.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> this patch depends on:
>   https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
>   [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index e5e03f902802..330f7294bf34 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -120,6 +120,11 @@ properties:
>      description: The mask to disable u3ports, bit0 for u3port0,
>        bit1 for u3port1, ... etc
>  
> +  mediatek,broken_streams_quirk:

You should imply this from the compatible property.

(Also, don't use '_' in property names).

> +    description: set if the controller doesn't support bulk stream but
> +      HCCPARAMS says support.
> +    type: boolean
> +
>    "#address-cells":
>      const: 1
>  
> -- 
> 2.18.0

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams
@ 2020-12-21 19:29   ` Rob Herring
  0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:29 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: devicetree, Mathias Nyman, Greg Kroah-Hartman, linux-usb,
	linux-kernel, linux-mediatek, Hsin-Yi Wang, Matthias Brugger,
	Ikjoon Jang, linux-arm-kernel

On Wed, Dec 16, 2020 at 07:51:23PM +0800, Chunfeng Yun wrote:
> The 0.96 xHCI controller on some platforms does not support
> bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> is set a non-zero default value by mistake, so add a new
> property "mediatek,broken_streams_quirk" to fix it.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> this patch depends on:
>   https://patchwork.kernel.org/project/linux-mediatek/list/?series=402773
>   [v4,09/11] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index e5e03f902802..330f7294bf34 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -120,6 +120,11 @@ properties:
>      description: The mask to disable u3ports, bit0 for u3port0,
>        bit1 for u3port1, ... etc
>  
> +  mediatek,broken_streams_quirk:

You should imply this from the compatible property.

(Also, don't use '_' in property names).

> +    description: set if the controller doesn't support bulk stream but
> +      HCCPARAMS says support.
> +    type: boolean
> +
>    "#address-cells":
>      const: 1
>  
> -- 
> 2.18.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-17  6:23           ` Chunfeng Yun
  (?)
@ 2020-12-21 19:35             ` Rob Herring
  -1 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:35 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Nicolas Boichat, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger, linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > >
> > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > is set a non-zero default value by mistake, here use
> > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > >
> > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > ---
> > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > >         if (mtk->lpm_support)
> > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > +
> > > > > +       if (mtk->broken_streams)
> > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > >  }
> > > > >
> > > > >  /* called during probe() after chip reset completes */
> > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > >                 return ret;
> > > > >
> > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > +       mtk->broken_streams =
> > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > >
> > > > Would it be better to add a data field to struct of_device_id
> > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > when the controller don't support bulk stream. If enable this quirk only
> > > for mt8173, then for other SoCs, the compatible need include
> > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > different quirk, the way you suggested will not handle it.
> > 
> > It can, we do this regularly for many other components. One example:
> > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > 
> Got it. Indeed works when add compatible private data.
> 
> Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> adding new compatible in driver when support new SoCs, and leave the
> code as simple as possible.

No. The problem is adding new fixes requires updating the DT. That would 
be fine if you knew all possible issues and quirks up front. You may for 
some, but you won't catch them all.

Save DT properties for per board quirks/features, not per SoC ones.

> 
> > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > converting the binding to YMAL.
> > >
> > > >
> > > > (IMHO usb3-lpm-capable detection should also be done in the same way)

I tend to agree, but am more tolerable of standard USB features than 
specific IP block quirks.

Rob

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-21 19:35             ` Rob Herring
  0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:35 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > >
> > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > is set a non-zero default value by mistake, here use
> > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > >
> > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > ---
> > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > >         if (mtk->lpm_support)
> > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > +
> > > > > +       if (mtk->broken_streams)
> > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > >  }
> > > > >
> > > > >  /* called during probe() after chip reset completes */
> > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > >                 return ret;
> > > > >
> > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > +       mtk->broken_streams =
> > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > >
> > > > Would it be better to add a data field to struct of_device_id
> > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > when the controller don't support bulk stream. If enable this quirk only
> > > for mt8173, then for other SoCs, the compatible need include
> > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > different quirk, the way you suggested will not handle it.
> > 
> > It can, we do this regularly for many other components. One example:
> > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > 
> Got it. Indeed works when add compatible private data.
> 
> Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> adding new compatible in driver when support new SoCs, and leave the
> code as simple as possible.

No. The problem is adding new fixes requires updating the DT. That would 
be fine if you knew all possible issues and quirks up front. You may for 
some, but you won't catch them all.

Save DT properties for per board quirks/features, not per SoC ones.

> 
> > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > converting the binding to YMAL.
> > >
> > > >
> > > > (IMHO usb3-lpm-capable detection should also be done in the same way)

I tend to agree, but am more tolerable of standard USB features than 
specific IP block quirks.

Rob

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-21 19:35             ` Rob Herring
  0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2020-12-21 19:35 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > >
> > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > is set a non-zero default value by mistake, here use
> > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > >
> > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > ---
> > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > >         if (mtk->lpm_support)
> > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > +
> > > > > +       if (mtk->broken_streams)
> > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > >  }
> > > > >
> > > > >  /* called during probe() after chip reset completes */
> > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > >                 return ret;
> > > > >
> > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > +       mtk->broken_streams =
> > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > >
> > > > Would it be better to add a data field to struct of_device_id
> > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > when the controller don't support bulk stream. If enable this quirk only
> > > for mt8173, then for other SoCs, the compatible need include
> > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > different quirk, the way you suggested will not handle it.
> > 
> > It can, we do this regularly for many other components. One example:
> > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > 
> Got it. Indeed works when add compatible private data.
> 
> Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> adding new compatible in driver when support new SoCs, and leave the
> code as simple as possible.

No. The problem is adding new fixes requires updating the DT. That would 
be fine if you knew all possible issues and quirks up front. You may for 
some, but you won't catch them all.

Save DT properties for per board quirks/features, not per SoC ones.

> 
> > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > converting the binding to YMAL.
> > >
> > > >
> > > > (IMHO usb3-lpm-capable detection should also be done in the same way)

I tend to agree, but am more tolerable of standard USB features than 
specific IP block quirks.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-17  3:43         ` Rosen Penev
  (?)
@ 2020-12-24  7:18           ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-24  7:18 UTC (permalink / raw)
  To: Rosen Penev
  Cc: Nicolas Boichat, Devicetree List, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> I'm guessing this also applies to mt7621?
Yes, mt7621 doesn't support bulk stream

> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-24  7:18           ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-24  7:18 UTC (permalink / raw)
  To: Rosen Penev
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> I'm guessing this also applies to mt7621?
Yes, mt7621 doesn't support bulk stream

> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-24  7:18           ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-24  7:18 UTC (permalink / raw)
  To: Rosen Penev
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > The 0.96 xHCI controller on some platforms does not support
> > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > is set a non-zero default value by mistake, here use
> > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > >
> > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index 8f321f39ab96..08dab974d847 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > > +       if (mtk->broken_streams)
> > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 return ret;
> > > >
> > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > +       mtk->broken_streams =
> > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > >
> > > Would it be better to add a data field to struct of_device_id
> > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > when the controller don't support bulk stream. If enable this quirk only
> > for mt8173, then for other SoCs, the compatible need include
> > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > e.g. a new SoC has the broken stream as mt8173, but also has another
> > different quirk, the way you suggested will not handle it.
> > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > converting the binding to YMAL.
> I'm guessing this also applies to mt7621?
Yes, mt7621 doesn't support bulk stream

> >
> > >
> > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> > I prefer to provide a property for common issues, and use the way you
> > suggested for the issue only happened at a specific SoC.
> >
> > Thank you
> >
> > >
> > > Thanks,
> > >
> > > >         /* optional property, ignore the error if it does not exist */
> > > >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> > > >                              &mtk->u3p_dis_msk);
> > > > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto put_usb3_hcd;
> > > >
> > > > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > > >                 xhci->shared_hcd->can_do_streams = 1;
> > > >
> > > >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > > > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > > > index a93cfe817904..86aa4978915e 100644
> > > > --- a/drivers/usb/host/xhci-mtk.h
> > > > +++ b/drivers/usb/host/xhci-mtk.h
> > > > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> > > >         struct phy **phys;
> > > >         int num_phys;
> > > >         bool lpm_support;
> > > > +       bool broken_streams;
> > > >         /* usb remote wakeup */
> > > >         bool uwk_en;
> > > >         struct regmap *uwk;
> > > > --
> > > > 2.18.0
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
  2020-12-21 19:35             ` Rob Herring
  (?)
@ 2020-12-25  6:12               ` Chunfeng Yun
  -1 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-25  6:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nicolas Boichat, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger, linux-usb, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support, Devicetree List, lkml,
	Hsin-Yi Wang, Ikjoon Jang

On Mon, 2020-12-21 at 12:35 -0700, Rob Herring wrote:
> On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> > On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > > >
> > > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > > is set a non-zero default value by mistake, here use
> > > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > > >
> > > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > > ---
> > > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > > >         if (mtk->lpm_support)
> > > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > > +
> > > > > > +       if (mtk->broken_streams)
> > > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > > >  }
> > > > > >
> > > > > >  /* called during probe() after chip reset completes */
> > > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > > >                 return ret;
> > > > > >
> > > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > > +       mtk->broken_streams =
> > > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > > >
> > > > > Would it be better to add a data field to struct of_device_id
> > > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > > when the controller don't support bulk stream. If enable this quirk only
> > > > for mt8173, then for other SoCs, the compatible need include
> > > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > > different quirk, the way you suggested will not handle it.
> > > 
> > > It can, we do this regularly for many other components. One example:
> > > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > > 
> > Got it. Indeed works when add compatible private data.
> > 
> > Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> > adding new compatible in driver when support new SoCs, and leave the
> > code as simple as possible.
> 
> No. The problem is adding new fixes requires updating the DT. That would 
> be fine if you knew all possible issues and quirks up front. You may for 
> some, but you won't catch them all.
Yes.
But all SoCs use the generic compatible "mediatek,mtk-xhci" except
mtk8173. as following:
compatible = "mediatek,mt<SoC>-xhci", "mediatek,mtk-xhci";

It may be a little messy when the driver fill many compatibles after
supporting several tens or hundreds SoCs.

I'll think about it again. 
> 
> Save DT properties for per board quirks/features, not per SoC ones.
This is for USB IP quirks/feature, not board or SoC

Thanks

> 
> > 
> > > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > > converting the binding to YMAL.
> > > >
> > > > >
> > > > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> 
> I tend to agree, but am more tolerable of standard USB features than 
> specific IP block quirks.
> 
> Rob


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-25  6:12               ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-25  6:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Mon, 2020-12-21 at 12:35 -0700, Rob Herring wrote:
> On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> > On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > > >
> > > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > > is set a non-zero default value by mistake, here use
> > > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > > >
> > > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > > ---
> > > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > > >         if (mtk->lpm_support)
> > > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > > +
> > > > > > +       if (mtk->broken_streams)
> > > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > > >  }
> > > > > >
> > > > > >  /* called during probe() after chip reset completes */
> > > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > > >                 return ret;
> > > > > >
> > > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > > +       mtk->broken_streams =
> > > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > > >
> > > > > Would it be better to add a data field to struct of_device_id
> > > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > > when the controller don't support bulk stream. If enable this quirk only
> > > > for mt8173, then for other SoCs, the compatible need include
> > > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > > different quirk, the way you suggested will not handle it.
> > > 
> > > It can, we do this regularly for many other components. One example:
> > > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > > 
> > Got it. Indeed works when add compatible private data.
> > 
> > Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> > adding new compatible in driver when support new SoCs, and leave the
> > code as simple as possible.
> 
> No. The problem is adding new fixes requires updating the DT. That would 
> be fine if you knew all possible issues and quirks up front. You may for 
> some, but you won't catch them all.
Yes.
But all SoCs use the generic compatible "mediatek,mtk-xhci" except
mtk8173. as following:
compatible = "mediatek,mt<SoC>-xhci", "mediatek,mtk-xhci";

It may be a little messy when the driver fill many compatibles after
supporting several tens or hundreds SoCs.

I'll think about it again. 
> 
> Save DT properties for per board quirks/features, not per SoC ones.
This is for USB IP quirks/feature, not board or SoC

Thanks

> 
> > 
> > > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > > converting the binding to YMAL.
> > > >
> > > > >
> > > > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> 
> I tend to agree, but am more tolerable of standard USB features than 
> specific IP block quirks.
> 
> Rob

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2020-12-25  6:12               ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2020-12-25  6:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Matthias Brugger, Ikjoon Jang, linux-arm Mailing List

On Mon, 2020-12-21 at 12:35 -0700, Rob Herring wrote:
> On Thu, Dec 17, 2020 at 02:23:50PM +0800, Chunfeng Yun wrote:
> > On Thu, 2020-12-17 at 11:32 +0800, Nicolas Boichat wrote:
> > > On Thu, Dec 17, 2020 at 10:19 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> > > > > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > > >
> > > > > > The 0.96 xHCI controller on some platforms does not support
> > > > > > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > > > > > is set a non-zero default value by mistake, here use
> > > > > > XHCI_BROKEN_STREAMS quirk to fix it.
> > > > > >
> > > > > > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > > > ---
> > > > > >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> > > > > >  drivers/usb/host/xhci-mtk.h | 1 +
> > > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > > index 8f321f39ab96..08dab974d847 100644
> > > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > > >         if (mtk->lpm_support)
> > > > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > > +
> > > > > > +       if (mtk->broken_streams)
> > > > > > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > > > >  }
> > > > > >
> > > > > >  /* called during probe() after chip reset completes */
> > > > > > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > > > >                 return ret;
> > > > > >
> > > > > >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > > > > > +       mtk->broken_streams =
> > > > > > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> > > > >
> > > > > Would it be better to add a data field to struct of_device_id
> > > > > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> > > > This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> > > > when the controller don't support bulk stream. If enable this quirk only
> > > > for mt8173, then for other SoCs, the compatible need include
> > > > "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> > > > e.g. a new SoC has the broken stream as mt8173, but also has another
> > > > different quirk, the way you suggested will not handle it.
> > > 
> > > It can, we do this regularly for many other components. One example:
> > > https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-mt65xx.c#L402
> > > 
> > Got it. Indeed works when add compatible private data.
> > 
> > Due to many SoCs supports USB and not upstream, I'd prefer to avoid
> > adding new compatible in driver when support new SoCs, and leave the
> > code as simple as possible.
> 
> No. The problem is adding new fixes requires updating the DT. That would 
> be fine if you knew all possible issues and quirks up front. You may for 
> some, but you won't catch them all.
Yes.
But all SoCs use the generic compatible "mediatek,mtk-xhci" except
mtk8173. as following:
compatible = "mediatek,mt<SoC>-xhci", "mediatek,mtk-xhci";

It may be a little messy when the driver fill many compatibles after
supporting several tens or hundreds SoCs.

I'll think about it again. 
> 
> Save DT properties for per board quirks/features, not per SoC ones.
This is for USB IP quirks/feature, not board or SoC

Thanks

> 
> > 
> > > > And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> > > > converting the binding to YMAL.
> > > >
> > > > >
> > > > > (IMHO usb3-lpm-capable detection should also be done in the same way)
> 
> I tend to agree, but am more tolerable of standard USB features than 
> specific IP block quirks.
> 
> Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
       [not found]           ` <7ad022d3-ff83-9126-ee74-6d1e4d381366@gmail.com>
  2021-02-01  1:47               ` Chunfeng Yun
@ 2021-02-01  1:47               ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2021-02-01  1:47 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rosen Penev, Nicolas Boichat, Devicetree List, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring,
	moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
	Ikjoon Jang, linux-arm Mailing List

On Sun, 2021-01-31 at 15:13 +0100, Matthias Brugger wrote:
> 
> On 24/12/2020 08:18, Chunfeng Yun wrote:
> > On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> >> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>
> >>> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> >>>> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> [...]
> >>>>>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> >>>>> +       mtk->broken_streams =
> >>>>> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >>>>
> >>>> Would it be better to add a data field to struct of_device_id
> >>>> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> >>> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> >>> when the controller don't support bulk stream. If enable this quirk only
> >>> for mt8173, then for other SoCs, the compatible need include
> >>> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> >>> e.g. a new SoC has the broken stream as mt8173, but also has another
> >>> different quirk, the way you suggested will not handle it.
> >>> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> >>> converting the binding to YMAL.
> >> I'm guessing this also applies to mt7621?
> > Yes, mt7621 doesn't support bulk stream
> > 
> 
> Then please provide patches to the DTSI for all SoCs that have this problem.
> Either as a follow-up or as part of this series, if you need to resubmit.
Ok, I'll send new version, and also try other way to fix it without add
property in DTS, thanks

> 
> Regards,
> Matthias


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2021-02-01  1:47               ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2021-02-01  1:47 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring, Rosen Penev,
	Hsin-Yi Wang, moderated list:ARM/Mediatek SoC support,
	Ikjoon Jang, linux-arm Mailing List

On Sun, 2021-01-31 at 15:13 +0100, Matthias Brugger wrote:
> 
> On 24/12/2020 08:18, Chunfeng Yun wrote:
> > On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> >> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>
> >>> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> >>>> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> [...]
> >>>>>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> >>>>> +       mtk->broken_streams =
> >>>>> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >>>>
> >>>> Would it be better to add a data field to struct of_device_id
> >>>> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> >>> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> >>> when the controller don't support bulk stream. If enable this quirk only
> >>> for mt8173, then for other SoCs, the compatible need include
> >>> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> >>> e.g. a new SoC has the broken stream as mt8173, but also has another
> >>> different quirk, the way you suggested will not handle it.
> >>> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> >>> converting the binding to YMAL.
> >> I'm guessing this also applies to mt7621?
> > Yes, mt7621 doesn't support bulk stream
> > 
> 
> Then please provide patches to the DTSI for all SoCs that have this problem.
> Either as a follow-up or as part of this series, if you need to resubmit.
Ok, I'll send new version, and also try other way to fix it without add
property in DTS, thanks

> 
> Regards,
> Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
@ 2021-02-01  1:47               ` Chunfeng Yun
  0 siblings, 0 replies; 39+ messages in thread
From: Chunfeng Yun @ 2021-02-01  1:47 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Devicetree List, Nicolas Boichat, Mathias Nyman,
	Greg Kroah-Hartman, linux-usb, lkml, Rob Herring, Rosen Penev,
	Hsin-Yi Wang, moderated list:ARM/Mediatek SoC support,
	Ikjoon Jang, linux-arm Mailing List

On Sun, 2021-01-31 at 15:13 +0100, Matthias Brugger wrote:
> 
> On 24/12/2020 08:18, Chunfeng Yun wrote:
> > On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote:
> >> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>
> >>> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> >>>> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> [...]
> >>>>>         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> >>>>> +       mtk->broken_streams =
> >>>>> +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> >>>>
> >>>> Would it be better to add a data field to struct of_device_id
> >>>> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
> >>> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
> >>> when the controller don't support bulk stream. If enable this quirk only
> >>> for mt8173, then for other SoCs, the compatible need include
> >>> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
> >>> e.g. a new SoC has the broken stream as mt8173, but also has another
> >>> different quirk, the way you suggested will not handle it.
> >>> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
> >>> converting the binding to YMAL.
> >> I'm guessing this also applies to mt7621?
> > Yes, mt7621 doesn't support bulk stream
> > 
> 
> Then please provide patches to the DTSI for all SoCs that have this problem.
> Either as a follow-up or as part of this series, if you need to resubmit.
Ok, I'll send new version, and also try other way to fix it without add
property in DTS, thanks

> 
> Regards,
> Matthias

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2021-02-01  1:52 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 11:51 [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams Chunfeng Yun
2020-12-16 11:51 ` Chunfeng Yun
2020-12-16 11:51 ` Chunfeng Yun
2020-12-16 11:51 ` [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk Chunfeng Yun
2020-12-16 11:51   ` Chunfeng Yun
2020-12-16 11:51   ` Chunfeng Yun
2020-12-16 12:28   ` Nicolas Boichat
2020-12-16 12:28     ` Nicolas Boichat
2020-12-16 12:28     ` Nicolas Boichat
2020-12-17  2:19     ` Chunfeng Yun
2020-12-17  2:19       ` Chunfeng Yun
2020-12-17  2:19       ` Chunfeng Yun
2020-12-17  3:32       ` Nicolas Boichat
2020-12-17  3:32         ` Nicolas Boichat
2020-12-17  3:32         ` Nicolas Boichat
2020-12-17  6:23         ` Chunfeng Yun
2020-12-17  6:23           ` Chunfeng Yun
2020-12-17  6:23           ` Chunfeng Yun
2020-12-21 19:35           ` Rob Herring
2020-12-21 19:35             ` Rob Herring
2020-12-21 19:35             ` Rob Herring
2020-12-25  6:12             ` Chunfeng Yun
2020-12-25  6:12               ` Chunfeng Yun
2020-12-25  6:12               ` Chunfeng Yun
2020-12-17  3:43       ` Rosen Penev
2020-12-17  3:43         ` Rosen Penev
2020-12-17  3:43         ` Rosen Penev
2020-12-24  7:18         ` Chunfeng Yun
2020-12-24  7:18           ` Chunfeng Yun
2020-12-24  7:18           ` Chunfeng Yun
     [not found]           ` <7ad022d3-ff83-9126-ee74-6d1e4d381366@gmail.com>
2021-02-01  1:47             ` Chunfeng Yun
2021-02-01  1:47               ` Chunfeng Yun
2021-02-01  1:47               ` Chunfeng Yun
2020-12-16 11:51 ` [PATCH 3/3] arm64: dts: mt8173: add a broken streams quirk property for xHCI node Chunfeng Yun
2020-12-16 11:51   ` Chunfeng Yun
2020-12-16 11:51   ` Chunfeng Yun
2020-12-21 19:29 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a new property for broken streams Rob Herring
2020-12-21 19:29   ` Rob Herring
2020-12-21 19:29   ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.