On Wed, Oct 08, 2014 at 03:33:28PM +0300, Octavian Purdila wrote:

On Wed, Oct 8, 2014 at 3:04 PM, Johan Hovold [off-list ref] wrote:

quoted

On Wed, Oct 08, 2014 at 01:54:07PM +0300, Octavian Purdila wrote:

quoted

On Wed, Oct 8, 2014 at 12:23 PM, Johan Hovold [off-list ref] wrote:

quoted

On Thu, Sep 25, 2014 at 07:07:31PM +0300, Octavian Purdila wrote:

quoted

+static void dln2_rx_transfer(struct dln2_dev *dln2, struct urb *urb,
+                          u16 handle, u16 rx_slot)
+{
+     struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
+     struct dln2_rx_context *rxc;
+     struct device *dev = &dln2->interface->dev;
+
+     spin_lock(&rxs->lock);

You must use spin_lock_irqsave here as you call it from the completion
handler.

Why? AFAICS the completion handler gets called from the HCD irq handler:

The completion handler is currently called with local interrupts
disabled but that is about to change once all drivers have been updated:

        http://marc.info/?l=linux-usb&m=137353360511003&w=2

In this case you could probably get away with not disabling interrupts
anyway, but using the irqsave versions would make it obvious.

I was not assuming that interrupts are disabled while running the
completion handler. Since that spinlock is not touched by any other
interrupt context code I don't think irqsave is necessary.

No, it isn't necessary so leave it as it is.

But you are exporting interfaces to other drivers and it may appear to
someone that those could possibly end up indirectly calling a function
taking that lock in IRQ context. We know that isn't the case now, but I
bet someone will post conversion patch for that spinlock at some point.
;)

Johan