Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Peter Chen <peter.chen@nxp.com>
To:     Pawel Laszczak <pawell@cadence.com>
CC:     "felipe.balbi@linux.intel.com" <felipe.balbi@linux.intel.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "rogerq@ti.com" <rogerq@ti.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "jbergsagel@ti.com" <jbergsagel@ti.com>,
        "nsekhar@ti.com" <nsekhar@ti.com>, "nm@ti.com" <nm@ti.com>,
        "sureshp@cadence.com" <sureshp@cadence.com>,
        "kurahul@cadence.com" <kurahul@cadence.com>
Subject: Re: [PATCH] usb: cdns3: Fix dequeue implementation.
Thread-Topic: [PATCH] usb: cdns3: Fix dequeue implementation.
Thread-Index: AQHVfQepqrRDdKh5qUCiZNqhRPzBa6dQW3eA
Date:   Tue, 8 Oct 2019 07:34:21 +0000
Message-ID: <20191008073405.GF5670@b29397-desktop>
References: <1570449978-19145-1-git-send-email-pawell@cadence.com>
In-Reply-To: <1570449978-19145-1-git-send-email-pawell@cadence.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C7D888988DD18440B60A93D08C607F49@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 61c58794-1754-4397-97e0-08d74bc1ef7f
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Oct 2019 07:34:21.4612
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jB6VAWTv7hZhlS2QdJl6/6Gh018TURgcdGUHpubTVsYfXsRu8yoL+j7i0++ZTUXc09afBWAY4ts30Ey2h1esBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5374
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 19-10-07 13:06:18, Pawel Laszczak wrote:
> Dequeuing implementation in cdns3_gadget_ep_dequeu gets first request fro=
m

%s/cdns3_gadget_ep_dequeu/cdns3_gadget_ep_dequeue

> deferred_req_list and changed TRB associated with it to LINK TRB.
> This approach is incorrect because deferred_req_list contains requests
> that have not been placed on hardware RING.  In this case driver should
> just giveback this request to gadget driver.
>=20
> The patch implements new approach that first checks where dequeuing
> request is located and only when it's on Transfer Ring then changes TRB
> associated with it to LINK TRB.
> During processing completed transfers such LINK TRB will be ignored.
>=20
> Reported-by: Peter Chen <peter.chen@nxp.com>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> ---
>  drivers/usb/cdns3/gadget.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>=20
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 2ca280f4c054..9050b380ab83 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -1145,6 +1145,14 @@ static void cdns3_transfer_completed(struct cdns3_=
device *priv_dev,
>  		request =3D cdns3_next_request(&priv_ep->pending_req_list);
>  		priv_req =3D to_cdns3_request(request);
> =20
> +		trb =3D priv_ep->trb_pool + priv_ep->dequeue;
> +
> +		/* Request was dequeued and TRB was changed to TRB_LINK. */
> +		if (TRB_FIELD_TO_TYPE(trb->control) =3D=3D TRB_LINK) {
> +			trace_cdns3_complete_trb(priv_ep, trb);
> +			cdns3_move_deq_to_next_trb(priv_req);
> +		}

If the request was dequeued, it should not be at request list, isn't it?

Peter
> +
>  		/* Re-select endpoint. It could be changed by other CPU during
>  		 * handling usb_gadget_giveback_request.
>  		 */
> @@ -2067,6 +2075,7 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>  	struct usb_request *req, *req_temp;
>  	struct cdns3_request *priv_req;
>  	struct cdns3_trb *link_trb;
> +	u8 req_on_hw_ring =3D 0;
>  	unsigned long flags;
>  	int ret =3D 0;
> =20
> @@ -2083,8 +2092,10 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
> =20
>  	list_for_each_entry_safe(req, req_temp, &priv_ep->pending_req_list,
>  				 list) {
> -		if (request =3D=3D req)
> +		if (request =3D=3D req) {
> +			req_on_hw_ring =3D 1;
>  			goto found;
> +		}
>  	}
> =20
>  	list_for_each_entry_safe(req, req_temp, &priv_ep->deferred_req_list,
> @@ -2096,27 +2107,21 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>  	goto not_found;
> =20
>  found:
> -
> -	if (priv_ep->wa1_trb =3D=3D priv_req->trb)
> -		cdns3_wa1_restore_cycle_bit(priv_ep);
> -
>  	link_trb =3D priv_req->trb;
> -	cdns3_move_deq_to_next_trb(priv_req);
> -	cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
> -
> -	/* Update ring */
> -	request =3D cdns3_next_request(&priv_ep->deferred_req_list);
> -	if (request) {
> -		priv_req =3D to_cdns3_request(request);
> =20
> +	/* Update ring only if removed request is on pending_req_list list */
> +	if (req_on_hw_ring) {
>  		link_trb->buffer =3D TRB_BUFFER(priv_ep->trb_pool_dma +
>  					      (priv_req->start_trb * TRB_SIZE));
>  		link_trb->control =3D (link_trb->control & TRB_CYCLE) |
> -				    TRB_TYPE(TRB_LINK) | TRB_CHAIN | TRB_TOGGLE;
> -	} else {
> -		priv_ep->flags |=3D EP_UPDATE_EP_TRBADDR;
> +				    TRB_TYPE(TRB_LINK) | TRB_CHAIN;
> +
> +		if (priv_ep->wa1_trb =3D=3D priv_req->trb)
> +			cdns3_wa1_restore_cycle_bit(priv_ep);
>  	}
> =20
> +	cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
> +
>  not_found:
>  	spin_unlock_irqrestore(&priv_dev->lock, flags);
>  	return ret;

--=20

Thanks,
Peter Chen=