On 22-07-04 01:59:45, Xu Yang wrote:
> 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
You mean even software clear the OP_ENDPTSETUPSTAT bits, but controller
itself doesn't clear its interrupt, and trigger the same
interrupt handler again?
> or received other setup token rather IN token.
That's possible. Have you found that at bus analyzer?
> 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);
>
Would you please add your above description and send patch again?
Peter
> 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
>
--
Thanks,
Peter Chen
