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>,
        Suresh Punnoose <sureshp@cadence.com>,
        Rahul Kumar <kurahul@cadence.com>
Subject: Re: [PATCH] usb: cdns3: Fix dequeue implementation.
Thread-Topic: [PATCH] usb: cdns3: Fix dequeue implementation.
Thread-Index: AQHVfQepqrRDdKh5qUCiZNqhRPzBa6dQW3eAgAAHm4CAATY8gA==
Date:   Wed, 9 Oct 2019 02:31:57 +0000
Message-ID: <20191009023141.GJ5670@b29397-desktop>
References: <1570449978-19145-1-git-send-email-pawell@cadence.com>
 <20191008073405.GF5670@b29397-desktop>
 <BN7PR07MB4705E02D18AE939D16BDDBC0DD9A0@BN7PR07MB4705.namprd07.prod.outlook.com>
In-Reply-To: <BN7PR07MB4705E02D18AE939D16BDDBC0DD9A0@BN7PR07MB4705.namprd07.prod.outlook.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: <E954E9D3A050454A8092B538DB911804@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b39ac17-8e79-4e57-6735-08d74c60db43
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Oct 2019 02:31:57.4963
 (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: fkppQ784euqlHpBolN+6/kx+/NKDbDDx+mC6/v+iVZeF+S4lTFVyIKgmaqcn8iS0w3LjaJWGxaGhpWd4ToB/XA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB3198
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-08 08:01:19, Pawel Laszczak wrote:
> >>
> >> 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 cdn=
s3_device *priv_dev,
> >>  		request =3D cdns3_next_request(&priv_ep->pending_req_list);
> >>  		priv_req =3D to_cdns3_request(request);
> >>
> >> +		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?
>=20
> Yes
> The dequeued request was removed from pending list but TRB associated wit=
h it=20
> was changed to Link TRB so it should be ignored in cdns3_transfer_complet=
ed function.=20
>=20

I see, you are right. Except for the typo, otherwise:

Reviewed-by: Peter Chen <peter.chen@nxp.com>

Peter


> Pawel
>=20
> >
> >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;
> >>
> >> @@ -2083,8 +2092,10 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
> >>
> >>  	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;
> >> +		}
> >>  	}
> >>
> >>  	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;
> >>
> >>  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);
> >>
> >> +	/* 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);
> >>  	}
> >>
> >> +	cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
> >> +
> >>  not_found:
> >>  	spin_unlock_irqrestore(&priv_dev->lock, flags);
> >>  	return ret;
> >
> >--
> >
> >Thanks,
> >Peter Chen

--=20

Thanks,
Peter Chen=