[PATCH] mmc: sdhci: queue work after sdhci_defer_done()

[PATCH] mmc: sdhci: queue work after sdhci_defer_done()

[Brian Masney]

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")
---
See 'sdhci@f98a4900' in qcom-msm8974-lge-nexus5-hammerhead.dts for
details about how the WiFi is wired into sdhci on this platform.

bisect log:

 git bisect start
 # bad: [4dde821e4296e156d133b98ddc4c45861935a4fb] Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
 git bisect bad 4dde821e4296e156d133b98ddc4c45861935a4fb
 # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
 git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
 # bad: [8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf] Merge tag 'docs-5.2' of git://git.lwn.net/linux
 git bisect bad 8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf
 # bad: [67a242223958d628f0ba33283668e3ddd192d057] Merge tag 'for-5.2/block-20190507' of git://git.kernel.dk/linux-block
 git bisect bad 67a242223958d628f0ba33283668e3ddd192d057
 # good: [8ff468c29e9a9c3afe9152c10c7b141343270bf3] Merge branch 'x86-fpu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
 git bisect good 8ff468c29e9a9c3afe9152c10c7b141343270bf3
 # good: [e2a5be107f52cefb9010ccae6f569c3ddaa954cc] staging: kpc2000: kpc_spi: Fix build error for {read,write}q
 git bisect good e2a5be107f52cefb9010ccae6f569c3ddaa954cc
 # bad: [cf482a49af564a3044de3178ea28f10ad5921b38] Merge tag 'driver-core-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
 git bisect bad cf482a49af564a3044de3178ea28f10ad5921b38
 # good: [9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e] Merge tag 'for-5.2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
 git bisect good 9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e
 # good: [b4b52b881cf08e13d110eac811d4becc0775abbf] Merge tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
 git bisect good b4b52b881cf08e13d110eac811d4becc0775abbf
 # bad: [d5f758f2df8015b8dcf47b6403cc192e4cef734d] mmc: meson-gx: disable HS400
 git bisect bad d5f758f2df8015b8dcf47b6403cc192e4cef734d
 # good: [b3fb9d64b497b890f7b779a9f0b40b5cc269ea18] mmc: mmci: define get_dctrl_cfg for legacy variant
 git bisect good b3fb9d64b497b890f7b779a9f0b40b5cc269ea18
 # good: [ade024f130f742725da9219624b01666f04bc4a6] memstick: jmb38x_ms: remove set but not used variable 'data'
 git bisect good ade024f130f742725da9219624b01666f04bc4a6
 # bad: [42c38d4a1bc41e78dedbf73b0fb35e44007789bb] mmc: core: Fix warning and undefined behavior in mmc voltage handling
 git bisect bad 42c38d4a1bc41e78dedbf73b0fb35e44007789bb
 # good: [19d2f695f4e82794df7465b029c02b104d1b9903] mmc: sdhci: Call mmc_request_done() from IRQ handler if possible
 git bisect good 19d2f695f4e82794df7465b029c02b104d1b9903
 # bad: [71c733c4e1aeb83e8221e89caeec893d51f88b7b] mmc: tegra: add sdhci tegra suspend and resume
 git bisect bad 71c733c4e1aeb83e8221e89caeec893d51f88b7b
 # bad: [c07a48c2651965e84d35cf193dfc0e5f7892d612] mmc: sdhci: Remove finish_tasklet
 git bisect bad c07a48c2651965e84d35cf193dfc0e5f7892d612
 # first bad commit: [c07a48c2651965e84d35cf193dfc0e5f7892d612] 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);
 		} else {
 			mrqs_done[i] = mrq;
 			host->mrqs_done[i] = NULL;
-- 
2.20.1

Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done()

[Adrian Hunter]

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")
> ---
> See 'sdhci@f98a4900' in qcom-msm8974-lge-nexus5-hammerhead.dts for
> details about how the WiFi is wired into sdhci on this platform.
> 
> bisect log:
> 
>  git bisect start
>  # bad: [4dde821e4296e156d133b98ddc4c45861935a4fb] Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
>  git bisect bad 4dde821e4296e156d133b98ddc4c45861935a4fb
>  # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
>  git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
>  # bad: [8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf] Merge tag 'docs-5.2' of git://git.lwn.net/linux
>  git bisect bad 8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf
>  # bad: [67a242223958d628f0ba33283668e3ddd192d057] Merge tag 'for-5.2/block-20190507' of git://git.kernel.dk/linux-block
>  git bisect bad 67a242223958d628f0ba33283668e3ddd192d057
>  # good: [8ff468c29e9a9c3afe9152c10c7b141343270bf3] Merge branch 'x86-fpu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
>  git bisect good 8ff468c29e9a9c3afe9152c10c7b141343270bf3
>  # good: [e2a5be107f52cefb9010ccae6f569c3ddaa954cc] staging: kpc2000: kpc_spi: Fix build error for {read,write}q
>  git bisect good e2a5be107f52cefb9010ccae6f569c3ddaa954cc
>  # bad: [cf482a49af564a3044de3178ea28f10ad5921b38] Merge tag 'driver-core-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
>  git bisect bad cf482a49af564a3044de3178ea28f10ad5921b38
>  # good: [9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e] Merge tag 'for-5.2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
>  git bisect good 9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e
>  # good: [b4b52b881cf08e13d110eac811d4becc0775abbf] Merge tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
>  git bisect good b4b52b881cf08e13d110eac811d4becc0775abbf
>  # bad: [d5f758f2df8015b8dcf47b6403cc192e4cef734d] mmc: meson-gx: disable HS400
>  git bisect bad d5f758f2df8015b8dcf47b6403cc192e4cef734d
>  # good: [b3fb9d64b497b890f7b779a9f0b40b5cc269ea18] mmc: mmci: define get_dctrl_cfg for legacy variant
>  git bisect good b3fb9d64b497b890f7b779a9f0b40b5cc269ea18
>  # good: [ade024f130f742725da9219624b01666f04bc4a6] memstick: jmb38x_ms: remove set but not used variable 'data'
>  git bisect good ade024f130f742725da9219624b01666f04bc4a6
>  # bad: [42c38d4a1bc41e78dedbf73b0fb35e44007789bb] mmc: core: Fix warning and undefined behavior in mmc voltage handling
>  git bisect bad 42c38d4a1bc41e78dedbf73b0fb35e44007789bb
>  # good: [19d2f695f4e82794df7465b029c02b104d1b9903] mmc: sdhci: Call mmc_request_done() from IRQ handler if possible
>  git bisect good 19d2f695f4e82794df7465b029c02b104d1b9903
>  # bad: [71c733c4e1aeb83e8221e89caeec893d51f88b7b] mmc: tegra: add sdhci tegra suspend and resume
>  git bisect bad 71c733c4e1aeb83e8221e89caeec893d51f88b7b
>  # bad: [c07a48c2651965e84d35cf193dfc0e5f7892d612] mmc: sdhci: Remove finish_tasklet
>  git bisect bad c07a48c2651965e84d35cf193dfc0e5f7892d612
>  # first bad commit: [c07a48c2651965e84d35cf193dfc0e5f7892d612] 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?

>  		} else {
>  			mrqs_done[i] = mrq;
>  			host->mrqs_done[i] = NULL;
> 

Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done()

[Brian Masney]

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")
> > ---
> > See 'sdhci@f98a4900' in qcom-msm8974-lge-nexus5-hammerhead.dts for
> > details about how the WiFi is wired into sdhci on this platform.
> > 
> > bisect log:
> > 
> >  git bisect start
> >  # bad: [4dde821e4296e156d133b98ddc4c45861935a4fb] Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
> >  git bisect bad 4dde821e4296e156d133b98ddc4c45861935a4fb
> >  # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
> >  git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
> >  # bad: [8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf] Merge tag 'docs-5.2' of git://git.lwn.net/linux
> >  git bisect bad 8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf
> >  # bad: [67a242223958d628f0ba33283668e3ddd192d057] Merge tag 'for-5.2/block-20190507' of git://git.kernel.dk/linux-block
> >  git bisect bad 67a242223958d628f0ba33283668e3ddd192d057
> >  # good: [8ff468c29e9a9c3afe9152c10c7b141343270bf3] Merge branch 'x86-fpu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> >  git bisect good 8ff468c29e9a9c3afe9152c10c7b141343270bf3
> >  # good: [e2a5be107f52cefb9010ccae6f569c3ddaa954cc] staging: kpc2000: kpc_spi: Fix build error for {read,write}q
> >  git bisect good e2a5be107f52cefb9010ccae6f569c3ddaa954cc
> >  # bad: [cf482a49af564a3044de3178ea28f10ad5921b38] Merge tag 'driver-core-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
> >  git bisect bad cf482a49af564a3044de3178ea28f10ad5921b38
> >  # good: [9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e] Merge tag 'for-5.2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
> >  git bisect good 9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e
> >  # good: [b4b52b881cf08e13d110eac811d4becc0775abbf] Merge tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
> >  git bisect good b4b52b881cf08e13d110eac811d4becc0775abbf
> >  # bad: [d5f758f2df8015b8dcf47b6403cc192e4cef734d] mmc: meson-gx: disable HS400
> >  git bisect bad d5f758f2df8015b8dcf47b6403cc192e4cef734d
> >  # good: [b3fb9d64b497b890f7b779a9f0b40b5cc269ea18] mmc: mmci: define get_dctrl_cfg for legacy variant
> >  git bisect good b3fb9d64b497b890f7b779a9f0b40b5cc269ea18
> >  # good: [ade024f130f742725da9219624b01666f04bc4a6] memstick: jmb38x_ms: remove set but not used variable 'data'
> >  git bisect good ade024f130f742725da9219624b01666f04bc4a6
> >  # bad: [42c38d4a1bc41e78dedbf73b0fb35e44007789bb] mmc: core: Fix warning and undefined behavior in mmc voltage handling
> >  git bisect bad 42c38d4a1bc41e78dedbf73b0fb35e44007789bb
> >  # good: [19d2f695f4e82794df7465b029c02b104d1b9903] mmc: sdhci: Call mmc_request_done() from IRQ handler if possible
> >  git bisect good 19d2f695f4e82794df7465b029c02b104d1b9903
> >  # bad: [71c733c4e1aeb83e8221e89caeec893d51f88b7b] mmc: tegra: add sdhci tegra suspend and resume
> >  git bisect bad 71c733c4e1aeb83e8221e89caeec893d51f88b7b
> >  # bad: [c07a48c2651965e84d35cf193dfc0e5f7892d612] mmc: sdhci: Remove finish_tasklet
> >  git bisect bad c07a48c2651965e84d35cf193dfc0e5f7892d612
> >  # first bad commit: [c07a48c2651965e84d35cf193dfc0e5f7892d612] 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?

What I meant by the equivalent change was that tasklet_schedule() used
to be called in this situation rather than returning IRQ_WAKE_THREAD.

I'm honestly not sure exactly what's going on yet. Without the patch I
sent out, wlan0 is not detected on the phone. Perhaps there is a subtle
race condition somewhere that is exposed with the reduction in latency
since I assume tasklet_schedule() is more expensive than doing the
processing in the bottom half?

I'll do some more digging and see if I can find more information. I sent
this out to get a discussion started.

Brian

Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done()

[Brian Masney]

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")
> > ---
> > See 'sdhci@f98a4900' in qcom-msm8974-lge-nexus5-hammerhead.dts for
> > details about how the WiFi is wired into sdhci on this platform.
> > 
> > bisect log:
> > 
> >  git bisect start
> >  # bad: [4dde821e4296e156d133b98ddc4c45861935a4fb] Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
> >  git bisect bad 4dde821e4296e156d133b98ddc4c45861935a4fb
> >  # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
> >  git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
> >  # bad: [8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf] Merge tag 'docs-5.2' of git://git.lwn.net/linux
> >  git bisect bad 8c79f4cd441b27df6cadd11b70a50e06b3b3a2bf
> >  # bad: [67a242223958d628f0ba33283668e3ddd192d057] Merge tag 'for-5.2/block-20190507' of git://git.kernel.dk/linux-block
> >  git bisect bad 67a242223958d628f0ba33283668e3ddd192d057
> >  # good: [8ff468c29e9a9c3afe9152c10c7b141343270bf3] Merge branch 'x86-fpu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> >  git bisect good 8ff468c29e9a9c3afe9152c10c7b141343270bf3
> >  # good: [e2a5be107f52cefb9010ccae6f569c3ddaa954cc] staging: kpc2000: kpc_spi: Fix build error for {read,write}q
> >  git bisect good e2a5be107f52cefb9010ccae6f569c3ddaa954cc
> >  # bad: [cf482a49af564a3044de3178ea28f10ad5921b38] Merge tag 'driver-core-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
> >  git bisect bad cf482a49af564a3044de3178ea28f10ad5921b38
> >  # good: [9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e] Merge tag 'for-5.2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
> >  git bisect good 9f2e3a53f7ec9ef55e9d01bc29a6285d291c151e
> >  # good: [b4b52b881cf08e13d110eac811d4becc0775abbf] Merge tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
> >  git bisect good b4b52b881cf08e13d110eac811d4becc0775abbf
> >  # bad: [d5f758f2df8015b8dcf47b6403cc192e4cef734d] mmc: meson-gx: disable HS400
> >  git bisect bad d5f758f2df8015b8dcf47b6403cc192e4cef734d
> >  # good: [b3fb9d64b497b890f7b779a9f0b40b5cc269ea18] mmc: mmci: define get_dctrl_cfg for legacy variant
> >  git bisect good b3fb9d64b497b890f7b779a9f0b40b5cc269ea18
> >  # good: [ade024f130f742725da9219624b01666f04bc4a6] memstick: jmb38x_ms: remove set but not used variable 'data'
> >  git bisect good ade024f130f742725da9219624b01666f04bc4a6
> >  # bad: [42c38d4a1bc41e78dedbf73b0fb35e44007789bb] mmc: core: Fix warning and undefined behavior in mmc voltage handling
> >  git bisect bad 42c38d4a1bc41e78dedbf73b0fb35e44007789bb
> >  # good: [19d2f695f4e82794df7465b029c02b104d1b9903] mmc: sdhci: Call mmc_request_done() from IRQ handler if possible
> >  git bisect good 19d2f695f4e82794df7465b029c02b104d1b9903
> >  # bad: [71c733c4e1aeb83e8221e89caeec893d51f88b7b] mmc: tegra: add sdhci tegra suspend and resume
> >  git bisect bad 71c733c4e1aeb83e8221e89caeec893d51f88b7b
> >  # bad: [c07a48c2651965e84d35cf193dfc0e5f7892d612] mmc: sdhci: Remove finish_tasklet
> >  git bisect bad c07a48c2651965e84d35cf193dfc0e5f7892d612
> >  # first bad commit: [c07a48c2651965e84d35cf193dfc0e5f7892d612] 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.

I'm not going to be able to look into this more today but I should have
time this weekend to dig in more with ftrace.

Brian

Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

+ 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.

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.

Brian

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Arend Van Spriel]

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

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

[-- Attachment #1: Type: text/plain, Size: 7013 bytes --]

On Sun, May 26, 2019 at 08:42:21PM +0200, Arend Van Spriel wrote:
> 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 attached a patch that shows how I was able to determine what had
already claimed the host. It's messy; please don't judge me negatively
for this. :) Anyways, sdio_claim_host() is mostly a wrapper for
__mmc_claim_host() and there is a mmc_ctx structure that contains a
task struct. This context can be NULL. I added a description field
to the context structure and put the function name that claimed the
host in there. The mmc_host structure already contained a 'claimer'
member, so that made it easy.

I see the following messages in dmesg that shows what has already
claimed the host when loading the brcmfmac module in 5.2rc1:

cfg80211: Loading compiled-in X.509 certificates for regulatory database
cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
platform regulatory.0: Direct firmware load for regulatory.db failed with error -2
cfg80211: failed to load regulatory.db
brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4339-sdio for chip BCM4339/2
brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac4339-sdio.lge,hammerhead.txt failed with error -2
------------[ cut here ]------------
WARNING: CPU: 0 PID: 5 at drivers/mmc/core/core.c:819 __mmc_claim_host+0x28c/0x2c0
Modules linked in: brcmfmac brcmutil cfg80211 dm_mod
CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.2.0-rc1-00175-g9899510d2cd1-dirty #420
Hardware name: Generic DT based system
Workqueue: events request_firmware_work_func
[<c03122dc>] (unwind_backtrace) from [<c030d5dc>] (show_stack+0x10/0x14)
[<c030d5dc>] (show_stack) from [<c0ac284c>] (dump_stack+0x78/0x8c)
[<c0ac284c>] (dump_stack) from [<c0321438>] (__warn.part.3+0xb8/0xd4)
[<c0321438>] (__warn.part.3) from [<c03215b0>] (warn_slowpath_null+0x44/0x4c)
[<c03215b0>] (warn_slowpath_null) from [<c093e608>] (__mmc_claim_host+0x28c/0x2c0)
[<c093e608>] (__mmc_claim_host) from [<bf115018>] (brcmf_sdiod_ramrw+0x9c/0x200 [brcmfmac])
[<bf115018>] (brcmf_sdiod_ramrw [brcmfmac]) from [<bf110508>] (brcmf_sdio_firmware_callback+0xe8/0x7b4 [brcmfmac])
[<bf110508>] (brcmf_sdio_firmware_callback [brcmfmac]) from [<bf108830>] (brcmf_fw_request_done+0xf0/0x110 [brcmfmac])
[<bf108830>] (brcmf_fw_request_done [brcmfmac]) from [<c081a4e8>] (request_firmware_work_func+0x4c/0x88)
[<c081a4e8>] (request_firmware_work_func) from [<c033c260>] (process_one_work+0x1fc/0x564)
[<c033c260>] (process_one_work) from [<c033ceb8>] (worker_thread+0x44/0x584)
[<c033ceb8>] (worker_thread) from [<c034226c>] (kthread+0x148/0x150)
[<c034226c>] (kthread) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
Exception stack(0xee8bdfb0 to 0xee8bdff8)
dfa0:                                     00000000 00000000 00000000 00000000
dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 4ab1b01efc876120 ]---
mmc_host mmc1: __mmc_claim_host: FIXME - before schedule() - descr=brcmf_sdiod_ramrw, claimer=brcmf_sdio_download_firmware

The 'after schedule()' line is not shown and WiFi doesn't work.

> > 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.

My .config has CONFIG_LOCKDEP_SUPPORT enabled. I haven't used lockdep
but my understanding is that it should print something in dmesg if a
deadlock occurs. I assume it won't pick up cases like this where
schedule() is called.

Brian

[-- Attachment #2: 0001-troubleshoot-broadcom-wireless-lockup-in-5.2rc1.patch --]
[-- Type: text/x-diff, Size: 18268 bytes --]

From 496c4deaa3787bf619baff58493142e11cd6757f Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@onstation.org>
Date: Sun, 26 May 2019 15:36:40 -0400
Subject: [PATCH] troubleshoot broadcom wireless lockup in 5.2rc1

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/mmc/core/core.c                       | 15 ++++
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      | 32 ++++---
 .../broadcom/brcm80211/brcmfmac/sdio.c        | 85 ++++++++++++-------
 include/linux/mmc/host.h                      |  1 +
 4 files changed, 93 insertions(+), 40 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..768acabf029b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -814,7 +814,22 @@ int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
 		if (stop || !host->claimed || mmc_ctx_matches(host, ctx, task))
 			break;
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			WARN_ON(1);
+			dev_info(&host->class_dev, "%s: FIXME - before schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		schedule();
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			dev_info(&host->class_dev, "%s: FIXME - after schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 60aede5abb4d..aa947fcea736 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,6 +47,8 @@
 #include "sdio.h"
 #include "core.h"
 #include "common.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define SDIOH_API_ACCESS_RETRY_LIMIT	2
 
@@ -59,6 +61,16 @@
 
 #define BRCMF_DEFAULT_RXGLOM_SIZE	32  /* max rx frames in glom chain */
 
+static void sdio_claim_host_with_descr(struct sdio_func *func,
+				       const char *descr)
+{
+	struct mmc_ctx mmc_ctx;
+
+	mmc_ctx.task = NULL;
+	mmc_ctx.descr = descr;
+	__mmc_claim_host(func->card->host, &mmc_ctx, NULL);
+}
+
 struct brcmf_sdiod_freezer {
 	atomic_t freezing;
 	atomic_t thread_count;
@@ -132,7 +144,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		}
 		sdiodev->irq_wake = true;
 
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 		if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
 			/* assign GPIO to SDIO core */
@@ -162,7 +174,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		sdio_release_host(sdiodev->func1);
 	} else {
 		brcmf_dbg(SDIO, "Entering\n");
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		sdio_claim_irq(sdiodev->func1, brcmf_sdiod_ib_irqhandler);
 		sdio_claim_irq(sdiodev->func2, brcmf_sdiod_dummy_irqhandler);
 		sdio_release_host(sdiodev->func1);
@@ -183,7 +195,7 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 		struct brcmfmac_sdio_pd *pdata;
 
 		pdata = &sdiodev->settings->bus.sdio;
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
 		sdio_release_host(sdiodev->func1);
@@ -199,7 +211,7 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 	}
 
 	if (sdiodev->sd_irq_requested) {
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		sdio_release_irq(sdiodev->func2);
 		sdio_release_irq(sdiodev->func1);
 		sdio_release_host(sdiodev->func1);
@@ -695,7 +707,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	else
 		dsize = size;
 
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	/* Do the transfer(s) */
 	while (size) {
@@ -827,7 +839,7 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdio_trigger_dpc(sdiodev->bus);
 	wait_event(sdiodev->freezer->thread_freeze,
 		   atomic_read(expect) == sdiodev->freezer->frozen_count);
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	res = brcmf_sdio_sleep(sdiodev->bus, true);
 	sdio_release_host(sdiodev->func1);
 	return res;
@@ -835,7 +847,7 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 
 static void brcmf_sdiod_freezer_off(struct brcmf_sdio_dev *sdiodev)
 {
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	brcmf_sdio_sleep(sdiodev->bus, false);
 	sdio_release_host(sdiodev->func1);
 	atomic_set(&sdiodev->freezer->freezing, 0);
@@ -887,12 +899,12 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdiod_freezer_detach(sdiodev);
 
 	/* Disable Function 2 */
-	sdio_claim_host(sdiodev->func2);
+	sdio_claim_host_with_descr(sdiodev->func2, __func__);
 	sdio_disable_func(sdiodev->func2);
 	sdio_release_host(sdiodev->func2);
 
 	/* Disable Function 1 */
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	sdio_disable_func(sdiodev->func1);
 	sdio_release_host(sdiodev->func1);
 
@@ -915,7 +927,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 {
 	int ret = 0;
 
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	ret = sdio_set_block_size(sdiodev->func1, SDIO_FUNC1_BLOCKSIZE);
 	if (ret) {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 22b73da42822..31acdb347e59 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -45,6 +45,8 @@
 #include "core.h"
 #include "common.h"
 #include "bcdc.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define DCMD_RESP_TIMEOUT	msecs_to_jiffies(2500)
 #define CTL_DONE_TIMEOUT	msecs_to_jiffies(2500)
@@ -651,6 +653,16 @@ static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
 	BRCMF_FW_ENTRY(CY_CC_43012_CHIP_ID, 0xFFFFFFFF, 43012)
 };
 
+static void sdio_claim_host_with_descr(struct sdio_func *func,
+				       const char *descr)
+{
+	struct mmc_ctx mmc_ctx;
+
+	mmc_ctx.task = NULL;
+	mmc_ctx.descr = descr;
+	__mmc_claim_host(func->card->host, &mmc_ctx, NULL);
+}
+
 static void pkt_align(struct sk_buff *p, int len, int align)
 {
 	uint datalign;
@@ -995,7 +1007,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	struct sdpcm_shared_le sh_le;
 	__le32 addr_le;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	brcmf_sdio_bus_sleep(bus, false, false);
 
 	/*
@@ -1583,7 +1595,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 		 * read directly into the chained packet, or allocate a large
 		 * packet and and copy into the chain.
 		 */
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdiod_recv_chain(bus->sdiodev,
 						 &bus->glom, dlen);
 		sdio_release_host(bus->sdiodev->func1);
@@ -1594,7 +1606,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 			brcmf_err("glom read of %d bytes failed: %d\n",
 				  dlen, errcode);
 
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
@@ -1608,7 +1621,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		rd_new.seq_num = rxseq;
 		rd_new.len = dlen;
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdio_hdparse(bus, pfirst->data, &rd_new,
 					     BRCMF_SDIO_FT_SUPER);
 		sdio_release_host(bus->sdiodev->func1);
@@ -1626,7 +1639,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 			rd_new.len = pnext->len;
 			rd_new.seq_num = rxseq++;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			errcode = brcmf_sdio_hdparse(bus, pnext->data, &rd_new,
 						     BRCMF_SDIO_FT_SUB);
 			sdio_release_host(bus->sdiodev->func1);
@@ -1638,7 +1652,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		if (errcode) {
 			/* Terminate frame on error */
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
@@ -1849,7 +1864,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		rd->len_left = rd->len;
 		/* read header first for unknow frame length */
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (!rd->len) {
 			ret = brcmf_sdiod_recv_buf(bus->sdiodev,
 						   bus->rxhdr, BRCMF_FIRSTREAD);
@@ -1916,7 +1931,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			brcmf_err("read %d bytes from channel %d failed: %d\n",
 				  rd->len, rd->channel, ret);
 			brcmu_pkt_buf_free_skb(pkt);
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true,
 					    RETRYCHAN(rd->channel));
 			sdio_release_host(bus->sdiodev->func1);
@@ -1930,7 +1946,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 		} else {
 			memcpy(bus->rxhdr, pkt->data, SDPCM_HDRLEN);
 			rd_new.seq_num = rd->seq_num;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, &rd_new,
 					       BRCMF_SDIO_FT_NORMAL)) {
 				rd->len = 0;
@@ -1963,7 +1980,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  rd_new.seq_num);
 				/* Force retry w/normal header read */
 				rd->len = 0;
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_rxfail(bus, false, true);
 				sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
@@ -1988,7 +2006,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			} else {
 				brcmf_err("%s: glom superframe w/o "
 					  "descriptor!\n", __func__);
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_rxfail(bus, false, false);
 				sdio_release_host(bus->sdiodev->func1);
 			}
@@ -2267,7 +2286,7 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 	if (ret)
 		goto done;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdiod_send_pkt(bus->sdiodev, pktq);
 	bus->sdcnt.f2txdata++;
 
@@ -2330,7 +2349,8 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		/* In poll mode, need to check for other events */
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			intstatus = brcmf_sdiod_readl(bus->sdiodev,
 						      intstat_addr, &ret);
 			sdio_release_host(bus->sdiodev->func1);
@@ -2442,7 +2462,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 	}
 
 	if (sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 		/* Enable clock for device interrupts */
 		brcmf_sdio_bus_sleep(bus, false, false);
@@ -2552,7 +2572,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	brcmf_dbg(SDIO, "Enter\n");
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 
 	/* If waiting for HTAVAIL, check status */
 	if (!bus->sr_enabled && bus->clkstate == CLK_PENDING) {
@@ -2658,7 +2678,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	if (bus->ctrl_frame_stat && (bus->clkstate == CLK_AVAIL) &&
 	    data_ok(bus)) {
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			err = brcmf_sdio_tx_ctrlframe(bus,  bus->ctrl_frame_buf,
 						      bus->ctrl_frame_len);
@@ -2682,7 +2702,8 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		brcmf_err("failed backplane access over SDIO, halting operation\n");
 		atomic_set(&bus->intstatus, 0);
 		if (bus->ctrl_frame_stat) {
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			if (bus->ctrl_frame_stat) {
 				bus->ctrl_frame_err = -ENODEV;
 				wmb();
@@ -2904,7 +2925,7 @@ brcmf_sdio_bus_txctl(struct device *dev, unsigned char *msg, uint msglen)
 					 CTL_DONE_TIMEOUT);
 	ret = 0;
 	if (bus->ctrl_frame_stat) {
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			brcmf_dbg(SDIO, "ctrl_frame timeout\n");
 			bus->ctrl_frame_stat = false;
@@ -3048,7 +3069,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		return 0;
 	}
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	if (sh->assert_file_addr != 0) {
 		error = brcmf_sdiod_ramrw(bus->sdiodev, false,
 					  sh->assert_file_addr, (u8 *)file, 80);
@@ -3340,7 +3361,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 	int bcmerror;
 	u32 rstvec;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 
 	rstvec = get_unaligned_le32(fw->data);
@@ -3564,7 +3585,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 
 	address = bus->ci->rambase;
 	offset = err = 0;
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	while (offset < mem_size) {
 		len = ((offset + MEMBLOCK) < mem_size) ? MEMBLOCK :
 		      mem_size - offset;
@@ -3637,7 +3658,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			if (!bus->dpc_triggered) {
 				u8 devpend;
 
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				devpend = brcmf_sdiod_func0_rb(bus->sdiodev,
 						  SDIO_CCCR_INTx, NULL);
 				sdio_release_host(bus->sdiodev->func1);
@@ -3666,7 +3688,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 		bus->console.count += jiffies_to_msecs(BRCMF_WD_POLL);
 		if (bus->console.count >= bus->console_interval) {
 			bus->console.count -= bus->console_interval;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			/* Make sure backplane clock is on */
 			brcmf_sdio_bus_sleep(bus, false, false);
 			if (brcmf_sdio_readconsole(bus) < 0)
@@ -3685,7 +3708,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			bus->idlecount++;
 			if (bus->idlecount > bus->idletime) {
 				brcmf_dbg(SDIO, "idle\n");
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				bus->idlecount = 0;
 				brcmf_sdio_bus_sleep(bus, true, false);
@@ -3903,7 +3927,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	u32 drivestrength;
 
 	sdiodev = bus->sdiodev;
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	pr_debug("F1 signature read @0x18000000=0x%4x\n",
 		 brcmf_sdiod_readl(sdiodev, SI_ENUM_BASE, NULL));
@@ -4147,7 +4171,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	bus->sdcnt.tickcnt = 0;
 	brcmf_sdio_wd_timer(bus, true);
 
-	sdio_claim_host(sdiod->func1);
+	sdio_claim_host_with_descr(sdiod->func1, __func__);
 
 	/* Make sure backplane clock is on, needed to generate F2 interrupt */
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
@@ -4255,7 +4279,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	err = brcmf_attach(sdiod->dev, sdiod->settings);
 	if (err != 0) {
 		brcmf_err("brcmf_attach failed\n");
-		sdio_claim_host(sdiod->func1);
+		sdio_claim_host_with_descr(sdiod->func1, __func__);
 		goto checkdied;
 	}
 
@@ -4361,7 +4385,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	bus->blocksize = bus->sdiodev->func2->cur_blksize;
 	bus->roundup = min(max_roundup, bus->blocksize);
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 
 	/* Disable F2 to clear any intermediate frame state on the dongle */
 	sdio_disable_func(bus->sdiodev->func2);
@@ -4428,7 +4452,8 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 
 		if (bus->ci) {
 			if (bus->sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 				/* Leave the device in state where it is
@@ -4485,7 +4510,7 @@ int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep)
 {
 	int ret;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdio_bus_sleep(bus, sleep, false);
 	sdio_release_host(bus->sdiodev->func1);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..6ccc76150f45 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -267,6 +267,7 @@ struct mmc_supply {
 };
 
 struct mmc_ctx {
+	const char *descr;
 	struct task_struct *task;
 };
 
-- 
2.20.1

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

[-- Attachment #1: Type: text/plain, Size: 7013 bytes --]

On Sun, May 26, 2019 at 08:42:21PM +0200, Arend Van Spriel wrote:
> 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 attached a patch that shows how I was able to determine what had
already claimed the host. It's messy; please don't judge me negatively
for this. :) Anyways, sdio_claim_host() is mostly a wrapper for
__mmc_claim_host() and there is a mmc_ctx structure that contains a
task struct. This context can be NULL. I added a description field
to the context structure and put the function name that claimed the
host in there. The mmc_host structure already contained a 'claimer'
member, so that made it easy.

I see the following messages in dmesg that shows what has already
claimed the host when loading the brcmfmac module in 5.2rc1:

cfg80211: Loading compiled-in X.509 certificates for regulatory database
cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
platform regulatory.0: Direct firmware load for regulatory.db failed with error -2
cfg80211: failed to load regulatory.db
brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4339-sdio for chip BCM4339/2
brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac4339-sdio.lge,hammerhead.txt failed with error -2
------------[ cut here ]------------
WARNING: CPU: 0 PID: 5 at drivers/mmc/core/core.c:819 __mmc_claim_host+0x28c/0x2c0
Modules linked in: brcmfmac brcmutil cfg80211 dm_mod
CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.2.0-rc1-00175-g9899510d2cd1-dirty #420
Hardware name: Generic DT based system
Workqueue: events request_firmware_work_func
[<c03122dc>] (unwind_backtrace) from [<c030d5dc>] (show_stack+0x10/0x14)
[<c030d5dc>] (show_stack) from [<c0ac284c>] (dump_stack+0x78/0x8c)
[<c0ac284c>] (dump_stack) from [<c0321438>] (__warn.part.3+0xb8/0xd4)
[<c0321438>] (__warn.part.3) from [<c03215b0>] (warn_slowpath_null+0x44/0x4c)
[<c03215b0>] (warn_slowpath_null) from [<c093e608>] (__mmc_claim_host+0x28c/0x2c0)
[<c093e608>] (__mmc_claim_host) from [<bf115018>] (brcmf_sdiod_ramrw+0x9c/0x200 [brcmfmac])
[<bf115018>] (brcmf_sdiod_ramrw [brcmfmac]) from [<bf110508>] (brcmf_sdio_firmware_callback+0xe8/0x7b4 [brcmfmac])
[<bf110508>] (brcmf_sdio_firmware_callback [brcmfmac]) from [<bf108830>] (brcmf_fw_request_done+0xf0/0x110 [brcmfmac])
[<bf108830>] (brcmf_fw_request_done [brcmfmac]) from [<c081a4e8>] (request_firmware_work_func+0x4c/0x88)
[<c081a4e8>] (request_firmware_work_func) from [<c033c260>] (process_one_work+0x1fc/0x564)
[<c033c260>] (process_one_work) from [<c033ceb8>] (worker_thread+0x44/0x584)
[<c033ceb8>] (worker_thread) from [<c034226c>] (kthread+0x148/0x150)
[<c034226c>] (kthread) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
Exception stack(0xee8bdfb0 to 0xee8bdff8)
dfa0:                                     00000000 00000000 00000000 00000000
dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 4ab1b01efc876120 ]---
mmc_host mmc1: __mmc_claim_host: FIXME - before schedule() - descr=brcmf_sdiod_ramrw, claimer=brcmf_sdio_download_firmware

The 'after schedule()' line is not shown and WiFi doesn't work.

> > 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.

My .config has CONFIG_LOCKDEP_SUPPORT enabled. I haven't used lockdep
but my understanding is that it should print something in dmesg if a
deadlock occurs. I assume it won't pick up cases like this where
schedule() is called.

Brian

[-- Attachment #2: 0001-troubleshoot-broadcom-wireless-lockup-in-5.2rc1.patch --]
[-- Type: text/x-diff, Size: 18269 bytes --]

>From 496c4deaa3787bf619baff58493142e11cd6757f Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@onstation.org>
Date: Sun, 26 May 2019 15:36:40 -0400
Subject: [PATCH] troubleshoot broadcom wireless lockup in 5.2rc1

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/mmc/core/core.c                       | 15 ++++
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      | 32 ++++---
 .../broadcom/brcm80211/brcmfmac/sdio.c        | 85 ++++++++++++-------
 include/linux/mmc/host.h                      |  1 +
 4 files changed, 93 insertions(+), 40 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..768acabf029b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -814,7 +814,22 @@ int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
 		if (stop || !host->claimed || mmc_ctx_matches(host, ctx, task))
 			break;
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			WARN_ON(1);
+			dev_info(&host->class_dev, "%s: FIXME - before schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		schedule();
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			dev_info(&host->class_dev, "%s: FIXME - after schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 60aede5abb4d..aa947fcea736 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,6 +47,8 @@
 #include "sdio.h"
 #include "core.h"
 #include "common.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define SDIOH_API_ACCESS_RETRY_LIMIT	2
 
@@ -59,6 +61,16 @@
 
 #define BRCMF_DEFAULT_RXGLOM_SIZE	32  /* max rx frames in glom chain */
 
+static void sdio_claim_host_with_descr(struct sdio_func *func,
+				       const char *descr)
+{
+	struct mmc_ctx mmc_ctx;
+
+	mmc_ctx.task = NULL;
+	mmc_ctx.descr = descr;
+	__mmc_claim_host(func->card->host, &mmc_ctx, NULL);
+}
+
 struct brcmf_sdiod_freezer {
 	atomic_t freezing;
 	atomic_t thread_count;
@@ -132,7 +144,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		}
 		sdiodev->irq_wake = true;
 
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 		if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
 			/* assign GPIO to SDIO core */
@@ -162,7 +174,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		sdio_release_host(sdiodev->func1);
 	} else {
 		brcmf_dbg(SDIO, "Entering\n");
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		sdio_claim_irq(sdiodev->func1, brcmf_sdiod_ib_irqhandler);
 		sdio_claim_irq(sdiodev->func2, brcmf_sdiod_dummy_irqhandler);
 		sdio_release_host(sdiodev->func1);
@@ -183,7 +195,7 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 		struct brcmfmac_sdio_pd *pdata;
 
 		pdata = &sdiodev->settings->bus.sdio;
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
 		sdio_release_host(sdiodev->func1);
@@ -199,7 +211,7 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 	}
 
 	if (sdiodev->sd_irq_requested) {
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 		sdio_release_irq(sdiodev->func2);
 		sdio_release_irq(sdiodev->func1);
 		sdio_release_host(sdiodev->func1);
@@ -695,7 +707,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	else
 		dsize = size;
 
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	/* Do the transfer(s) */
 	while (size) {
@@ -827,7 +839,7 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdio_trigger_dpc(sdiodev->bus);
 	wait_event(sdiodev->freezer->thread_freeze,
 		   atomic_read(expect) == sdiodev->freezer->frozen_count);
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	res = brcmf_sdio_sleep(sdiodev->bus, true);
 	sdio_release_host(sdiodev->func1);
 	return res;
@@ -835,7 +847,7 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 
 static void brcmf_sdiod_freezer_off(struct brcmf_sdio_dev *sdiodev)
 {
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	brcmf_sdio_sleep(sdiodev->bus, false);
 	sdio_release_host(sdiodev->func1);
 	atomic_set(&sdiodev->freezer->freezing, 0);
@@ -887,12 +899,12 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdiod_freezer_detach(sdiodev);
 
 	/* Disable Function 2 */
-	sdio_claim_host(sdiodev->func2);
+	sdio_claim_host_with_descr(sdiodev->func2, __func__);
 	sdio_disable_func(sdiodev->func2);
 	sdio_release_host(sdiodev->func2);
 
 	/* Disable Function 1 */
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	sdio_disable_func(sdiodev->func1);
 	sdio_release_host(sdiodev->func1);
 
@@ -915,7 +927,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 {
 	int ret = 0;
 
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	ret = sdio_set_block_size(sdiodev->func1, SDIO_FUNC1_BLOCKSIZE);
 	if (ret) {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 22b73da42822..31acdb347e59 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -45,6 +45,8 @@
 #include "core.h"
 #include "common.h"
 #include "bcdc.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define DCMD_RESP_TIMEOUT	msecs_to_jiffies(2500)
 #define CTL_DONE_TIMEOUT	msecs_to_jiffies(2500)
@@ -651,6 +653,16 @@ static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
 	BRCMF_FW_ENTRY(CY_CC_43012_CHIP_ID, 0xFFFFFFFF, 43012)
 };
 
+static void sdio_claim_host_with_descr(struct sdio_func *func,
+				       const char *descr)
+{
+	struct mmc_ctx mmc_ctx;
+
+	mmc_ctx.task = NULL;
+	mmc_ctx.descr = descr;
+	__mmc_claim_host(func->card->host, &mmc_ctx, NULL);
+}
+
 static void pkt_align(struct sk_buff *p, int len, int align)
 {
 	uint datalign;
@@ -995,7 +1007,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	struct sdpcm_shared_le sh_le;
 	__le32 addr_le;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	brcmf_sdio_bus_sleep(bus, false, false);
 
 	/*
@@ -1583,7 +1595,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 		 * read directly into the chained packet, or allocate a large
 		 * packet and and copy into the chain.
 		 */
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdiod_recv_chain(bus->sdiodev,
 						 &bus->glom, dlen);
 		sdio_release_host(bus->sdiodev->func1);
@@ -1594,7 +1606,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 			brcmf_err("glom read of %d bytes failed: %d\n",
 				  dlen, errcode);
 
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
@@ -1608,7 +1621,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		rd_new.seq_num = rxseq;
 		rd_new.len = dlen;
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdio_hdparse(bus, pfirst->data, &rd_new,
 					     BRCMF_SDIO_FT_SUPER);
 		sdio_release_host(bus->sdiodev->func1);
@@ -1626,7 +1639,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 			rd_new.len = pnext->len;
 			rd_new.seq_num = rxseq++;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			errcode = brcmf_sdio_hdparse(bus, pnext->data, &rd_new,
 						     BRCMF_SDIO_FT_SUB);
 			sdio_release_host(bus->sdiodev->func1);
@@ -1638,7 +1652,8 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		if (errcode) {
 			/* Terminate frame on error */
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
@@ -1849,7 +1864,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		rd->len_left = rd->len;
 		/* read header first for unknow frame length */
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (!rd->len) {
 			ret = brcmf_sdiod_recv_buf(bus->sdiodev,
 						   bus->rxhdr, BRCMF_FIRSTREAD);
@@ -1916,7 +1931,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			brcmf_err("read %d bytes from channel %d failed: %d\n",
 				  rd->len, rd->channel, ret);
 			brcmu_pkt_buf_free_skb(pkt);
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			brcmf_sdio_rxfail(bus, true,
 					    RETRYCHAN(rd->channel));
 			sdio_release_host(bus->sdiodev->func1);
@@ -1930,7 +1946,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 		} else {
 			memcpy(bus->rxhdr, pkt->data, SDPCM_HDRLEN);
 			rd_new.seq_num = rd->seq_num;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, &rd_new,
 					       BRCMF_SDIO_FT_NORMAL)) {
 				rd->len = 0;
@@ -1963,7 +1980,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  rd_new.seq_num);
 				/* Force retry w/normal header read */
 				rd->len = 0;
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_rxfail(bus, false, true);
 				sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
@@ -1988,7 +2006,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			} else {
 				brcmf_err("%s: glom superframe w/o "
 					  "descriptor!\n", __func__);
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_rxfail(bus, false, false);
 				sdio_release_host(bus->sdiodev->func1);
 			}
@@ -2267,7 +2286,7 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 	if (ret)
 		goto done;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdiod_send_pkt(bus->sdiodev, pktq);
 	bus->sdcnt.f2txdata++;
 
@@ -2330,7 +2349,8 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		/* In poll mode, need to check for other events */
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			intstatus = brcmf_sdiod_readl(bus->sdiodev,
 						      intstat_addr, &ret);
 			sdio_release_host(bus->sdiodev->func1);
@@ -2442,7 +2462,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 	}
 
 	if (sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-		sdio_claim_host(sdiodev->func1);
+		sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 		/* Enable clock for device interrupts */
 		brcmf_sdio_bus_sleep(bus, false, false);
@@ -2552,7 +2572,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	brcmf_dbg(SDIO, "Enter\n");
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 
 	/* If waiting for HTAVAIL, check status */
 	if (!bus->sr_enabled && bus->clkstate == CLK_PENDING) {
@@ -2658,7 +2678,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	if (bus->ctrl_frame_stat && (bus->clkstate == CLK_AVAIL) &&
 	    data_ok(bus)) {
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			err = brcmf_sdio_tx_ctrlframe(bus,  bus->ctrl_frame_buf,
 						      bus->ctrl_frame_len);
@@ -2682,7 +2702,8 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		brcmf_err("failed backplane access over SDIO, halting operation\n");
 		atomic_set(&bus->intstatus, 0);
 		if (bus->ctrl_frame_stat) {
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			if (bus->ctrl_frame_stat) {
 				bus->ctrl_frame_err = -ENODEV;
 				wmb();
@@ -2904,7 +2925,7 @@ brcmf_sdio_bus_txctl(struct device *dev, unsigned char *msg, uint msglen)
 					 CTL_DONE_TIMEOUT);
 	ret = 0;
 	if (bus->ctrl_frame_stat) {
-		sdio_claim_host(bus->sdiodev->func1);
+		sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			brcmf_dbg(SDIO, "ctrl_frame timeout\n");
 			bus->ctrl_frame_stat = false;
@@ -3048,7 +3069,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		return 0;
 	}
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	if (sh->assert_file_addr != 0) {
 		error = brcmf_sdiod_ramrw(bus->sdiodev, false,
 					  sh->assert_file_addr, (u8 *)file, 80);
@@ -3340,7 +3361,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 	int bcmerror;
 	u32 rstvec;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 
 	rstvec = get_unaligned_le32(fw->data);
@@ -3564,7 +3585,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 
 	address = bus->ci->rambase;
 	offset = err = 0;
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 	while (offset < mem_size) {
 		len = ((offset + MEMBLOCK) < mem_size) ? MEMBLOCK :
 		      mem_size - offset;
@@ -3637,7 +3658,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			if (!bus->dpc_triggered) {
 				u8 devpend;
 
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				devpend = brcmf_sdiod_func0_rb(bus->sdiodev,
 						  SDIO_CCCR_INTx, NULL);
 				sdio_release_host(bus->sdiodev->func1);
@@ -3666,7 +3688,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 		bus->console.count += jiffies_to_msecs(BRCMF_WD_POLL);
 		if (bus->console.count >= bus->console_interval) {
 			bus->console.count -= bus->console_interval;
-			sdio_claim_host(bus->sdiodev->func1);
+			sdio_claim_host_with_descr(bus->sdiodev->func1,
+						   __func__);
 			/* Make sure backplane clock is on */
 			brcmf_sdio_bus_sleep(bus, false, false);
 			if (brcmf_sdio_readconsole(bus) < 0)
@@ -3685,7 +3708,8 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			bus->idlecount++;
 			if (bus->idlecount > bus->idletime) {
 				brcmf_dbg(SDIO, "idle\n");
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				bus->idlecount = 0;
 				brcmf_sdio_bus_sleep(bus, true, false);
@@ -3903,7 +3927,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	u32 drivestrength;
 
 	sdiodev = bus->sdiodev;
-	sdio_claim_host(sdiodev->func1);
+	sdio_claim_host_with_descr(sdiodev->func1, __func__);
 
 	pr_debug("F1 signature read @0x18000000=0x%4x\n",
 		 brcmf_sdiod_readl(sdiodev, SI_ENUM_BASE, NULL));
@@ -4147,7 +4171,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	bus->sdcnt.tickcnt = 0;
 	brcmf_sdio_wd_timer(bus, true);
 
-	sdio_claim_host(sdiod->func1);
+	sdio_claim_host_with_descr(sdiod->func1, __func__);
 
 	/* Make sure backplane clock is on, needed to generate F2 interrupt */
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
@@ -4255,7 +4279,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	err = brcmf_attach(sdiod->dev, sdiod->settings);
 	if (err != 0) {
 		brcmf_err("brcmf_attach failed\n");
-		sdio_claim_host(sdiod->func1);
+		sdio_claim_host_with_descr(sdiod->func1, __func__);
 		goto checkdied;
 	}
 
@@ -4361,7 +4385,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	bus->blocksize = bus->sdiodev->func2->cur_blksize;
 	bus->roundup = min(max_roundup, bus->blocksize);
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 
 	/* Disable F2 to clear any intermediate frame state on the dongle */
 	sdio_disable_func(bus->sdiodev->func2);
@@ -4428,7 +4452,8 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 
 		if (bus->ci) {
 			if (bus->sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-				sdio_claim_host(bus->sdiodev->func1);
+				sdio_claim_host_with_descr(bus->sdiodev->func1,
+							   __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 				/* Leave the device in state where it is
@@ -4485,7 +4510,7 @@ int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep)
 {
 	int ret;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	sdio_claim_host_with_descr(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdio_bus_sleep(bus, sleep, false);
 	sdio_release_host(bus->sdiodev->func1);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..6ccc76150f45 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -267,6 +267,7 @@ struct mmc_supply {
 };
 
 struct mmc_ctx {
+	const char *descr;
 	struct task_struct *task;
 };
 
-- 
2.20.1

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Adrian Hunter]

On 26/05/19 10:58 PM, Brian Masney wrote:
> On Sun, May 26, 2019 at 08:42:21PM +0200, Arend Van Spriel wrote:
>> 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.

This is because SDHCI is using the IRQ thread to process the SDIO card
interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
finish_tasklet") has moved the tasklet processing to the IRQ thread.

I would expect to be able to use the IRQ thread to complete requests, and it
is desirable to do so because it is lower latency.

Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
is what other drivers are doing.

I will investigate some more and send a patch.

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
> I attached a patch that shows how I was able to determine what had
> already claimed the host.

I realized this morning that I had a flaw with my test patch that
diagnosed what was deadlocked. The mmc_ctx structure was allocated on
the stack. I attached a second version of that patch that uses
kmalloc() for that structure. It didn't change what I reported
yesterday: brcmf_sdiod_ramrw is deadlocked by
brcmf_sdio_download_firmware.


On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
> This is because SDHCI is using the IRQ thread to process the SDIO card
> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
> finish_tasklet") has moved the tasklet processing to the IRQ thread.
> 
> I would expect to be able to use the IRQ thread to complete requests, and it
> is desirable to do so because it is lower latency.
> 
> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
> is what other drivers are doing.
> 
> I will investigate some more and send a patch.

Thank you!

Brian


[-- Attachment #2: 0001-troubleshoot-broadcom-wireless-lockup-in-5.2rc1-v2.patch --]
[-- Type: text/x-diff, Size: 28427 bytes --]

From acc78b5e581d2c3ebc994a6ad6e7b367f2b81935 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@onstation.org>
Date: Sun, 26 May 2019 15:36:40 -0400
Subject: [PATCH] troubleshoot broadcom wireless lockup in 5.2rc1 (v2)

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/mmc/core/core.c                       |  15 ++
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      |  46 ++---
 .../broadcom/brcm80211/brcmfmac/sdio.c        | 160 ++++++++++--------
 .../broadcom/brcm80211/brcmfmac/sdio.h        |   3 +
 include/linux/mmc/host.h                      |   1 +
 5 files changed, 136 insertions(+), 89 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..768acabf029b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -814,7 +814,22 @@ int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
 		if (stop || !host->claimed || mmc_ctx_matches(host, ctx, task))
 			break;
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			WARN_ON(1);
+			dev_info(&host->class_dev, "%s: FIXME - before schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		schedule();
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			dev_info(&host->class_dev, "%s: FIXME - after schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 60aede5abb4d..fdeaeb1353af 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,6 +47,8 @@
 #include "sdio.h"
 #include "core.h"
 #include "common.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define SDIOH_API_ACCESS_RETRY_LIMIT	2
 
@@ -132,7 +134,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		}
 		sdiodev->irq_wake = true;
 
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 		if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
 			/* assign GPIO to SDIO core */
@@ -159,13 +161,13 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 			data |= SDIO_CCCR_BRCM_SEPINT_ACT_HI;
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT,
 				     data, &ret);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 	} else {
 		brcmf_dbg(SDIO, "Entering\n");
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		sdio_claim_irq(sdiodev->func1, brcmf_sdiod_ib_irqhandler);
 		sdio_claim_irq(sdiodev->func2, brcmf_sdiod_dummy_irqhandler);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		sdiodev->sd_irq_requested = true;
 	}
 
@@ -183,10 +185,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 		struct brcmfmac_sdio_pd *pdata;
 
 		pdata = &sdiodev->settings->bus.sdio;
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 
 		sdiodev->oob_irq_requested = false;
 		if (sdiodev->irq_wake) {
@@ -199,10 +201,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 	}
 
 	if (sdiodev->sd_irq_requested) {
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		sdio_release_irq(sdiodev->func2);
 		sdio_release_irq(sdiodev->func1);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		sdiodev->sd_irq_requested = false;
 	}
 }
@@ -695,7 +697,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	else
 		dsize = size;
 
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	/* Do the transfer(s) */
 	while (size) {
@@ -742,7 +744,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 	dev_kfree_skb(pkt);
 
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	return err;
 }
@@ -827,17 +829,17 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdio_trigger_dpc(sdiodev->bus);
 	wait_event(sdiodev->freezer->thread_freeze,
 		   atomic_read(expect) == sdiodev->freezer->frozen_count);
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	res = brcmf_sdio_sleep(sdiodev->bus, true);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return res;
 }
 
 static void brcmf_sdiod_freezer_off(struct brcmf_sdio_dev *sdiodev)
 {
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	brcmf_sdio_sleep(sdiodev->bus, false);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	atomic_set(&sdiodev->freezer->freezing, 0);
 	complete_all(&sdiodev->freezer->resumed);
 }
@@ -887,14 +889,14 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdiod_freezer_detach(sdiodev);
 
 	/* Disable Function 2 */
-	sdio_claim_host(sdiodev->func2);
+	brcmf_sdio_claim_host(sdiodev->func2, __func__);
 	sdio_disable_func(sdiodev->func2);
-	sdio_release_host(sdiodev->func2);
+	brcmf_sdio_release_host(sdiodev->func2);
 
 	/* Disable Function 1 */
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	sdio_disable_func(sdiodev->func1);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	sg_free_table(&sdiodev->sgtable);
 	sdiodev->sbwad = 0;
@@ -915,18 +917,18 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 {
 	int ret = 0;
 
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	ret = sdio_set_block_size(sdiodev->func1, SDIO_FUNC1_BLOCKSIZE);
 	if (ret) {
 		brcmf_err("Failed to set F1 blocksize\n");
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		goto out;
 	}
 	ret = sdio_set_block_size(sdiodev->func2, SDIO_FUNC2_BLOCKSIZE);
 	if (ret) {
 		brcmf_err("Failed to set F2 blocksize\n");
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		goto out;
 	}
 
@@ -935,7 +937,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 
 	/* Enable Function 1 */
 	ret = sdio_enable_func(sdiodev->func1);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	if (ret) {
 		brcmf_err("Failed to enable F1: err=%d\n", ret);
 		goto out;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 22b73da42822..ba836aa5da78 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -45,6 +45,8 @@
 #include "core.h"
 #include "common.h"
 #include "bcdc.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define DCMD_RESP_TIMEOUT	msecs_to_jiffies(2500)
 #define CTL_DONE_TIMEOUT	msecs_to_jiffies(2500)
@@ -651,6 +653,25 @@ static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
 	BRCMF_FW_ENTRY(CY_CC_43012_CHIP_ID, 0xFFFFFFFF, 43012)
 };
 
+void brcmf_sdio_claim_host(struct sdio_func *func, const char *descr)
+{
+	struct mmc_ctx *mmc_ctx;
+
+	mmc_ctx = kmalloc(sizeof(*mmc_ctx), GFP_KERNEL);
+	mmc_ctx->task = NULL;
+	mmc_ctx->descr = descr;
+	__mmc_claim_host(func->card->host, mmc_ctx, NULL);
+}
+
+void brcmf_sdio_release_host(struct sdio_func *func)
+{
+	struct mmc_ctx *tofree;
+
+	tofree = func->card->host->claimer;
+	sdio_release_host(func);
+	kfree(tofree);
+}
+
 static void pkt_align(struct sk_buff *p, int len, int align)
 {
 	uint datalign;
@@ -995,7 +1016,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	struct sdpcm_shared_le sh_le;
 	__le32 addr_le;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	brcmf_sdio_bus_sleep(bus, false, false);
 
 	/*
@@ -1029,7 +1050,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	if (rv < 0)
 		goto fail;
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* Endianness */
 	sh->flags = le32_to_cpu(sh_le.flags);
@@ -1051,7 +1072,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 fail:
 	brcmf_err("unable to obtain sdpcm_shared info: rv=%d (addr=0x%x)\n",
 		  rv, addr);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 	return rv;
 }
 
@@ -1583,10 +1604,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 		 * read directly into the chained packet, or allocate a large
 		 * packet and and copy into the chain.
 		 */
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdiod_recv_chain(bus->sdiodev,
 						 &bus->glom, dlen);
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		bus->sdcnt.f2rxdata++;
 
 		/* On failure, kill the superframe */
@@ -1594,11 +1615,11 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 			brcmf_err("glom read of %d bytes failed: %d\n",
 				  dlen, errcode);
 
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			return 0;
 		}
 
@@ -1608,10 +1629,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		rd_new.seq_num = rxseq;
 		rd_new.len = dlen;
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdio_hdparse(bus, pfirst->data, &rd_new,
 					     BRCMF_SDIO_FT_SUPER);
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		bus->cur_read.len = rd_new.len_nxtfrm << 4;
 
 		/* Remove superframe header, remember offset */
@@ -1626,10 +1647,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 			rd_new.len = pnext->len;
 			rd_new.seq_num = rxseq++;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			errcode = brcmf_sdio_hdparse(bus, pnext->data, &rd_new,
 						     BRCMF_SDIO_FT_SUB);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			brcmf_dbg_hex_dump(BRCMF_GLOM_ON(),
 					   pnext->data, 32, "subframe:\n");
 
@@ -1638,11 +1659,11 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		if (errcode) {
 			/* Terminate frame on error */
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			bus->cur_read.len = 0;
 			return 0;
 		}
@@ -1849,7 +1870,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		rd->len_left = rd->len;
 		/* read header first for unknow frame length */
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (!rd->len) {
 			ret = brcmf_sdiod_recv_buf(bus->sdiodev,
 						   bus->rxhdr, BRCMF_FIRSTREAD);
@@ -1859,7 +1880,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  ret);
 				bus->sdcnt.rx_hdrfail++;
 				brcmf_sdio_rxfail(bus, true, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				continue;
 			}
 
@@ -1869,7 +1890,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, rd,
 					       BRCMF_SDIO_FT_NORMAL)) {
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				if (!bus->rxpending)
 					break;
 				else
@@ -1885,7 +1906,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 				rd->len_nxtfrm = 0;
 				/* treat all packet as event if we don't know */
 				rd->channel = SDPCM_EVENT_CHANNEL;
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				continue;
 			}
 			rd->len_left = rd->len > BRCMF_FIRSTREAD ?
@@ -1902,7 +1923,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			brcmf_err("brcmu_pkt_buf_get_skb failed\n");
 			brcmf_sdio_rxfail(bus, false,
 					    RETRYCHAN(rd->channel));
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			continue;
 		}
 		skb_pull(pkt, head_read);
@@ -1910,16 +1931,16 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		ret = brcmf_sdiod_recv_pkt(bus->sdiodev, pkt);
 		bus->sdcnt.f2rxdata++;
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 
 		if (ret < 0) {
 			brcmf_err("read %d bytes from channel %d failed: %d\n",
 				  rd->len, rd->channel, ret);
 			brcmu_pkt_buf_free_skb(pkt);
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true,
 					    RETRYCHAN(rd->channel));
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			continue;
 		}
 
@@ -1930,7 +1951,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 		} else {
 			memcpy(bus->rxhdr, pkt->data, SDPCM_HDRLEN);
 			rd_new.seq_num = rd->seq_num;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, &rd_new,
 					       BRCMF_SDIO_FT_NORMAL)) {
 				rd->len = 0;
@@ -1943,11 +1964,11 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  roundup(rd_new.len, 16) >> 4);
 				rd->len = 0;
 				brcmf_sdio_rxfail(bus, true, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
 				continue;
 			}
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			rd->len_nxtfrm = rd_new.len_nxtfrm;
 			rd->channel = rd_new.channel;
 			rd->dat_offset = rd_new.dat_offset;
@@ -1963,9 +1984,10 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  rd_new.seq_num);
 				/* Force retry w/normal header read */
 				rd->len = 0;
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_rxfail(bus, false, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
 				continue;
 			}
@@ -1988,9 +2010,10 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			} else {
 				brcmf_err("%s: glom superframe w/o "
 					  "descriptor!\n", __func__);
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_rxfail(bus, false, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 			/* prepare the descriptor for the next read */
 			rd->len = rd->len_nxtfrm << 4;
@@ -2267,14 +2290,14 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 	if (ret)
 		goto done;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdiod_send_pkt(bus->sdiodev, pktq);
 	bus->sdcnt.f2txdata++;
 
 	if (ret < 0)
 		brcmf_sdio_txfail(bus);
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 done:
 	brcmf_sdio_txpkt_postp(bus, pktq);
@@ -2330,10 +2353,10 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		/* In poll mode, need to check for other events */
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			intstatus = brcmf_sdiod_readl(bus->sdiodev,
 						      intstat_addr, &ret);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 
 			bus->sdcnt.f2txdata++;
 			if (ret != 0)
@@ -2442,7 +2465,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 	}
 
 	if (sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 		/* Enable clock for device interrupts */
 		brcmf_sdio_bus_sleep(bus, false, false);
@@ -2477,7 +2500,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		brcmf_sdiod_writel(sdiodev, core->base + SD_REG(intstatus),
 				   local_hostintmask, NULL);
 
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 	}
 	/* Clear the data packet queues */
 	brcmu_pktq_flush(&bus->txq, true, NULL, NULL);
@@ -2552,7 +2575,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	brcmf_dbg(SDIO, "Enter\n");
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 
 	/* If waiting for HTAVAIL, check status */
 	if (!bus->sr_enabled && bus->clkstate == CLK_PENDING) {
@@ -2615,7 +2638,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		intstatus |= brcmf_sdio_hostmail(bus);
 	}
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* Generally don't ask for these, can get CRC errors... */
 	if (intstatus & I_WR_OOSYNC) {
@@ -2658,7 +2681,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	if (bus->ctrl_frame_stat && (bus->clkstate == CLK_AVAIL) &&
 	    data_ok(bus)) {
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			err = brcmf_sdio_tx_ctrlframe(bus,  bus->ctrl_frame_buf,
 						      bus->ctrl_frame_len);
@@ -2666,7 +2689,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 			wmb();
 			bus->ctrl_frame_stat = false;
 		}
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		brcmf_sdio_wait_event_wakeup(bus);
 	}
 	/* Send queued frames (limit 1 if rx may still be pending) */
@@ -2682,14 +2705,14 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		brcmf_err("failed backplane access over SDIO, halting operation\n");
 		atomic_set(&bus->intstatus, 0);
 		if (bus->ctrl_frame_stat) {
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			if (bus->ctrl_frame_stat) {
 				bus->ctrl_frame_err = -ENODEV;
 				wmb();
 				bus->ctrl_frame_stat = false;
 				brcmf_sdio_wait_event_wakeup(bus);
 			}
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 		}
 	} else if (atomic_read(&bus->intstatus) ||
 		   atomic_read(&bus->ipend) > 0 ||
@@ -2904,13 +2927,13 @@ brcmf_sdio_bus_txctl(struct device *dev, unsigned char *msg, uint msglen)
 					 CTL_DONE_TIMEOUT);
 	ret = 0;
 	if (bus->ctrl_frame_stat) {
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			brcmf_dbg(SDIO, "ctrl_frame timeout\n");
 			bus->ctrl_frame_stat = false;
 			ret = -ETIMEDOUT;
 		}
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 	}
 	if (!ret) {
 		brcmf_dbg(SDIO, "ctrl_frame complete, err=%d\n",
@@ -3048,7 +3071,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		return 0;
 	}
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	if (sh->assert_file_addr != 0) {
 		error = brcmf_sdiod_ramrw(bus->sdiodev, false,
 					  sh->assert_file_addr, (u8 *)file, 80);
@@ -3061,7 +3084,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		if (error < 0)
 			return error;
 	}
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	seq_printf(seq, "dongle assert: %s:%d: assert(%s)\n",
 		   file, sh->assert_line, expr);
@@ -3340,7 +3363,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 	int bcmerror;
 	u32 rstvec;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 
 	rstvec = get_unaligned_le32(fw->data);
@@ -3369,7 +3392,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 
 err:
 	brcmf_sdio_clkctl(bus, CLK_SDONLY, false);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 	return bcmerror;
 }
 
@@ -3564,7 +3587,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 
 	address = bus->ci->rambase;
 	offset = err = 0;
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	while (offset < mem_size) {
 		len = ((offset + MEMBLOCK) < mem_size) ? MEMBLOCK :
 		      mem_size - offset;
@@ -3580,7 +3603,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 	}
 
 done:
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return err;
 }
 
@@ -3637,10 +3660,11 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			if (!bus->dpc_triggered) {
 				u8 devpend;
 
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				devpend = brcmf_sdiod_func0_rb(bus->sdiodev,
 						  SDIO_CCCR_INTx, NULL);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				intstatus = devpend & (INTR_STATUS_FUNC1 |
 						       INTR_STATUS_FUNC2);
 			}
@@ -3666,13 +3690,13 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 		bus->console.count += jiffies_to_msecs(BRCMF_WD_POLL);
 		if (bus->console.count >= bus->console_interval) {
 			bus->console.count -= bus->console_interval;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			/* Make sure backplane clock is on */
 			brcmf_sdio_bus_sleep(bus, false, false);
 			if (brcmf_sdio_readconsole(bus) < 0)
 				/* stop on error */
 				bus->console_interval = 0;
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 		}
 	}
 #endif				/* DEBUG */
@@ -3685,11 +3709,12 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			bus->idlecount++;
 			if (bus->idlecount > bus->idletime) {
 				brcmf_dbg(SDIO, "idle\n");
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				bus->idlecount = 0;
 				brcmf_sdio_bus_sleep(bus, true, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 		} else {
 			bus->idlecount = 0;
@@ -3903,7 +3928,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	u32 drivestrength;
 
 	sdiodev = bus->sdiodev;
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	pr_debug("F1 signature read @0x18000000=0x%4x\n",
 		 brcmf_sdiod_readl(sdiodev, SI_ENUM_BASE, NULL));
@@ -4010,7 +4035,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	if (err)
 		goto fail;
 
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	brcmu_pktq_init(&bus->txq, (PRIOMASK + 1), TXQLEN);
 
@@ -4031,7 +4056,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	return true;
 
 fail:
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return false;
 }
 
@@ -4147,7 +4172,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	bus->sdcnt.tickcnt = 0;
 	brcmf_sdio_wd_timer(bus, true);
 
-	sdio_claim_host(sdiod->func1);
+	brcmf_sdio_claim_host(sdiod->func1, __func__);
 
 	/* Make sure backplane clock is on, needed to generate F2 interrupt */
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
@@ -4243,7 +4268,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 		goto checkdied;
 	}
 
-	sdio_release_host(sdiod->func1);
+	brcmf_sdio_release_host(sdiod->func1);
 
 	/* Assign bus interface call back */
 	sdiod->bus_if->dev = sdiod->dev;
@@ -4255,7 +4280,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	err = brcmf_attach(sdiod->dev, sdiod->settings);
 	if (err != 0) {
 		brcmf_err("brcmf_attach failed\n");
-		sdio_claim_host(sdiod->func1);
+		brcmf_sdio_claim_host(sdiod->func1, __func__);
 		goto checkdied;
 	}
 
@@ -4265,7 +4290,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 checkdied:
 	brcmf_sdio_checkdied(bus);
 release:
-	sdio_release_host(sdiod->func1);
+	brcmf_sdio_release_host(sdiod->func1);
 fail:
 	brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), err);
 	device_release_driver(&sdiod->func2->dev);
@@ -4361,7 +4386,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	bus->blocksize = bus->sdiodev->func2->cur_blksize;
 	bus->roundup = min(max_roundup, bus->blocksize);
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 
 	/* Disable F2 to clear any intermediate frame state on the dongle */
 	sdio_disable_func(bus->sdiodev->func2);
@@ -4371,7 +4396,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	/* Done with backplane-dependent accesses, can drop clock... */
 	brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, 0, NULL);
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* ...and initialize clock/power states */
 	bus->clkstate = CLK_SDONLY;
@@ -4428,7 +4453,8 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 
 		if (bus->ci) {
 			if (bus->sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 				/* Leave the device in state where it is
@@ -4438,7 +4464,7 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 				msleep(20);
 				brcmf_chip_set_passive(bus->ci);
 				brcmf_sdio_clkctl(bus, CLK_NONE, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 			brcmf_chip_detach(bus->ci);
 		}
@@ -4485,9 +4511,9 @@ int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep)
 {
 	int ret;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdio_bus_sleep(bus, sleep, false);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 34b031154da9..51ade937f5b0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -388,4 +388,7 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled);
 int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep);
 void brcmf_sdio_trigger_dpc(struct brcmf_sdio *bus);
 
+void brcmf_sdio_claim_host(struct sdio_func *func, const char *descr);
+void brcmf_sdio_release_host(struct sdio_func *func);
+
 #endif /* BRCMFMAC_SDIO_H */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..6ccc76150f45 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -267,6 +267,7 @@ struct mmc_supply {
 };
 
 struct mmc_ctx {
+	const char *descr;
 	struct task_struct *task;
 };
 
-- 
2.20.1

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
> I attached a patch that shows how I was able to determine what had
> already claimed the host.

I realized this morning that I had a flaw with my test patch that
diagnosed what was deadlocked. The mmc_ctx structure was allocated on
the stack. I attached a second version of that patch that uses
kmalloc() for that structure. It didn't change what I reported
yesterday: brcmf_sdiod_ramrw is deadlocked by
brcmf_sdio_download_firmware.


On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
> This is because SDHCI is using the IRQ thread to process the SDIO card
> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
> finish_tasklet") has moved the tasklet processing to the IRQ thread.
> 
> I would expect to be able to use the IRQ thread to complete requests, and it
> is desirable to do so because it is lower latency.
> 
> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
> is what other drivers are doing.
> 
> I will investigate some more and send a patch.

Thank you!

Brian


[-- Attachment #2: 0001-troubleshoot-broadcom-wireless-lockup-in-5.2rc1-v2.patch --]
[-- Type: text/x-diff, Size: 28428 bytes --]

>From acc78b5e581d2c3ebc994a6ad6e7b367f2b81935 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@onstation.org>
Date: Sun, 26 May 2019 15:36:40 -0400
Subject: [PATCH] troubleshoot broadcom wireless lockup in 5.2rc1 (v2)

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/mmc/core/core.c                       |  15 ++
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      |  46 ++---
 .../broadcom/brcm80211/brcmfmac/sdio.c        | 160 ++++++++++--------
 .../broadcom/brcm80211/brcmfmac/sdio.h        |   3 +
 include/linux/mmc/host.h                      |   1 +
 5 files changed, 136 insertions(+), 89 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..768acabf029b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -814,7 +814,22 @@ int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
 		if (stop || !host->claimed || mmc_ctx_matches(host, ctx, task))
 			break;
 		spin_unlock_irqrestore(&host->lock, flags);
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			WARN_ON(1);
+			dev_info(&host->class_dev, "%s: FIXME - before schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		schedule();
+
+		if (ctx != NULL && ctx->descr != NULL) {
+			dev_info(&host->class_dev, "%s: FIXME - after schedule() - descr=%s, claimer=%s\n",
+				 __func__, ctx->descr,
+				 host->claimer != NULL ? host->claimer->descr : NULL);
+		}
+
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 60aede5abb4d..fdeaeb1353af 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -47,6 +47,8 @@
 #include "sdio.h"
 #include "core.h"
 #include "common.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define SDIOH_API_ACCESS_RETRY_LIMIT	2
 
@@ -132,7 +134,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 		}
 		sdiodev->irq_wake = true;
 
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 		if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
 			/* assign GPIO to SDIO core */
@@ -159,13 +161,13 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 			data |= SDIO_CCCR_BRCM_SEPINT_ACT_HI;
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT,
 				     data, &ret);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 	} else {
 		brcmf_dbg(SDIO, "Entering\n");
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		sdio_claim_irq(sdiodev->func1, brcmf_sdiod_ib_irqhandler);
 		sdio_claim_irq(sdiodev->func2, brcmf_sdiod_dummy_irqhandler);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		sdiodev->sd_irq_requested = true;
 	}
 
@@ -183,10 +185,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 		struct brcmfmac_sdio_pd *pdata;
 
 		pdata = &sdiodev->settings->bus.sdio;
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
 		brcmf_sdiod_func0_wb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 
 		sdiodev->oob_irq_requested = false;
 		if (sdiodev->irq_wake) {
@@ -199,10 +201,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 	}
 
 	if (sdiodev->sd_irq_requested) {
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 		sdio_release_irq(sdiodev->func2);
 		sdio_release_irq(sdiodev->func1);
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		sdiodev->sd_irq_requested = false;
 	}
 }
@@ -695,7 +697,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	else
 		dsize = size;
 
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	/* Do the transfer(s) */
 	while (size) {
@@ -742,7 +744,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 	dev_kfree_skb(pkt);
 
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	return err;
 }
@@ -827,17 +829,17 @@ static int brcmf_sdiod_freezer_on(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdio_trigger_dpc(sdiodev->bus);
 	wait_event(sdiodev->freezer->thread_freeze,
 		   atomic_read(expect) == sdiodev->freezer->frozen_count);
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	res = brcmf_sdio_sleep(sdiodev->bus, true);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return res;
 }
 
 static void brcmf_sdiod_freezer_off(struct brcmf_sdio_dev *sdiodev)
 {
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	brcmf_sdio_sleep(sdiodev->bus, false);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	atomic_set(&sdiodev->freezer->freezing, 0);
 	complete_all(&sdiodev->freezer->resumed);
 }
@@ -887,14 +889,14 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 	brcmf_sdiod_freezer_detach(sdiodev);
 
 	/* Disable Function 2 */
-	sdio_claim_host(sdiodev->func2);
+	brcmf_sdio_claim_host(sdiodev->func2, __func__);
 	sdio_disable_func(sdiodev->func2);
-	sdio_release_host(sdiodev->func2);
+	brcmf_sdio_release_host(sdiodev->func2);
 
 	/* Disable Function 1 */
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	sdio_disable_func(sdiodev->func1);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	sg_free_table(&sdiodev->sgtable);
 	sdiodev->sbwad = 0;
@@ -915,18 +917,18 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 {
 	int ret = 0;
 
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	ret = sdio_set_block_size(sdiodev->func1, SDIO_FUNC1_BLOCKSIZE);
 	if (ret) {
 		brcmf_err("Failed to set F1 blocksize\n");
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		goto out;
 	}
 	ret = sdio_set_block_size(sdiodev->func2, SDIO_FUNC2_BLOCKSIZE);
 	if (ret) {
 		brcmf_err("Failed to set F2 blocksize\n");
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 		goto out;
 	}
 
@@ -935,7 +937,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 
 	/* Enable Function 1 */
 	ret = sdio_enable_func(sdiodev->func1);
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	if (ret) {
 		brcmf_err("Failed to enable F1: err=%d\n", ret);
 		goto out;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 22b73da42822..ba836aa5da78 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -45,6 +45,8 @@
 #include "core.h"
 #include "common.h"
 #include "bcdc.h"
+#include "../../../../../mmc/core/host.h"
+#include "../../../../../mmc/core/core.h"
 
 #define DCMD_RESP_TIMEOUT	msecs_to_jiffies(2500)
 #define CTL_DONE_TIMEOUT	msecs_to_jiffies(2500)
@@ -651,6 +653,25 @@ static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
 	BRCMF_FW_ENTRY(CY_CC_43012_CHIP_ID, 0xFFFFFFFF, 43012)
 };
 
+void brcmf_sdio_claim_host(struct sdio_func *func, const char *descr)
+{
+	struct mmc_ctx *mmc_ctx;
+
+	mmc_ctx = kmalloc(sizeof(*mmc_ctx), GFP_KERNEL);
+	mmc_ctx->task = NULL;
+	mmc_ctx->descr = descr;
+	__mmc_claim_host(func->card->host, mmc_ctx, NULL);
+}
+
+void brcmf_sdio_release_host(struct sdio_func *func)
+{
+	struct mmc_ctx *tofree;
+
+	tofree = func->card->host->claimer;
+	sdio_release_host(func);
+	kfree(tofree);
+}
+
 static void pkt_align(struct sk_buff *p, int len, int align)
 {
 	uint datalign;
@@ -995,7 +1016,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	struct sdpcm_shared_le sh_le;
 	__le32 addr_le;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	brcmf_sdio_bus_sleep(bus, false, false);
 
 	/*
@@ -1029,7 +1050,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 	if (rv < 0)
 		goto fail;
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* Endianness */
 	sh->flags = le32_to_cpu(sh_le.flags);
@@ -1051,7 +1072,7 @@ static int brcmf_sdio_readshared(struct brcmf_sdio *bus,
 fail:
 	brcmf_err("unable to obtain sdpcm_shared info: rv=%d (addr=0x%x)\n",
 		  rv, addr);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 	return rv;
 }
 
@@ -1583,10 +1604,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 		 * read directly into the chained packet, or allocate a large
 		 * packet and and copy into the chain.
 		 */
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdiod_recv_chain(bus->sdiodev,
 						 &bus->glom, dlen);
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		bus->sdcnt.f2rxdata++;
 
 		/* On failure, kill the superframe */
@@ -1594,11 +1615,11 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 			brcmf_err("glom read of %d bytes failed: %d\n",
 				  dlen, errcode);
 
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			return 0;
 		}
 
@@ -1608,10 +1629,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		rd_new.seq_num = rxseq;
 		rd_new.len = dlen;
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		errcode = brcmf_sdio_hdparse(bus, pfirst->data, &rd_new,
 					     BRCMF_SDIO_FT_SUPER);
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		bus->cur_read.len = rd_new.len_nxtfrm << 4;
 
 		/* Remove superframe header, remember offset */
@@ -1626,10 +1647,10 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 			rd_new.len = pnext->len;
 			rd_new.seq_num = rxseq++;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			errcode = brcmf_sdio_hdparse(bus, pnext->data, &rd_new,
 						     BRCMF_SDIO_FT_SUB);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			brcmf_dbg_hex_dump(BRCMF_GLOM_ON(),
 					   pnext->data, 32, "subframe:\n");
 
@@ -1638,11 +1659,11 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 
 		if (errcode) {
 			/* Terminate frame on error */
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true, false);
 			bus->sdcnt.rxglomfail++;
 			brcmf_sdio_free_glom(bus);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			bus->cur_read.len = 0;
 			return 0;
 		}
@@ -1849,7 +1870,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		rd->len_left = rd->len;
 		/* read header first for unknow frame length */
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (!rd->len) {
 			ret = brcmf_sdiod_recv_buf(bus->sdiodev,
 						   bus->rxhdr, BRCMF_FIRSTREAD);
@@ -1859,7 +1880,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  ret);
 				bus->sdcnt.rx_hdrfail++;
 				brcmf_sdio_rxfail(bus, true, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				continue;
 			}
 
@@ -1869,7 +1890,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, rd,
 					       BRCMF_SDIO_FT_NORMAL)) {
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				if (!bus->rxpending)
 					break;
 				else
@@ -1885,7 +1906,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 				rd->len_nxtfrm = 0;
 				/* treat all packet as event if we don't know */
 				rd->channel = SDPCM_EVENT_CHANNEL;
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				continue;
 			}
 			rd->len_left = rd->len > BRCMF_FIRSTREAD ?
@@ -1902,7 +1923,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			brcmf_err("brcmu_pkt_buf_get_skb failed\n");
 			brcmf_sdio_rxfail(bus, false,
 					    RETRYCHAN(rd->channel));
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			continue;
 		}
 		skb_pull(pkt, head_read);
@@ -1910,16 +1931,16 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 
 		ret = brcmf_sdiod_recv_pkt(bus->sdiodev, pkt);
 		bus->sdcnt.f2rxdata++;
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 
 		if (ret < 0) {
 			brcmf_err("read %d bytes from channel %d failed: %d\n",
 				  rd->len, rd->channel, ret);
 			brcmu_pkt_buf_free_skb(pkt);
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			brcmf_sdio_rxfail(bus, true,
 					    RETRYCHAN(rd->channel));
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			continue;
 		}
 
@@ -1930,7 +1951,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 		} else {
 			memcpy(bus->rxhdr, pkt->data, SDPCM_HDRLEN);
 			rd_new.seq_num = rd->seq_num;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			if (brcmf_sdio_hdparse(bus, bus->rxhdr, &rd_new,
 					       BRCMF_SDIO_FT_NORMAL)) {
 				rd->len = 0;
@@ -1943,11 +1964,11 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  roundup(rd_new.len, 16) >> 4);
 				rd->len = 0;
 				brcmf_sdio_rxfail(bus, true, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
 				continue;
 			}
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 			rd->len_nxtfrm = rd_new.len_nxtfrm;
 			rd->channel = rd_new.channel;
 			rd->dat_offset = rd_new.dat_offset;
@@ -1963,9 +1984,10 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					  rd_new.seq_num);
 				/* Force retry w/normal header read */
 				rd->len = 0;
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_rxfail(bus, false, true);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				brcmu_pkt_buf_free_skb(pkt);
 				continue;
 			}
@@ -1988,9 +2010,10 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 			} else {
 				brcmf_err("%s: glom superframe w/o "
 					  "descriptor!\n", __func__);
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_rxfail(bus, false, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 			/* prepare the descriptor for the next read */
 			rd->len = rd->len_nxtfrm << 4;
@@ -2267,14 +2290,14 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 	if (ret)
 		goto done;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdiod_send_pkt(bus->sdiodev, pktq);
 	bus->sdcnt.f2txdata++;
 
 	if (ret < 0)
 		brcmf_sdio_txfail(bus);
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 done:
 	brcmf_sdio_txpkt_postp(bus, pktq);
@@ -2330,10 +2353,10 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		/* In poll mode, need to check for other events */
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			intstatus = brcmf_sdiod_readl(bus->sdiodev,
 						      intstat_addr, &ret);
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 
 			bus->sdcnt.f2txdata++;
 			if (ret != 0)
@@ -2442,7 +2465,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 	}
 
 	if (sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-		sdio_claim_host(sdiodev->func1);
+		brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 		/* Enable clock for device interrupts */
 		brcmf_sdio_bus_sleep(bus, false, false);
@@ -2477,7 +2500,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		brcmf_sdiod_writel(sdiodev, core->base + SD_REG(intstatus),
 				   local_hostintmask, NULL);
 
-		sdio_release_host(sdiodev->func1);
+		brcmf_sdio_release_host(sdiodev->func1);
 	}
 	/* Clear the data packet queues */
 	brcmu_pktq_flush(&bus->txq, true, NULL, NULL);
@@ -2552,7 +2575,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	brcmf_dbg(SDIO, "Enter\n");
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 
 	/* If waiting for HTAVAIL, check status */
 	if (!bus->sr_enabled && bus->clkstate == CLK_PENDING) {
@@ -2615,7 +2638,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		intstatus |= brcmf_sdio_hostmail(bus);
 	}
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* Generally don't ask for these, can get CRC errors... */
 	if (intstatus & I_WR_OOSYNC) {
@@ -2658,7 +2681,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 	if (bus->ctrl_frame_stat && (bus->clkstate == CLK_AVAIL) &&
 	    data_ok(bus)) {
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			err = brcmf_sdio_tx_ctrlframe(bus,  bus->ctrl_frame_buf,
 						      bus->ctrl_frame_len);
@@ -2666,7 +2689,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 			wmb();
 			bus->ctrl_frame_stat = false;
 		}
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 		brcmf_sdio_wait_event_wakeup(bus);
 	}
 	/* Send queued frames (limit 1 if rx may still be pending) */
@@ -2682,14 +2705,14 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		brcmf_err("failed backplane access over SDIO, halting operation\n");
 		atomic_set(&bus->intstatus, 0);
 		if (bus->ctrl_frame_stat) {
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			if (bus->ctrl_frame_stat) {
 				bus->ctrl_frame_err = -ENODEV;
 				wmb();
 				bus->ctrl_frame_stat = false;
 				brcmf_sdio_wait_event_wakeup(bus);
 			}
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 		}
 	} else if (atomic_read(&bus->intstatus) ||
 		   atomic_read(&bus->ipend) > 0 ||
@@ -2904,13 +2927,13 @@ brcmf_sdio_bus_txctl(struct device *dev, unsigned char *msg, uint msglen)
 					 CTL_DONE_TIMEOUT);
 	ret = 0;
 	if (bus->ctrl_frame_stat) {
-		sdio_claim_host(bus->sdiodev->func1);
+		brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 		if (bus->ctrl_frame_stat) {
 			brcmf_dbg(SDIO, "ctrl_frame timeout\n");
 			bus->ctrl_frame_stat = false;
 			ret = -ETIMEDOUT;
 		}
-		sdio_release_host(bus->sdiodev->func1);
+		brcmf_sdio_release_host(bus->sdiodev->func1);
 	}
 	if (!ret) {
 		brcmf_dbg(SDIO, "ctrl_frame complete, err=%d\n",
@@ -3048,7 +3071,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		return 0;
 	}
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	if (sh->assert_file_addr != 0) {
 		error = brcmf_sdiod_ramrw(bus->sdiodev, false,
 					  sh->assert_file_addr, (u8 *)file, 80);
@@ -3061,7 +3084,7 @@ static int brcmf_sdio_assert_info(struct seq_file *seq, struct brcmf_sdio *bus,
 		if (error < 0)
 			return error;
 	}
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	seq_printf(seq, "dongle assert: %s:%d: assert(%s)\n",
 		   file, sh->assert_line, expr);
@@ -3340,7 +3363,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 	int bcmerror;
 	u32 rstvec;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 
 	rstvec = get_unaligned_le32(fw->data);
@@ -3369,7 +3392,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 
 err:
 	brcmf_sdio_clkctl(bus, CLK_SDONLY, false);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 	return bcmerror;
 }
 
@@ -3564,7 +3587,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 
 	address = bus->ci->rambase;
 	offset = err = 0;
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 	while (offset < mem_size) {
 		len = ((offset + MEMBLOCK) < mem_size) ? MEMBLOCK :
 		      mem_size - offset;
@@ -3580,7 +3603,7 @@ static int brcmf_sdio_bus_get_memdump(struct device *dev, void *data,
 	}
 
 done:
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return err;
 }
 
@@ -3637,10 +3660,11 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			if (!bus->dpc_triggered) {
 				u8 devpend;
 
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				devpend = brcmf_sdiod_func0_rb(bus->sdiodev,
 						  SDIO_CCCR_INTx, NULL);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 				intstatus = devpend & (INTR_STATUS_FUNC1 |
 						       INTR_STATUS_FUNC2);
 			}
@@ -3666,13 +3690,13 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 		bus->console.count += jiffies_to_msecs(BRCMF_WD_POLL);
 		if (bus->console.count >= bus->console_interval) {
 			bus->console.count -= bus->console_interval;
-			sdio_claim_host(bus->sdiodev->func1);
+			brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 			/* Make sure backplane clock is on */
 			brcmf_sdio_bus_sleep(bus, false, false);
 			if (brcmf_sdio_readconsole(bus) < 0)
 				/* stop on error */
 				bus->console_interval = 0;
-			sdio_release_host(bus->sdiodev->func1);
+			brcmf_sdio_release_host(bus->sdiodev->func1);
 		}
 	}
 #endif				/* DEBUG */
@@ -3685,11 +3709,12 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			bus->idlecount++;
 			if (bus->idlecount > bus->idletime) {
 				brcmf_dbg(SDIO, "idle\n");
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				bus->idlecount = 0;
 				brcmf_sdio_bus_sleep(bus, true, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 		} else {
 			bus->idlecount = 0;
@@ -3903,7 +3928,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	u32 drivestrength;
 
 	sdiodev = bus->sdiodev;
-	sdio_claim_host(sdiodev->func1);
+	brcmf_sdio_claim_host(sdiodev->func1, __func__);
 
 	pr_debug("F1 signature read @0x18000000=0x%4x\n",
 		 brcmf_sdiod_readl(sdiodev, SI_ENUM_BASE, NULL));
@@ -4010,7 +4035,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	if (err)
 		goto fail;
 
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 
 	brcmu_pktq_init(&bus->txq, (PRIOMASK + 1), TXQLEN);
 
@@ -4031,7 +4056,7 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	return true;
 
 fail:
-	sdio_release_host(sdiodev->func1);
+	brcmf_sdio_release_host(sdiodev->func1);
 	return false;
 }
 
@@ -4147,7 +4172,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	bus->sdcnt.tickcnt = 0;
 	brcmf_sdio_wd_timer(bus, true);
 
-	sdio_claim_host(sdiod->func1);
+	brcmf_sdio_claim_host(sdiod->func1, __func__);
 
 	/* Make sure backplane clock is on, needed to generate F2 interrupt */
 	brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
@@ -4243,7 +4268,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 		goto checkdied;
 	}
 
-	sdio_release_host(sdiod->func1);
+	brcmf_sdio_release_host(sdiod->func1);
 
 	/* Assign bus interface call back */
 	sdiod->bus_if->dev = sdiod->dev;
@@ -4255,7 +4280,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	err = brcmf_attach(sdiod->dev, sdiod->settings);
 	if (err != 0) {
 		brcmf_err("brcmf_attach failed\n");
-		sdio_claim_host(sdiod->func1);
+		brcmf_sdio_claim_host(sdiod->func1, __func__);
 		goto checkdied;
 	}
 
@@ -4265,7 +4290,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 checkdied:
 	brcmf_sdio_checkdied(bus);
 release:
-	sdio_release_host(sdiod->func1);
+	brcmf_sdio_release_host(sdiod->func1);
 fail:
 	brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), err);
 	device_release_driver(&sdiod->func2->dev);
@@ -4361,7 +4386,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	bus->blocksize = bus->sdiodev->func2->cur_blksize;
 	bus->roundup = min(max_roundup, bus->blocksize);
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 
 	/* Disable F2 to clear any intermediate frame state on the dongle */
 	sdio_disable_func(bus->sdiodev->func2);
@@ -4371,7 +4396,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	/* Done with backplane-dependent accesses, can drop clock... */
 	brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, 0, NULL);
 
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	/* ...and initialize clock/power states */
 	bus->clkstate = CLK_SDONLY;
@@ -4428,7 +4453,8 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 
 		if (bus->ci) {
 			if (bus->sdiodev->state != BRCMF_SDIOD_NOMEDIUM) {
-				sdio_claim_host(bus->sdiodev->func1);
+				brcmf_sdio_claim_host(bus->sdiodev->func1,
+						      __func__);
 				brcmf_sdio_wd_timer(bus, false);
 				brcmf_sdio_clkctl(bus, CLK_AVAIL, false);
 				/* Leave the device in state where it is
@@ -4438,7 +4464,7 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
 				msleep(20);
 				brcmf_chip_set_passive(bus->ci);
 				brcmf_sdio_clkctl(bus, CLK_NONE, false);
-				sdio_release_host(bus->sdiodev->func1);
+				brcmf_sdio_release_host(bus->sdiodev->func1);
 			}
 			brcmf_chip_detach(bus->ci);
 		}
@@ -4485,9 +4511,9 @@ int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep)
 {
 	int ret;
 
-	sdio_claim_host(bus->sdiodev->func1);
+	brcmf_sdio_claim_host(bus->sdiodev->func1, __func__);
 	ret = brcmf_sdio_bus_sleep(bus, sleep, false);
-	sdio_release_host(bus->sdiodev->func1);
+	brcmf_sdio_release_host(bus->sdiodev->func1);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 34b031154da9..51ade937f5b0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -388,4 +388,7 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled);
 int brcmf_sdio_sleep(struct brcmf_sdio *bus, bool sleep);
 void brcmf_sdio_trigger_dpc(struct brcmf_sdio *bus);
 
+void brcmf_sdio_claim_host(struct sdio_func *func, const char *descr);
+void brcmf_sdio_release_host(struct sdio_func *func);
+
 #endif /* BRCMFMAC_SDIO_H */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..6ccc76150f45 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -267,6 +267,7 @@ struct mmc_supply {
 };
 
 struct mmc_ctx {
+	const char *descr;
 	struct task_struct *task;
 };
 
-- 
2.20.1

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Adrian Hunter]

On 27/05/19 12:37 PM, Brian Masney wrote:
> On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
>> I attached a patch that shows how I was able to determine what had
>> already claimed the host.
> On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
>> This is because SDHCI is using the IRQ thread to process the SDIO card
>> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
>> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
>> finish_tasklet") has moved the tasklet processing to the IRQ thread.
>>
>> I would expect to be able to use the IRQ thread to complete requests, and it
>> is desirable to do so because it is lower latency.
>>
>> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
>> is what other drivers are doing.
>>
>> I will investigate some more and send a patch.

Please try the patch below:

From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 27 May 2019 14:45:55 +0300
Subject: [PATCH] mmc: sdhci: Fix SDIO IRQ thread deadlock

Since commit c07a48c26519 ("mmc: sdhci: Remove finish_tasklet"), the IRQ
thread might be used to complete requests, but the IRQ thread is also used
to process SDIO card interrupts. This can cause a deadlock when the SDIO
processing tries to access the card since that would also require the IRQ
thread. Change SDHCI to use sdio_signal_irq() to schedule a work item
instead. That also requires implementing the ->ack_sdio_irq() mmc host op.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Fixes: c07a48c26519 ("mmc: sdhci: Remove finish_tasklet")
---
 drivers/mmc/host/sdhci.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 97158344b862..0cd5f2ce98df 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2137,6 +2137,17 @@ void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 }
 EXPORT_SYMBOL_GPL(sdhci_enable_sdio_irq);
 
+static void sdhci_ack_sdio_irq(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->lock, flags);
+	if (host->flags & SDHCI_SDIO_IRQ_ENABLED)
+		sdhci_enable_sdio_irq_nolock(host, true);
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+
 int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 				      struct mmc_ios *ios)
 {
@@ -2585,6 +2596,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.get_ro		= sdhci_get_ro,
 	.hw_reset	= sdhci_hw_reset,
 	.enable_sdio_irq = sdhci_enable_sdio_irq,
+	.ack_sdio_irq    = sdhci_ack_sdio_irq,
 	.start_signal_voltage_switch	= sdhci_start_signal_voltage_switch,
 	.prepare_hs400_tuning		= sdhci_prepare_hs400_tuning,
 	.execute_tuning			= sdhci_execute_tuning,
@@ -3087,8 +3099,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 		if ((intmask & SDHCI_INT_CARD_INT) &&
 		    (host->ier & SDHCI_INT_CARD_INT)) {
 			sdhci_enable_sdio_irq_nolock(host, false);
-			host->thread_isr |= SDHCI_INT_CARD_INT;
-			result = IRQ_WAKE_THREAD;
+			sdio_signal_irq(host->mmc);
 		}
 
 		intmask &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
@@ -3160,15 +3171,6 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
 		mmc_detect_change(mmc, msecs_to_jiffies(200));
 	}
 
-	if (isr & SDHCI_INT_CARD_INT) {
-		sdio_run_irqs(host->mmc);
-
-		spin_lock_irqsave(&host->lock, flags);
-		if (host->flags & SDHCI_SDIO_IRQ_ENABLED)
-			sdhci_enable_sdio_irq_nolock(host, true);
-		spin_unlock_irqrestore(&host->lock, flags);
-	}
-
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Brian Masney]

On Mon, May 27, 2019 at 03:08:07PM +0300, Adrian Hunter wrote:
> On 27/05/19 12:37 PM, Brian Masney wrote:
> > On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
> >> I attached a patch that shows how I was able to determine what had
> >> already claimed the host.
> > On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
> >> This is because SDHCI is using the IRQ thread to process the SDIO card
> >> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
> >> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
> >> finish_tasklet") has moved the tasklet processing to the IRQ thread.
> >>
> >> I would expect to be able to use the IRQ thread to complete requests, and it
> >> is desirable to do so because it is lower latency.
> >>
> >> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
> >> is what other drivers are doing.
> >>
> >> I will investigate some more and send a patch.
> 
> Please try the patch below:
> 
> From: Adrian Hunter <adrian.hunter@intel.com>
> Date: Mon, 27 May 2019 14:45:55 +0300
> Subject: [PATCH] mmc: sdhci: Fix SDIO IRQ thread deadlock
> 
> Since commit c07a48c26519 ("mmc: sdhci: Remove finish_tasklet"), the IRQ
> thread might be used to complete requests, but the IRQ thread is also used
> to process SDIO card interrupts. This can cause a deadlock when the SDIO
> processing tries to access the card since that would also require the IRQ
> thread. Change SDHCI to use sdio_signal_irq() to schedule a work item
> instead. That also requires implementing the ->ack_sdio_irq() mmc host op.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Fixes: c07a48c26519 ("mmc: sdhci: Remove finish_tasklet")

Yes, this fixes the issue for me. You can add my:

Reported-by: Brian Masney <masneyb@onstation.org>
Tested-by: Brian Masney <masneyb@onstation.org>

Thanks,

Brian

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Ulf Hansson]

On Mon, 27 May 2019 at 14:50, Brian Masney <masneyb@onstation.org> wrote:
>
> On Mon, May 27, 2019 at 03:08:07PM +0300, Adrian Hunter wrote:
> > On 27/05/19 12:37 PM, Brian Masney wrote:
> > > On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
> > >> I attached a patch that shows how I was able to determine what had
> > >> already claimed the host.
> > > On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
> > >> This is because SDHCI is using the IRQ thread to process the SDIO card
> > >> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
> > >> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
> > >> finish_tasklet") has moved the tasklet processing to the IRQ thread.
> > >>
> > >> I would expect to be able to use the IRQ thread to complete requests, and it
> > >> is desirable to do so because it is lower latency.
> > >>
> > >> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
> > >> is what other drivers are doing.
> > >>
> > >> I will investigate some more and send a patch.
> >
> > Please try the patch below:
> >
> > From: Adrian Hunter <adrian.hunter@intel.com>
> > Date: Mon, 27 May 2019 14:45:55 +0300
> > Subject: [PATCH] mmc: sdhci: Fix SDIO IRQ thread deadlock
> >
> > Since commit c07a48c26519 ("mmc: sdhci: Remove finish_tasklet"), the IRQ
> > thread might be used to complete requests, but the IRQ thread is also used
> > to process SDIO card interrupts. This can cause a deadlock when the SDIO
> > processing tries to access the card since that would also require the IRQ
> > thread. Change SDHCI to use sdio_signal_irq() to schedule a work item
> > instead. That also requires implementing the ->ack_sdio_irq() mmc host op.
> >
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > Fixes: c07a48c26519 ("mmc: sdhci: Remove finish_tasklet")
>
> Yes, this fixes the issue for me. You can add my:
>
> Reported-by: Brian Masney <masneyb@onstation.org>
> Tested-by: Brian Masney <masneyb@onstation.org>
>

Applied for fixes, thanks!

Kind regards
Uffe

Re: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer_done())

[Arend Van Spriel]

On 5/27/2019 2:08 PM, Adrian Hunter wrote:
> On 27/05/19 12:37 PM, Brian Masney wrote:
>> On Sun, May 26, 2019 at 03:58:19PM -0400, Brian Masney wrote:
>>> I attached a patch that shows how I was able to determine what had
>>> already claimed the host.
>> On Mon, May 27, 2019 at 10:48:24AM +0300, Adrian Hunter wrote:
>>> This is because SDHCI is using the IRQ thread to process the SDIO card
>>> interrupt (sdio_run_irqs()).  When the card driver tries to use the card, it
>>> causes interrupts which deadlocks since c07a48c26519 ("mmc: sdhci: Remove
>>> finish_tasklet") has moved the tasklet processing to the IRQ thread.
>>>
>>> I would expect to be able to use the IRQ thread to complete requests, and it
>>> is desirable to do so because it is lower latency.
>>>
>>> Probably, SDHCI should use sdio_signal_irq() which queues a work item, and
>>> is what other drivers are doing.
>>>
>>> I will investigate some more and send a patch.
> 
> Please try the patch below:

Finally got time to update my kernel to 5.2-rc2. This patch indeed 
resolves the issue.

Thanks,
Arend