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@xxxxxxxxxxxx>
>
> 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
