On Mon, Mar 25, 2013 at 01:33:25PM +0200, Luciano Coelho wrote: > On Mon, 2013-03-25 at 13:11 +0200, Felipe Balbi wrote: > > On Mon, Mar 25, 2013 at 12:53:44PM +0200, Luciano Coelho wrote: > > > Spin locks and completions are expensive in hard IRQ context and cause > > > problems with RT kernels. In RT kernels, both spin locks and > > > completions can schedule(), so we can't use them in hard irq context. > > > > > > Move handling code into the irq thread function to avoid that. > > > > > > Reported-by: Gregoire Gentil <gregoire@xxxxxxxxxxxxxxxxxxxx> > > > Signed-off-by: Luciano Coelho <coelho@xxxxxx> > > > --- > > > drivers/net/wireless/ti/wlcore/main.c | 53 +++++++++++++-------------------- > > > 1 file changed, 21 insertions(+), 32 deletions(-) > > > > > > diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c > > > index 248daa9..c2730a7 100644 > > > --- a/drivers/net/wireless/ti/wlcore/main.c > > > +++ b/drivers/net/wireless/ti/wlcore/main.c > > > @@ -651,6 +651,25 @@ static irqreturn_t wlcore_irq(int irq, void *cookie) > > > unsigned long flags; > > > struct wl1271 *wl = cookie; > > > > > > + /* complete the ELP completion */ > > > + spin_lock_irqsave(&wl->wl_lock, flags); > > > + set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags); > > > + if (wl->elp_compl) { > > > + complete(wl->elp_compl); > > > + wl->elp_compl = NULL; > > > + } > > > + > > > + if (test_bit(WL1271_FLAG_SUSPENDED, &wl->flags)) { > > > + /* don't enqueue a work right now. mark it as pending */ > > > + set_bit(WL1271_FLAG_PENDING_WORK, &wl->flags); > > > + wl1271_debug(DEBUG_IRQ, "should not enqueue work"); > > > + disable_irq_nosync(wl->irq); > > > + pm_wakeup_event(wl->dev, 0); > > > + spin_unlock_irqrestore(&wl->wl_lock, flags); > > > + return IRQ_HANDLED; > > > + } > > > + spin_unlock_irqrestore(&wl->wl_lock, flags); > > > > I still think _irqrestore() here is wrong, since it will reenable the > > IRQ line... other than that, it looks alright. > > As we discussed earlier, it won't re-enable, since it was not enabled > when we saved. But, in any case, as we agreed, I'll send a separate > patch removing it. Don't want to mix two things in the same patch. alright... Reviewed-by: Felipe Balbi <balbi@xxxxxx> -- balbi
Attachment:
signature.asc
Description: Digital signature