2023-04-17 02:52:55

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 1/7] usb: mtu3: give back request when rx error happens

When the Rx enconnter errors, currently, only print error logs, that
may cause class driver's RX halt, shall give back the request with
error status meanwhile.

Signed-off-by: Chunfeng Yun <[email protected]>
---
v3: no changes
v2: remove @req suggested by AngeloGioacchino
---
drivers/usb/mtu3/mtu3_qmu.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_qmu.c b/drivers/usb/mtu3/mtu3_qmu.c
index a2fdab8b63b2..a4da1af0b2c0 100644
--- a/drivers/usb/mtu3/mtu3_qmu.c
+++ b/drivers/usb/mtu3/mtu3_qmu.c
@@ -466,6 +466,37 @@ static void qmu_tx_zlp_error_handler(struct mtu3 *mtu, u8 epnum)
mtu3_qmu_resume(mep);
}

+/*
+ * when rx error happens (except zlperr), QMU will stop, and RQCPR saves
+ * the GPD encountered error, Done irq will arise after resuming QMU again.
+ */
+static void qmu_error_rx(struct mtu3 *mtu, u8 epnum)
+{
+ struct mtu3_ep *mep = mtu->out_eps + epnum;
+ struct mtu3_gpd_ring *ring = &mep->gpd_ring;
+ struct qmu_gpd *gpd_current = NULL;
+ struct mtu3_request *mreq;
+ dma_addr_t cur_gpd_dma;
+
+ cur_gpd_dma = read_rxq_cur_addr(mtu->mac_base, epnum);
+ gpd_current = gpd_dma_to_virt(ring, cur_gpd_dma);
+
+ mreq = next_request(mep);
+ if (!mreq || mreq->gpd != gpd_current) {
+ dev_err(mtu->dev, "no correct RX req is found\n");
+ return;
+ }
+
+ mreq->request.status = -EAGAIN;
+
+ /* by pass the current GDP */
+ gpd_current->dw0_info |= cpu_to_le32(GPD_FLAGS_BPS | GPD_FLAGS_HWO);
+ mtu3_qmu_resume(mep);
+
+ dev_dbg(mtu->dev, "%s EP%d, current=%p, req=%p\n",
+ __func__, epnum, gpd_current, mreq);
+}
+
/*
* NOTE: request list maybe is already empty as following case:
* queue_tx --> qmu_interrupt(clear interrupt pending, schedule tasklet)-->
@@ -571,14 +602,18 @@ static void qmu_exception_isr(struct mtu3 *mtu, u32 qmu_status)

if ((qmu_status & RXQ_CSERR_INT) || (qmu_status & RXQ_LENERR_INT)) {
errval = mtu3_readl(mbase, U3D_RQERRIR0);
+ mtu3_writel(mbase, U3D_RQERRIR0, errval);
+
for (i = 1; i < mtu->num_eps; i++) {
if (errval & QMU_RX_CS_ERR(i))
dev_err(mtu->dev, "Rx %d CS error!\n", i);

if (errval & QMU_RX_LEN_ERR(i))
dev_err(mtu->dev, "RX %d Length error\n", i);
+
+ if (errval & (QMU_RX_CS_ERR(i) | QMU_RX_LEN_ERR(i)))
+ qmu_error_rx(mtu, i);
}
- mtu3_writel(mbase, U3D_RQERRIR0, errval);
}

if (qmu_status & RXQ_ZLPERR_INT) {
--
2.18.0


2023-04-17 02:53:11

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 4/7] usb: mtu3: unlock @mtu->lock just before giving back request

No need unlock @mtu->lock when unmap request, unlock it just before
giving back request, due to we do not lock this spinlock when map
the request.

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
v3: no changes
v2: add Reviewed-by AngeloGioacchino
---
drivers/usb/mtu3/mtu3_gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
index c0264d5426bf..ad0eeac4332d 100644
--- a/drivers/usb/mtu3/mtu3_gadget.c
+++ b/drivers/usb/mtu3/mtu3_gadget.c
@@ -23,7 +23,6 @@ __acquires(mep->mtu->lock)
req->status = status;

trace_mtu3_req_complete(mreq);
- spin_unlock(&mtu->lock);

/* ep0 makes use of PIO, needn't unmap it */
if (mep->epnum)
@@ -32,6 +31,7 @@ __acquires(mep->mtu->lock)
dev_dbg(mtu->dev, "%s complete req: %p, sts %d, %d/%d\n",
mep->name, req, req->status, req->actual, req->length);

+ spin_unlock(&mtu->lock);
usb_gadget_giveback_request(&mep->ep, req);
spin_lock(&mtu->lock);
}
--
2.18.0

2023-04-17 02:53:27

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 3/7] usb: mtu3: fix kernel panic at qmu transfer done irq handler

When handle qmu transfer irq, it will unlock @mtu->lock before give back
request, if another thread handle disconnect event at the same time, and
try to disable ep, it may lock @mtu->lock and free qmu ring, then qmu
irq hanlder may get a NULL gpd, avoid the KE by checking gpd's value before
handling it.

e.g.
qmu done irq on cpu0 thread running on cpu1

qmu_done_tx()
handle gpd [0]
mtu3_requ_complete() mtu3_gadget_ep_disable()
unlock @mtu->lock
give back request lock @mtu->lock
mtu3_ep_disable()
mtu3_gpd_ring_free()
unlock @mtu->lock
lock @mtu->lock
get next gpd [1]

[1]: goto [0] to handle next gpd, and next gpd may be NULL.

Fixes: 48e0d3735aa5 ("usb: mtu3: supports new QMU format")
Signed-off-by: Chunfeng Yun <[email protected]>
---
v3: add Fixes tag suggested by AngeloGioacchino
v2: fix typo suggested by AngeloGioacchino
---
drivers/usb/mtu3/mtu3_qmu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_qmu.c b/drivers/usb/mtu3/mtu3_qmu.c
index 6be4977a5db5..3d77408e3133 100644
--- a/drivers/usb/mtu3/mtu3_qmu.c
+++ b/drivers/usb/mtu3/mtu3_qmu.c
@@ -210,6 +210,7 @@ static struct qmu_gpd *advance_enq_gpd(struct mtu3_gpd_ring *ring)
return ring->enqueue;
}

+/* @dequeue may be NULL if ring is unallocated or freed */
static struct qmu_gpd *advance_deq_gpd(struct mtu3_gpd_ring *ring)
{
if (ring->dequeue < ring->end)
@@ -522,7 +523,7 @@ static void qmu_done_tx(struct mtu3 *mtu, u8 epnum)
dev_dbg(mtu->dev, "%s EP%d, last=%p, current=%p, enq=%p\n",
__func__, epnum, gpd, gpd_current, ring->enqueue);

- while (gpd != gpd_current && !GET_GPD_HWO(gpd)) {
+ while (gpd && gpd != gpd_current && !GET_GPD_HWO(gpd)) {

mreq = next_request(mep);

@@ -561,7 +562,7 @@ static void qmu_done_rx(struct mtu3 *mtu, u8 epnum)
dev_dbg(mtu->dev, "%s EP%d, last=%p, current=%p, enq=%p\n",
__func__, epnum, gpd, gpd_current, ring->enqueue);

- while (gpd != gpd_current && !GET_GPD_HWO(gpd)) {
+ while (gpd && gpd != gpd_current && !GET_GPD_HWO(gpd)) {

mreq = next_request(mep);

--
2.18.0

2023-04-17 02:53:39

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 2/7] usb: mtu3: use boolean return value

Prefer to use boolean value due to gpd_ring_empty() return true or false.
See "16) Function return values and names" in coding-style.rst

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
v3: no changes
v2: add Reviewed-by AngeloGioacchino
---
drivers/usb/mtu3/mtu3_qmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_qmu.c b/drivers/usb/mtu3/mtu3_qmu.c
index a4da1af0b2c0..6be4977a5db5 100644
--- a/drivers/usb/mtu3/mtu3_qmu.c
+++ b/drivers/usb/mtu3/mtu3_qmu.c
@@ -221,7 +221,7 @@ static struct qmu_gpd *advance_deq_gpd(struct mtu3_gpd_ring *ring)
}

/* check if a ring is emtpy */
-static int gpd_ring_empty(struct mtu3_gpd_ring *ring)
+static bool gpd_ring_empty(struct mtu3_gpd_ring *ring)
{
struct qmu_gpd *enq = ring->enqueue;
struct qmu_gpd *next;
--
2.18.0

2023-04-17 02:53:41

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 6/7] dt-bindings: usb: mtu3: add two optional clocks

Add optional clock 'xhci_ck' and 'frmcnt_ck';

Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
v3: add reviewed-by Rob
v2: remove assigned-clocks* properties suggested by Rob
---
Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml
index d2655173e108..3d403d944453 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtu3.yaml
@@ -66,6 +66,8 @@ properties:
- description: Reference clock used by low power mode etc
- description: Mcu bus clock for register access
- description: DMA bus clock for data transfer
+ - description: DRD controller clock
+ - description: Frame count clock

clock-names:
minItems: 1
@@ -74,6 +76,8 @@ properties:
- const: ref_ck
- const: mcu_ck
- const: dma_ck
+ - const: xhci_ck
+ - const: frmcnt_ck

phys:
description:
--
2.18.0

2023-04-17 02:54:29

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v3 7/7] usb: mtu3: add optional clock xhci_ck and frmcnt_ck

Add optional clock 'xhci_ck' which is usually the same as sys_ck, but
some SoC use two separated clocks when the controller supports dual
role mode;
Add optional clock 'frmcnt_ck' used on 4nm or advanced process SoC.

Signed-off-by: Chunfeng Yun <[email protected]>
---
v2~v3: no changes
---
drivers/usb/mtu3/mtu3.h | 2 +-
drivers/usb/mtu3/mtu3_plat.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 2d7b57e07eee..b4a7662dded5 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -90,7 +90,7 @@ struct mtu3_request;
*/
#define EP0_RESPONSE_BUF 6

-#define BULK_CLKS_CNT 4
+#define BULK_CLKS_CNT 6

/* device operated link and speed got from DEVICE_CONF register */
enum mtu3_speed {
diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index d78ae52b4e26..6f264b129243 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -234,6 +234,8 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
clks[1].id = "ref_ck";
clks[2].id = "mcu_ck";
clks[3].id = "dma_ck";
+ clks[4].id = "xhci_ck";
+ clks[5].id = "frmcnt_ck";
ret = devm_clk_bulk_get_optional(dev, BULK_CLKS_CNT, clks);
if (ret)
return ret;
--
2.18.0