RE: [PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

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

 



(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




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

  Powered by Linux