Add property 'tpl-support' to support targeted peripheral list
for USB-IF Embedded Host Compliance Test
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
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 <[email protected]>
---
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
Add support property 'tpl-support' for targeted peripheral list
(USB-IF Embedded Host Compliance Plan)
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
Add a member num_esit to save the number of esit, then no need
caculate it in some functions.
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
check_sch_tt() will access fs_bus_bw[] array, check boundary
firstly to avoid out-of-bounds issue.
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
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: <[email protected]>
Reported-by: Stan Lu <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Hi Chunfeng,
On Fri, Jul 30, 2021 at 4:51 PM Chunfeng Yun <[email protected]> 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 <[email protected]>
> ---
> 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
>
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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
Hi,
On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
> > >
>
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 <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
> ---
> 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
>
>
Hi Chunfeng,
On Fri, Jul 30, 2021 at 4:50 PM Chunfeng Yun <[email protected]> wrote:
>
> check_sch_tt() will access fs_bus_bw[] array, check boundary
> firstly to avoid out-of-bounds issue.
>
> Signed-off-by: Chunfeng Yun <[email protected]>
> ---
> 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
>
On Wed, 2021-08-04 at 22:06 +0800, Ikjoon Jang wrote:
> Hi,
>
> On Wed, Aug 4, 2021 at 1:19 PM Chunfeng Yun <
> [email protected]> 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 <
> > > [email protected]> 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 <[email protected]>
> > > > ---
> > > > 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
> > > >
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 <[email protected]>
> > ---
> > 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
> >
> >
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 <[email protected]>
> > ---
> > 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
> >
> >
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 <
> [email protected]> wrote:
> >
> > check_sch_tt() will access fs_bus_bw[] array, check boundary
> > firstly to avoid out-of-bounds issue.
> >
> > Signed-off-by: Chunfeng Yun <[email protected]>
> > ---
> > 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
> >
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 <[email protected]>
> > ---
> > 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
> >
> >
On Fri, Jul 30, 2021 at 4:50 PM Chunfeng Yun <[email protected]> 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 <[email protected]>
> Signed-off-by: Chunfeng Yun <[email protected]>
Reviewed-and-Tested-by: Ikjoon Jang <[email protected]>
> ---
> 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
>
On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)
<[email protected]> 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
On Wed, 2021-08-18 at 09:20 -0500, Rob Herring wrote:
> On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)
> <[email protected]> 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