Hi Peter,
> -----Original Message-----
> From: Peter Chen <peter.chen@xxxxxxxxxx>
> Sent: Saturday, July 9, 2022 6:00 PM
> 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: Re: [EXT] Re: [PATCH] usb: chipidea: udc: check request status
> before setting device address
>
> Caution: EXT Email
>
> 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?
Yes, still not sure the root cause of this werid behavior. Will dig it out later.
>
> > or received other setup token rather IN token.
> That's possible. Have you found that at bus analyzer?
Not yet.
>
>
> > 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?
This patch has been received by Greg. Do I still need to send patch again?
Xu Yang
>
> 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
