From: Kalle Valo <kvalo@codeaurora.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Douglas Anderson" <dianders@chromium.org>,
"Adrian Hunter" <adrian.hunter@intel.com>,
"Arend van Spriel" <arend.vanspriel@broadcom.com>,
brcm80211-dev-list.pdl@broadcom.com,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
"Double Lo" <double.lo@cypress.com>,
"Brian Norris" <briannorris@chromium.org>,
linux-wireless <linux-wireless@vger.kernel.org>,
"Naveen Gupta" <naveen.gupta@cypress.com>,
"Madhan Mohan R" <madhanmohan.r@cypress.com>,
"Matthias Kaehlcke" <mka@chromium.org>,
"Wright Feng" <wright.feng@cypress.com>,
"Chi-Hsien Lin" <chi-hsien.lin@cypress.com>,
netdev <netdev@vger.kernel.org>,
brcm80211-dev-list <brcm80211-dev-list@cypress.com>,
YueHaibing <yuehaibing@huawei.com>,
"Allison Randal" <allison@lohutok.net>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Hante Meuleman" <hante.meuleman@broadcom.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
"Ritesh Harjani" <riteshh@codeaurora.org>,
"Michael Trimarchi" <michael@amarulasolutions.com>,
"Wolfram Sang" <wsa+renesas@sang-engineering.com>,
"Franky Lin" <franky.lin@broadcom.com>,
"Ondrej Jirman" <megous@megous.com>,
"Jiong Wu" <lohengrin1024@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Avri Altman" <avri.altman@wdc.com>
Subject: Re: [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
Date: Tue, 18 Jun 2019 14:47:27 +0300 [thread overview]
Message-ID: <87ef3r9kts.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <CAPDyKFoE0+KNBT5j3_VpJKcztghVa-eFJhy8887bZcUk8bfN2Q@mail.gmail.com> (Ulf Hansson's message of "Tue, 18 Jun 2019 13:28:10 +0200")
Ulf Hansson <ulf.hansson@linaro.org> writes:
> On Tue, 18 Jun 2019 at 13:02, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
> > On Mon, 17 Jun 2019 at 19:57, Douglas Anderson
> <dianders@chromium.org> wrote:
> >>
> >> This series attempts to deal better with the expected
> transmission
> >> errors related to the idle states (handled by the
> Always-On-Subsystem
> >> or AOS) on the SDIO-based WiFi on rk3288-veyron-minnie,
> >> rk3288-veyron-speedy, and rk3288-veyron-mickey.
> >>
> >> Some details about those errors can be found in
> >> <https://crbug.com/960222>, but to summarize it here: if we try
> to
> >> send the wakeup command to the WiFi card at the same time it
> has
> >> decided to wake up itself then it will behave badly on the SDIO
> bus.
> >> This can cause timeouts or CRC errors.
> >>
> >> When I tested on 4.19 and 4.20 these CRC errors can be seen to
> cause
> >> re-tuning. Since I am currently developing on 4.19 this was the
> >> original problem I attempted to solve.
> >>
> >> On mainline it turns out that you don't see the retuning errors
> but
> >> you see tons of spam about timeouts trying to wakeup from
> sleep. I
> >> tracked down the commit that was causing that and have
> partially
> >> reverted it here. I have no real knowledge about Broadcom WiFi,
> but
> >> the commit that was causing problems sounds (from the
> descriptioin) to
> >> be a hack commit penalizing all Broadcom WiFi users because of
> a bug
> >> in a Cypress SD controller. I will let others comment if this
> is
> >> truly the case and, if so, what the right solution should be.
> >>
> >> For v3 of this series I have added 2 patches to the end of the
> series
> >> to address errors that would show up on systems with these same
> SDIO
> >> WiFi cards when used on controllers that do periodic retuning.
> These
> >> systems need an extra fix to prevent the retuning from
> happening when
> >> the card is asleep.
> >>
> >> I believe v5 of this series is all ready to go assuming Kalle
> Valo is
> >> good with it. I've added after-the-cut notes to patches
> awaiting his
> >> Ack and have added other tags collected so far.
> >>
> >> Changes in v5:
> >> - Add missing sdio_retune_crc_enable() in comments (Ulf).
> >> - /s/reneable/re-enable (Ulf).
> >> - Remove leftover prototypes: mmc_expect_errors_begin() / end()
> (Ulf).
> >> - Rewording of "sleep command" in commit message (Arend).
> >>
> >> Changes in v4:
> >> - Moved to SDIO API only (Adrian, Ulf).
> >> - Renamed to make it less generic, now retune_crc_disable
> (Ulf).
> >> - Function header makes it clear host must be claimed (Ulf).
> >> - No more WARN_ON (Ulf).
> >> - Adjust to API rename (Adrian, Ulf).
> >> - Moved retune hold/release to SDIO API (Adrian).
> >> - Adjust to API rename (Adrian).
> >>
> >> Changes in v3:
> >> - Took out the spinlock since I believe this is all in one
> context.
> >> - Expect errors for all of brcmf_sdio_kso_control() (Adrian).
> >> - ("mmc: core: Export mmc_retune_hold_now() mmc_retune_release
> ()") new for v3.
> >> - ("brcmfmac: sdio: Don't tune while the card is off") new for
> v3.
> >>
> >> Changes in v2:
> >> - A full revert, not just a partial one (Arend). ...with
> explicit Cc.
> >> - Updated commit message to clarify based on discussion of v1.
> >>
> >> Douglas Anderson (5):
> >> Revert "brcmfmac: disable command decode in sdio_aos"
> >> mmc: core: API to temporarily disable retuning for SDIO CRC
> errors
> >> brcmfmac: sdio: Disable auto-tuning around commands expected to
> fail
> >> mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
> >> brcmfmac: sdio: Don't tune while the card is off
> >>
> >> drivers/mmc/core/core.c | 5 +-
> >> drivers/mmc/core/sdio_io.c | 77 +++++++++++++++++++
> >> .../broadcom/brcm80211/brcmfmac/sdio.c | 17 ++--
> >> include/linux/mmc/host.h | 1 +
> >> include/linux/mmc/sdio_func.h | 6 ++
> >> 5 files changed, 99 insertions(+), 7 deletions(-)
> >>
> >> --
> >> 2.22.0.410.gd8fdbe21b5-goog
> >>
> >
> > Applied for fixes, thanks!
> >
> > Some minor changes:
> > 1) Dropped the a few "commit notes", that was more related to
> version
> > and practical information about the series.
> > 2) Dropped fixes tags for patch 2->5, but instead put a stable
> tag
> > targeted for v4.18+.
> >
> > Awaiting an ack from Kalle before sending the PR to Linus.
> >
> > Kalle, perhaps you prefer to pick patch 1, as it could go
> separate.
> > Then please tell - and/or if there is anything else you want me
> to
> > change.
>
> TBH I haven't followed the thread (or patches) that closely :) So
> feel
> free to take them and push them to Linus.
>
>
> I take that as an ack and will add your tag for it, thanks!
Yes, it was an ack :) I forgot to add:
Acked-by: Kalle Valo <kvalo@codeaurora.org>
BTW, your previous mail was in HTML so most likely it didn't reach the
list.
--
Kalle Valo
prev parent reply other threads:[~2019-06-18 11:47 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 ` [PATCH v5 5/5] brcmfmac: sdio: Don't tune while the card is off Douglas Anderson
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 [this message]
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=87ef3r9kts.fsf@kamboji.qca.qualcomm.com \
--to=kvalo@codeaurora.org \
--cc=adrian.hunter@intel.com \
--cc=allison@lohutok.net \
--cc=arend.vanspriel@broadcom.com \
--cc=avri.altman@wdc.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=dianders@chromium.org \
--cc=double.lo@cypress.com \
--cc=franky.lin@broadcom.com \
--cc=gregkh@linuxfoundation.org \
--cc=hante.meuleman@broadcom.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lohengrin1024@gmail.com \
--cc=madhanmohan.r@cypress.com \
--cc=megous@megous.com \
--cc=michael@amarulasolutions.com \
--cc=mka@chromium.org \
--cc=naveen.gupta@cypress.com \
--cc=netdev@vger.kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=riteshh@codeaurora.org \
--cc=tglx@linutronix.de \
--cc=ulf.hansson@linaro.org \
--cc=wright.feng@cypress.com \
--cc=wsa+renesas@sang-engineering.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).