RE: [PATCHv2 10/13] usb: dwc2: initialize the spin_lock for both host and gadget

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux