From abdd5dcc00207e7c38680f3754d1bfffafff1093 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 17 Jun 2019 10:56:49 -0700 Subject: Revert "brcmfmac: disable command decode in sdio_aos" This reverts commit 29f6589140a10ece8c1d73f58043ea5b3473ab3e. After that patch landed I find that my kernel log on rk3288-veyron-minnie and rk3288-veyron-speedy is filled with: brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep state -110 This seems to happen every time the Broadcom WiFi transitions out of sleep mode. Reverting the commit fixes the problem for me, so that's what this patch does. Note that, in general, the justification in the original commit seemed a little weak. It looked like someone was testing on a SD card controller that would sometimes die if there were CRC errors on the bus. This used to happen back in early days of dw_mmc (the controller on my boards), but we fixed it. Disabling a feature on all boards just because one SD card controller is broken seems bad. Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos") Cc: Wright Feng Cc: Double Lo Cc: Madhan Mohan R Cc: Chi-Hsien Lin Signed-off-by: Douglas Anderson Cc: stable@vger.kernel.org Acked-by: Kalle Valo Signed-off-by: Ulf Hansson --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 22b73da42822..b6466f3b38f4 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -3375,11 +3375,7 @@ err: static bool brcmf_sdio_aos_no_decode(struct brcmf_sdio *bus) { - if (bus->ci->chip == CY_CC_43012_CHIP_ID || - bus->ci->chip == CY_CC_4373_CHIP_ID || - bus->ci->chip == BRCM_CC_4339_CHIP_ID || - bus->ci->chip == BRCM_CC_4345_CHIP_ID || - bus->ci->chip == BRCM_CC_4354_CHIP_ID) + if (bus->ci->chip == CY_CC_43012_CHIP_ID) return true; else return false; -- cgit v1.2.3 From 2de0b42da263c97d330d276f5ccf7c4470e3324f Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 17 Jun 2019 10:56:51 -0700 Subject: brcmfmac: sdio: Disable auto-tuning around commands expected to fail There are certain cases, notably when transitioning between sleep and active state, when Broadcom SDIO WiFi cards will produce errors on the SDIO bus. This is evident from the source code where you can see that we try commands in a loop until we either get success or we've tried too many times. The comment in the code reinforces this by saying "just one write attempt may fail" Unfortunately these failures sometimes end up causing an "-EILSEQ" back to the core which triggers a retuning of the SDIO card and that blocks all traffic to the card until it's done. Let's disable retuning around the commands we expect might fail. Cc: stable@vger.kernel.org #v4.18+ Signed-off-by: Douglas Anderson Acked-by: Adrian Hunter Reviewed-by: Arend van Spriel Acked-by: Kalle Valo Signed-off-by: Ulf Hansson --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/net') diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index b6466f3b38f4..2ad8bdb7e073 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -678,6 +678,8 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on) brcmf_dbg(TRACE, "Enter: on=%d\n", on); + sdio_retune_crc_disable(bus->sdiodev->func1); + wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT); /* 1st KSO write goes to AOS wake up core if device is asleep */ brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err); @@ -738,6 +740,8 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on) if (try_cnt > MAX_KSO_ATTEMPTS) brcmf_err("max tries: rd_val=0x%x err=%d\n", rd_val, err); + sdio_retune_crc_enable(bus->sdiodev->func1); + return err; } -- cgit v1.2.3 From 65dade6044079a5c206fd1803642ff420061417a Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 17 Jun 2019 10:56:53 -0700 Subject: brcmfmac: sdio: Don't tune while the card is off When Broadcom SDIO cards are idled they go to sleep and a whole separate subsystem takes over their SDIO communication. This is the Always-On-Subsystem (AOS) and it can't handle tuning requests. Specifically, as tested on rk3288-veyron-minnie (which reports having BCM4354/1 in dmesg), if I force a retune in brcmf_sdio_kso_control() when "on = 1" (aka we're transition from sleep to wake) by whacking: bus->sdiodev->func1->card->host->need_retune = 1 ...then I can often see tuning fail. In this case dw_mmc reports "All phases bad!"). Note that I don't get 100% failure, presumably because sometimes the card itself has already transitioned away from the AOS itself by the time we try to wake it up. If I force retuning when "on = 0" (AKA force retuning right before sending the command to go to sleep) then retuning is always OK. NOTE: we need _both_ this patch and the patch to avoid triggering tuning due to CRC errors in the sleep/wake transition, AKA ("brcmfmac: sdio: Disable auto-tuning around commands expected to fail"). Though both patches handle issues with Broadcom's AOS, the problems are distinct: 1. We want to defer (but not ignore) asynchronous (like timer-requested) tuning requests till the card is awake. However, we want to ignore CRC errors during the transition, we don't want to queue deferred tuning request. 2. You could imagine that the AOS could implement retuning but we could still get errors while transitioning in and out of the AOS. Similarly you could imagine a seamless transition into and out of the AOS (with no CRC errors) even if the AOS couldn't handle tuning. ALSO NOTE: presumably there is never a desperate need to retune in order to wake up the card, since doing so is impossible. Luckily the only way the card can get into sleep state is if we had a good enough tuning to send it the command to put it into sleep, so presumably that "good enough" tuning is enough to wake us up, at least with a few retries. Cc: stable@vger.kernel.org #v4.18+ Signed-off-by: Douglas Anderson Acked-by: Adrian Hunter Reviewed-by: Arend van Spriel Acked-by: Kalle Valo Signed-off-by: Ulf Hansson --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/net') diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 2ad8bdb7e073..9a51f1ba87c3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -680,6 +680,10 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on) sdio_retune_crc_disable(bus->sdiodev->func1); + /* Cannot re-tune if device is asleep; defer till we're awake */ + if (on) + sdio_retune_hold_now(bus->sdiodev->func1); + wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT); /* 1st KSO write goes to AOS wake up core if device is asleep */ brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err); @@ -740,6 +744,9 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on) if (try_cnt > MAX_KSO_ATTEMPTS) brcmf_err("max tries: rd_val=0x%x err=%d\n", rd_val, err); + if (on) + sdio_retune_release(bus->sdiodev->func1); + sdio_retune_crc_enable(bus->sdiodev->func1); return err; -- cgit v1.2.3