Hi Peter,
> -----Original Message-----
> From: Peter Chen <peter.chen@xxxxxxxxxx>
> Sent: Sunday, July 3, 2022 8:42 AM
> To: Xu Yang <xu.yang_2@xxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Jun Li
> <jun.li@xxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: [EXT] Re: [PATCH] usb: chipidea: udc: check request status before
> setting device address
>
> Caution: EXT Email
>
> On 22-06-23 11:02:42, Xu Yang wrote:
> > The complete() function may be called even though request is not
> > completed.
>
> Would you please explain more when the complete() is called but the
> request has not completed, and this happens before set_address has
> finished?
The following sequence will be a case:
1. the HOST sent SET_ADDRESS control message to DEVICE.
2. the DEVICE acked this message and queued a usb_request (0 data length) for future IN transfer.
3. somehow the setup interrupt is not cleared by USB controller or received other setup token rather IN token.
4. the driver will go through below functions:
isr_setup_packet_handler(ci);
_ep_nuke(ci->ep0in);
usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
req->complete(ep, req);
isr_setup_status_complete();
hw_usb_set_address(ci, ci->address);
usb_gadget_set_state(&ci->gadget, USB_STATE_ADDRESS);
Xu Yang
>
> Peter
>
> > In this case, it's necessary to check request status so as not to set
> > device address wrongly.
> >
> > Fixes: 10775eb17bee ("usb: chipidea: udc: update gadget states
> > according to ch9")
> > cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > ---
> > drivers/usb/chipidea/udc.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 0c9ae9768a67..8c3e3a635ac2 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1048,6 +1048,9 @@ isr_setup_status_complete(struct usb_ep *ep,
> struct usb_request *req)
> > struct ci_hdrc *ci = req->context;
> > unsigned long flags;
> >
> > + if (req->status < 0)
> > + return;
> > +
> > if (ci->setaddr) {
> > hw_usb_set_address(ci, ci->address);
> > ci->setaddr = false;
> > --
> > 2.25.1
> >
>
> --
>
> Thanks,
> Peter Chen
