| Message ID | 20191007153731.58045-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wlcore: fix race for WL1271_FLAG_IRQ_RUNNING | expand |
* Tony Lindgren <tony@atomide.com> [191007 08:38]: > --- a/drivers/net/wireless/ti/wlcore/main.c > +++ b/drivers/net/wireless/ti/wlcore/main.c > @@ -692,6 +687,9 @@ static irqreturn_t wlcore_irq(int irq, void *cookie) > > mutex_unlock(&wl->mutex); > > +out_handled: > + clear_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags); > + > return IRQ_HANDLED; > } Sorry looks like I again sent out the patch too fast. The clear_bit() here should have spin_lock_irqsave() and spin_unlock_irqrestore() around it like we do elsewhere in the code currently. Regards, Tony
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c --- a/drivers/net/wireless/ti/wlcore/main.c +++ b/drivers/net/wireless/ti/wlcore/main.c @@ -544,11 +544,6 @@ static int wlcore_irq_locked(struct wl1271 *wl) } while (!done && loopcount--) { - /* - * In order to avoid a race with the hardirq, clear the flag - * before acknowledging the chip. - */ - clear_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags); smp_mb__after_atomic(); ret = wlcore_fw_status(wl, wl->fw_status); @@ -668,7 +663,7 @@ static irqreturn_t wlcore_irq(int irq, void *cookie) disable_irq_nosync(wl->irq); pm_wakeup_event(wl->dev, 0); spin_unlock_irqrestore(&wl->wl_lock, flags); - return IRQ_HANDLED; + goto out_handled; } spin_unlock_irqrestore(&wl->wl_lock, flags); @@ -692,6 +687,9 @@ static irqreturn_t wlcore_irq(int irq, void *cookie) mutex_unlock(&wl->mutex); +out_handled: + clear_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags); + return IRQ_HANDLED; }
We set WL1271_FLAG_IRQ_RUNNING in the beginning of wlcore_irq(), and test for it in wlcore_runtime_resume(). But WL1271_FLAG_IRQ_RUNNING currently gets cleared too early by wlcore_irq_locked() before wlcore_irq() is done calling it. And this will race against wlcore_runtime_resume() testing it. Let's set and clear IRQ_RUNNING in wlcore_irq() so wlcore_runtime_resume() can rely on it. And let's remove old comments about hardirq, that's no longer the case as we're using request_threaded_irq(). This fixes occasional annoying wlcore firmware reboots stat start with "wlcore: WARNING ELP wakeup timeout!" followed by a multisecond latency when the wlcore firmware gets wrongly rebooted waiting for an ELP wake interrupt that won't be coming. Note that I also suspect some form of this issue was the root cause why the wlcore GPIO interrupt has been often configured as a level interrupt instead of edge as an attempt to work around the ELP wake timeout errors. Fixes: fa2648a34e73 ("wlcore: Add support for runtime PM") Cc: Anders Roxell <anders.roxell@linaro.org> Cc: Eyal Reizer <eyalr@ti.com> Cc: Guy Mishol <guym@ti.com> Cc: John Stultz <john.stultz@linaro.org> Cc: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/net/wireless/ti/wlcore/main.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)