Message ID | cover.1609865348.git.Thinh.Nguyen@synopsys.com |
---|---|
Headers | show |
Series | usb: dwc3: gadget: Check for multiple start/stop | expand |
Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: > If the gadget had already stopped, don't try to stop again. Otherwise > we'd get a warning for trying to free an already freed irq. This can > happen if a user tries to trigger a soft-disconnect from soft_connect > sysfs multiple times. The fix is to check if there's a bounded gadget > driver to determined if the gadget had stopped. > > Cc: stable@vger.kernel.org > Fixes: 8698e2acf3a5 ("usb: dwc3: gadget: introduce and use enable/disable irq methods") > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > drivers/usb/dwc3/gadget.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 51d81a32ce78..9ec70282e610 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2338,6 +2338,10 @@ static int dwc3_gadget_stop(struct usb_gadget *g) > struct dwc3 *dwc = gadget_to_dwc(g); > unsigned long flags; > > + /* The controller is already stopped if there's no gadget driver */ > + if (!dwc->gadget_driver) > + return 0; same here. Better done at the framework -- balbi
On 21-01-05 08:56:28, Thinh Nguyen wrote: > Add some checks to avoid going through the start/stop sequence if the gadget > had already started/stopped. This series base-commit is Greg's usb-linus > branch. > Hi Thinh, What's the sequence your could reproduce it? Peter > > > Thinh Nguyen (2): > usb: dwc3: gadget: Check if the gadget had started > usb: dwc3: gadget: Check if the gadget had stopped > > drivers/usb/dwc3/gadget.c | 28 ++++++++++++---------------- > 1 file changed, 12 insertions(+), 16 deletions(-) > > > base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482 > -- > 2.28.0 > -- Thanks, Peter Chen
Hi Peter, Peter Chen wrote: > On 21-01-05 08:56:28, Thinh Nguyen wrote: >> Add some checks to avoid going through the start/stop sequence if the gadget >> had already started/stopped. This series base-commit is Greg's usb-linus >> branch. >> > Hi Thinh, > > What's the sequence your could reproduce it? > > Peter You can test as follow: # echo connect > /sys/class/udc/<UDC>/soft_connect # echo connect > /sys/class/udc/<UDC>/soft_connect and # echo disconnect > /sys/class/udc/<UDC>/soft_connect # echo disconnect > /sys/class/udc/<UDC>/soft_connect Thinh >> >> Thinh Nguyen (2): >> usb: dwc3: gadget: Check if the gadget had started >> usb: dwc3: gadget: Check if the gadget had stopped >> >> drivers/usb/dwc3/gadget.c | 28 ++++++++++++---------------- >> 1 file changed, 12 insertions(+), 16 deletions(-) >> >> >> base-commit: 96ebc9c871d8a28fb22aa758dd9188a4732df482 >> -- >> 2.28.0 >>
On 21-01-08 02:40:55, Thinh Nguyen wrote: > Hi Peter, > > Peter Chen wrote: > > On 21-01-05 08:56:28, Thinh Nguyen wrote: > >> Add some checks to avoid going through the start/stop sequence if the gadget > >> had already started/stopped. This series base-commit is Greg's usb-linus > >> branch. > >> > > Hi Thinh, > > > > What's the sequence your could reproduce it? > > > > Peter > > You can test as follow: > > # echo connect > /sys/class/udc/<UDC>/soft_connect > # echo connect > /sys/class/udc/<UDC>/soft_connect > > and > > # echo disconnect > /sys/class/udc/<UDC>/soft_connect > # echo disconnect > /sys/class/udc/<UDC>/soft_connect > > Thinh > Thanks, now I reproduce the issue. Another improvement you might consider adding is checking return value for usb_gadget_udc_start at soft_connect_store. -- Thanks, Peter Chen