(sorry for html reply) ________________________________________ From: Paul Zimmerman [Paul.Zimmerman@xxxxxxxxxxxx] Sent: Friday, December 06, 2013 5:23 PM To: Shilimkar, Santosh; Balbi, Felipe; Kwok, WingMan Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Greg Kroah-Hartman Subject: RE: [PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Santosh Shilimkar > Sent: Friday, December 06, 2013 1:40 PM > > On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote: > > Hi, > > > > On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote: > >> Add Keystone platform specific glue layer to support > >> USB3 Host mode. > >> > >> Cc: Santosh Shilimkar <santosh.shilimkar@xxxxxx> > >> Cc: Felipe Balbi <balbi@xxxxxx> > >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > >> Signed-off-by: WingMan Kwok <w-kwok2@xxxxxx> > >> --- > >> drivers/usb/dwc3/Kconfig | 7 ++ > >> drivers/usb/dwc3/Makefile | 1 + > >> drivers/usb/dwc3/dwc3-keystone.c | 210 ++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 218 insertions(+) > >> create mode 100644 drivers/usb/dwc3/dwc3-keystone.c > > [..] > > >> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc) > >> +{ > >> + struct dwc3_keystone *kdwc = _kdwc; > >> + > >> + spin_lock(&kdwc->lock); > > > > Isn't this lock unnecessary ? This handler will run with IRQs disabled > > anyway. > > > Indeed. ------------------------------- Say what? AFAIK it's necessary to take the driver lock inside the interrupt handler, because of SMP. Here is the equivalent routine from dwc3-omap.c: static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) { struct dwc3_omap *omap = _omap; u32 reg; spin_lock(&omap->lock); ...-- Paul } Has this now become unnecessary? ---------------- [Santosh] Not sure if i am missing your point, but this lock is not needed IMHO. The register update which is protected by the lock happens only in ISR where the source of the irq is disabled, so even on SMP machines there is no race possible which needs to be protected. I would have agreed to you if there was some additional code outside isr and the code in ISR were both needed lock and that would have been issue on SMP machine without spin lock. On OMAP code as well the lock is not needed but I let Felipe comment about it. Regards, Santosh-- 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
