> From: Dinh Nguyen [mailto:dinh.linux@xxxxxxxxx] > Sent: Monday, August 25, 2014 3:58 AM > > On 8/22/14, 3:57 PM, Felipe Balbi wrote: > > Hi, > > > > On Fri, Aug 22, 2014 at 08:52:23PM +0000, Paul Zimmerman wrote: > >>> From: dinguyen@xxxxxxxxxx [mailto:dinguyen@xxxxxxxxxx] > >>> Sent: Wednesday, July 30, 2014 8:21 AM > >>> > >>> Move spin_lock_init to common location for both host and gadget. > >>> > >>> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxx> > >>> --- > >>> drivers/usb/dwc2/hcd.c | 1 - > >>> drivers/usb/dwc2/platform.c | 1 + > >>> 2 files changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > >>> index 07a7bcd..c6778d9 100644 > >>> --- a/drivers/usb/dwc2/hcd.c > >>> +++ b/drivers/usb/dwc2/hcd.c > >>> @@ -2824,7 +2824,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, > >>> > >>> hcd->has_tt = 1; > >>> > >>> - spin_lock_init(&hsotg->lock); > >>> ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg; > >>> hsotg->priv = hcd; > >>> > >>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > >>> index eb2a131..4898268 100644 > >>> --- a/drivers/usb/dwc2/platform.c > >>> +++ b/drivers/usb/dwc2/platform.c > >>> @@ -197,6 +197,7 @@ static int dwc2_driver_probe(struct platform_device *dev) > >>> } > >>> > >>> platform_set_drvdata(dev, hsotg); > >>> + spin_lock_init(&hsotg->lock); > >>> > >>> return retval; > >>> } > >> > >> Hi Dinh, > >> > >> I don't have a copy of your v3 patches in my mailbox anymore, so I am > >> replying to the v2 one instead. > >> > >> Are you absolutely sure that no code that takes the spinlock can be > >> called before this point? This is the last line in the probe() > >> function, so I have a hard time believing it is safe to initialize > >> the spinlock this late. > >> > >> In particular, the IRQ has already been attached, and > >> usb_add_gadget_udc() has already been called. So it seems entirely > >> possible that some other entity could try to access the driver before > >> this point. > > > > you're right with this comment. request_irq() enables the IRQ line and > > it could be that we already have a pending event to handle which fires > > as soon as we enable that IRQ line. > > > > Yes, thanks for catching this. I will move the call up in v4. OK, I think that was the last issue I had with the series. So you can add my acked-by to any other patches in the series that I haven't acked yet. Except I want to see the changes for spin_lock_init() before I ack those. And again, thanks for all your work on this. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
