From patchwork Wed Mar 9 09:14:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hante Meuleman X-Patchwork-Id: 8544251 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id C7E4C9F38C for ; Wed, 9 Mar 2016 09:14:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BC1CF2026F for ; Wed, 9 Mar 2016 09:14:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9D44020263 for ; Wed, 9 Mar 2016 09:14:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752834AbcCIJO5 (ORCPT ); Wed, 9 Mar 2016 04:14:57 -0500 Received: from mail-oi0-f43.google.com ([209.85.218.43]:35842 "EHLO mail-oi0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752244AbcCIJOw (ORCPT ); Wed, 9 Mar 2016 04:14:52 -0500 Received: by mail-oi0-f43.google.com with SMTP id r187so30735108oih.3 for ; Wed, 09 Mar 2016 01:14:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=h2u2etpu/JvfPh5npKDU/jw76qF28nEDNOobORWrVJ8=; b=U+5dBvFJq+LNFq6/r7Sp4FOpx7QvzHNGaUd/P4ThmODY45AesGUI0AtbyfYmgqztaG t4mQ0RfInVaaXoblZaXJdySnYSdV0fgzMyLeIvS6tpLcor/NGQJ4pzEFpdZZL2Gjdt83 eEjV1klwpD49deV4WzMQHNksNXP7wKBL7boNQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=h2u2etpu/JvfPh5npKDU/jw76qF28nEDNOobORWrVJ8=; b=EdW1fcOdcDPTh85K4k53a+CoFoTh/BxVUGfT59b+swgq3lz3ju7saTmQ2FVYz5XyFQ wjB05jsNYJNPKilA2Vao/gS1d/p4P41JVqGnQT+7on3UK59x3A+h+13AkYVaYe7JhtMo dtt5VWsdkqH517ZqsGOgr7uovR6r75zdQ6Gt+u7muBvmeb9Pkyk2N68XEKC/+Tcq3n4o K2BZbyTQVf0wCs6xOa48D0eBzy1joI/HRpEa09TBSkHydgDc8iGoyDdkXkTR8lU+B7Yq 4hbcTEv3gYRDddbArGUP/iEHds6J2qxyuUMTDR73vUht3oyUmd3sWWuCZ9481dStgevK fTsA== X-Gm-Message-State: AD7BkJKllIjx+aRBZ2eK31ga92YSutYR3WCEcFsfz3vEQ2GXbtVawLhU4QOgakPNIPM0ANir7iK189FUiSwyUbue X-Received: by 10.202.186.67 with SMTP id k64mr9944891oif.51.1457514891955; Wed, 09 Mar 2016 01:14:51 -0800 (PST) From: Hante Meuleman References: <1457508326-24420-1-git-send-email-hui.wang@canonical.com> In-Reply-To: <1457508326-24420-1-git-send-email-hui.wang@canonical.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQMfsY7bl9cR6/ot3xkIx0vpu7t0v5y0ZIYw Date: Wed, 9 Mar 2016 10:14:51 +0100 Message-ID: <6d617d09cc3f449be4c6ce593ab84cd4@mail.gmail.com> Subject: RE: [PATCH] brcmfmac: Remove waitqueue_active check To: Hui Wang , 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 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Cc: Hante Meuleman Cc: Franky (Zhenhui) Lin Cc: Pieter-Paul Giesberts Cc: Arend van Spriel Signed-off-by: Hui Wang --- 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); } }