Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 1/2] mac80211: add tx dequeue function for process context
From: Erik Stromdahl @ 2019-06-17 20:01 UTC (permalink / raw)
  To: johannes, kvalo, davem, linux-wireless, ath10k, linux-kernel
  Cc: Erik Stromdahl

Since ieee80211_tx_dequeue() must not be called with softirqs enabled
(i.e. from process context without proper disable of bottom halves),
we add a wrapper that disables bottom halves before calling
ieee80211_tx_dequeue()

The new function is named ieee80211_tx_dequeue_ni() just as all other
from-process-context versions found in mac80211.

The documentation of ieee80211_tx_dequeue() is also updated so it
mentions that the function should not be called from process context.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 include/net/mac80211.h | 26 ++++++++++++++++++++++++++
 net/mac80211/tx.c      |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 72080d9d617e..c47990d8db79 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6251,10 +6251,36 @@ void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
  * but for the duration of the frame handling.
  * However, also note that while in the wake_tx_queue() method,
  * rcu_read_lock() is already held.
+ *
+ * softirqs must also be disabled when this function is called.
+ * In process context, use ieee80211_tx_dequeue_ni() instead.
  */
 struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 				     struct ieee80211_txq *txq);
 
+/**
+ * ieee80211_tx_dequeue_ni - dequeue a packet from a software tx queue
+ * (in process context)
+ *
+ * Like ieee80211_tx_dequeue() but can be called in process context
+ * (internally disables bottom halves).
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface, or from
+ *	ieee80211_next_txq()
+ */
+static inline struct sk_buff *ieee80211_tx_dequeue_ni(struct ieee80211_hw *hw,
+						      struct ieee80211_txq *txq)
+{
+	struct sk_buff *skb;
+
+	local_bh_disable();
+	skb = ieee80211_tx_dequeue(hw, txq);
+	local_bh_enable();
+
+	return skb;
+}
+
 /**
  * ieee80211_next_txq - get next tx queue to pull packets from
  *
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index dd220b977025..4bd0066ea7cd 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3550,6 +3550,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	ieee80211_tx_result r;
 	struct ieee80211_vif *vif = txq->vif;
 
+	WARN_ON_ONCE(softirq_count() == 0);
+
 begin:
 	spin_lock_bh(&fq->lock);
 
-- 
2.22.0


^ permalink raw reply related

* [PATCH 2/2] ath10k: switch to ieee80211_tx_dequeue_ni
From: Erik Stromdahl @ 2019-06-17 20:01 UTC (permalink / raw)
  To: johannes, kvalo, davem, linux-wireless, ath10k, linux-kernel
  Cc: Erik Stromdahl
In-Reply-To: <20190617200140.6189-1-erik.stromdahl@gmail.com>

Since ath10k_mac_tx_push_txq() can be called from process context, we
must explicitly disable softirqs before the call into mac80211.

By calling ieee80211_tx_dequeue_ni() instead of ieee80211_tx_dequeue()
we make sure softirqs are always disabled even in the case when
ath10k_mac_tx_push_txq() is called from process context.

Calling ieee80211_tx_dequeue_ni() with softirq's already disabled
(e.g., from softirq context) should be safe as the local_bh_disable()
and local_bh_enable() functions (called from ieee80211_tx_dequeue_ni)
are fully reentrant.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 2d503d6cdcd2..bbed9f1b1778 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4033,7 +4033,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
 	if (ret)
 		return ret;
 
-	skb = ieee80211_tx_dequeue(hw, txq);
+	skb = ieee80211_tx_dequeue_ni(hw, txq);
 	if (!skb) {
 		spin_lock_bh(&ar->htt.tx_lock);
 		ath10k_htt_tx_dec_pending(htt);
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
From: Doug Anderson @ 2019-06-17 18:46 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kalle Valo, Adrian Hunter, Arend van Spriel,
	brcm80211-dev-list.pdl, open list:ARM/Rockchip SoC..., Double Lo,
	Brian Norris, linux-wireless, Naveen Gupta, Madhan Mohan R,
	Matthias Kaehlcke, Wright Feng, Chi-Hsien Lin, netdev,
	brcm80211-dev-list, YueHaibing, Allison Randal, Thomas Gleixner,
	Hante Meuleman, Greg Kroah-Hartman, Niklas Söderlund,
	Ritesh Harjani, Michael Trimarchi, Wolfram Sang, Franky Lin,
	Ondrej Jirman, Jiong Wu, David S. Miller,
	linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Avri Altman
In-Reply-To: <CAPDyKFpaX6DSM_BjtghAHUf7qYCyEG+wMagXPUdgz3Eutovqfw@mail.gmail.com>

Hi,

On Mon, Jun 17, 2019 at 11:39 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> 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+.

OK, sounds good.  Thanks!  :-)

I guess when I see the # v4.18+ in the commit message it makes me
believe that the problem only existed on 4.18+, but maybe that's just
me reading too much into it.  ;-)  In any case, presumably anyone who
had these problems on earlier kernels already has solved them with
local patches.


-Doug

^ permalink raw reply

* Re: [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
From: Ulf Hansson @ 2019-06-17 18:38 UTC (permalink / raw)
  To: Douglas Anderson, Kalle Valo
  Cc: Adrian Hunter, Arend van Spriel, brcm80211-dev-list.pdl,
	open list:ARM/Rockchip SoC..., Double Lo, Brian Norris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, Matthias Kaehlcke,
	Wright Feng, Chi-Hsien Lin, netdev, brcm80211-dev-list,
	YueHaibing, Allison Randal, Thomas Gleixner, Hante Meuleman,
	Greg Kroah-Hartman, Niklas Söderlund, Ritesh Harjani,
	Michael Trimarchi, Wolfram Sang, Franky Lin, Ondrej Jirman,
	Jiong Wu, David S. Miller, linux-mmc@vger.kernel.org,
	Linux Kernel Mailing List, Avri Altman
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

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.

Kind regards
Uffe

^ permalink raw reply

* [PATCH v5 5/5] brcmfmac: sdio: Don't tune while the card is off
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	stable, Franky Lin, linux-kernel, Hante Meuleman, Ondrej Jirman,
	YueHaibing, David S. Miller
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


^ permalink raw reply related

* [PATCH v5 4/5] mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	stable, Allison Randal, linux-mmc, linux-kernel, Thomas Gleixner,
	Greg Kroah-Hartman
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

We want SDIO drivers to be able to temporarily stop retuning when the
driver knows that the SDIO card is not in a state where retuning will
work (maybe because the card is asleep).  We'll move the relevant
functions to a place where drivers can call them.

Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Patches #2 - #5 will go through 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: None
Changes in v4:
- Moved retune hold/release to SDIO API (Adrian).

Changes in v3:
- ("mmc: core: Export mmc_retune_hold_now() mmc_retune_release()") new for v3.

Changes in v2: None

 drivers/mmc/core/sdio_io.c    | 40 +++++++++++++++++++++++++++++++++++
 include/linux/mmc/sdio_func.h |  3 +++
 2 files changed, 43 insertions(+)

diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index 0acb1a29c968..2ba00acf64e6 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -15,6 +15,7 @@
 #include "sdio_ops.h"
 #include "core.h"
 #include "card.h"
+#include "host.h"
 
 /**
  *	sdio_claim_host - exclusively claim a bus for a certain SDIO function
@@ -771,3 +772,42 @@ void sdio_retune_crc_enable(struct sdio_func *func)
 	func->card->host->retune_crc_disable = false;
 }
 EXPORT_SYMBOL_GPL(sdio_retune_crc_enable);
+
+/**
+ *	sdio_retune_hold_now - start deferring retuning requests till release
+ *	@func: SDIO function attached to host
+ *
+ *	This function can be called if it's currently a bad time to do
+ *	a retune of the SDIO card.  Retune requests made during this time
+ *	will be held and we'll actually do the retune sometime after the
+ *	release.
+ *
+ *	This function could be useful if an SDIO card is in a power state
+ *	where it can respond to a small subset of commands that doesn't
+ *	include the retuning command.  Care should be taken when using
+ *	this function since (presumably) the retuning request we might be
+ *	deferring was made for a good reason.
+ *
+ *	This function should be called while the host is claimed.
+ */
+void sdio_retune_hold_now(struct sdio_func *func)
+{
+	mmc_retune_hold_now(func->card->host);
+}
+EXPORT_SYMBOL_GPL(sdio_retune_hold_now);
+
+/**
+ *	sdio_retune_release - signal that it's OK to retune now
+ *	@func: SDIO function attached to host
+ *
+ *	This is the complement to sdio_retune_hold_now().  Calling this
+ *	function won't make a retune happen right away but will allow
+ *	them to be scheduled normally.
+ *
+ *	This function should be called while the host is claimed.
+ */
+void sdio_retune_release(struct sdio_func *func)
+{
+	mmc_retune_release(func->card->host);
+}
+EXPORT_SYMBOL_GPL(sdio_retune_release);
diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
index 4820e6d09dac..5a177f7a83c3 100644
--- a/include/linux/mmc/sdio_func.h
+++ b/include/linux/mmc/sdio_func.h
@@ -170,4 +170,7 @@ extern int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags);
 extern void sdio_retune_crc_disable(struct sdio_func *func);
 extern void sdio_retune_crc_enable(struct sdio_func *func);
 
+extern void sdio_retune_hold_now(struct sdio_func *func);
+extern void sdio_retune_release(struct sdio_func *func);
+
 #endif /* LINUX_MMC_SDIO_FUNC_H */
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v5 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	stable, David S. Miller, Franky Lin, linux-kernel, Hante Meuleman,
	YueHaibing, Michael Trimarchi
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

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.

Commit notes:
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.
END

Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
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>
---

Changes in v5: None
Changes in v4:
- Adjust to API rename (Adrian, Ulf).

Changes in v3:
- Expect errors for all of brcmf_sdio_kso_control() (Adrian).

Changes in v2: None

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4a750838d8cd..ee76593259a7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -667,6 +667,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);
@@ -727,6 +729,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;
 }
 
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v5 2/5] mmc: core: API to temporarily disable retuning for SDIO CRC errors
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	stable, Jiong Wu, Ritesh Harjani, Allison Randal, linux-mmc,
	linux-kernel, Thomas Gleixner, Greg Kroah-Hartman, Wolfram Sang,
	Avri Altman, Niklas Söderlund
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

Normally when the MMC core sees an "-EILSEQ" error returned by a host
controller then it will trigger a retuning of the card.  This is
generally a good idea.

However, if a command is expected to sometimes cause transfer errors
then these transfer errors shouldn't cause a re-tuning.  This
re-tuning will be a needless waste of time.  One example case where a
transfer is expected to cause errors is when transitioning between
idle (sometimes referred to as "sleep" in Broadcom code) and active
state on certain Broadcom WiFi SDIO cards.  Specifically if the card
was already transitioning between states when the command was sent it
could cause an error on the SDIO bus.

Let's add an API that the SDIO function drivers can call that will
temporarily disable the auto-tuning functionality.  Then we can add a
call to this in the Broadcom WiFi driver and any other driver that
might have similar needs.

NOTE: this makes the assumption that the card is already tuned well
enough that it's OK to disable the auto-retuning during one of these
error-prone situations.  Presumably the driver code performing the
error-prone transfer knows how to recover / retry from errors.  ...and
after we can get back to a state where transfers are no longer
error-prone then we can enable the auto-retuning again.  If we truly
find ourselves in a case where the card needs to be retuned sometimes
to handle one of these error-prone transfers then we can always try a
few transfers first without auto-retuning and then re-try with
auto-retuning if the first few fail.

Without this change on rk3288-veyron-minnie I periodically see this in
the logs of a machine just sitting there idle:
  dwmmc_rockchip ff0d0000.dwmmc: Successfully tuned phase to XYZ

Commit notes:
Patches #2 - #5 will go through Ulf's tree.
END

Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---

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

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

Changes in v3:
- Took out the spinlock since I believe this is all in one context.

Changes in v2:
- Updated commit message to clarify based on discussion of v1.

 drivers/mmc/core/core.c       |  5 +++--
 drivers/mmc/core/sdio_io.c    | 37 +++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h      |  1 +
 include/linux/mmc/sdio_func.h |  3 +++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..9020cb2490f7 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -144,8 +144,9 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
 	int err = cmd->error;
 
 	/* Flag re-tuning needed on CRC errors */
-	if ((cmd->opcode != MMC_SEND_TUNING_BLOCK &&
-	    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200) &&
+	if (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
+	    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
+	    !host->retune_crc_disable &&
 	    (err == -EILSEQ || (mrq->sbc && mrq->sbc->error == -EILSEQ) ||
 	    (mrq->data && mrq->data->error == -EILSEQ) ||
 	    (mrq->stop && mrq->stop->error == -EILSEQ)))
diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index f79f0b0caab8..0acb1a29c968 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -734,3 +734,40 @@ int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(sdio_set_host_pm_flags);
+
+/**
+ *	sdio_retune_crc_disable - temporarily disable retuning on CRC errors
+ *	@func: SDIO function attached to host
+ *
+ *	If the SDIO card is known to be in a state where it might produce
+ *	CRC errors on the bus in response to commands (like if we know it is
+ *	transitioning between power states), an SDIO function driver can
+ *	call this function to temporarily disable the SD/MMC core behavior of
+ *	triggering an automatic retuning.
+ *
+ *	This function should be called while the host is claimed and the host
+ *	should remain claimed until sdio_retune_crc_enable() is called.
+ *	Specifically, the expected sequence of calls is:
+ *	- sdio_claim_host()
+ *	- sdio_retune_crc_disable()
+ *	- some number of calls like sdio_writeb() and sdio_readb()
+ *	- sdio_retune_crc_enable()
+ *	- sdio_release_host()
+ */
+void sdio_retune_crc_disable(struct sdio_func *func)
+{
+	func->card->host->retune_crc_disable = true;
+}
+EXPORT_SYMBOL_GPL(sdio_retune_crc_disable);
+
+/**
+ *	sdio_retune_crc_enable - re-enable retuning on CRC errors
+ *	@func: SDIO function attached to host
+ *
+ *	This is the compement to sdio_retune_crc_disable().
+ */
+void sdio_retune_crc_enable(struct sdio_func *func)
+{
+	func->card->host->retune_crc_disable = false;
+}
+EXPORT_SYMBOL_GPL(sdio_retune_crc_enable);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..ecb7972e2423 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -398,6 +398,7 @@ struct mmc_host {
 	unsigned int		retune_now:1;	/* do re-tuning at next req */
 	unsigned int		retune_paused:1; /* re-tuning is temporarily disabled */
 	unsigned int		use_blk_mq:1;	/* use blk-mq */
+	unsigned int		retune_crc_disable:1; /* don't trigger retune upon crc */
 
 	int			rescan_disable;	/* disable card detection */
 	int			rescan_entered;	/* used with nonremovable devices */
diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
index e9dfdd501cd1..4820e6d09dac 100644
--- a/include/linux/mmc/sdio_func.h
+++ b/include/linux/mmc/sdio_func.h
@@ -167,4 +167,7 @@ extern void sdio_f0_writeb(struct sdio_func *func, unsigned char b,
 extern mmc_pm_flag_t sdio_get_host_pm_caps(struct sdio_func *func);
 extern int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags);
 
+extern void sdio_retune_crc_disable(struct sdio_func *func);
+extern void sdio_retune_crc_enable(struct sdio_func *func);
+
 #endif /* LINUX_MMC_SDIO_FUNC_H */
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v5 1/5] Revert "brcmfmac: disable command decode in sdio_aos"
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	Franky Lin, linux-kernel, Hante Meuleman, YueHaibing,
	David S. Miller
In-Reply-To: <20190617175653.21756-1-dianders@chromium.org>

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 <wright.feng@cypress.com>
Cc: Double Lo <double.lo@cypress.com>
Cc: Madhan Mohan R <madhanmohan.r@cypress.com>
Cc: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
This commit to land in the wireless tree.  OK to land it separately
from the rest of the series.

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- A full revert, not just a partial one (Arend).  ...with explicit Cc.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4e15ea57d4f5..4a750838d8cd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3364,11 +3364,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 
 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;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
From: Douglas Anderson @ 2019-06-17 17:56 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	YueHaibing, Allison Randal, Thomas Gleixner, Hante Meuleman,
	Greg Kroah-Hartman, Niklas Söderlund, Ritesh Harjani,
	Michael Trimarchi, Wolfram Sang, Franky Lin, Ondrej Jirman,
	Jiong Wu, David S. Miller, linux-mmc, linux-kernel, Avri Altman

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


^ permalink raw reply

* Re: [mac80211-next:master 17/20] drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
From: Johannes Berg @ 2019-06-17 16:09 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Greg Kroah-Hartman, kbuild test robot, kbuild-all, linux-wireless
In-Reply-To: <87muigtg2q.fsf@purkki.adurom.net>

On Mon, 2019-06-17 at 17:57 +0300, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > On Fri, 2019-06-14 at 16:33 +0200, Greg Kroah-Hartman wrote:
> > 
> > > Did you apply my "[PATCH 3/5] iwlwifi: dvm: no need to check return
> > > value of debugfs_create function" patch also to this tree?  The 5th
> > > patch in the series depended on it :(
> > 
> > Yeah, my bad, sorry about that. I was not paying attention to the "5/5"
> > in patchwork, and the other patches got assigned to other maintainers so
> > I didn't see them there.
> > 
> > I've dropped the patch for now, until that's all sorted out. Or maybe
> > Kalle will just take them all together.
> 
> Maybe it's easier that you just wait for other patches to trickle down
> your tree?

Sure, that works too, whichever you prefer.

johannes


^ permalink raw reply

* Re: [PATCH] ath10k: add mic bytes for pmf management packet
From: Ben Greear @ 2019-06-17 16:03 UTC (permalink / raw)
  To: Wen Gong, ath10k; +Cc: linux-wireless
In-Reply-To: <1560757079-19266-1-git-send-email-wgong@codeaurora.org>

On 6/17/19 12:37 AM, Wen Gong wrote:
> For PMF case, the action,deauth,disassoc management need to encrypt
> by hardware, it need to reserve 8 bytes for encryption, otherwise
> the packet will be sent out with error format, then PMF case will
> fail.
> 
> After add the 8 bytes, it will pass the PMF case.
> 
> Tested with QCA6174 SDIO with firmware
> WLAN.RMH.4.4.1-00005-QCARMSWP-1.
> 
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> ---
>   drivers/net/wireless/ath/ath10k/htt_tx.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index d8e9cc0..7bef9d9 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -1236,6 +1236,7 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
>   	struct ath10k *ar = htt->ar;
>   	int res, data_len;
>   	struct htt_cmd_hdr *cmd_hdr;
> +	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)msdu->data;
>   	struct htt_data_tx_desc *tx_desc;
>   	struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(msdu);
>   	struct sk_buff *tmp_skb;
> @@ -1245,6 +1246,13 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
>   	u8 flags0 = 0;
>   	u16 flags1 = 0;
>   
> +	if ((ieee80211_is_action(hdr->frame_control) ||
> +	     ieee80211_is_deauth(hdr->frame_control) ||
> +	     ieee80211_is_disassoc(hdr->frame_control)) &&
> +	     ieee80211_has_protected(hdr->frame_control)) {
> +		skb_put(msdu, IEEE80211_CCMP_MIC_LEN);
> +	}

I was looking at mac80211 code recently, and it seems some action
frames are NOT supposed to be protected.  I added my own helper
method to my local ath10k.  Maybe you want to use this?


/* Copied from ieee80211_is_robust_mgmt_frame, but disable the check for has_protected
  * since we do tx hw crypt, and it won't actually be encrypted even when this flag is
  * set.
  */
bool ieee80211_is_robust_mgmt_frame_tx(struct ieee80211_hdr *hdr)
{
         if (ieee80211_is_disassoc(hdr->frame_control) ||
             ieee80211_is_deauth(hdr->frame_control))
                 return true;

         if (ieee80211_is_action(hdr->frame_control)) {
                 u8 *category;

                 /*
                  * Action frames, excluding Public Action frames, are Robust
                  * Management Frames. However, if we are looking at a Protected
                  * frame, skip the check since the data may be encrypted and
                  * the frame has already been found to be a Robust Management
                  * Frame (by the other end).
                  */
		/*
		if (ieee80211_has_protected(hdr->frame_control))
                         return true;
		*/
                 category = ((u8 *) hdr) + 24;
                 return *category != WLAN_CATEGORY_PUBLIC &&
                         *category != WLAN_CATEGORY_HT &&
                         *category != WLAN_CATEGORY_WNM_UNPROTECTED &&
                         *category != WLAN_CATEGORY_SELF_PROTECTED &&
                         *category != WLAN_CATEGORY_UNPROT_DMG &&
                         *category != WLAN_CATEGORY_VHT &&
                         *category != WLAN_CATEGORY_VENDOR_SPECIFIC;
         }

         return false;
}

Thanks,
Ben

> +
>   	data_len = msdu->len;
>   
>   	switch (txmode) {
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [mac80211-next:master 17/20] drivers/net/wireless/intel/iwlwifi/dvm/rs.c:3317:3: error: 'const struct rate_control_ops' has no member named 'remove_sta_debugfs'; did you mean 'add_sta_debugfs'?
From: Kalle Valo @ 2019-06-17 14:57 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Greg Kroah-Hartman, kbuild test robot, kbuild-all, linux-wireless
In-Reply-To: <330312ef061715d2beba89d0337bfe1a6698f36e.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2019-06-14 at 16:33 +0200, Greg Kroah-Hartman wrote:
>
>> Did you apply my "[PATCH 3/5] iwlwifi: dvm: no need to check return
>> value of debugfs_create function" patch also to this tree?  The 5th
>> patch in the series depended on it :(
>
> Yeah, my bad, sorry about that. I was not paying attention to the "5/5"
> in patchwork, and the other patches got assigned to other maintainers so
> I didn't see them there.
>
> I've dropped the patch for now, until that's all sorted out. Or maybe
> Kalle will just take them all together.

Maybe it's easier that you just wait for other patches to trickle down
your tree?

-- 
Kalle Valo

^ permalink raw reply

* Re: wpa_supplicant 2.8 fails in brcmf_cfg80211_set_pmk
From: Marcel Holtmann @ 2019-06-17 14:33 UTC (permalink / raw)
  To: Chi-Hsien Lin
  Cc: Stefan Wahren, Stanley Hsu, Arend van Spriel, Franky Lin,
	Hante Meuleman, Wright Feng, linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com, brcm80211-dev-list
In-Reply-To: <605ea0a8-3303-b810-6223-18ccc7eb7af4@cypress.com>

Hi Chi-hsien,

>>> i was able to reproduce an (maybe older issue) with 4-way handshake
>>> offloading for 802.1X in the brcmfmac driver. My setup consists of
>>> Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a
>>> Raspberry Pi 3 A+ (Linux 4.19) on AP side.
>> 
>> Looks like Raspberry Pi isn't the only affected platform [3], [4].
>> 
>> [3] - https://bugzilla.redhat.com/show_bug.cgi?id=1665608
>> [4] - https://bugzilla.kernel.org/show_bug.cgi?id=202521
> 
> Stefan,
> 
> Could you please try the attached patch for your wpa_supplicant? We'll 
> upstream if it works for you.

I hope that someone is also providing a kernel patch to fix the issue. Hacking around a kernel issue in userspace is not enough. Fix the root cause in the kernel.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 12/16] staging/comedi: mark as broken
From: Ian Abbott @ 2019-06-17 13:15 UTC (permalink / raw)
  To: Christoph Hellwig, Greg KH
  Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	H Hartley Sweeten, devel, linux-s390, Intel Linux Wireless,
	linux-rdma, netdev, intel-gfx, linux-wireless, linux-kernel,
	dri-devel, linux-mm, iommu, moderated list:ARM PORT, linux-media
In-Reply-To: <20190614153428.GA10008@lst.de>

On 14/06/2019 16:34, Christoph Hellwig wrote:
> On Fri, Jun 14, 2019 at 05:30:32PM +0200, Greg KH wrote:
>> On Fri, Jun 14, 2019 at 04:48:57PM +0200, Christoph Hellwig wrote:
>>> On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote:
>>>> Perhaps a hint as to how we can fix this up?  This is the first time
>>>> I've heard of the comedi code not handling dma properly.
>>>
>>> It can be fixed by:
>>>
>>>   a) never calling virt_to_page (or vmalloc_to_page for that matter)
>>>      on dma allocation
>>>   b) never remapping dma allocation with conflicting cache modes
>>>      (no remapping should be doable after a) anyway).
>>
>> Ok, fair enough, have any pointers of drivers/core code that does this
>> correctly?  I can put it on my todo list, but might take a week or so...
> 
> Just about everyone else.  They just need to remove the vmap and
> either do one large allocation, or live with the fact that they need
> helpers to access multiple array elements instead of one net vmap,
> which most of the users already seem to do anyway, with just a few
> using the vmap (which might explain why we didn't see blowups yet).

Avoiding the vmap in comedi should be do-able as it already has other 
means to get at the buffer pages.

When comedi makes the buffer from DMA coherent memory, it currently 
allocates it as a series of page-sized chunks.  That cannot be mmap'ed 
in one go with dma_mmap_coherent(), so I see the following solutions.

1. Change the buffer allocation to allocate a single chunk of DMA 
coherent memory and use dma_mmap_coherent() to mmap it.

2. Call dma_mmap_coherent() in a loop, adjusting vma->vm_start and 
vma->vm_end for each iteration (vma->vm_pgoff will be 0), and restoring 
the vma->vm_start and vma->vm_end at the end.

I'm not sure if 2 is a legal option.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-

^ permalink raw reply

* Re: [PATCH v3] {nl,mac}80211: allow 4addr AP operation on crypto controlled devices
From: Tom Psyborg @ 2019-06-17 11:32 UTC (permalink / raw)
  To: Stefan Lippers-Hollmann
  Cc: Johannes Berg, Manikanta Pubbisetty, linux-wireless
In-Reply-To: <20190617093658.46591254@mir>

Hi

Checked out r10011 from openwrt git on May, 15; built images and
flashed devices; tried to set up WDS (AP) + WDS (Client); got this in
logs: https://forum.openwrt.org/t/ath10k-wds-support-missing/30346/15?u=psyborg

Saw this patch in my inbox and checked openwrt sources, to find out
there was v1 used:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=package/kernel/mac80211/patches/subsys/390-nl-mac-80211-allow-4addr-AP-operation-on-crypto-cont.patch

Reverted v1 of the patch and patched manually to v3 (double checked),
did not help.

Found patch that fixes problem and applied;
https://github.com/openwrt/openwrt/pull/2048
Now I got this in logs:
https://forum.openwrt.org/t/wds-client-wont-stay-connected-prev-auth-not-valid-using-recent-snapshot-builds/38194/20?u=psyborg

Reverted again v3 of this patch to v1, and the connection worked as expected.

v4.19.32 is current in openwrt master, and testing against ath9k
devices may not reveal exact issue. notice in one of my posts on
forums that i was able to bridge it with archer c7 running tp-link fw
without any of these patches

^ permalink raw reply

* Re: [PATCH v4 5/5] brcmfmac: sdio: Don't tune while the card is off
From: Arend Van Spriel @ 2019-06-17 10:52 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Adrian Hunter
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Franky Lin,
	linux-kernel, Hante Meuleman, YueHaibing, David S. Miller
In-Reply-To: <20190613234153.59309-6-dianders@chromium.org>

On 6/14/2019 1:41 AM, Douglas Anderson wrote:
> 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 a sleep command, so presumably that "good enough"
> tuning is enough to wake us up, at least with a few retries.

The term "sleep command" is a bit confusing. There actually is a CMD14 
defined in the eSD spec, but that is not what we are using (unless we 
program the chip to do so) here. It is simply a specific register access 
using CMD52.

Apart from that....

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

I think the stable version is mostly determined by change in MMC/SDIO so 
4.18 as mentioned Adrian seems most sensible. bcm4354 support was 
introduced in 3.14 and there were some earlier devices (4335) using same 
sleep mechanism.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v4 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Arend Van Spriel @ 2019-06-17 10:35 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Adrian Hunter
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Hans de Goede,
	Franky Lin, linux-kernel, Hante Meuleman, YueHaibing,
	David S. Miller
In-Reply-To: <20190613234153.59309-4-dianders@chromium.org>

On 6/14/2019 1:41 AM, Douglas Anderson wrote:
> 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.
> 
> Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply

* Re: [PATCH] mmc: core: Prevent processing SDIO IRQs when the card is suspended
From: Ulf Hansson @ 2019-06-17  9:56 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
	Guenter Roeck, Heiko Stuebner, Kalle Valo, linux-wireless, # 4.0+
In-Reply-To: <CAD=FV=Ujb=_jEFMdxLPW6tYwb9DZo5-RZ8BVuyq5DdFQ5jJbQw@mail.gmail.com>

On Fri, 14 Jun 2019 at 17:42, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Fri, Jun 14, 2019 at 4:56 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > > I was more worried about the safety of mmc_card_set_suspended()
> > > itself.  That is:
> > >
> > > #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
> > >
> > > ...so it's doing a read-modify-write of "state".  Is that safe to do
> > > without any type of locking?
> >
> > In this case, yes I think so.
> >
> > The point is, it really doesn't matter if the reader (work or thread),
> > reads a non-updated value, because the synchronization is managed by
> > the later mmc_claim_host() and the cancel_delayed_work_sync().
>
> If this were just an "int" then perhaps, but this is a bitfield.  So
> if someone else updates the bitfield at the same time then we can
> fully clobber their modification or they can clobber ours, right?
>
> task 1: load "state" from memory into CPU register on cpu0
> task 2: load "state" from memory into CPU register on cpu1
> task 1: OR in MMC_CARD_REMOVED
> task 1: write "state" from CPU register on cpu0
> task 2: OR in MMC_STATE_SUSPENDED
> task 2: write "state" from CPU register on cpu1
>
> ...so now we've clobbered MMC_CARD_REMOVED.  ...or am I just being
> paranoid here and everything else in "state" is somehow guaranteed to
> not be touched at the same time this function is running?

I understand your concern. It's not obvious by looking at the code,
but yes, there should be no other writing to the "state" at the same
time mmc_sdio_supend() is running.

MMC_CARD_REMOVED for example, is set from  _mmc_detect_card_removed(),
but because the detect work (mmc_recan()) has been disabled and the
block device driver has been suspended, it can't be called.

Anyway, perhaps we get reasons to add a lock to the card struct when
going forward, but at this point I think we are fine.

Kind regards
Uffe

^ permalink raw reply

* [PATCH] rt2x00: fix rx queue hang
From: Soeren Moch @ 2019-06-17  9:46 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Soeren Moch, Helmut Schaa, Kalle Valo, David S. Miller,
	linux-wireless, netdev, linux-kernel, stable

Since commit ed194d136769 ("usb: core: remove local_irq_save() around
 ->complete() handler") the handlers rt2x00usb_interrupt_rxdone() and
rt2x00usb_interrupt_txdone() are not running with interrupts disabled
anymore. So these handlers are not guaranteed to run completely before
workqueue processing starts. So only mark entries ready for workqueue
processing after proper accounting in the dma done queue.
Note that rt2x00usb_work_rxdone() processes all available entries, not
only such for which queue_work() was called.

This fixes a regression on a RT5370 based wifi stick in AP mode, which
suddenly stopped data transmission after some period of heavy load. Also
stopping the hanging hostapd resulted in the error message "ieee80211
phy0: rt2x00queue_flush_queue: Warning - Queue 14 failed to flush".
Other operation modes are probably affected as well, this just was
the used testcase.

Fixes: ed194d136769 ("usb: core: remove local_irq_save() around ->complete() handler")
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Helmut Schaa <helmut.schaa@googlemail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org # 4.20+
Signed-off-by: Soeren Moch <smoch@web.de>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 1b08b01db27b..9c102a501ee6 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -263,9 +263,9 @@ EXPORT_SYMBOL_GPL(rt2x00lib_dmastart);

 void rt2x00lib_dmadone(struct queue_entry *entry)
 {
-	set_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags);
 	clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
 	rt2x00queue_index_inc(entry, Q_INDEX_DMA_DONE);
+	set_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags);
 }
 EXPORT_SYMBOL_GPL(rt2x00lib_dmadone);

--
2.17.1


^ permalink raw reply related

* [PATCH v2] lib80211: use crypto API ccm(aes) transform for CCMP processing
From: Ard Biesheuvel @ 2019-06-17  9:19 UTC (permalink / raw)
  To: linux-wireless; +Cc: linux-crypto, herbert, ebiggers, johannes, Ard Biesheuvel

Instead of open coding the CCM aead mode in the driver, and invoking
the AES block cipher block by block, use a ccm(aes) aead transform
which already encapsulates this functionality. This is a cleaner use
of the crypto API, and permits optimized implementations to be used,
which are typically much faster and deal more efficiently with the
SIMD register file, which usually needs to be preserved/restored in
order to use special AES instructions.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: Address lots of style related issues flagged by Eric. Still untested on
    actual hardware.

 net/wireless/Kconfig               |   2 +
 net/wireless/lib80211_crypt_ccmp.c | 197 +++++++++-----------
 2 files changed, 87 insertions(+), 112 deletions(-)

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 6310ddede220..cf8ba192249e 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -216,6 +216,8 @@ config LIB80211_CRYPT_WEP
 
 config LIB80211_CRYPT_CCMP
 	tristate
+	select CRYPTO_AES
+	select CRYPTO_CCM
 
 config LIB80211_CRYPT_TKIP
 	tristate
diff --git a/net/wireless/lib80211_crypt_ccmp.c b/net/wireless/lib80211_crypt_ccmp.c
index 55214fe925b2..7297646c084a 100644
--- a/net/wireless/lib80211_crypt_ccmp.c
+++ b/net/wireless/lib80211_crypt_ccmp.c
@@ -26,6 +26,7 @@
 #include <linux/ieee80211.h>
 
 #include <linux/crypto.h>
+#include <crypto/aead.h>
 
 #include <net/lib80211.h>
 
@@ -52,20 +53,13 @@ struct lib80211_ccmp_data {
 
 	int key_idx;
 
-	struct crypto_cipher *tfm;
+	struct crypto_aead *tfm;
 
 	/* scratch buffers for virt_to_page() (crypto API) */
-	u8 tx_b0[AES_BLOCK_LEN], tx_b[AES_BLOCK_LEN],
-	    tx_e[AES_BLOCK_LEN], tx_s0[AES_BLOCK_LEN];
-	u8 rx_b0[AES_BLOCK_LEN], rx_b[AES_BLOCK_LEN], rx_a[AES_BLOCK_LEN];
+	u8 tx_aad[2 * AES_BLOCK_LEN];
+	u8 rx_aad[2 * AES_BLOCK_LEN];
 };
 
-static inline void lib80211_ccmp_aes_encrypt(struct crypto_cipher *tfm,
-					      const u8 pt[16], u8 ct[16])
-{
-	crypto_cipher_encrypt_one(tfm, ct, pt);
-}
-
 static void *lib80211_ccmp_init(int key_idx)
 {
 	struct lib80211_ccmp_data *priv;
@@ -75,7 +69,7 @@ static void *lib80211_ccmp_init(int key_idx)
 		goto fail;
 	priv->key_idx = key_idx;
 
-	priv->tfm = crypto_alloc_cipher("aes", 0, 0);
+	priv->tfm = crypto_alloc_aead("ccm(aes)", 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(priv->tfm)) {
 		priv->tfm = NULL;
 		goto fail;
@@ -86,7 +80,7 @@ static void *lib80211_ccmp_init(int key_idx)
       fail:
 	if (priv) {
 		if (priv->tfm)
-			crypto_free_cipher(priv->tfm);
+			crypto_free_aead(priv->tfm);
 		kfree(priv);
 	}
 
@@ -97,25 +91,16 @@ static void lib80211_ccmp_deinit(void *priv)
 {
 	struct lib80211_ccmp_data *_priv = priv;
 	if (_priv && _priv->tfm)
-		crypto_free_cipher(_priv->tfm);
+		crypto_free_aead(_priv->tfm);
 	kfree(priv);
 }
 
-static inline void xor_block(u8 * b, u8 * a, size_t len)
-{
-	int i;
-	for (i = 0; i < len; i++)
-		b[i] ^= a[i];
-}
-
-static void ccmp_init_blocks(struct crypto_cipher *tfm,
-			     struct ieee80211_hdr *hdr,
-			     u8 * pn, size_t dlen, u8 * b0, u8 * auth, u8 * s0)
+static int ccmp_init_iv_and_aad(const struct ieee80211_hdr *hdr,
+				const u8 *pn, u8 *iv, u8 *aad)
 {
 	u8 *pos, qc = 0;
 	size_t aad_len;
 	int a4_included, qc_included;
-	u8 aad[2 * AES_BLOCK_LEN];
 
 	a4_included = ieee80211_has_a4(hdr->frame_control);
 	qc_included = ieee80211_is_data_qos(hdr->frame_control);
@@ -131,17 +116,19 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
 		aad_len += 2;
 	}
 
-	/* CCM Initial Block:
-	 * Flag (Include authentication header, M=3 (8-octet MIC),
-	 *       L=1 (2-octet Dlen))
-	 * Nonce: 0x00 | A2 | PN
-	 * Dlen */
-	b0[0] = 0x59;
-	b0[1] = qc;
-	memcpy(b0 + 2, hdr->addr2, ETH_ALEN);
-	memcpy(b0 + 8, pn, CCMP_PN_LEN);
-	b0[14] = (dlen >> 8) & 0xff;
-	b0[15] = dlen & 0xff;
+	/* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
+	 * mode authentication are not allowed to collide, yet both are derived
+	 * from the same vector. We only set L := 1 here to indicate that the
+	 * data size can be represented in (L+1) bytes. The CCM layer will take
+	 * care of storing the data length in the top (L+1) bytes and setting
+	 * and clearing the other bits as is required to derive the two IVs.
+	 */
+	iv[0] = 0x1;
+
+	/* Nonce: QC | A2 | PN */
+	iv[1] = qc;
+	memcpy(iv + 2, hdr->addr2, ETH_ALEN);
+	memcpy(iv + 8, pn, CCMP_PN_LEN);
 
 	/* AAD:
 	 * FC with bits 4..6 and 11..13 masked to zero; 14 is always one
@@ -151,31 +138,20 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
 	 * QC (if present)
 	 */
 	pos = (u8 *) hdr;
-	aad[0] = 0;		/* aad_len >> 8 */
-	aad[1] = aad_len & 0xff;
-	aad[2] = pos[0] & 0x8f;
-	aad[3] = pos[1] & 0xc7;
-	memcpy(aad + 4, hdr->addr1, 3 * ETH_ALEN);
+	aad[0] = pos[0] & 0x8f;
+	aad[1] = pos[1] & 0xc7;
+	memcpy(aad + 2, hdr->addr1, 3 * ETH_ALEN);
 	pos = (u8 *) & hdr->seq_ctrl;
-	aad[22] = pos[0] & 0x0f;
-	aad[23] = 0;		/* all bits masked */
-	memset(aad + 24, 0, 8);
+	aad[20] = pos[0] & 0x0f;
+	aad[21] = 0;		/* all bits masked */
+	memset(aad + 22, 0, 8);
 	if (a4_included)
-		memcpy(aad + 24, hdr->addr4, ETH_ALEN);
+		memcpy(aad + 22, hdr->addr4, ETH_ALEN);
 	if (qc_included) {
-		aad[a4_included ? 30 : 24] = qc;
+		aad[a4_included ? 28 : 22] = qc;
 		/* rest of QC masked */
 	}
-
-	/* Start with the first block and AAD */
-	lib80211_ccmp_aes_encrypt(tfm, b0, auth);
-	xor_block(auth, aad, AES_BLOCK_LEN);
-	lib80211_ccmp_aes_encrypt(tfm, auth, auth);
-	xor_block(auth, &aad[AES_BLOCK_LEN], AES_BLOCK_LEN);
-	lib80211_ccmp_aes_encrypt(tfm, auth, auth);
-	b0[0] &= 0x07;
-	b0[14] = b0[15] = 0;
-	lib80211_ccmp_aes_encrypt(tfm, b0, s0);
+	return aad_len;
 }
 
 static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
@@ -218,13 +194,13 @@ static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
 static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 {
 	struct lib80211_ccmp_data *key = priv;
-	int data_len, i, blocks, last, len;
-	u8 *pos, *mic;
 	struct ieee80211_hdr *hdr;
-	u8 *b0 = key->tx_b0;
-	u8 *b = key->tx_b;
-	u8 *e = key->tx_e;
-	u8 *s0 = key->tx_s0;
+	struct aead_request *req;
+	struct scatterlist sg[2];
+	u8 *aad = key->tx_aad;
+	u8 iv[AES_BLOCK_LEN];
+	int len, data_len, aad_len;
+	int ret;
 
 	if (skb_tailroom(skb) < CCMP_MIC_LEN || skb->len < hdr_len)
 		return -1;
@@ -234,31 +210,28 @@ static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	if (len < 0)
 		return -1;
 
-	pos = skb->data + hdr_len + CCMP_HDR_LEN;
+	req = aead_request_alloc(key->tfm, GFP_ATOMIC);
+	if (!req)
+		return -ENOMEM;
+
 	hdr = (struct ieee80211_hdr *)skb->data;
-	ccmp_init_blocks(key->tfm, hdr, key->tx_pn, data_len, b0, b, s0);
-
-	blocks = DIV_ROUND_UP(data_len, AES_BLOCK_LEN);
-	last = data_len % AES_BLOCK_LEN;
-
-	for (i = 1; i <= blocks; i++) {
-		len = (i == blocks && last) ? last : AES_BLOCK_LEN;
-		/* Authentication */
-		xor_block(b, pos, len);
-		lib80211_ccmp_aes_encrypt(key->tfm, b, b);
-		/* Encryption, with counter */
-		b0[14] = (i >> 8) & 0xff;
-		b0[15] = i & 0xff;
-		lib80211_ccmp_aes_encrypt(key->tfm, b0, e);
-		xor_block(pos, e, len);
-		pos += len;
-	}
+	aad_len = ccmp_init_iv_and_aad(hdr, key->tx_pn, iv, aad);
 
-	mic = skb_put(skb, CCMP_MIC_LEN);
-	for (i = 0; i < CCMP_MIC_LEN; i++)
-		mic[i] = b[i] ^ s0[i];
+	skb_put(skb, CCMP_MIC_LEN);
 
-	return 0;
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], aad, aad_len);
+	sg_set_buf(&sg[1], skb->data + hdr_len + CCMP_HDR_LEN,
+		   data_len + CCMP_MIC_LEN);
+
+	aead_request_set_callback(req, 0, NULL, NULL);
+	aead_request_set_ad(req, aad_len);
+	aead_request_set_crypt(req, sg, sg, data_len, iv);
+
+	ret = crypto_aead_encrypt(req);
+	aead_request_free(req);
+
+	return ret;
 }
 
 /*
@@ -287,13 +260,13 @@ static int lib80211_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	struct lib80211_ccmp_data *key = priv;
 	u8 keyidx, *pos;
 	struct ieee80211_hdr *hdr;
-	u8 *b0 = key->rx_b0;
-	u8 *b = key->rx_b;
-	u8 *a = key->rx_a;
+	struct aead_request *req;
+	struct scatterlist sg[2];
+	u8 *aad = key->rx_aad;
+	u8 iv[AES_BLOCK_LEN];
 	u8 pn[6];
-	int i, blocks, last, len;
-	size_t data_len = skb->len - hdr_len - CCMP_HDR_LEN - CCMP_MIC_LEN;
-	u8 *mic = skb->data + skb->len - CCMP_MIC_LEN;
+	int aad_len, ret;
+	size_t data_len = skb->len - hdr_len - CCMP_HDR_LEN;
 
 	if (skb->len < hdr_len + CCMP_HDR_LEN + CCMP_MIC_LEN) {
 		key->dot11RSNAStatsCCMPFormatErrors++;
@@ -341,28 +314,26 @@ static int lib80211_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 		return -4;
 	}
 
-	ccmp_init_blocks(key->tfm, hdr, pn, data_len, b0, a, b);
-	xor_block(mic, b, CCMP_MIC_LEN);
-
-	blocks = DIV_ROUND_UP(data_len, AES_BLOCK_LEN);
-	last = data_len % AES_BLOCK_LEN;
-
-	for (i = 1; i <= blocks; i++) {
-		len = (i == blocks && last) ? last : AES_BLOCK_LEN;
-		/* Decrypt, with counter */
-		b0[14] = (i >> 8) & 0xff;
-		b0[15] = i & 0xff;
-		lib80211_ccmp_aes_encrypt(key->tfm, b0, b);
-		xor_block(pos, b, len);
-		/* Authentication */
-		xor_block(a, pos, len);
-		lib80211_ccmp_aes_encrypt(key->tfm, a, a);
-		pos += len;
-	}
+	req = aead_request_alloc(key->tfm, GFP_ATOMIC);
+	if (!req)
+		return -ENOMEM;
 
-	if (memcmp(mic, a, CCMP_MIC_LEN) != 0) {
-		net_dbg_ratelimited("CCMP: decrypt failed: STA=%pM\n",
-				    hdr->addr2);
+	aad_len = ccmp_init_iv_and_aad(hdr, pn, iv, aad);
+
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], aad, aad_len);
+	sg_set_buf(&sg[1], pos, data_len);
+
+	aead_request_set_callback(req, 0, NULL, NULL);
+	aead_request_set_ad(req, aad_len);
+	aead_request_set_crypt(req, sg, sg, data_len, iv);
+
+	ret = crypto_aead_decrypt(req);
+	aead_request_free(req);
+
+	if (ret) {
+		net_dbg_ratelimited("CCMP: decrypt failed: STA=%pM (%d)\n",
+				    hdr->addr2, ret);
 		key->dot11RSNAStatsCCMPDecryptErrors++;
 		return -5;
 	}
@@ -381,7 +352,7 @@ static int lib80211_ccmp_set_key(void *key, int len, u8 * seq, void *priv)
 {
 	struct lib80211_ccmp_data *data = priv;
 	int keyidx;
-	struct crypto_cipher *tfm = data->tfm;
+	struct crypto_aead *tfm = data->tfm;
 
 	keyidx = data->key_idx;
 	memset(data, 0, sizeof(*data));
@@ -398,7 +369,9 @@ static int lib80211_ccmp_set_key(void *key, int len, u8 * seq, void *priv)
 			data->rx_pn[4] = seq[1];
 			data->rx_pn[5] = seq[0];
 		}
-		crypto_cipher_setkey(data->tfm, data->key, CCMP_TK_LEN);
+		if (crypto_aead_setauthsize(data->tfm, CCMP_MIC_LEN) ||
+		    crypto_aead_setkey(data->tfm, data->key, CCMP_TK_LEN))
+			return -1;
 	} else if (len == 0)
 		data->key_set = 0;
 	else
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4 4/5] mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
From: Adrian Hunter @ 2019-06-17  8:46 UTC (permalink / raw)
  To: Doug Anderson, Ulf Hansson, Arend van Spriel
  Cc: Kalle Valo, brcm80211-dev-list.pdl, open list:ARM/Rockchip SoC...,
	Double Lo, Brian Norris, linux-wireless, Naveen Gupta,
	Madhan Mohan R, Matthias Kaehlcke, Wright Feng, Chi-Hsien Lin,
	netdev, brcm80211-dev-list, linux-mmc@vger.kernel.org,
	Linux Kernel Mailing List, Thomas Gleixner, Greg Kroah-Hartman,
	Avri Altman
In-Reply-To: <CAD=FV=Wuj=gANR2im_o4ZnoLEB+U6FqzKe4noLdQyi1vw+K2xw@mail.gmail.com>

On 14/06/19 7:38 PM, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jun 14, 2019 at 5:10 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On Fri, 14 Jun 2019 at 01:42, Douglas Anderson <dianders@chromium.org> wrote:
>>>
>>> We want SDIO drivers to be able to temporarily stop retuning when the
>>> driver knows that the SDIO card is not in a state where retuning will
>>> work (maybe because the card is asleep).  We'll move the relevant
>>> functions to a place where drivers can call them.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>
>> This looks good to me.
>>
>> BTW, seems like this series is best funneled via my mmc tree, no? In
>> such case, I need acks for the brcmfmac driver patches.
> 
> For patch #1 I think it could just go in directly to the wireless
> tree.  It should be fine to land the rest of the patches separately.
> 
> For patch #2 - #5 then what you say makes sense to me.  I suppose
> you'd want at least a Reviewed-by from Arend and an Ack from Kalle on
> the Broadcom patches?
> 
> I'd also suggest that we Cc stable explicitly when applying.  That's
> easy for #2 and #3 since they have a Fixes tag.  For #4 and #5 I guess
> the question is how far back to go.  Maybe Adrian has an opinion here
> since I think he's the one who experienced these problems.

V4 seemed to apply cleanly back to v4.18

^ permalink raw reply

* [PATCH v2] wireless: airo: switch to skcipher interface
From: Ard Biesheuvel @ 2019-06-17  8:43 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-crypto, herbert, ebiggers, kvalo, johannes, linux,
	Ard Biesheuvel

The AIRO driver applies a ctr(aes) on a buffer of considerable size
(2400 bytes), and instead of invoking the crypto API to handle this
in its entirety, it open codes the counter manipulation and invokes
the AES block cipher directly.

Let's fix this, by switching to the sync skcipher API instead.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: - fix Kconfig dependencies
    - zero the output buffer and use it as input instead of the zero page

 drivers/net/wireless/cisco/Kconfig |  2 +
 drivers/net/wireless/cisco/airo.c  | 57 ++++++++++----------
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/cisco/Kconfig b/drivers/net/wireless/cisco/Kconfig
index 7329830ed7cc..01e173ede894 100644
--- a/drivers/net/wireless/cisco/Kconfig
+++ b/drivers/net/wireless/cisco/Kconfig
@@ -17,6 +17,7 @@ config AIRO
 	depends on CFG80211 && ISA_DMA_API && (PCI || BROKEN)
 	select WIRELESS_EXT
 	select CRYPTO
+	select CRYPTO_BLKCIPHER
 	select WEXT_SPY
 	select WEXT_PRIV
 	---help---
@@ -40,6 +41,7 @@ config AIRO_CS
 	select WEXT_PRIV
 	select CRYPTO
 	select CRYPTO_AES
+	select CRYPTO_CTR
 	---help---
 	  This is the standard Linux driver to support Cisco/Aironet PCMCIA
 	  802.11 wireless cards.  This driver is the same as the Aironet
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index 3f5a14112c6b..9342ffbe1e81 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -49,6 +49,9 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 
+#include <crypto/aes.h>
+#include <crypto/skcipher.h>
+
 #include <net/cfg80211.h>
 #include <net/iw_handler.h>
 
@@ -951,7 +954,7 @@ typedef struct {
 } mic_statistics;
 
 typedef struct {
-	u32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
+	__be32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
 	u64 accum;	// accumulated mic, reduced to u32 in final()
 	int position;	// current position (byte offset) in message
 	union {
@@ -1216,7 +1219,7 @@ struct airo_info {
 	struct iw_spy_data	spy_data;
 	struct iw_public_data	wireless_data;
 	/* MIC stuff */
-	struct crypto_cipher	*tfm;
+	struct crypto_sync_skcipher	*tfm;
 	mic_module		mod[2];
 	mic_statistics		micstats;
 	HostRxDesc rxfids[MPI_MAX_FIDS]; // rx/tx/config MPI350 descriptors
@@ -1291,14 +1294,14 @@ static int flashrestart(struct airo_info *ai,struct net_device *dev);
 static int RxSeqValid (struct airo_info *ai,miccntx *context,int mcast,u32 micSeq);
 static void MoveWindow(miccntx *context, u32 micSeq);
 static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
-			   struct crypto_cipher *tfm);
+			   struct crypto_sync_skcipher *tfm);
 static void emmh32_init(emmh32_context *context);
 static void emmh32_update(emmh32_context *context, u8 *pOctets, int len);
 static void emmh32_final(emmh32_context *context, u8 digest[4]);
 static int flashpchar(struct airo_info *ai,int byte,int dwelltime);
 
 static void age_mic_context(miccntx *cur, miccntx *old, u8 *key, int key_len,
-			    struct crypto_cipher *tfm)
+			    struct crypto_sync_skcipher *tfm)
 {
 	/* If the current MIC context is valid and its key is the same as
 	 * the MIC register, there's nothing to do.
@@ -1359,7 +1362,7 @@ static int micsetup(struct airo_info *ai) {
 	int i;
 
 	if (ai->tfm == NULL)
-		ai->tfm = crypto_alloc_cipher("aes", 0, 0);
+		ai->tfm = crypto_alloc_sync_skcipher("ctr(aes)", 0, 0);
 
         if (IS_ERR(ai->tfm)) {
                 airo_print_err(ai->dev->name, "failed to load transform for AES");
@@ -1624,37 +1627,31 @@ static void MoveWindow(miccntx *context, u32 micSeq)
 
 /* mic accumulate */
 #define MIC_ACCUM(val)	\
-	context->accum += (u64)(val) * context->coeff[coeff_position++];
-
-static unsigned char aes_counter[16];
+	context->accum += (u64)(val) * be32_to_cpu(context->coeff[coeff_position++]);
 
 /* expand the key to fill the MMH coefficient array */
 static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
-			   struct crypto_cipher *tfm)
+			   struct crypto_sync_skcipher *tfm)
 {
   /* take the keying material, expand if necessary, truncate at 16-bytes */
   /* run through AES counter mode to generate context->coeff[] */
   
-	int i,j;
-	u32 counter;
-	u8 *cipher, plain[16];
-
-	crypto_cipher_setkey(tfm, pkey, 16);
-	counter = 0;
-	for (i = 0; i < ARRAY_SIZE(context->coeff); ) {
-		aes_counter[15] = (u8)(counter >> 0);
-		aes_counter[14] = (u8)(counter >> 8);
-		aes_counter[13] = (u8)(counter >> 16);
-		aes_counter[12] = (u8)(counter >> 24);
-		counter++;
-		memcpy (plain, aes_counter, 16);
-		crypto_cipher_encrypt_one(tfm, plain, plain);
-		cipher = plain;
-		for (j = 0; (j < 16) && (i < ARRAY_SIZE(context->coeff)); ) {
-			context->coeff[i++] = ntohl(*(__be32 *)&cipher[j]);
-			j += 4;
-		}
-	}
+	SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
+	struct scatterlist sg;
+	u8 iv[AES_BLOCK_SIZE] = {};
+	int ret;
+
+	crypto_sync_skcipher_setkey(tfm, pkey, 16);
+
+	memset(context->coeff, 0, sizeof(context->coeff));
+	sg_init_one(&sg, context->coeff, sizeof(context->coeff));
+
+	skcipher_request_set_sync_tfm(req, tfm);
+	skcipher_request_set_callback(req, 0, NULL, NULL);
+	skcipher_request_set_crypt(req, &sg, &sg, sizeof(context->coeff), iv);
+
+	ret = crypto_skcipher_encrypt(req);
+	WARN_ON_ONCE(ret);
 }
 
 /* prepare for calculation of a new mic */
@@ -2415,7 +2412,7 @@ void stop_airo_card( struct net_device *dev, int freeres )
 				ai->shared, ai->shared_dma);
 		}
         }
-	crypto_free_cipher(ai->tfm);
+	crypto_free_sync_skcipher(ai->tfm);
 	del_airo_dev(ai);
 	free_netdev( dev );
 }
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4 4/5] mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
From: Adrian Hunter @ 2019-06-17  8:34 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, linux-mmc,
	linux-kernel, Thomas Gleixner, Greg Kroah-Hartman, Avri Altman
In-Reply-To: <20190613234153.59309-5-dianders@chromium.org>

On 14/06/19 2:41 AM, Douglas Anderson wrote:
> We want SDIO drivers to be able to temporarily stop retuning when the
> driver knows that the SDIO card is not in a state where retuning will
> work (maybe because the card is asleep).  We'll move the relevant
> functions to a place where drivers can call them.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> 
> Changes in v4:
> - Moved retune hold/release to SDIO API (Adrian).
> 
> Changes in v3:
> - ("mmc: core: Export mmc_retune_hold_now() mmc_retune_release()") new for v3.
> 
> Changes in v2: None
> 
>  drivers/mmc/core/sdio_io.c    | 40 +++++++++++++++++++++++++++++++++++
>  include/linux/mmc/sdio_func.h |  3 +++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
> index f822a9630b0e..1b6fe737bd72 100644
> --- a/drivers/mmc/core/sdio_io.c
> +++ b/drivers/mmc/core/sdio_io.c
> @@ -15,6 +15,7 @@
>  #include "sdio_ops.h"
>  #include "core.h"
>  #include "card.h"
> +#include "host.h"
>  
>  /**
>   *	sdio_claim_host - exclusively claim a bus for a certain SDIO function
> @@ -770,3 +771,42 @@ void sdio_retune_crc_enable(struct sdio_func *func)
>  	func->card->host->retune_crc_disable = false;
>  }
>  EXPORT_SYMBOL_GPL(sdio_retune_crc_enable);
> +
> +/**
> + *	sdio_retune_hold_now - start deferring retuning requests till release
> + *	@func: SDIO function attached to host
> + *
> + *	This function can be called if it's currently a bad time to do
> + *	a retune of the SDIO card.  Retune requests made during this time
> + *	will be held and we'll actually do the retune sometime after the
> + *	release.
> + *
> + *	This function could be useful if an SDIO card is in a power state
> + *	where it can respond to a small subset of commands that doesn't
> + *	include the retuning command.  Care should be taken when using
> + *	this function since (presumably) the retuning request we might be
> + *	deferring was made for a good reason.
> + *
> + *	This function should be called while the host is claimed.
> + */
> +void sdio_retune_hold_now(struct sdio_func *func)
> +{
> +	mmc_retune_hold_now(func->card->host);
> +}
> +EXPORT_SYMBOL_GPL(sdio_retune_hold_now);
> +
> +/**
> + *	sdio_retune_release - signal that it's OK to retune now
> + *	@func: SDIO function attached to host
> + *
> + *	This is the complement to sdio_retune_hold_now().  Calling this
> + *	function won't make a retune happen right away but will allow
> + *	them to be scheduled normally.
> + *
> + *	This function should be called while the host is claimed.
> + */
> +void sdio_retune_release(struct sdio_func *func)
> +{
> +	mmc_retune_release(func->card->host);
> +}
> +EXPORT_SYMBOL_GPL(sdio_retune_release);
> diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h
> index 4820e6d09dac..5a177f7a83c3 100644
> --- a/include/linux/mmc/sdio_func.h
> +++ b/include/linux/mmc/sdio_func.h
> @@ -170,4 +170,7 @@ extern int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags);
>  extern void sdio_retune_crc_disable(struct sdio_func *func);
>  extern void sdio_retune_crc_enable(struct sdio_func *func);
>  
> +extern void sdio_retune_hold_now(struct sdio_func *func);
> +extern void sdio_retune_release(struct sdio_func *func);
> +
>  #endif /* LINUX_MMC_SDIO_FUNC_H */
> 


^ permalink raw reply

* Re: [PATCH v4 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Adrian Hunter @ 2019-06-17  8:33 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Hans de Goede,
	Franky Lin, linux-kernel, Hante Meuleman, YueHaibing,
	David S. Miller
In-Reply-To: <20190613234153.59309-4-dianders@chromium.org>

On 14/06/19 2:41 AM, Douglas Anderson wrote:
> 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.
> 
> Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> 
> Changes in v4:
> - Adjust to API rename (Adrian, Ulf).
> 
> Changes in v3:
> - Expect errors for all of brcmf_sdio_kso_control() (Adrian).
> 
> Changes in v2: None
> 
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 4a750838d8cd..ee76593259a7 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -667,6 +667,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);
> @@ -727,6 +729,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;
>  }
>  
> 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox