[PATCH 01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support'

[PATCH 01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support'

[Chunfeng Yun]

Add property 'tpl-support' to support targeted peripheral list
for USB-IF Embedded Host Compliance Test

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

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index 240882b12565..49729aab6d1a 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -115,6 +115,8 @@ properties:
 
   usb2-lpm-disable: true
 
+  tpl-support: true
+
   imod-interval-ns:
     description:
       Interrupt moderation interval value, it is 8 times as much as that
-- 
2.18.0

[PATCH 02/11] dt-bindings: usb: mtk-xhci: add optional property to disable usb2 ports

[Chunfeng Yun]

Add support to disable specific usb2 host ports, it's useful when
a usb2 port is disabled on some platforms, but enabled on others for
the same SoC, another case is that the different package may support
different number of ports.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 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 49729aab6d1a..61a0e550b5d6 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -154,6 +154,11 @@ properties:
     description: The mask to disable u3ports, bit0 for u3port0,
       bit1 for u3port1, ... etc
 
+  mediatek,u2p-dis-msk:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The mask to disable u2ports, bit0 for u2port0,
+      bit1 for u2port1, ... etc
+
   "#address-cells":
     const: 1
 
-- 
2.18.0

Re: [PATCH 02/11] dt-bindings: usb: mtk-xhci: add optional property to disable usb2 ports

[Rob Herring]

On Fri, Jul 30, 2021 at 04:49:53PM +0800, Chunfeng Yun wrote:
> Add support to disable specific usb2 host ports, it's useful when
> a usb2 port is disabled on some platforms, but enabled on others for
> the same SoC, another case is that the different package may support
> different number of ports.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  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 49729aab6d1a..61a0e550b5d6 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -154,6 +154,11 @@ properties:
>      description: The mask to disable u3ports, bit0 for u3port0,
>        bit1 for u3port1, ... etc
>  
> +  mediatek,u2p-dis-msk:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: The mask to disable u2ports, bit0 for u2port0,
> +      bit1 for u2port1, ... etc

You already have this property in the parent node, right? Why are we 
duplicating it in both places?

Rob

Re: [PATCH 02/11] dt-bindings: usb: mtk-xhci: add optional property to disable usb2 ports

[Chunfeng Yun (云春峰)]

On Fri, 2021-08-06 at 14:41 -0600, Rob Herring wrote:
> On Fri, Jul 30, 2021 at 04:49:53PM +0800, Chunfeng Yun wrote:
> > Add support to disable specific usb2 host ports, it's useful when
> > a usb2 port is disabled on some platforms, but enabled on others
> > for
> > the same SoC, another case is that the different package may
> > support
> > different number of ports.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  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 49729aab6d1a..61a0e550b5d6 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -154,6 +154,11 @@ properties:
> >      description: The mask to disable u3ports, bit0 for u3port0,
> >        bit1 for u3port1, ... etc
> >  
> > +  mediatek,u2p-dis-msk:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: The mask to disable u2ports, bit0 for u2port0,
> > +      bit1 for u2port1, ... etc
> 
> You already have this property in the parent node, right? Why are we 
> duplicating it in both places?
Some SoC may only support xHCI controller, then only use mtk-xhci node
without parent mtu3 node which is used for dual-role controller.
it's mainly for backward compatible.

Thanks a lot

> 
> Rob

[PATCH 03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

[Chunfeng Yun]

There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
exclude IP0) have a wrong default SOF/ITP interval which is
calculated from the frame counter clock 24Mhz by default, but
in fact, the frame counter clock is 48Mhz, so we should set
the accurate interval according to 48Mhz. Here add a new compatible
for MT8195, it's also supported in driver. But the first controller
(IP0) has no such issue, we prefer to use generic compatible,
e.g. mt8192's compatible.

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

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index 61a0e550b5d6..753e043e5327 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -31,6 +31,7 @@ properties:
           - mediatek,mt8173-xhci
           - mediatek,mt8183-xhci
           - mediatek,mt8192-xhci
+          - mediatek,mt8195-xhci
       - const: mediatek,mtk-xhci
 
   reg:
-- 
2.18.0

Re: [PATCH 03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

[Rob Herring]

On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:
> There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
> exclude IP0) have a wrong default SOF/ITP interval which is
> calculated from the frame counter clock 24Mhz by default, but
> in fact, the frame counter clock is 48Mhz, so we should set
> the accurate interval according to 48Mhz. Here add a new compatible
> for MT8195, it's also supported in driver. But the first controller
> (IP0) has no such issue, we prefer to use generic compatible,
> e.g. mt8192's compatible.

That only works until you find some 8195 bug common to all instances. 
Can't you read the clock frequency?

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index 61a0e550b5d6..753e043e5327 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -31,6 +31,7 @@ properties:
>            - mediatek,mt8173-xhci
>            - mediatek,mt8183-xhci
>            - mediatek,mt8192-xhci
> +          - mediatek,mt8195-xhci
>        - const: mediatek,mtk-xhci
>  
>    reg:
> -- 
> 2.18.0
> 
> 

Re: [PATCH 03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

[Chunfeng Yun (云春峰)]

On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:
> On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:
> > There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
> > exclude IP0) have a wrong default SOF/ITP interval which is
> > calculated from the frame counter clock 24Mhz by default, but
> > in fact, the frame counter clock is 48Mhz, so we should set
> > the accurate interval according to 48Mhz. Here add a new compatible
> > for MT8195, it's also supported in driver. But the first controller
> > (IP0) has no such issue, we prefer to use generic compatible,
> > e.g. mt8192's compatible.
> 
> That only works until you find some 8195 bug common to all
> instances. 
It's also OK for IP0 to use mt8195's compatible, these setting value is
the same as IP0's default value, use mt8192's may avoid these dummy
setting.

> Can't you read the clock frequency?
No, it may be divided internally, not the same as the clock source.

> 
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml
> > index 61a0e550b5d6..753e043e5327 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -31,6 +31,7 @@ properties:
> >            - mediatek,mt8173-xhci
> >            - mediatek,mt8183-xhci
> >            - mediatek,mt8192-xhci
> > +          - mediatek,mt8195-xhci
> >        - const: mediatek,mtk-xhci
> >  
> >    reg:
> > -- 
> > 2.18.0
> > 
> > 

Re: [PATCH 03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

[Rob Herring]

On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)
<Chunfeng.Yun@mediatek.com> wrote:
>
> On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:
> > On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:
> > > There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
> > > exclude IP0) have a wrong default SOF/ITP interval which is
> > > calculated from the frame counter clock 24Mhz by default, but
> > > in fact, the frame counter clock is 48Mhz, so we should set
> > > the accurate interval according to 48Mhz. Here add a new compatible
> > > for MT8195, it's also supported in driver. But the first controller
> > > (IP0) has no such issue, we prefer to use generic compatible,
> > > e.g. mt8192's compatible.
> >
> > That only works until you find some 8195 bug common to all
> > instances.
> It's also OK for IP0 to use mt8195's compatible, these setting value is
> the same as IP0's default value, use mt8192's may avoid these dummy
> setting.

I still don't understand. By use mt8192's compatible, that means you
have for IP0:

compatible = "mediatek,mt8192-xhci", "mediatek,mtk-xhci";

And for the rest:
compatible = "mediatek,mt8195-xhci", "mediatek,mtk-xhci";

If there's a 8195 quirk you need to work around, then you can't on
IP0. You need to be able to address quirks in the future without
changing the DTB. That is why we require SoC specific compatibles even
when IP blocks are 'the same'.

Rob

Re: [PATCH 03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

[Chunfeng Yun (云春峰)]

On Wed, 2021-08-18 at 09:20 -0500, Rob Herring wrote:
> On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)
> <Chunfeng.Yun@mediatek.com> wrote:
> > 
> > On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:
> > > On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:
> > > > There are 4 USB controllers on MT8195, the controllers
> > > > (IP1~IP3,
> > > > exclude IP0) have a wrong default SOF/ITP interval which is
> > > > calculated from the frame counter clock 24Mhz by default, but
> > > > in fact, the frame counter clock is 48Mhz, so we should set
> > > > the accurate interval according to 48Mhz. Here add a new
> > > > compatible
> > > > for MT8195, it's also supported in driver. But the first
> > > > controller
> > > > (IP0) has no such issue, we prefer to use generic compatible,
> > > > e.g. mt8192's compatible.
> > > 
> > > That only works until you find some 8195 bug common to all
> > > instances.
> > 
> > It's also OK for IP0 to use mt8195's compatible, these setting
> > value is
> > the same as IP0's default value, use mt8192's may avoid these dummy
> > setting.
> 
> I still don't understand. By use mt8192's compatible, that means you
> have for IP0:
> 
> compatible = "mediatek,mt8192-xhci", "mediatek,mtk-xhci";
> 
> And for the rest:
> compatible = "mediatek,mt8195-xhci", "mediatek,mtk-xhci";
No, use "mediatek,mt8195-xhci" is also ok for IP0.

Seems need modify commit log and remove last sentence to avoid
misunderstanding.

Thanks

> 
> If there's a 8195 quirk you need to work around, then you can't on
> IP0. You need to be able to address quirks in the future without
> changing the DTB. That is why we require SoC specific compatibles
> even
> when IP blocks are 'the same'.
> 
> Rob

[PATCH 04/11] usb: xhci-mtk: fix use-after-free of mtk->hcd

[Chunfeng Yun]

 BUG: KASAN: use-after-free in usb_hcd_is_primary_hcd+0x38/0x60
 Call trace:
  dump_backtrace+0x0/0x3dc
  show_stack+0x20/0x2c
  dump_stack+0x15c/0x1d4
  print_address_description+0x7c/0x510
  kasan_report+0x164/0x1ac
  __asan_report_load8_noabort+0x44/0x50
  usb_hcd_is_primary_hcd+0x38/0x60
  xhci_mtk_runtime_suspend+0x68/0x148
  pm_generic_runtime_suspend+0x90/0xac
  __rpm_callback+0xb8/0x1f4
  rpm_callback+0x54/0x1d0
  rpm_suspend+0x4e0/0xc84
  __pm_runtime_suspend+0xc4/0x114
  xhci_mtk_probe+0xa58/0xd00

This may happen when probe fails, needn't suspend it synchronously,
fix it by using pm_runtime_put_noidle().

Reported-by: Pi Hsun <pihsun@google.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 2548976bcf05..cb27569186a0 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -569,7 +569,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	xhci_mtk_ldos_disable(mtk);
 
 disable_pm:
-	pm_runtime_put_sync_autosuspend(dev);
+	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
 	return ret;
 }
-- 
2.18.0

Re: [PATCH 04/11] usb: xhci-mtk: fix use-after-free of mtk->hcd

[Ikjoon Jang]

On Fri, Jul 30, 2021 at 4:50 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
>  BUG: KASAN: use-after-free in usb_hcd_is_primary_hcd+0x38/0x60
>  Call trace:
>   dump_backtrace+0x0/0x3dc
>   show_stack+0x20/0x2c
>   dump_stack+0x15c/0x1d4
>   print_address_description+0x7c/0x510
>   kasan_report+0x164/0x1ac
>   __asan_report_load8_noabort+0x44/0x50
>   usb_hcd_is_primary_hcd+0x38/0x60
>   xhci_mtk_runtime_suspend+0x68/0x148
>   pm_generic_runtime_suspend+0x90/0xac
>   __rpm_callback+0xb8/0x1f4
>   rpm_callback+0x54/0x1d0
>   rpm_suspend+0x4e0/0xc84
>   __pm_runtime_suspend+0xc4/0x114
>   xhci_mtk_probe+0xa58/0xd00
>
> This may happen when probe fails, needn't suspend it synchronously,
> fix it by using pm_runtime_put_noidle().
>
> Reported-by: Pi Hsun <pihsun@google.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Reviewed-and-Tested-by: Ikjoon Jang <ikjn@chromium.org>


> ---
>  drivers/usb/host/xhci-mtk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 2548976bcf05..cb27569186a0 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -569,7 +569,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         xhci_mtk_ldos_disable(mtk);
>
>  disable_pm:
> -       pm_runtime_put_sync_autosuspend(dev);
> +       pm_runtime_put_noidle(dev);
>         pm_runtime_disable(dev);
>         return ret;
>  }
> --
> 2.18.0
>

[PATCH 05/11] usb: xhci-mtk: add support 'tpl-support'

[Chunfeng Yun]

Add support property 'tpl-support' for targeted peripheral list
(USB-IF Embedded Host Compliance Plan)

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index cb27569186a0..f6d161670c4e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -18,6 +18,7 @@
 #include <linux/pm_wakeirq.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/usb/of.h>
 
 #include "xhci.h"
 #include "xhci-mtk.h"
@@ -517,6 +518,9 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		goto disable_device_wakeup;
 	}
 
+	hcd->tpl_support = of_usb_host_tpl_support(node);
+	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto put_usb3_hcd;
-- 
2.18.0

[PATCH 06/11] usb: xhci-mtk: support option to disable usb2 ports

[Chunfeng Yun]

Add support to disable specific usb2 host ports, it's useful when
a usb2 port is disabled on some platforms, but enabled on others
for the same SoC.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 12 ++++++++++--
 drivers/usb/host/xhci-mtk.h |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index f6d161670c4e..12b691547438 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -116,8 +116,11 @@ static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
 		writel(value, &ippc->u3_ctrl_p[i]);
 	}
 
-	/* power on and enable all u2 ports */
+	/* power on and enable all u2 ports except skipped ones */
 	for (i = 0; i < mtk->num_u2_ports; i++) {
+		if (BIT(i) & mtk->u2p_dis_msk)
+			continue;
+
 		value = readl(&ippc->u2_ctrl_p[i]);
 		value &= ~(CTRL_U2_PORT_PDN | CTRL_U2_PORT_DIS);
 		value |= CTRL_U2_PORT_HOST_SEL;
@@ -164,8 +167,11 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
 		writel(value, &ippc->u3_ctrl_p[i]);
 	}
 
-	/* power down all u2 ports */
+	/* power down all u2 ports except skipped ones */
 	for (i = 0; i < mtk->num_u2_ports; i++) {
+		if (BIT(i) & mtk->u2p_dis_msk)
+			continue;
+
 		value = readl(&ippc->u2_ctrl_p[i]);
 		value |= CTRL_U2_PORT_PDN;
 		writel(value, &ippc->u2_ctrl_p[i]);
@@ -445,6 +451,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	/* optional property, ignore the error if it does not exist */
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
+	of_property_read_u32(node, "mediatek,u2p-dis-msk",
+			     &mtk->u2p_dis_msk);
 
 	ret = usb_wakeup_of_property_parse(mtk, node);
 	if (ret) {
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index ace432356c41..0466bc8f7500 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -138,6 +138,7 @@ struct xhci_hcd_mtk {
 	struct mu3c_ippc_regs __iomem *ippc_regs;
 	int num_u2_ports;
 	int num_u3_ports;
+	int u2p_dis_msk;
 	int u3p_dis_msk;
 	struct regulator *vusb33;
 	struct regulator *vbus;
-- 
2.18.0

[PATCH 07/11] usb: xhci-mtk: fix issue of out-of-bounds array access

[Chunfeng Yun]

Bus bandwidth array access is based on esit, increase one
will cause out-of-bounds issue; for example, when esit is
XHCI_MTK_MAX_ESIT, will overstep boundary.

Fixes: 7c986fbc16ae ("usb: xhci-mtk: get the microframe boundary for ESIT")
Cc: <stable@vger.kernel.org>
Reported-by: Stan Lu <stan.lu@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk-sch.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index cffcaf4dfa9f..0bb1a6295d64 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -575,10 +575,12 @@ static u32 get_esit_boundary(struct mu3h_sch_ep_info *sch_ep)
 	u32 boundary = sch_ep->esit;
 
 	if (sch_ep->sch_tt) { /* LS/FS with TT */
-		/* tune for CS */
-		if (sch_ep->ep_type != ISOC_OUT_EP)
-			boundary++;
-		else if (boundary > 1) /* normally esit >= 8 for FS/LS */
+		/*
+		 * tune for CS, normally esit >= 8 for FS/LS,
+		 * not add one for other types to avoid access array
+		 * out of boundary
+		 */
+		if (sch_ep->ep_type == ISOC_OUT_EP && boundary > 1)
 			boundary--;
 	}
 
-- 
2.18.0

[PATCH 08/11] usb: xhci-mtk: update fs bus bandwidth by bw_budget_table

[Chunfeng Yun]

Use @bw_budget_table[] to update fs bus bandwidth due to
not all microframes consume @bw_cost_per_microframe, see
setup_sch_info().

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk-sch.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 0bb1a6295d64..10c0f0f6461f 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
 		 * Compared with hs bus, no matter what ep type,
 		 * the hub will always delay one uframe to send data
 		 */
-		for (j = 0; j < sch_ep->cs_count; j++) {
-			tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe;
+		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+			tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j];
 			if (tmp > FS_PAYLOAD_MAX)
 				return -ESCH_BW_OVERFLOW;
 		}
@@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
 	u32 base, num_esit;
-	int bw_updated;
 	int i, j;
 
 	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
 
-	if (used)
-		bw_updated = sch_ep->bw_cost_per_microframe;
-	else
-		bw_updated = -sch_ep->bw_cost_per_microframe;
-
 	for (i = 0; i < num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
 
-		for (j = 0; j < sch_ep->cs_count; j++)
-			tt->fs_bus_bw[base + j] += bw_updated;
+		for (j = 0; j < sch_ep->num_budget_microframes; j++)
+			if (used)
+				tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j];
+			else
+				tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j];
 	}
 
 	if (used)
-- 
2.18.0

Re: [PATCH 08/11] usb: xhci-mtk: update fs bus bandwidth by bw_budget_table

[Ikjoon Jang]

Hi Chunfeng,

On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Use @bw_budget_table[] to update fs bus bandwidth due to
> not all microframes consume @bw_cost_per_microframe, see
> setup_sch_info().
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk-sch.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> index 0bb1a6295d64..10c0f0f6461f 100644
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
>                  * Compared with hs bus, no matter what ep type,
>                  * the hub will always delay one uframe to send data
>                  */
> -               for (j = 0; j < sch_ep->cs_count; j++) {
> -                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe;
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j];
>                         if (tmp > FS_PAYLOAD_MAX)
>                                 return -ESCH_BW_OVERFLOW;
>                 }
> @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
>  {
>         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
>         u32 base, num_esit;
> -       int bw_updated;
>         int i, j;
>
>         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
>
> -       if (used)
> -               bw_updated = sch_ep->bw_cost_per_microframe;
> -       else
> -               bw_updated = -sch_ep->bw_cost_per_microframe;
> -
>         for (i = 0; i < num_esit; i++) {
>                 base = sch_ep->offset + i * sch_ep->esit;
>
> -               for (j = 0; j < sch_ep->cs_count; j++)
> -                       tt->fs_bus_bw[base + j] += bw_updated;
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> +                       if (used)
> +                               tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j];
> +                       else
> +                               tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j];

I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth
but I think this approach could trigger a problem.

for example, if there are two endpoints scheduled in the same u-frame index,
* ep1out  = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0
* ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... }  --> y0

(If this is possible allocation from this patch),
I guess xhci-mtk could have some problems on internal scheduling?

>         }
>
>         if (used)

> --
> 2.18.0
>

Re: [PATCH 08/11] usb: xhci-mtk: update fs bus bandwidth by bw_budget_table

[Chunfeng Yun]

On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote:
> Hi Chunfeng,
> 
> On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > Use @bw_budget_table[] to update fs bus bandwidth due to
> > not all microframes consume @bw_cost_per_microframe, see
> > setup_sch_info().
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk-sch.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> > index 0bb1a6295d64..10c0f0f6461f 100644
> > --- a/drivers/usb/host/xhci-mtk-sch.c
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
> >                  * Compared with hs bus, no matter what ep type,
> >                  * the hub will always delay one uframe to send data
> >                  */
> > -               for (j = 0; j < sch_ep->cs_count; j++) {
> > -                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe;
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j];
> >                         if (tmp > FS_PAYLOAD_MAX)
> >                                 return -ESCH_BW_OVERFLOW;
> >                 }
> > @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
> >  {
> >         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> >         u32 base, num_esit;
> > -       int bw_updated;
> >         int i, j;
> >
> >         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> >
> > -       if (used)
> > -               bw_updated = sch_ep->bw_cost_per_microframe;
> > -       else
> > -               bw_updated = -sch_ep->bw_cost_per_microframe;
> > -
> >         for (i = 0; i < num_esit; i++) {
> >                 base = sch_ep->offset + i * sch_ep->esit;
> >
> > -               for (j = 0; j < sch_ep->cs_count; j++)
> > -                       tt->fs_bus_bw[base + j] += bw_updated;
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> > +                       if (used)
> > +                               tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j];
> > +                       else
> > +                               tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j];
> 
> I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth
> but I think this approach could trigger a problem.
See updat_bus_bw(), when add fs ep's bandwidth, it uses
bw_budget_table[], so prefer to use the same way

> 
> for example, if there are two endpoints scheduled in the same u-frame index,
> * ep1out  = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0
> * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... }  --> y0
> 
> (If this is possible allocation from this patch),
> I guess xhci-mtk could have some problems on internal scheduling?

Test it on dvt env. don't encounter issues;

Thanks

> 
> >         }
> >
> >         if (used)
> 
> > --
> > 2.18.0
> >

Re: [PATCH 08/11] usb: xhci-mtk: update fs bus bandwidth by bw_budget_table

[Ikjoon Jang]

Hi,

On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote:
> > Hi Chunfeng,
> >
> > On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > Use @bw_budget_table[] to update fs bus bandwidth due to
> > > not all microframes consume @bw_cost_per_microframe, see
> > > setup_sch_info().
> > >
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > ---
> > >  drivers/usb/host/xhci-mtk-sch.c | 17 +++++++----------
> > >  1 file changed, 7 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> > > index 0bb1a6295d64..10c0f0f6461f 100644
> > > --- a/drivers/usb/host/xhci-mtk-sch.c
> > > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
> > >                  * Compared with hs bus, no matter what ep type,
> > >                  * the hub will always delay one uframe to send data
> > >                  */
> > > -               for (j = 0; j < sch_ep->cs_count; j++) {
> > > -                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_cost_per_microframe;
> > > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > > +                       tmp = tt->fs_bus_bw[base + j] + sch_ep->bw_budget_table[j];
> > >                         if (tmp > FS_PAYLOAD_MAX)
> > >                                 return -ESCH_BW_OVERFLOW;
> > >                 }
> > > @@ -534,21 +534,18 @@ static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
> > >  {
> > >         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> > >         u32 base, num_esit;
> > > -       int bw_updated;
> > >         int i, j;
> > >
> > >         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> > >
> > > -       if (used)
> > > -               bw_updated = sch_ep->bw_cost_per_microframe;
> > > -       else
> > > -               bw_updated = -sch_ep->bw_cost_per_microframe;
> > > -
> > >         for (i = 0; i < num_esit; i++) {
> > >                 base = sch_ep->offset + i * sch_ep->esit;
> > >
> > > -               for (j = 0; j < sch_ep->cs_count; j++)
> > > -                       tt->fs_bus_bw[base + j] += bw_updated;
> > > +               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> > > +                       if (used)
> > > +                               tt->fs_bus_bw[base + j] += sch_ep->bw_budget_table[j];
> > > +                       else
> > > +                               tt->fs_bus_bw[base + j] -= sch_ep->bw_budget_table[j];
> >
> > I agree that xhci-mtk-sch still has more rooms for tt periodic bandwidth
> > but I think this approach could trigger a problem.
> See updat_bus_bw(), when add fs ep's bandwidth, it uses
> bw_budget_table[], so prefer to use the same way
>
> >
> > for example, if there are two endpoints scheduled in the same u-frame index,
> > * ep1out  = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...} --> y0
> > * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... }  --> y0
> >
> > (If this is possible allocation from this patch),
> > I guess xhci-mtk could have some problems on internal scheduling?
>
> Test it on dvt env. don't encounter issues;
>

As you can see In the above example, this patch starts to allow that allocation.
Do you mean that we don't have to worry about such a case (on all MTK
platforms)?

thanks

> Thanks
>
> >
> > >         }
> > >
> > >         if (used)
> >
> > > --
> > > 2.18.0
> > >
>

Re: [PATCH 08/11] usb: xhci-mtk: update fs bus bandwidth by bw_budget_table

[Chunfeng Yun (云春峰)]

On Wed, 2021-08-04 at 22:06 +0800, Ikjoon Jang wrote:
> Hi,
> 
> On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> > 
> > On Tue, 2021-08-03 at 14:05 +0800, Ikjoon Jang wrote:
> > > Hi Chunfeng,
> > > 
> > > On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <
> > > chunfeng.yun@mediatek.com> wrote:
> > > > 
> > > > Use @bw_budget_table[] to update fs bus bandwidth due to
> > > > not all microframes consume @bw_cost_per_microframe, see
> > > > setup_sch_info().
> > > > 
> > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk-sch.c | 17 +++++++----------
> > > >  1 file changed, 7 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/host/xhci-mtk-sch.c
> > > > b/drivers/usb/host/xhci-mtk-sch.c
> > > > index 0bb1a6295d64..10c0f0f6461f 100644
> > > > --- a/drivers/usb/host/xhci-mtk-sch.c
> > > > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > > > @@ -458,8 +458,8 @@ static int check_fs_bus_bw(struct
> > > > mu3h_sch_ep_info *sch_ep, int offset)
> > > >                  * Compared with hs bus, no matter what ep
> > > > type,
> > > >                  * the hub will always delay one uframe to send
> > > > data
> > > >                  */
> > > > -               for (j = 0; j < sch_ep->cs_count; j++) {
> > > > -                       tmp = tt->fs_bus_bw[base + j] + sch_ep-
> > > > >bw_cost_per_microframe;
> > > > +               for (j = 0; j < sch_ep->num_budget_microframes; 
> > > > j++) {
> > > > +                       tmp = tt->fs_bus_bw[base + j] + sch_ep-
> > > > >bw_budget_table[j];
> > > >                         if (tmp > FS_PAYLOAD_MAX)
> > > >                                 return -ESCH_BW_OVERFLOW;
> > > >                 }
> > > > @@ -534,21 +534,18 @@ static void update_sch_tt(struct
> > > > mu3h_sch_ep_info *sch_ep, bool used)
> > > >  {
> > > >         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> > > >         u32 base, num_esit;
> > > > -       int bw_updated;
> > > >         int i, j;
> > > > 
> > > >         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> > > > 
> > > > -       if (used)
> > > > -               bw_updated = sch_ep->bw_cost_per_microframe;
> > > > -       else
> > > > -               bw_updated = -sch_ep->bw_cost_per_microframe;
> > > > -
> > > >         for (i = 0; i < num_esit; i++) {
> > > >                 base = sch_ep->offset + i * sch_ep->esit;
> > > > 
> > > > -               for (j = 0; j < sch_ep->cs_count; j++)
> > > > -                       tt->fs_bus_bw[base + j] += bw_updated;
> > > > +               for (j = 0; j < sch_ep->num_budget_microframes; 
> > > > j++)
> > > > +                       if (used)
> > > > +                               tt->fs_bus_bw[base + j] +=
> > > > sch_ep->bw_budget_table[j];
> > > > +                       else
> > > > +                               tt->fs_bus_bw[base + j] -=
> > > > sch_ep->bw_budget_table[j];
> > > 
> > > I agree that xhci-mtk-sch still has more rooms for tt periodic
> > > bandwidth
> > > but I think this approach could trigger a problem.
> > 
> > See updat_bus_bw(), when add fs ep's bandwidth, it uses
> > bw_budget_table[], so prefer to use the same way
> > 
> > > 
> > > for example, if there are two endpoints scheduled in the same u-
> > > frame index,
> > > * ep1out  = iso 192bytes bw_budget_table[] = { 188, 188, 0, ...}
> > > --> y0
> > > * ep2in = int 64bytes, bw_budget_table[] = { 0, 0, 64, ... }  -->
> > > y0
> > > 
> > > (If this is possible allocation from this patch),
> > > I guess xhci-mtk could have some problems on internal scheduling?
> > 
> > Test it on dvt env. don't encounter issues;
> > 
> 
> As you can see In the above example, this patch starts to allow that
> allocation.
> Do you mean that we don't have to worry about such a case (on all MTK
> platforms)?
No, that is another question, when update bus_bw[] and fs_bus_bw[] for
FS with TT should use the same bw_budget_table[] which is filled in
setup_sch_info(). If the bw_budget_table[] is something wrong, we can
prepare new patch to fix it.

> 
> thanks
> 
> > Thanks
> > 
> > > 
> > > >         }
> > > > 
> > > >         if (used)
> > > > --
> > > > 2.18.0
> > > > 

[PATCH 09/11] usb: xhci-mtk: check boundary before check tt

[Chunfeng Yun]

check_sch_tt() will access fs_bus_bw[] array, check boundary
firstly to avoid out-of-bounds issue.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk-sch.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 10c0f0f6461f..c2f13d69c607 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -600,13 +600,14 @@ static int check_sch_bw(struct mu3h_sch_bw_info *sch_bw,
 	 * and find a microframe where its worst bandwidth is minimum.
 	 */
 	for (offset = 0; offset < sch_ep->esit; offset++) {
-		ret = check_sch_tt(sch_ep, offset);
-		if (ret)
-			continue;
 
 		if ((offset + sch_ep->num_budget_microframes) > esit_boundary)
 			break;
 
+		ret = check_sch_tt(sch_ep, offset);
+		if (ret)
+			continue;
+
 		worst_bw = get_max_bw(sch_bw, sch_ep, offset);
 		if (worst_bw > bw_boundary)
 			continue;
-- 
2.18.0

Re: [PATCH 09/11] usb: xhci-mtk: check boundary before check tt

[Ikjoon Jang]

Hi Chunfeng,

On Fri, Jul 30, 2021 at 4:50 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> check_sch_tt() will access fs_bus_bw[] array, check boundary
> firstly to avoid out-of-bounds issue.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk-sch.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> index 10c0f0f6461f..c2f13d69c607 100644
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -600,13 +600,14 @@ static int check_sch_bw(struct mu3h_sch_bw_info *sch_bw,
>          * and find a microframe where its worst bandwidth is minimum.
>          */
>         for (offset = 0; offset < sch_ep->esit; offset++) {
> -               ret = check_sch_tt(sch_ep, offset);
> -               if (ret)
> -                       continue;
>
>                 if ((offset + sch_ep->num_budget_microframes) > esit_boundary)
>                         break;

Instead of dropping it,
I'm wondering if it should be checked against (offset & 63) == 0 when it's 64?

>
> +               ret = check_sch_tt(sch_ep, offset);
> +               if (ret)
> +                       continue;
> +
>                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
>                 if (worst_bw > bw_boundary)
>                         continue;
> --
> 2.18.0
>

Re: [PATCH 09/11] usb: xhci-mtk: check boundary before check tt

[Chunfeng Yun (云春峰)]

On Mon, 2021-08-09 at 15:32 +0800, Ikjoon Jang wrote:
> Hi Chunfeng,
> 
> On Fri, Jul 30, 2021 at 4:50 PM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> > 
> > check_sch_tt() will access fs_bus_bw[] array, check boundary
> > firstly to avoid out-of-bounds issue.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk-sch.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c
> > b/drivers/usb/host/xhci-mtk-sch.c
> > index 10c0f0f6461f..c2f13d69c607 100644
> > --- a/drivers/usb/host/xhci-mtk-sch.c
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > @@ -600,13 +600,14 @@ static int check_sch_bw(struct
> > mu3h_sch_bw_info *sch_bw,
> >          * and find a microframe where its worst bandwidth is
> > minimum.
> >          */
> >         for (offset = 0; offset < sch_ep->esit; offset++) {
> > -               ret = check_sch_tt(sch_ep, offset);
> > -               if (ret)
> > -                       continue;
> > 
> >                 if ((offset + sch_ep->num_budget_microframes) >
> > esit_boundary)
> >                         break;
> 
> Instead of dropping it,
> I'm wondering if it should be checked against (offset & 63) == 0 when
> it's 64?
No, sch_ep->esit already ensure it, it's <= 64, see get_esit()

> 
> > 
> > +               ret = check_sch_tt(sch_ep, offset);
> > +               if (ret)
> > +                       continue;
> > +
> >                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
> >                 if (worst_bw > bw_boundary)
> >                         continue;
> > --
> > 2.18.0
> > 

[PATCH 10/11] usb: xhci-mtk: add a member of num_esit

[Chunfeng Yun]

Add a member num_esit to save the number of esit, then no need
caculate it in some functions.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk-sch.c | 20 +++++++-------------
 drivers/usb/host/xhci-mtk.h     |  2 ++
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index c2f13d69c607..a9fcf7e30c41 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -297,6 +297,7 @@ static void setup_sch_info(struct xhci_ep_ctx *ep_ctx,
 		 CTX_TO_MAX_ESIT_PAYLOAD(le32_to_cpu(ep_ctx->tx_info));
 
 	sch_ep->esit = get_esit(ep_ctx);
+	sch_ep->num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
 	sch_ep->ep_type = ep_type;
 	sch_ep->maxpkt = maxpkt;
 	sch_ep->offset = 0;
@@ -401,14 +402,12 @@ static void setup_sch_info(struct xhci_ep_ctx *ep_ctx,
 static u32 get_max_bw(struct mu3h_sch_bw_info *sch_bw,
 	struct mu3h_sch_ep_info *sch_ep, u32 offset)
 {
-	u32 num_esit;
 	u32 max_bw = 0;
 	u32 bw;
 	int i;
 	int j;
 
-	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
-	for (i = 0; i < num_esit; i++) {
+	for (i = 0; i < sch_ep->num_esit; i++) {
 		u32 base = offset + i * sch_ep->esit;
 
 		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
@@ -424,13 +423,11 @@ static u32 get_max_bw(struct mu3h_sch_bw_info *sch_bw,
 static void update_bus_bw(struct mu3h_sch_bw_info *sch_bw,
 	struct mu3h_sch_ep_info *sch_ep, bool used)
 {
-	u32 num_esit;
 	u32 base;
 	int i;
 	int j;
 
-	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
-	for (i = 0; i < num_esit; i++) {
+	for (i = 0; i < sch_ep->num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
 		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
 			if (used)
@@ -446,12 +443,11 @@ static void update_bus_bw(struct mu3h_sch_bw_info *sch_bw,
 static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
-	u32 num_esit, tmp;
+	u32 tmp;
 	int base;
 	int i, j;
 
-	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
-	for (i = 0; i < num_esit; i++) {
+	for (i = 0; i < sch_ep->num_esit; i++) {
 		base = offset + i * sch_ep->esit;
 
 		/*
@@ -533,12 +529,10 @@ static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
 static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
-	u32 base, num_esit;
+	u32 base;
 	int i, j;
 
-	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
-
-	for (i = 0; i < num_esit; i++) {
+	for (i = 0; i < sch_ep->num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
 
 		for (j = 0; j < sch_ep->num_budget_microframes; j++)
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 0466bc8f7500..56dc348349af 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -50,6 +50,7 @@ struct mu3h_sch_bw_info {
  * struct mu3h_sch_ep_info: schedule information for endpoint
  *
  * @esit: unit is 125us, equal to 2 << Interval field in ep-context
+ * @num_esit: number of @esit in a period
  * @num_budget_microframes: number of continuous uframes
  *		(@repeat==1) scheduled within the interval
  * @bw_cost_per_microframe: bandwidth cost per microframe
@@ -79,6 +80,7 @@ struct mu3h_sch_bw_info {
  */
 struct mu3h_sch_ep_info {
 	u32 esit;
+	u32 num_esit;
 	u32 num_budget_microframes;
 	u32 bw_cost_per_microframe;
 	struct list_head endpoint;
-- 
2.18.0

[PATCH 11/11] usb: xhci-mtk: modify the SOF/ITP interval for mt8195

[Chunfeng Yun]

There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
exclude IP0) have a wrong default SOF/ITP interval which is
calculated from the frame counter clock 24Mhz by default, but
in fact, the frame counter clock is 48Mhz, so we should set
the accurate interval according to 48Mhz for those controllers.
Note: the first controller no need set it.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 65 +++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 12b691547438..7ff0cd707ba1 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -57,6 +57,27 @@
 /* u2_phy_pll register */
 #define CTRL_U2_FORCE_PLL_STB	BIT(28)
 
+/* xHCI CSR */
+#define LS_EOF_CFG		0x930
+#define LSEOF_OFFSET		0x89
+
+#define FS_EOF_CFG		0x934
+#define FSEOF_OFFSET		0x2e
+
+#define SS_GEN1_EOF_CFG		0x93c
+#define SSG1EOF_OFFSET		0x78
+
+#define HFCNTR_CFG		0x944
+#define ITP_DELTA_CLK		(0xa << 1)
+#define ITP_DELTA_CLK_MASK	GENMASK(5, 1)
+#define FRMCNT_LEV1_RANG	(0x12b << 8)
+#define FRMCNT_LEV1_RANG_MASK	GENMASK(19, 8)
+
+#define SS_GEN2_EOF_CFG		0x990
+#define SSG2EOF_OFFSET		0x3c
+
+#define XSEOF_OFFSET_MASK	GENMASK(11, 0)
+
 /* usb remote wakeup registers in syscon */
 
 /* mt8173 etc */
@@ -87,6 +108,46 @@ enum ssusb_uwk_vers {
 	SSUSB_UWK_V1_2,		/* specific revision 1.2 */
 };
 
+/*
+ * MT8195 has 4 controllers, the controller1~3's default SOF/ITP interval
+ * is calculated from the frame counter clock 24M, but in fact, the clock
+ * is 48M, add workaround for it.
+ */
+static void xhci_mtk_set_frame_interval(struct xhci_hcd_mtk *mtk)
+{
+	struct device *dev = mtk->dev;
+	struct usb_hcd *hcd = mtk->hcd;
+	u32 value;
+
+	if (!of_device_is_compatible(dev->of_node, "mediatek,mt8195-xhci"))
+		return;
+
+	value = readl(hcd->regs + HFCNTR_CFG);
+	value &= ~(ITP_DELTA_CLK_MASK | FRMCNT_LEV1_RANG_MASK);
+	value |= (ITP_DELTA_CLK | FRMCNT_LEV1_RANG);
+	writel(value, hcd->regs + HFCNTR_CFG);
+
+	value = readl(hcd->regs + LS_EOF_CFG);
+	value &= ~XSEOF_OFFSET_MASK;
+	value |= LSEOF_OFFSET;
+	writel(value, hcd->regs + LS_EOF_CFG);
+
+	value = readl(hcd->regs + FS_EOF_CFG);
+	value &= ~XSEOF_OFFSET_MASK;
+	value |= FSEOF_OFFSET;
+	writel(value, hcd->regs + FS_EOF_CFG);
+
+	value = readl(hcd->regs + SS_GEN1_EOF_CFG);
+	value &= ~XSEOF_OFFSET_MASK;
+	value |= SSG1EOF_OFFSET;
+	writel(value, hcd->regs + SS_GEN1_EOF_CFG);
+
+	value = readl(hcd->regs + SS_GEN2_EOF_CFG);
+	value &= ~XSEOF_OFFSET_MASK;
+	value |= SSG2EOF_OFFSET;
+	writel(value, hcd->regs + SS_GEN2_EOF_CFG);
+}
+
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
 {
 	struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
@@ -368,6 +429,9 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
 		ret = xhci_mtk_ssusb_config(mtk);
 		if (ret)
 			return ret;
+
+		/* workaround only for mt8195 */
+		xhci_mtk_set_frame_interval(mtk);
 	}
 
 	ret = xhci_gen_setup(hcd, xhci_mtk_quirks);
@@ -716,6 +780,7 @@ static const struct dev_pm_ops xhci_mtk_pm_ops = {
 
 static const struct of_device_id mtk_xhci_of_match[] = {
 	{ .compatible = "mediatek,mt8173-xhci"},
+	{ .compatible = "mediatek,mt8195-xhci"},
 	{ .compatible = "mediatek,mtk-xhci"},
 	{ },
 };
-- 
2.18.0

Re: [PATCH 01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support'

[Rob Herring]

On Fri, Jul 30, 2021 at 04:49:52PM +0800, Chunfeng Yun wrote:
> Add property 'tpl-support' to support targeted peripheral list
> for USB-IF Embedded Host Compliance Test

Given you have to configure the TPL somehow, how is this useful to be in 
DT? And no, that's not a suggestion to put all the TPL config into DT.

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index 240882b12565..49729aab6d1a 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -115,6 +115,8 @@ properties:
>  
>    usb2-lpm-disable: true
>  
> +  tpl-support: true
> +
>    imod-interval-ns:
>      description:
>        Interrupt moderation interval value, it is 8 times as much as that
> -- 
> 2.18.0
> 
> 

Re: [PATCH 01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support'

[Chunfeng Yun (云春峰)]

On Fri, 2021-08-06 at 14:37 -0600, Rob Herring wrote:
> On Fri, Jul 30, 2021 at 04:49:52PM +0800, Chunfeng Yun wrote:
> > Add property 'tpl-support' to support targeted peripheral list
> > for USB-IF Embedded Host Compliance Test
> 
> Given you have to configure the TPL somehow, how is this useful to be
> in 
> DT? And no, that's not a suggestion to put all the TPL config into
> DT.
Ok, will abandon this patch

Thanks

> 
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 2
> > ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml
> > index 240882b12565..49729aab6d1a 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -115,6 +115,8 @@ properties:
> >  
> >    usb2-lpm-disable: true
> >  
> > +  tpl-support: true
> > +
> >    imod-interval-ns:
> >      description:
> >        Interrupt moderation interval value, it is 8 times as much
> > as that
> > -- 
> > 2.18.0
> > 
> > 

Re: [PATCH 01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support'

[Chunfeng Yun (云春峰)]

On Fri, 2021-08-06 at 14:37 -0600, Rob Herring wrote:
> On Fri, Jul 30, 2021 at 04:49:52PM +0800, Chunfeng Yun wrote:
> > Add property 'tpl-support' to support targeted peripheral list
> > for USB-IF Embedded Host Compliance Test
> 
> Given you have to configure the TPL somehow, how is this useful to be
> in 
> DT? And no, that's not a suggestion to put all the TPL config into
> DT.
It's indeed not flexible here, abandon this patch.

Thanks

> 
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 2
> > ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-
> > xhci.yaml
> > index 240882b12565..49729aab6d1a 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -115,6 +115,8 @@ properties:
> >  
> >    usb2-lpm-disable: true
> >  
> > +  tpl-support: true
> > +
> >    imod-interval-ns:
> >      description:
> >        Interrupt moderation interval value, it is 8 times as much
> > as that
> > -- 
> > 2.18.0
> > 
> >