LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
@ 2023-12-13  6:35 Chunfeng Yun
  2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Chunfeng Yun @ 2023-12-13  6:35 UTC (permalink / raw
  To: Greg Kroah-Hartman, Rob Herring
  Cc: Chunfeng Yun, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Mathias Nyman, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Macpaul Lin, Eddie Hung

For Gen1 isoc-in endpoint on controller before about SSUSB IPM v1.6.0, it
still send out unexpected ACK after receiving a short packet in burst
transfer, this will cause an exception on connected device, specially for
a 4k camera.
Add a quirk property "mediatek,rxfifo-depth" to work around this hardware
issue;
The side-effect is that may cause performance drop about 10%, including
bulk transfer.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e9644e333d78..b8ed68574ba4 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -124,6 +124,16 @@ properties:
       defined in the xHCI spec on MTK's controller.
     default: 5000
 
+  mediatek,rxfifo-depth:
+    description:
+      It is a quirk flag used to work around Gen1 isoc-in endpoint transfer
+      issue that still send out unexpected ACK after device finish the burst
+      transfer with a short packet and cause an exception, specially on a 4K
+      camera device, it happens on controller before about IPM v1.6.0; the
+      side-effect is that may cause performance drop about 10%, include bulk
+      transfer.
+    type: boolean
+
   # the following properties are only used for case 1
   wakeup-source:
     description: enable USB remote wakeup, see power/wakeup-source.txt
-- 
2.25.1


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

* [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer
  2023-12-13  6:35 [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Chunfeng Yun
@ 2023-12-13  6:35 ` Chunfeng Yun
  2023-12-13 21:03   ` kernel test robot
  2023-12-14 18:08   ` kernel test robot
  2023-12-13  6:35 ` [PATCH 3/3] arm64: dts: mediatek: mt8195: Add mediatek,rxfifo-depth to cherry Chunfeng Yun
  2023-12-13  8:05 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Krzysztof Kozlowski
  2 siblings, 2 replies; 11+ messages in thread
From: Chunfeng Yun @ 2023-12-13  6:35 UTC (permalink / raw
  To: Greg Kroah-Hartman, Rob Herring
  Cc: Chunfeng Yun, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Mathias Nyman, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Macpaul Lin, Eddie Hung

For Gen1 isoc-in transfer, host still send out unexpected ACK after device
finish the burst with a short packet, this will cause an exception on the
connected device, such as, a usb 4k camera.
It can be fixed by setting rxfifo depth less than 3, prefer to use 2 here,
the side-effect is that may cause performance drop about 10%, including
bulk transfer.

Fixes: 926d60ae64a6 ("usb: xhci-mtk: modify the SOF/ITP interval for mt8195")
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 35 +++++++++++++++++++++++++++++++++--
 drivers/usb/host/xhci-mtk.h |  1 +
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bbdf1b0b7be1..a54535877e90 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -73,6 +73,9 @@
 #define FRMCNT_LEV1_RANG	(0x12b << 8)
 #define FRMCNT_LEV1_RANG_MASK	GENMASK(19, 8)
 
+#define HSCH_CFG1		0x960
+#define SCH3_RXFIFO_DEPTH_MASK	GENMASK(21, 20)
+
 #define SS_GEN2_EOF_CFG		0x990
 #define SSG2EOF_OFFSET		0x3c
 
@@ -165,6 +168,33 @@ static void xhci_mtk_set_frame_interval(struct xhci_hcd_mtk *mtk)
 	writel(value, hcd->regs + SS_GEN2_EOF_CFG);
 }
 
+/*
+ * workaround: usb3.2 gen1 isoc rx hw issue
+ * host send out unexpected ACK afer device fininsh a burst transfer with
+ * a short packet.
+ */
+static void xhci_mtk_rxfifo_depth_set(struct xhci_hcd_mtk *mtk)
+{
+	struct usb_hcd *hcd = mtk->hcd;
+	u32 value;
+
+	if (mtk->rxfifo_depth_quirk) {
+		value = readl(hcd->regs + HSCH_CFG1);
+		value &= ~SCH3_RXFIFO_DEPTH_MASK;
+		value |= FIELD_PREP(SCH3_RXFIFO_DEPTH_MASK, 2);
+		writel(value, hcd->regs + HSCH_CFG1);
+	}
+}
+
+static void xhci_mtk_init_quirk(struct xhci_hcd_mtk *mtk)
+{
+	/* workaround only for mt8195 */
+	xhci_mtk_set_frame_interval(mtk);
+
+	/* workaround for SoCs using SSUSB about before IPM v1.6.0 */
+	xhci_mtk_rxfifo_depth_set(mtk);
+}
+
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
 {
 	struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
@@ -448,8 +478,7 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
 		if (ret)
 			return ret;
 
-		/* workaround only for mt8195 */
-		xhci_mtk_set_frame_interval(mtk);
+		xhci_mtk_init_quirk(mtk);
 	}
 
 	ret = xhci_gen_setup(hcd, xhci_mtk_quirks);
@@ -527,6 +556,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	of_property_read_u32(node, "mediatek,u2p-dis-msk",
 			     &mtk->u2p_dis_msk);
 
+	mtk->rxfifo_depth_quirk = of_property_read_bool(node, "mediatek,rxfifo-depth");
+
 	ret = usb_wakeup_of_property_parse(mtk, node);
 	if (ret) {
 		dev_err(dev, "failed to parse uwk property\n");
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 39f7ae7d3087..2443075530e3 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -166,6 +166,7 @@ struct xhci_hcd_mtk {
 	unsigned int has_ippc:1;
 	unsigned int lpm_support:1;
 	unsigned int u2_lpm_disable:1;
+	unsigned int rxfifo_depth_quirk:1;
 	/* usb remote wakeup */
 	unsigned int uwk_en:1;
 	struct regmap *uwk;
-- 
2.25.1


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

* [PATCH 3/3] arm64: dts: mediatek: mt8195: Add mediatek,rxfifo-depth to cherry
  2023-12-13  6:35 [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Chunfeng Yun
  2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
@ 2023-12-13  6:35 ` Chunfeng Yun
  2023-12-13  8:05 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Krzysztof Kozlowski
  2 siblings, 0 replies; 11+ messages in thread
From: Chunfeng Yun @ 2023-12-13  6:35 UTC (permalink / raw
  To: Greg Kroah-Hartman, Rob Herring
  Cc: Chunfeng Yun, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Mathias Nyman, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Macpaul Lin, Eddie Hung

Add the quirk property "mediatek,rxfifo-depth" to work around Gen1 isoc-in
transfer issue which send out unexpected ACK even after device already
finished the burst transfer with a short patcket, specially for a 4K
camera device.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
index dd5b89b73190..2f2133931846 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
@@ -1185,6 +1185,7 @@ &xhci0 {
 
 	vusb33-supply = <&mt6359_vusb_ldo_reg>;
 	vbus-supply = <&usb_vbus>;
+	mediatek,rxfifo-depth;
 };
 
 &xhci1 {
@@ -1192,6 +1193,7 @@ &xhci1 {
 
 	vusb33-supply = <&mt6359_vusb_ldo_reg>;
 	vbus-supply = <&usb_vbus>;
+	mediatek,rxfifo-depth;
 };
 
 &xhci2 {
-- 
2.25.1


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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13  6:35 [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Chunfeng Yun
  2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
  2023-12-13  6:35 ` [PATCH 3/3] arm64: dts: mediatek: mt8195: Add mediatek,rxfifo-depth to cherry Chunfeng Yun
@ 2023-12-13  8:05 ` Krzysztof Kozlowski
  2023-12-13 11:17   ` AngeloGioacchino Del Regno
  2023-12-15  6:10   ` Chunfeng Yun (云春峰)
  2 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2023-12-13  8:05 UTC (permalink / raw
  To: Chunfeng Yun, Greg Kroah-Hartman, Rob Herring
  Cc: Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Mathias Nyman, linux-usb,
	linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
	Macpaul Lin, Eddie Hung

On 13/12/2023 07:35, Chunfeng Yun wrote:
> For Gen1 isoc-in endpoint on controller before about SSUSB IPM v1.6.0, it
> still send out unexpected ACK after receiving a short packet in burst
> transfer, this will cause an exception on connected device, specially for
> a 4k camera.
> Add a quirk property "mediatek,rxfifo-depth" to work around this hardware
> issue;
> The side-effect is that may cause performance drop about 10%, including
> bulk transfer.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index e9644e333d78..b8ed68574ba4 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -124,6 +124,16 @@ properties:
>        defined in the xHCI spec on MTK's controller.
>      default: 5000
>  
> +  mediatek,rxfifo-depth:
> +    description:

The property description and driver patch suggest you configure the
depth of FIFO, so this should be not bool, but some uint32. And then,
use generic "fifo-depth" property to set desired depth.


Best regards,
Krzysztof


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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13  8:05 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Krzysztof Kozlowski
@ 2023-12-13 11:17   ` AngeloGioacchino Del Regno
  2023-12-13 11:24     ` Krzysztof Kozlowski
                       ` (2 more replies)
  2023-12-15  6:10   ` Chunfeng Yun (云春峰)
  1 sibling, 3 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-12-13 11:17 UTC (permalink / raw
  To: Krzysztof Kozlowski, Chunfeng Yun, Greg Kroah-Hartman,
	Rob Herring
  Cc: Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	Mathias Nyman, linux-usb, linux-arm-kernel, linux-mediatek,
	devicetree, linux-kernel, Macpaul Lin, Eddie Hung

Il 13/12/23 09:05, Krzysztof Kozlowski ha scritto:
> On 13/12/2023 07:35, Chunfeng Yun wrote:
>> For Gen1 isoc-in endpoint on controller before about SSUSB IPM v1.6.0, it
>> still send out unexpected ACK after receiving a short packet in burst
>> transfer, this will cause an exception on connected device, specially for
>> a 4k camera.
>> Add a quirk property "mediatek,rxfifo-depth" to work around this hardware
>> issue;
>> The side-effect is that may cause performance drop about 10%, including
>> bulk transfer.
>>
>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> ---
>>   .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>> index e9644e333d78..b8ed68574ba4 100644
>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>> @@ -124,6 +124,16 @@ properties:
>>         defined in the xHCI spec on MTK's controller.
>>       default: 5000
>>   
>> +  mediatek,rxfifo-depth:
>> +    description:
> 
> The property description and driver patch suggest you configure the
> depth of FIFO, so this should be not bool, but some uint32. And then,
> use generic "fifo-depth" property to set desired depth.

Did you mean "rx-fifo-depth" instead, as used in net?
This FIFO depth parameter is for RX only.

Cheers,
Angelo



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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13 11:17   ` AngeloGioacchino Del Regno
@ 2023-12-13 11:24     ` Krzysztof Kozlowski
  2023-12-14 22:43     ` Rob Herring
  2023-12-15  6:14     ` Chunfeng Yun (云春峰)
  2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2023-12-13 11:24 UTC (permalink / raw
  To: AngeloGioacchino Del Regno, Chunfeng Yun, Greg Kroah-Hartman,
	Rob Herring
  Cc: Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	Mathias Nyman, linux-usb, linux-arm-kernel, linux-mediatek,
	devicetree, linux-kernel, Macpaul Lin, Eddie Hung

On 13/12/2023 12:17, AngeloGioacchino Del Regno wrote:
> Il 13/12/23 09:05, Krzysztof Kozlowski ha scritto:
>> On 13/12/2023 07:35, Chunfeng Yun wrote:
>>> For Gen1 isoc-in endpoint on controller before about SSUSB IPM v1.6.0, it
>>> still send out unexpected ACK after receiving a short packet in burst
>>> transfer, this will cause an exception on connected device, specially for
>>> a 4k camera.
>>> Add a quirk property "mediatek,rxfifo-depth" to work around this hardware
>>> issue;
>>> The side-effect is that may cause performance drop about 10%, including
>>> bulk transfer.
>>>
>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>> ---
>>>   .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>>> index e9644e333d78..b8ed68574ba4 100644
>>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
>>> @@ -124,6 +124,16 @@ properties:
>>>         defined in the xHCI spec on MTK's controller.
>>>       default: 5000
>>>   
>>> +  mediatek,rxfifo-depth:
>>> +    description:
>>
>> The property description and driver patch suggest you configure the
>> depth of FIFO, so this should be not bool, but some uint32. And then,
>> use generic "fifo-depth" property to set desired depth.
> 
> Did you mean "rx-fifo-depth" instead, as used in net?
> This FIFO depth parameter is for RX only.

I guess yes, if it is specific to RX.

Best regards,
Krzysztof


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

* Re: [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer
  2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
@ 2023-12-13 21:03   ` kernel test robot
  2023-12-14 18:08   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-12-13 21:03 UTC (permalink / raw
  To: Chunfeng Yun, Greg Kroah-Hartman, Rob Herring
  Cc: llvm, oe-kbuild-all, Chunfeng Yun, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Mathias Nyman, linux-usb, linux-arm-kernel, linux-mediatek,
	devicetree, linux-kernel, Macpaul Lin, Eddie Hung

Hi Chunfeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next krzk/for-next linus/master v6.7-rc5 next-20231213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chunfeng-Yun/usb-xhci-mtk-fix-a-short-packet-issue-of-gen1-isoc-in-transfer/20231213-143959
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20231213063543.12435-2-chunfeng.yun%40mediatek.com
patch subject: [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer
config: arm-randconfig-001-20231213 (https://download.01.org/0day-ci/archive/20231214/202312140415.r09Wbh8D-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/202312140415.r09Wbh8D-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312140415.r09Wbh8D-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/host/xhci-mtk.c:184:12: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     184 |                 value |= FIELD_PREP(SCH3_RXFIFO_DEPTH_MASK, 2);
         |                          ^
   1 error generated.


vim +/FIELD_PREP +184 drivers/usb/host/xhci-mtk.c

   170	
   171	/*
   172	 * workaround: usb3.2 gen1 isoc rx hw issue
   173	 * host send out unexpected ACK afer device fininsh a burst transfer with
   174	 * a short packet.
   175	 */
   176	static void xhci_mtk_rxfifo_depth_set(struct xhci_hcd_mtk *mtk)
   177	{
   178		struct usb_hcd *hcd = mtk->hcd;
   179		u32 value;
   180	
   181		if (mtk->rxfifo_depth_quirk) {
   182			value = readl(hcd->regs + HSCH_CFG1);
   183			value &= ~SCH3_RXFIFO_DEPTH_MASK;
 > 184			value |= FIELD_PREP(SCH3_RXFIFO_DEPTH_MASK, 2);
   185			writel(value, hcd->regs + HSCH_CFG1);
   186		}
   187	}
   188	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer
  2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
  2023-12-13 21:03   ` kernel test robot
@ 2023-12-14 18:08   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-12-14 18:08 UTC (permalink / raw
  To: Chunfeng Yun, Greg Kroah-Hartman, Rob Herring
  Cc: oe-kbuild-all, Chunfeng Yun, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Mathias Nyman,
	linux-usb, linux-arm-kernel, linux-mediatek, devicetree,
	linux-kernel, Macpaul Lin, Eddie Hung

Hi Chunfeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next krzk/for-next linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chunfeng-Yun/usb-xhci-mtk-fix-a-short-packet-issue-of-gen1-isoc-in-transfer/20231213-143959
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20231213063543.12435-2-chunfeng.yun%40mediatek.com
patch subject: [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer
config: openrisc-allyesconfig (https://download.01.org/0day-ci/archive/20231215/202312150146.ffBFCrhf-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231215/202312150146.ffBFCrhf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312150146.ffBFCrhf-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/usb/host/xhci-mtk.c: In function 'xhci_mtk_rxfifo_depth_set':
>> drivers/usb/host/xhci-mtk.c:184:26: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     184 |                 value |= FIELD_PREP(SCH3_RXFIFO_DEPTH_MASK, 2);
         |                          ^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/FIELD_PREP +184 drivers/usb/host/xhci-mtk.c

   170	
   171	/*
   172	 * workaround: usb3.2 gen1 isoc rx hw issue
   173	 * host send out unexpected ACK afer device fininsh a burst transfer with
   174	 * a short packet.
   175	 */
   176	static void xhci_mtk_rxfifo_depth_set(struct xhci_hcd_mtk *mtk)
   177	{
   178		struct usb_hcd *hcd = mtk->hcd;
   179		u32 value;
   180	
   181		if (mtk->rxfifo_depth_quirk) {
   182			value = readl(hcd->regs + HSCH_CFG1);
   183			value &= ~SCH3_RXFIFO_DEPTH_MASK;
 > 184			value |= FIELD_PREP(SCH3_RXFIFO_DEPTH_MASK, 2);
   185			writel(value, hcd->regs + HSCH_CFG1);
   186		}
   187	}
   188	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13 11:17   ` AngeloGioacchino Del Regno
  2023-12-13 11:24     ` Krzysztof Kozlowski
@ 2023-12-14 22:43     ` Rob Herring
  2023-12-15  6:14     ` Chunfeng Yun (云春峰)
  2 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2023-12-14 22:43 UTC (permalink / raw
  To: AngeloGioacchino Del Regno
  Cc: Krzysztof Kozlowski, Chunfeng Yun, Greg Kroah-Hartman,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	Mathias Nyman, linux-usb, linux-arm-kernel, linux-mediatek,
	devicetree, linux-kernel, Macpaul Lin, Eddie Hung

On Wed, Dec 13, 2023 at 12:17:17PM +0100, AngeloGioacchino Del Regno wrote:
> Il 13/12/23 09:05, Krzysztof Kozlowski ha scritto:
> > On 13/12/2023 07:35, Chunfeng Yun wrote:
> > > For Gen1 isoc-in endpoint on controller before about SSUSB IPM v1.6.0, it
> > > still send out unexpected ACK after receiving a short packet in burst
> > > transfer, this will cause an exception on connected device, specially for
> > > a 4k camera.
> > > Add a quirk property "mediatek,rxfifo-depth" to work around this hardware
> > > issue;
> > > The side-effect is that may cause performance drop about 10%, including
> > > bulk transfer.
> > > 
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >   .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10 ++++++++++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > > index e9644e333d78..b8ed68574ba4 100644
> > > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > > @@ -124,6 +124,16 @@ properties:
> > >         defined in the xHCI spec on MTK's controller.
> > >       default: 5000
> > > +  mediatek,rxfifo-depth:
> > > +    description:
> > 
> > The property description and driver patch suggest you configure the
> > depth of FIFO, so this should be not bool, but some uint32. And then,
> > use generic "fifo-depth" property to set desired depth.
> 
> Did you mean "rx-fifo-depth" instead, as used in net?
> This FIFO depth parameter is for RX only.

Or rx-fifo-size. There's so many to choose from.

Rob

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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13  8:05 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Krzysztof Kozlowski
  2023-12-13 11:17   ` AngeloGioacchino Del Regno
@ 2023-12-15  6:10   ` Chunfeng Yun (云春峰)
  1 sibling, 0 replies; 11+ messages in thread
From: Chunfeng Yun (云春峰) @ 2023-12-15  6:10 UTC (permalink / raw
  To: krzysztof.kozlowski@linaro.org, gregkh@linuxfoundation.org,
	robh+dt@kernel.org
  Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
	mathias.nyman@intel.com, Eddie Hung (洪正鑫),
	conor+dt@kernel.org, Macpaul Lin (林智斌),
	linux-arm-kernel@lists.infradead.org,
	krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
	angelogioacchino.delregno@collabora.com

On Wed, 2023-12-13 at 09:05 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 13/12/2023 07:35, Chunfeng Yun wrote:
> > For Gen1 isoc-in endpoint on controller before about SSUSB IPM
> v1.6.0, it
> > still send out unexpected ACK after receiving a short packet in
> burst
> > transfer, this will cause an exception on connected device,
> specially for
> > a 4k camera.
> > Add a quirk property "mediatek,rxfifo-depth" to work around this
> hardware
> > issue;
> > The side-effect is that may cause performance drop about 10%,
> including
> > bulk transfer.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10
> ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> xhci.yaml
> > index e9644e333d78..b8ed68574ba4 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -124,6 +124,16 @@ properties:
> >        defined in the xHCI spec on MTK's controller.
> >      default: 5000
> >  
> > +  mediatek,rxfifo-depth:
> > +    description:
> 
> The property description and driver patch suggest you configure the
> depth of FIFO, so this should be not bool, but some uint32. And then,
> use generic "fifo-depth" property to set desired depth.
I fix it as 2, it's OK to use a uint32, I'll modify it.

Thanks

> 
> 
> Best regards,
> Krzysztof
> 
> 

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

* Re: [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue
  2023-12-13 11:17   ` AngeloGioacchino Del Regno
  2023-12-13 11:24     ` Krzysztof Kozlowski
  2023-12-14 22:43     ` Rob Herring
@ 2023-12-15  6:14     ` Chunfeng Yun (云春峰)
  2 siblings, 0 replies; 11+ messages in thread
From: Chunfeng Yun (云春峰) @ 2023-12-15  6:14 UTC (permalink / raw
  To: krzysztof.kozlowski@linaro.org,
	angelogioacchino.delregno@collabora.com,
	gregkh@linuxfoundation.org, robh+dt@kernel.org
  Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
	mathias.nyman@intel.com, Eddie Hung (洪正鑫),
	conor+dt@kernel.org, Macpaul Lin (林智斌),
	linux-arm-kernel@lists.infradead.org,
	krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com

On Wed, 2023-12-13 at 12:17 +0100, AngeloGioacchino Del Regno wrote:
> Il 13/12/23 09:05, Krzysztof Kozlowski ha scritto:
> > On 13/12/2023 07:35, Chunfeng Yun wrote:
> > > For Gen1 isoc-in endpoint on controller before about SSUSB IPM
> > > v1.6.0, it
> > > still send out unexpected ACK after receiving a short packet in
> > > burst
> > > transfer, this will cause an exception on connected device,
> > > specially for
> > > a 4k camera.
> > > Add a quirk property "mediatek,rxfifo-depth" to work around this
> > > hardware
> > > issue;
> > > The side-effect is that may cause performance drop about 10%,
> > > including
> > > bulk transfer.
> > > 
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >   .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml     | 10
> > > ++++++++++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > > xhci.yaml
> > > index e9644e333d78..b8ed68574ba4 100644
> > > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > > xhci.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > > xhci.yaml
> > > @@ -124,6 +124,16 @@ properties:
> > >         defined in the xHCI spec on MTK's controller.
> > >       default: 5000
> > >   
> > > +  mediatek,rxfifo-depth:
> > > +    description:
> > 
> > The property description and driver patch suggest you configure the
> > depth of FIFO, so this should be not bool, but some uint32. And
> > then,
> > use generic "fifo-depth" property to set desired depth.
> 
> Did you mean "rx-fifo-depth" instead, as used in net?
> This FIFO depth parameter is for RX only.
Yes, only for RX

> 
> Cheers,
> Angelo
> 
> 

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

end of thread, other threads:[~2023-12-15  6:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-13  6:35 [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Chunfeng Yun
2023-12-13  6:35 ` [PATCH 2/3] usb: xhci-mtk: fix a short packet issue of gen1 isoc-in transfer Chunfeng Yun
2023-12-13 21:03   ` kernel test robot
2023-12-14 18:08   ` kernel test robot
2023-12-13  6:35 ` [PATCH 3/3] arm64: dts: mediatek: mt8195: Add mediatek,rxfifo-depth to cherry Chunfeng Yun
2023-12-13  8:05 ` [PATCH 1/3] dt-bindings: usb: mtk-xhci: add a property for Gen1 isoc-in transfer issue Krzysztof Kozlowski
2023-12-13 11:17   ` AngeloGioacchino Del Regno
2023-12-13 11:24     ` Krzysztof Kozlowski
2023-12-14 22:43     ` Rob Herring
2023-12-15  6:14     ` Chunfeng Yun (云春峰)
2023-12-15  6:10   ` Chunfeng Yun (云春峰)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).