* [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9
@ 2020-07-02 16:29 Tony Lindgren
2020-07-02 16:29 ` [PATCH 1/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Tony Lindgren @ 2020-07-02 16:29 UTC (permalink / raw)
To: Kalle Valo; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
Hi,
Here are few improvments for wlcore resume ELP handling and interrupt
handling for v5.9.
Regards,
Tony
Changes since v1:
- Update patch descriptions for simplifying the complicated locking
between interrupt handler and tx
- Change patch order for the ELP handling to be the first change
as the others are mostly clean-up type improvments
Tony Lindgren (4):
wlcore: Simplify runtime resume ELP path
wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue
wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx
wlcore: Remove pointless spinlock
drivers/net/wireless/ti/wlcore/main.c | 84 +++++++++++++--------------
1 file changed, 39 insertions(+), 45 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] wlcore: Simplify runtime resume ELP path
2020-07-02 16:29 [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
@ 2020-07-02 16:29 ` Tony Lindgren
2020-07-15 9:12 ` Kalle Valo
2020-07-02 16:29 ` [PATCH 2/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue Tony Lindgren
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2020-07-02 16:29 UTC (permalink / raw)
To: Kalle Valo; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
We can simplify the runtime resume ELP path by always setting and
clearing the completion in runtime resume. This way we can test for
WL1271_FLAG_IRQ_RUNNING after the resume write to see if we need
completion at all.
And in wlcore_irq(), we need to take spinlock for running the
completion and for the pm_wakeup_event(). Spinlock is not needed
around the bitops flags check for WL1271_FLAG_SUSPENDED so the
spinlocked sections get shorter.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/net/wireless/ti/wlcore/main.c | 43 ++++++++++-----------------
1 file changed, 15 insertions(+), 28 deletions(-)
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
@@ -649,24 +649,26 @@ 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;
+
+ /* complete the ELP completion */
+ if (test_bit(WL1271_FLAG_IN_ELP, &wl->flags)) {
+ spin_lock_irqsave(&wl->wl_lock, flags);
+ if (wl->elp_compl)
+ complete(wl->elp_compl);
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
}
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");
+ spin_lock_irqsave(&wl->wl_lock, flags);
disable_irq_nosync(wl->irq);
pm_wakeup_event(wl->dev, 0);
spin_unlock_irqrestore(&wl->wl_lock, flags);
goto out_handled;
}
- spin_unlock_irqrestore(&wl->wl_lock, flags);
/* TX might be handled here, avoid redundant work */
set_bit(WL1271_FLAG_TX_PENDING, &wl->flags);
@@ -6732,7 +6734,6 @@ static int __maybe_unused wlcore_runtime_resume(struct device *dev)
unsigned long flags;
int ret;
unsigned long start_time = jiffies;
- bool pending = false;
bool recovery = false;
/* Nothing to do if no ELP mode requested */
@@ -6742,49 +6743,35 @@ static int __maybe_unused wlcore_runtime_resume(struct device *dev)
wl1271_debug(DEBUG_PSM, "waking up chip from elp");
spin_lock_irqsave(&wl->wl_lock, flags);
- if (test_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags))
- pending = true;
- else
- wl->elp_compl = &compl;
+ wl->elp_compl = &compl;
spin_unlock_irqrestore(&wl->wl_lock, flags);
ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_WAKE_UP);
if (ret < 0) {
recovery = true;
- goto err;
- }
-
- if (!pending) {
+ } else if (!test_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags)) {
ret = wait_for_completion_timeout(&compl,
msecs_to_jiffies(WL1271_WAKEUP_TIMEOUT));
if (ret == 0) {
wl1271_warning("ELP wakeup timeout!");
-
- /* Return no error for runtime PM for recovery */
- ret = 0;
recovery = true;
- goto err;
}
}
- clear_bit(WL1271_FLAG_IN_ELP, &wl->flags);
-
- wl1271_debug(DEBUG_PSM, "wakeup time: %u ms",
- jiffies_to_msecs(jiffies - start_time));
-
- return 0;
-
-err:
spin_lock_irqsave(&wl->wl_lock, flags);
wl->elp_compl = NULL;
spin_unlock_irqrestore(&wl->wl_lock, flags);
+ clear_bit(WL1271_FLAG_IN_ELP, &wl->flags);
if (recovery) {
set_bit(WL1271_FLAG_INTENDED_FW_RECOVERY, &wl->flags);
wl12xx_queue_recovery_work(wl);
+ } else {
+ wl1271_debug(DEBUG_PSM, "wakeup time: %u ms",
+ jiffies_to_msecs(jiffies - start_time));
}
- return ret;
+ return 0;
}
static const struct dev_pm_ops wlcore_pm_ops = {
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue
2020-07-02 16:29 [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
2020-07-02 16:29 ` [PATCH 1/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
@ 2020-07-02 16:29 ` Tony Lindgren
2020-07-02 16:29 ` [PATCH 3/4] wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx Tony Lindgren
2020-07-02 16:29 ` [PATCH 4/4] wlcore: Remove pointless spinlock Tony Lindgren
3 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2020-07-02 16:29 UTC (permalink / raw)
To: Kalle Valo; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
We currently have a collection of flags and locking between the
threaded irq and tx work:
- wl->flags bitops
- wl->mutex
- wl->wl_lock spinlock
The bitops flags do not need a spinlock around them, and
wlcore_irq() already holds the mutex calling wlcore_irq_locked().
And we only need the spinlock to see if we need to run the queue
or not.
To simplify the locking, we can use spin_trylock and always run the
tx queue unless we know there's nothing to do.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/net/wireless/ti/wlcore/main.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
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
@@ -521,6 +521,7 @@ static int wlcore_irq_locked(struct wl1271 *wl)
int ret = 0;
u32 intr;
int loopcount = WL1271_IRQ_MAX_LOOPS;
+ bool run_tx_queue = true;
bool done = false;
unsigned int defer_count;
unsigned long flags;
@@ -586,19 +587,22 @@ static int wlcore_irq_locked(struct wl1271 *wl)
goto err_ret;
/* Check if any tx blocks were freed */
- spin_lock_irqsave(&wl->wl_lock, flags);
- if (!test_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags) &&
- wl1271_tx_total_queue_count(wl) > 0) {
- spin_unlock_irqrestore(&wl->wl_lock, flags);
+ if (!test_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags)) {
+ if (spin_trylock_irqsave(&wl->wl_lock, flags)) {
+ if (!wl1271_tx_total_queue_count(wl))
+ run_tx_queue = false;
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+ }
+
/*
* In order to avoid starvation of the TX path,
* call the work function directly.
*/
- ret = wlcore_tx_work_locked(wl);
- if (ret < 0)
- goto err_ret;
- } else {
- spin_unlock_irqrestore(&wl->wl_lock, flags);
+ if (run_tx_queue) {
+ ret = wlcore_tx_work_locked(wl);
+ if (ret < 0)
+ goto err_ret;
+ }
}
/* check for tx results */
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4] wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx
2020-07-02 16:29 [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
2020-07-02 16:29 ` [PATCH 1/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
2020-07-02 16:29 ` [PATCH 2/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue Tony Lindgren
@ 2020-07-02 16:29 ` Tony Lindgren
2020-07-02 16:29 ` [PATCH 4/4] wlcore: Remove pointless spinlock Tony Lindgren
3 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2020-07-02 16:29 UTC (permalink / raw)
To: Kalle Valo; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
We currently have a collection of flags and locking between the
threaded irq and tx work:
- wl->flags bitops
- wl->mutex
- wl->wl_lock spinlock
The bitops flags do not need a spinlock around them, and we only need
the spinlock to see if we need to queue tx work or not. And wlcore_irq()
holds the mutex.
To simplify the locking, we can use spin_trylock and always queue tx
work unless we know there's nothing to do.
Let's also update the comment a bit while at it.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/net/wireless/ti/wlcore/main.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
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
@@ -652,6 +652,7 @@ static irqreturn_t wlcore_irq(int irq, void *cookie)
int ret;
unsigned long flags;
struct wl1271 *wl = cookie;
+ bool queue_tx_work = true;
set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags);
@@ -684,13 +685,17 @@ static irqreturn_t wlcore_irq(int irq, void *cookie)
if (ret)
wl12xx_queue_recovery_work(wl);
- spin_lock_irqsave(&wl->wl_lock, flags);
- /* In case TX was not handled here, queue TX work */
+ /* In case TX was not handled in wlcore_irq_locked(), queue TX work */
clear_bit(WL1271_FLAG_TX_PENDING, &wl->flags);
- if (!test_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags) &&
- wl1271_tx_total_queue_count(wl) > 0)
- ieee80211_queue_work(wl->hw, &wl->tx_work);
- spin_unlock_irqrestore(&wl->wl_lock, flags);
+ if (!test_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags)) {
+ if (spin_trylock_irqsave(&wl->wl_lock, flags)) {
+ if (!wl1271_tx_total_queue_count(wl))
+ queue_tx_work = false;
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+ }
+ if (queue_tx_work)
+ ieee80211_queue_work(wl->hw, &wl->tx_work);
+ }
mutex_unlock(&wl->mutex);
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] wlcore: Remove pointless spinlock
2020-07-02 16:29 [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
` (2 preceding siblings ...)
2020-07-02 16:29 ` [PATCH 3/4] wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx Tony Lindgren
@ 2020-07-02 16:29 ` Tony Lindgren
3 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2020-07-02 16:29 UTC (permalink / raw)
To: Kalle Valo; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
No need to take a spinlock here for bitops.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/net/wireless/ti/wlcore/main.c | 2 --
1 file changed, 2 deletions(-)
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
@@ -700,9 +700,7 @@ static irqreturn_t wlcore_irq(int irq, void *cookie)
mutex_unlock(&wl->mutex);
out_handled:
- spin_lock_irqsave(&wl->wl_lock, flags);
clear_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags);
- spin_unlock_irqrestore(&wl->wl_lock, flags);
return IRQ_HANDLED;
}
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] wlcore: Simplify runtime resume ELP path
2020-07-02 16:29 ` [PATCH 1/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
@ 2020-07-15 9:12 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2020-07-15 9:12 UTC (permalink / raw)
To: Tony Lindgren; +Cc: Eyal Reizer, Guy Mishol, linux-wireless, linux-omap
Tony Lindgren <tony@atomide.com> wrote:
> We can simplify the runtime resume ELP path by always setting and
> clearing the completion in runtime resume. This way we can test for
> WL1271_FLAG_IRQ_RUNNING after the resume write to see if we need
> completion at all.
>
> And in wlcore_irq(), we need to take spinlock for running the
> completion and for the pm_wakeup_event(). Spinlock is not needed
> around the bitops flags check for WL1271_FLAG_SUSPENDED so the
> spinlocked sections get shorter.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
4 patches applied to wireless-drivers-next.git, thanks.
eb215c33f308 wlcore: Simplify runtime resume ELP path
f0325e38ab39 wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue
35fba0f0fd76 wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx
2c3601e6a340 wlcore: Remove pointless spinlock
--
https://patchwork.kernel.org/patch/11639577/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-15 9:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-02 16:29 [PATCHv2 0/4] Improvments for wlcore irq and resume for v5.9 Tony Lindgren
2020-07-02 16:29 ` [PATCH 1/4] wlcore: Simplify runtime resume ELP path Tony Lindgren
2020-07-15 9:12 ` Kalle Valo
2020-07-02 16:29 ` [PATCH 2/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue Tony Lindgren
2020-07-02 16:29 ` [PATCH 3/4] wlcore: Use spin_trylock in wlcore_irq() to see if we need to queue tx Tony Lindgren
2020-07-02 16:29 ` [PATCH 4/4] wlcore: Remove pointless spinlock Tony Lindgren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.