> 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. > > thanks, > > greg k-h