> From: Felipe Balbi <balbi@xxxxxxxxxx>
> Sent: Thursday, September 9, 2021 4:45 PM
> To: Linyu Yuan (QUIC) <quic_linyyuan@xxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-
> usb@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit
>
>
> "Linyu Yuan (QUIC)" <quic_linyyuan@xxxxxxxxxxx> writes:
>
> >> From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >> Sent: Thursday, September 9, 2021 3:38 PM
> >> To: Linyu Yuan (QUIC) <quic_linyyuan@xxxxxxxxxxx>
> >> Cc: Felipe Balbi <balbi@xxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit
> >>
> >> On Thu, Sep 09, 2021 at 07:17:57AM +0000, Linyu Yuan (QUIC) wrote:
> >> >
> >> >
> >> > > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >> > > Sent: Thursday, September 9, 2021 2:11 PM
> >> > > To: Linyu Yuan (QUIC) <quic_linyyuan@xxxxxxxxxxx>
> >> > > Cc: Felipe Balbi <balbi@xxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> >> > > Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit
> >> > >
> >> > > On Thu, Sep 09, 2021 at 01:45:47PM +0800, Linyu Yuan wrote:
> >> > > > change device release function to clear gadget pointer.
> >> > >
> >> > > That does not properly describe what and why this change is needed.
> >> > >
> >> > > >
> >> > > > Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
> >> > > > ---
> >> > > > drivers/usb/dwc3/gadget.c | 5 +++--
> >> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> >> > > >
> >> > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> > > > index 804b505..e2ab5f6 100644
> >> > > > --- a/drivers/usb/dwc3/gadget.c
> >> > > > +++ b/drivers/usb/dwc3/gadget.c
> >> > > > @@ -4188,9 +4188,10 @@ static int dwc3_gadget_get_irq(struct
> dwc3
> >> > > *dwc)
> >> > > >
> >> > > > static void dwc_gadget_release(struct device *dev)
> >> > > > {
> >> > > > - struct usb_gadget *gadget = container_of(dev, struct
> usb_gadget,
> >> > > dev);
> >> > > > + struct dwc3 *dwc = dev_get_platdata(dev);
> >> > >
> >> > > Are you sure this is the same?
> >> > Yes, in dwc3_gadget_init()
> >> > usb_initialize_gadget(dwc->dev, dwc->gadget, dwc_gadget_release);
> >> > dev = &dwc->gadget->dev;
> >> > dev->platform_data = dwc;
> >> >
> >> > here original code use follow container_of, it use same dev,
> >> > container_of(dev, struct usb_gadget, dev);
> >> > >
> >> > > >
> >> > > > - kfree(gadget);
> >> > > > + kfree(dwc->gadget);
> >> > > > + dwc->gadget = NULL;
> >> > >
> >> > > Why set this to NULL? Who cares about this now? What changed to
> >> make
> >> > > it required?
> >> > It better to set to NULL for better understanding.
> >>
> >> Understanding of what? What issue does this fix? You need a reason to
> >> make this, or any, kernel change.
> >
> > Sorry, let explain, for example, when do role switch, we can check it
> > value to make sure it switch complete,
> >
> > If we do not set to NULL, it will be invalid.
>
> Using this pointer as a role switch flag seems fragile, though.
Hi Felipe,
I update a v2 patch, no, I didn't use this pointer for role switch, just used to check role switch status, make sure it complete.
when do role switch from device to host mode,
first disable device mode, currently there is no better way to
confirm when gadget was complete removed.
change device release function to clear gadget pointer,
this will confirm device stop complete when it is NULL.
>
> --
> balbi
