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]>
---
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;
+
+ /* 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) {
--
2.18.0
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]>
---
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
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 <[email protected]>
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
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`, 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?
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 <[email protected]>
>
> 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: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
>
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 <[email protected]>
>
> 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