From: Arend Van Spriel <arend.vanspriel@broadcom.com>
To: Brian Masney <masneyb@onstation.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Franky Lin <franky.lin@broadcom.com>,
Hante Meuleman <hante.meuleman@broadcom.com>,
Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
Wright Feng <wright.feng@cypress.com>
Cc: ulf.hansson@linaro.org, faiz_abbas@ti.com,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Kalle Valo <kvalo@codeaurora.org>,
linux-wireless@vger.kernel.org,
brcm80211-dev-list.pdl@broadcom.com,
brcm80211-dev-list@cypress.com, netdev@vger.kernel.org
Subject: Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())
Date: Sun, 26 May 2019 20:42:21 +0200 [thread overview]
Message-ID: <e8c049ce-07e1-8b34-678d-41b3d6d41983@broadcom.com> (raw)
In-Reply-To: <20190526122136.GA26456@basecamp>
On 5/26/2019 2:21 PM, Brian Masney wrote:
> + Broadcom wireless maintainers
>
> On Fri, May 24, 2019 at 11:49:58AM -0400, Brian Masney wrote:
>> On Fri, May 24, 2019 at 03:17:13PM +0300, Adrian Hunter wrote:
>>> On 24/05/19 2:10 PM, Brian Masney wrote:
>>>> WiFi stopped working on the LG Nexus 5 phone and the issue was bisected
>>>> to the commit c07a48c26519 ("mmc: sdhci: Remove finish_tasklet") that
>>>> moved from using a tasklet to a work queue. That patch also changed
>>>> sdhci_irq() to return IRQ_WAKE_THREAD instead of finishing the work when
>>>> sdhci_defer_done() is true. Change it to queue work to the complete work
>>>> queue if sdhci_defer_done() is true so that the functionality is
>>>> equilivent to what was there when the finish_tasklet was present. This
>>>> corrects the WiFi breakage on the Nexus 5 phone.
>>>>
>>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
>>>> Fixes: c07a48c26519 ("mmc: sdhci: Remove finish_tasklet")
>>>> ---
>>>> [ ... ]
>>>>
>>>> drivers/mmc/host/sdhci.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 97158344b862..3563c3bc57c9 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -3115,7 +3115,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>> continue;
>>>>
>>>> if (sdhci_defer_done(host, mrq)) {
>>>> - result = IRQ_WAKE_THREAD;
>>>> + queue_work(host->complete_wq, &host->complete_work);
>>>
>>> The IRQ thread has a lot less latency than the work queue, which is why it
>>> is done that way.
>>>
>>> I am not sure why you say this change is equivalent to what was there
>>> before, nor why it fixes your problem.
>>>
>>> Can you explain some more?
>>
>> [ ... ]
>>
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c calls
>> sdio_claim_host() and it appears to never return.
>
> When the brcmfmac driver is loaded, the firmware is requested from disk,
> and that's when the deadlock occurs in 5.2rc1. Specifically:
>
> 1) brcmf_sdio_download_firmware() in
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c calls
> sdio_claim_host()
>
> 2) brcmf_sdio_firmware_callback() is called and brcmf_sdiod_ramrw()
> tries to claim the host, but has to wait since its already claimed
> in #1 and the deadlock occurs.
This does not make any sense to me. brcmf_sdio_download_firmware() is
called from brcmf_sdio_firmware_callback() so they are in the same
context. So #2 is not waiting for #1, but something else I would say.
Also #2 calls sdio_claim_host() after brcmf_sdio_download_firmware has
completed so definitely not waiting for #1.
> I tried to release the host before the firmware is requested, however
> parts of brcmf_chip_set_active() needs the host to be claimed, and a
> similar deadlock occurs in brcmf_sdiod_ramrw() if I claim the host
> before calling brcmf_chip_set_active().
>
> I started to look at moving the sdio_{claim,release}_host() calls out of
> brcmf_sdiod_ramrw() but there's a fair number of callers, so I'd like to
> get feedback about the best course of action here.
Long ago Franky reworked the sdio critical sections requiring sdio
claim/release and I am pretty sure they are correct.
Could you try with lockdep kernel and see if that brings any more
information. In the mean time I will update my dev branch to 5.2-rc1 and
see if I can find any clues.
Regards,
Arend
next prev parent reply other threads:[~2019-05-26 18:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-24 11:10 [PATCH] mmc: sdhci: queue work after sdhci_defer_done() Brian Masney
2019-05-24 12:17 ` Adrian Hunter
2019-05-24 13:02 ` Brian Masney
2019-05-24 15:49 ` Brian Masney
2019-05-26 12:21 ` Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done()) Brian Masney
2019-05-26 18:42 ` Arend Van Spriel [this message]
2019-05-26 19:58 ` Brian Masney
2019-05-27 7:48 ` Adrian Hunter
2019-05-27 9:37 ` Brian Masney
2019-05-27 12:08 ` Adrian Hunter
2019-05-27 12:50 ` Brian Masney
2019-05-28 20:11 ` Ulf Hansson
2019-06-04 11:33 ` Arend Van Spriel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e8c049ce-07e1-8b34-678d-41b3d6d41983@broadcom.com \
--to=arend.vanspriel@broadcom.com \
--cc=adrian.hunter@intel.com \
--cc=brcm80211-dev-list.pdl@broadcom.com \
--cc=brcm80211-dev-list@cypress.com \
--cc=chi-hsien.lin@cypress.com \
--cc=faiz_abbas@ti.com \
--cc=franky.lin@broadcom.com \
--cc=hante.meuleman@broadcom.com \
--cc=kvalo@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=masneyb@onstation.org \
--cc=netdev@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
--cc=wright.feng@cypress.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).