linux-kernel - Re: [PATCH 1/7] usb: mtu3: give back request when rx error happens

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6626cb88-9d9e-593f-f4c9-44c3035bad43@collabora.com>
Date:   Mon, 3 Apr 2023 14:31:44 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        linux-usb@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Eddie Hung <eddie.hung@...iatek.com>,
        Tianping Fang <tianping.fang@...iatek.com>
Subject: Re: [PATCH 1/7] usb: mtu3: give back request when rx error happens

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@...iatek.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