Hi Minas, On Fri, Oct 26, 2018 at 6:59 PM Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx> wrote: > Commit: dccf1bad4be7eaa096c1f3697bd37883f9a08ecb > Parent: 4c19cc14064d99ef0a20fb5ba0d45c94dbedb13c > Refname: refs/heads/master > Web: https://git.kernel.org/torvalds/c/dccf1bad4be7eaa096c1f3697bd37883f9a08ecb > Author: Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx> > AuthorDate: Wed Sep 19 18:13:52 2018 +0400 > Committer: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> > CommitDate: Tue Oct 2 10:33:15 2018 +0300 > > usb: dwc2: Disable all EP's on disconnect > > Disabling all EP's allow to reset EP's to initial state. > On disconnect disable all EP's instead of just killing > all requests. Because of some platform didn't catch > disconnect event, same stuff added to > dwc2_hsotg_core_init_disconnected() function when USB > reset detected on the bus. > > Changed from version 1: > Changed lock acquire flow in dwc2_hsotg_ep_disable() > function. Dan had some comments on v1, and requested to document why the locking games you're playing are safe, cfr. https://lists.01.org/pipermail/kbuild/2018-September/002945.html Unfortunately you didn't CC him for v2... > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -3993,6 +4004,7 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep) > unsigned long flags; > u32 epctrl_reg; > u32 ctrl; > + int locked; > > dev_dbg(hsotg->dev, "%s(ep %p)\n", __func__, ep); > > @@ -4008,7 +4020,9 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep) > > epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index); > > - spin_lock_irqsave(&hsotg->lock, flags); > + locked = spin_is_locked(&hsotg->lock); > + if (!locked) > + spin_lock_irqsave(&hsotg->lock, flags); This looks really fishy to me. What if another CPU takes the lock in between? Can you please explain, and fix if necessary? Thanks! > > ctrl = dwc2_readl(hsotg, epctrl_reg); > > @@ -4032,7 +4046,9 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep) > hs_ep->fifo_index = 0; > hs_ep->fifo_size = 0; > > - spin_unlock_irqrestore(&hsotg->lock, flags); > + if (!locked) > + spin_unlock_irqrestore(&hsotg->lock, flags); > + > return 0; > } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
