| Message ID | 20230403025230.25035-1-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/7] usb: mtu3: give back request when rx error happens | expand |
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. > --- > 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. > + > + /* 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. Regards, Angelo
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
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 --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) {
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(-)