* [PATCH] brcmfmac: Remove waitqueue_active check
@ 2016-03-09 7:25 Hui Wang
2016-03-09 9:14 ` Hante Meuleman
2016-03-10 12:58 ` Kalle Valo
0 siblings, 2 replies; 3+ messages in thread
From: Hui Wang @ 2016-03-09 7:25 UTC (permalink / raw)
To: brudley, arend, frankyl, meuleman, pieterpg
Cc: linux-wireless, brcm80211-dev-list, hui.wang
We met a problem of pm_suspend when repeated closing/opening the lid
on a Lenovo laptop (1/20 reproduce rate), below is the log:
[ 199.735876] PM: Entering mem sleep
[ 199.750516] e1000e: EEE TX LPI TIMER: 00000011
[ 199.856638] Trying to free nonexistent resource <000000000000d000-000000000000d0ff>
[ 201.753566] brcmfmac: brcmf_pcie_suspend: Timeout on response for entering D3 substate
[ 201.753581] pci_legacy_suspend(): brcmf_pcie_suspend+0x0/0x1f0 [brcmfmac] returns -5
[ 201.753585] dpm_run_callback(): pci_pm_suspend+0x0/0x160 returns -5
[ 201.753589] PM: Device 0000:04:00.0 failed to suspend async: error -5
Through debugging, we found when problem happens, it is not the device
fails to enter D3, but the signal D3_ACK comes too early to pass the
waitqueue_active() check.
Just like this:
brcmf_pcie_send_mb_data(devinfo, BRCMF_H2D_HOST_D3_INFORM);
// signal is triggered here
wait_event_timeout(devinfo->mbdata_resp_wait, devinfo->mbdata_completed,
BRCMF_PCIE_MBDATA_TIMEOUT);
So far I think it is safe to remove waitqueue_active check since there
is only one place to trigger this signal (sending
BRCMF_H2D_HOST_D3_INFORM). And it is not a problem calling wake_up
event earlier than calling wait_event.
Cc: Brett Rudley <brudley@broadcom.com>
Cc: Hante Meuleman <meuleman@broadcom.com>
Cc: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Cc: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 0480b70..5f12ff3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -675,10 +675,8 @@ static void brcmf_pcie_handle_mb_data(struct brcmf_pciedev_info *devinfo)
brcmf_dbg(PCIE, "D2H_MB_DATA: DEEP SLEEP EXIT\n");
if (dtoh_mb_data & BRCMF_D2H_DEV_D3_ACK) {
brcmf_dbg(PCIE, "D2H_MB_DATA: D3 ACK\n");
- if (waitqueue_active(&devinfo->mbdata_resp_wait)) {
- devinfo->mbdata_completed = true;
- wake_up(&devinfo->mbdata_resp_wait);
- }
+ devinfo->mbdata_completed = true;
+ wake_up(&devinfo->mbdata_resp_wait);
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH] brcmfmac: Remove waitqueue_active check
2016-03-09 7:25 [PATCH] brcmfmac: Remove waitqueue_active check Hui Wang
@ 2016-03-09 9:14 ` Hante Meuleman
2016-03-10 12:58 ` Kalle Valo
1 sibling, 0 replies; 3+ messages in thread
From: Hante Meuleman @ 2016-03-09 9:14 UTC (permalink / raw)
To: Hui Wang, brudley, arend, frankyl, meuleman, pieterpg
Cc: linux-wireless, brcm80211-dev-list
Hi Hui,
Excellent find. Looks like a perfect solution to me.
Regards,
Hante
-----Original Message-----
From: Hui Wang [mailto:hui.wang@canonical.com]
Sent: Wednesday, March 09, 2016 8:25 AM
To: brudley@broadcom.com; arend@broadcom.com; frankyl@broadcom.com;
meuleman@broadcom.com; pieterpg@broadcom.com
Cc: linux-wireless@vger.kernel.org; brcm80211-dev-list@broadcom.com;
hui.wang@canonical.com
Subject: [PATCH] brcmfmac: Remove waitqueue_active check
We met a problem of pm_suspend when repeated closing/opening the lid
on a Lenovo laptop (1/20 reproduce rate), below is the log:
[ 199.735876] PM: Entering mem sleep
[ 199.750516] e1000e: EEE TX LPI TIMER: 00000011
[ 199.856638] Trying to free nonexistent resource
<000000000000d000-000000000000d0ff>
[ 201.753566] brcmfmac: brcmf_pcie_suspend: Timeout on response for
entering D3 substate
[ 201.753581] pci_legacy_suspend(): brcmf_pcie_suspend+0x0/0x1f0
[brcmfmac] returns -5
[ 201.753585] dpm_run_callback(): pci_pm_suspend+0x0/0x160 returns -5
[ 201.753589] PM: Device 0000:04:00.0 failed to suspend async: error -5
Through debugging, we found when problem happens, it is not the device
fails to enter D3, but the signal D3_ACK comes too early to pass the
waitqueue_active() check.
Just like this:
brcmf_pcie_send_mb_data(devinfo, BRCMF_H2D_HOST_D3_INFORM);
// signal is triggered here
wait_event_timeout(devinfo->mbdata_resp_wait, devinfo->mbdata_completed,
BRCMF_PCIE_MBDATA_TIMEOUT);
So far I think it is safe to remove waitqueue_active check since there
is only one place to trigger this signal (sending
BRCMF_H2D_HOST_D3_INFORM). And it is not a problem calling wake_up
event earlier than calling wait_event.
Cc: Brett Rudley <brudley@broadcom.com>
Cc: Hante Meuleman <meuleman@broadcom.com>
Cc: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Cc: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 0480b70..5f12ff3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -675,10 +675,8 @@ static void brcmf_pcie_handle_mb_data(struct
brcmf_pciedev_info *devinfo)
brcmf_dbg(PCIE, "D2H_MB_DATA: DEEP SLEEP EXIT\n");
if (dtoh_mb_data & BRCMF_D2H_DEV_D3_ACK) {
brcmf_dbg(PCIE, "D2H_MB_DATA: D3 ACK\n");
- if (waitqueue_active(&devinfo->mbdata_resp_wait)) {
- devinfo->mbdata_completed = true;
- wake_up(&devinfo->mbdata_resp_wait);
- }
+ devinfo->mbdata_completed = true;
+ wake_up(&devinfo->mbdata_resp_wait);
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: brcmfmac: Remove waitqueue_active check
2016-03-09 7:25 [PATCH] brcmfmac: Remove waitqueue_active check Hui Wang
2016-03-09 9:14 ` Hante Meuleman
@ 2016-03-10 12:58 ` Kalle Valo
1 sibling, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2016-03-10 12:58 UTC (permalink / raw)
To: Hui Wang
Cc: brudley, arend, frankyl, meuleman, pieterpg, linux-wireless,
brcm80211-dev-list, hui.wang
> We met a problem of pm_suspend when repeated closing/opening the lid
> on a Lenovo laptop (1/20 reproduce rate), below is the log:
>
> [ 199.735876] PM: Entering mem sleep
> [ 199.750516] e1000e: EEE TX LPI TIMER: 00000011
> [ 199.856638] Trying to free nonexistent resource <000000000000d000-000000000000d0ff>
> [ 201.753566] brcmfmac: brcmf_pcie_suspend: Timeout on response for entering D3 substate
> [ 201.753581] pci_legacy_suspend(): brcmf_pcie_suspend+0x0/0x1f0 [brcmfmac] returns -5
> [ 201.753585] dpm_run_callback(): pci_pm_suspend+0x0/0x160 returns -5
> [ 201.753589] PM: Device 0000:04:00.0 failed to suspend async: error -5
>
> Through debugging, we found when problem happens, it is not the device
> fails to enter D3, but the signal D3_ACK comes too early to pass the
> waitqueue_active() check.
>
> Just like this:
> brcmf_pcie_send_mb_data(devinfo, BRCMF_H2D_HOST_D3_INFORM);
> // signal is triggered here
> wait_event_timeout(devinfo->mbdata_resp_wait, devinfo->mbdata_completed,
> BRCMF_PCIE_MBDATA_TIMEOUT);
>
> So far I think it is safe to remove waitqueue_active check since there
> is only one place to trigger this signal (sending
> BRCMF_H2D_HOST_D3_INFORM). And it is not a problem calling wake_up
> event earlier than calling wait_event.
>
> Cc: Brett Rudley <brudley@broadcom.com>
> Cc: Hante Meuleman <meuleman@broadcom.com>
> Cc: Franky (Zhenhui) Lin <frankyl@broadcom.com>
> Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
> Cc: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-10 12:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-09 7:25 [PATCH] brcmfmac: Remove waitqueue_active check Hui Wang
2016-03-09 9:14 ` Hante Meuleman
2016-03-10 12:58 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).