netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Ulf Hansson <ulf.hansson@linaro.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: brcm80211-dev-list.pdl@broadcom.com,
	linux-rockchip@lists.infradead.org,
	Double Lo <double.lo@cypress.com>,
	briannorris@chromium.org, linux-wireless@vger.kernel.org,
	Naveen Gupta <naveen.gupta@cypress.com>,
	Madhan Mohan R <madhanmohan.r@cypress.com>,
	mka@chromium.org, Wright Feng <wright.feng@cypress.com>,
	Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
	netdev@vger.kernel.org, brcm80211-dev-list@cypress.com,
	Douglas Anderson <dianders@chromium.org>,
	stable@vger.kernel.org, Franky Lin <franky.lin@broadcom.com>,
	linux-kernel@vger.kernel.org,
	Hante Meuleman <hante.meuleman@broadcom.com>,
	Ondrej Jirman <megous@megous.com>,
	YueHaibing <yuehaibing@huawei.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH v5 5/5] brcmfmac: sdio: Don't tune while the card is off
Date: Mon, 17 Jun 2019 10:56:53 -0700	[thread overview]
Message-ID: <20190617175653.21756-6-dianders@chromium.org> (raw)
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

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
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Patches #2 - #5 will go through Ulf's tree.

This patch is still lacking Kalle Valo's Ack, which should probably be
received before landing in Ulf's tree.

I've CCed stable@ here without a version tag.  As per Adrian Hunter
this patch applies cleanly to 4.18+ so that would be an easy first
target.  However, if someone were so inclined they could provide
further backports.  As per Adrian [1] the root problem has existed for
~4 years.

[1] https://lkml.kernel.org/r/4f39e152-04ba-a64e-985a-df93e6d15ff8@intel.com

Changes in v5:
- Rewording of "sleep command" in commit message (Arend).

Changes in v4:
- Adjust to API rename (Adrian).

Changes in v3:
- ("brcmfmac: sdio: Don't tune while the card is off") new for v3.

Changes in v2: None

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ee76593259a7..629140b6d7e2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -669,6 +669,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);
@@ -729,6 +733,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;
-- 
2.22.0.410.gd8fdbe21b5-goog


  parent reply	other threads:[~2019-06-17 17:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-17 17:56 [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle Douglas Anderson
2019-06-17 17:56 ` [PATCH v5 1/5] Revert "brcmfmac: disable command decode in sdio_aos" Douglas Anderson
2019-06-17 17:56 ` [PATCH v5 2/5] mmc: core: API to temporarily disable retuning for SDIO CRC errors Douglas Anderson
2019-06-17 17:56 ` [PATCH v5 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail Douglas Anderson
2019-06-17 17:56 ` [PATCH v5 4/5] mmc: core: Add sdio_retune_hold_now() and sdio_retune_release() Douglas Anderson
2019-06-17 17:56 ` Douglas Anderson [this message]
2019-06-17 18:38 ` [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle Ulf Hansson
2019-06-17 18:46   ` Doug Anderson
2019-06-18 11:02   ` Kalle Valo
2019-06-18 11:40     ` Ulf Hansson
     [not found]     ` <CAPDyKFoE0+KNBT5j3_VpJKcztghVa-eFJhy8887bZcUk8bfN2Q@mail.gmail.com>
2019-06-18 11:47       ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190617175653.21756-6-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=adrian.hunter@intel.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=brcm80211-dev-list@cypress.com \
    --cc=briannorris@chromium.org \
    --cc=chi-hsien.lin@cypress.com \
    --cc=davem@davemloft.net \
    --cc=double.lo@cypress.com \
    --cc=franky.lin@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=madhanmohan.r@cypress.com \
    --cc=megous@megous.com \
    --cc=mka@chromium.org \
    --cc=naveen.gupta@cypress.com \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=wright.feng@cypress.com \
    --cc=yuehaibing@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).