On Thu, Sep 09, 2021 at 08:02:19AM +0000, Linyu Yuan (QUIC) wrote:
> > 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.
Are you checking that already? If not, there is no need for this.
Please see the kernel documentation for how to write a changelog text,
otherwise we have no idea why you are wanting to make a change.
Please fix this all up when resubmitting the patch.
thanks,
greg k-h
