diff mbox series

[1/7] usb: mtu3: give back request when rx error happens

Message ID 20230403025230.25035-1-chunfeng.yun@mediatek.com
State Superseded
Headers show
Series [1/7] usb: mtu3: give back request when rx error happens | expand

Commit Message

Chunfeng Yun (云春峰) April 3, 2023, 2:52 a.m. UTC
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 <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_qmu.c | 39 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

Comments

Chunfeng Yun (云春峰) April 7, 2023, 8:12 a.m. UTC | #1
On Mon, 2023-04-03 at 14:34 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 03/04/23 04:52, Chunfeng Yun ha scritto:
> > 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.
> 
> This needs more details, because from what it seems, `xhci_ck` could
> be
> a children of `sys_ck`,
No, it's not child of 'sys_ck', they are all the 125Mhz clocks.

> in which case there would be no need to add that
> to this driver, as it'd be handled by the clock API instead.
> 
> What SoC is this for?
encounter the issue on mt8195

> 
> Regards,
> Angelo
>
Chunfeng Yun (云春峰) April 17, 2023, 1:07 a.m. UTC | #2
On Mon, 2023-04-03 at 14:31 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 03/04/23 04:52, Chunfeng Yun ha scritto:
> > 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 <chunfeng.yun@mediatek.com>
> 
>  From what I understand, this is not a new feature, but a fix for a
> unwanted QMU
> halt.
> This means that this commit needs a Fixes tag.
I did not take into account this cases when write this driver, it
caused the issue by the bug of host driver.

> 
> > ---
> >   drivers/usb/mtu3/mtu3_qmu.c | 39
> > ++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 38 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/mtu3/mtu3_qmu.c
> > b/drivers/usb/mtu3/mtu3_qmu.c
> > index a2fdab8b63b2..7be4e4be1a6a 100644
> > --- a/drivers/usb/mtu3/mtu3_qmu.c
> > +++ b/drivers/usb/mtu3/mtu3_qmu.c
> > @@ -466,6 +466,39 @@ 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 usb_request *req = 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;
> > +     }
> > +
> > +     req = &mreq->request;
> > +     req->status = -EAGAIN;
> 
> You don't need a *req pointer for just one simple assignment.
> 
>         mreq->request.status = -EAGAIN;
> 
> that'll do.
That's good, I'll modify it, thanks
> 
> > +
> > +     /* 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 +604,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);
> 
> Please mention in the commit description the reason why you're moving
> this register
> write here.
It clears irq status before handling the error;

> 
> Regards,
> Angelo
diff mbox series

Patch

diff --git a/drivers/usb/mtu3/mtu3_qmu.c b/drivers/usb/mtu3/mtu3_qmu.c
index a2fdab8b63b2..7be4e4be1a6a 100644
--- a/drivers/usb/mtu3/mtu3_qmu.c
+++ b/drivers/usb/mtu3/mtu3_qmu.c
@@ -466,6 +466,39 @@  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 usb_request *req = 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;
+	}
+
+	req = &mreq->request;
+	req->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 +604,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) {