Re: [PATCH] mwifiex: Use put_unaligned_le32

[Igor Mitsyanko]

On 10/05/2017 12:07 PM, Himanshu Jha wrote:


In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
seems that asm-generic/unaligned.h is set up to include different
headers, based on the expected architecture behavior.


Yes, asm-generic/unaligned.h looks more appopriate and is most generic
implementation of unaligned accesses and  arc specific.



Probably the idea is that each ARCH knows exactly what is the best way 
to do unaligned access, so common code (like drivers) should include 
ARCH-specific asm/unaligned.h only. It will then decide how to do that 
and will include asm-generic/unaligned.h itself, if required.

Re: converting mac80211 to TXQs entirely

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> On Thu, 2017-10-05 at 18:28 +0200, Toke Høiland-Jørgensen wrote:
>
>> I'm been thinking about how to move the airtime fairness scheduler
>> out of ath9k and into mac80211 (so more drivers can take advantage of
>> it). This will require some changes to the TXQ API that drivers speak
>> to, so wanted to add my thoughts here to make sure it's compatible
>> with your thinking.
>> 
>> I think the easiest way to have mac80211 handle airtime fairness is
>> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
>> signal which semantically means "there is no packet for this queue
>> right now, but please keep scheduling it" (which would be the result
>> of a TXQ belonging to a station that has used its airtime quota but
>> still has packets pending). This is different from the current
>> meaning of NULL, which will make the driver stop scheduling that TXQ
>> until it gets a new wake signal.
>
> I think that's reasonable. I'm not really sure it's *necessary*
> though? Couldn't mac80211 return NULL, and then simply call
> wake_tx_queue again when the TXQ becomes eligible for scheduling
> again? Otherwise the driver might end up doing a lot of polling for it
> to become eligible again?

Theoretically, but then there would have to be some kind of callback or
another mechanism to figure out when the queue is ready again. The neat
thing about DRR scheduling is that we just use the fact that there was
an attempt to schedule the queue as an opportunity to increase that
queue's deficit by one quantum. This does give a bit of "polling
overhead" as you say, but it has been hidden in the measurement noise in
all my tests.

The obvious alternative is to do a token-based airtime scheduler, where
the airtime used by one station is immediately divided out to all active
stations. But that requires walking over all active stations on every TX
completion, and there's a lot of housekeeping to make sure we don't
accidentally starve everyone. So I'd prefer to stick with the DRR
scheduler :)

>> And I'm wondering if this "is queue stopped" API could be the same
>> one used for the airtime DEFER case?
>
> I think these are two completely different cases.
>
> The "is queue stopped" I was thinking of would be basically checking
> the variable local->queue_stop_reasons, so that the driver could still
> use the stop_queue API(s). Yeah, this would be very roundabout, but as
> a conversion step I think that'd not be a bad thing.

Ah, I see. Yeah, these are different, and the existing packet/NULL
return is probably sufficient, then :)

>> Oh, and BTW: I see this is on the list of topics for the wireless
>> summit in Seoul. How do I go about signing up for that? I'll be at
>> netdev talking about some of this stuff anyway[1] :)
>
> Just show up there, or you can add yourself to the list on the wiki
> page :)

Cool, I did. See you in Seoul :)

-Toke

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Himanshu Jha]

On Thu, Oct 05, 2017 at 11:02:50AM -0700, Brian Norris wrote:
> On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> > There are various instances where a function used in file say for eg
> > int func_align (void* a)
> > is used and it is defined in align.h
> > But many files don't *directly* include align.h and rather include
> > any other header which includes align.h
> 
> I believe the general rule is that you should included headers for all
> symbols you use, and not rely on implicit includes.
> 
> The modification to the general rule is that not all headers are
> intended to be included directly, and in such cases there's likely a
> parent header that is the more appropriate target.
> 
> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
> seems that asm-generic/unaligned.h is set up to include different
> headers, based on the expected architecture behavior.
>
Yes, asm-generic/unaligned.h looks more appopriate and is most generic
implementation of unaligned accesses and  arc specific.

Let's see what Kalle Valo recommends! And then I will send v2 of the
patch.

Thanks for the information!

Himanshu Jha

> I wonder if include/linux/unaligned/access_ok.h should have a safety
> check (e.g., raise an #error if
> !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).
> 
> > Is compiling the file the only way to check if apppropriate header is
> > included or is there some other way to check for it.
> 
> I believe it's mostly manual. Implicit includes have been a problem for
> anyone who refactors header files.
> 
> Brian

Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned

[Marcelo Ricardo Leitner]

On Thu, Oct 05, 2017 at 09:16:31PM +0800, Herbert Xu wrote:
> On Thu, Oct 05, 2017 at 06:16:20PM +0800, Herbert Xu wrote:
> >
> > That was my point.  Functions like sctp_pack_cookie shouldn't be
> > setting the key in the first place.  The setkey should happen at
> > the point when the key is generated.  That's sctp_endpoint_init
> > which AFAICS only gets called in GFP_KERNEL context.
> > 
> > Or is there a code-path where sctp_endpoint_init is called in
> > softirq context?
> 
> OK, there are indeed code paths where the key is derived in softirq
> context.  Notably sctp_auth_calculate_hmac.
> 
> So I think this patch is the correct fix and I will push it upstream
> as well as back to stable.

Okay, thanks.

  Marcelo

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Brian Norris]

On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> There are various instances where a function used in file say for eg
> int func_align (void* a)
> is used and it is defined in align.h
> But many files don't *directly* include align.h and rather include
> any other header which includes align.h

I believe the general rule is that you should included headers for all
symbols you use, and not rely on implicit includes.

The modification to the general rule is that not all headers are
intended to be included directly, and in such cases there's likely a
parent header that is the more appropriate target.

In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
seems that asm-generic/unaligned.h is set up to include different
headers, based on the expected architecture behavior.

I wonder if include/linux/unaligned/access_ok.h should have a safety
check (e.g., raise an #error if
!CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).

> Is compiling the file the only way to check if apppropriate header is
> included or is there some other way to check for it.

I believe it's mostly manual. Implicit includes have been a problem for
anyone who refactors header files.

Brian

Re: [PATCH 00/18] use ARRAY_SIZE macro

[J. Bruce Fields]

On Mon, Oct 02, 2017 at 09:33:12PM -0400, Jérémy Lefaure wrote:
> On Mon, 2 Oct 2017 15:22:24 -0400
> bfie...@fieldses.org (J. Bruce Fields) wrote:
> 
> > Mainly I'd just like to know which you're asking for.  Do you want me to
> > apply this, or to ACK it so someone else can?  If it's sent as a series
> > I tend to assume the latter.
> > 
> > But in this case I'm assuming it's the former, so I'll pick up the nfsd
> > one
> Could you to apply the NFSD patch ("nfsd: use ARRAY_SIZE") with the
> Reviewed-by: Jeff Layton ) tag please ?
> 
> This patch is an individual patch and it should not have been sent in a
> series (sorry about that).

Applying for 4.15, thanks.--b.

[PATCH v2] net/mac80211/mesh_plink: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer back
to the sta_info since container_of() can't resolve the sta_info.

Cc: Johannes Berg 
Cc: "David S. Miller" 
Cc: linux-wireless@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: Thomas Gleixner 
Signed-off-by: Kees Cook 
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.

v2:
- make mesh_plink_timer non-static and use it in timer_setup() call directly.
---
 net/mac80211/mesh.h   |  1 +
 net/mac80211/mesh_plink.c | 10 --
 net/mac80211/sta_info.c   |  4 +++-
 net/mac80211/sta_info.h   |  2 ++
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 7e5f271e3c30..465b7853edc0 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -275,6 +275,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data 
*sdata,
   u8 *hw_addr, struct ieee802_11_elems *ie);
 bool mesh_peer_accepts_plinks(struct ieee802_11_elems *ie);
 u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata);
+void mesh_plink_timer(struct timer_list *t);
 void mesh_plink_broken(struct sta_info *sta);
 u32 mesh_plink_deactivate(struct sta_info *sta);
 u32 mesh_plink_open(struct sta_info *sta);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index f69c6c38ca43..e79adb4164f3 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -604,8 +604,9 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data 
*sdata,
ieee80211_mbss_info_change_notify(sdata, changed);
 }
 
-static void mesh_plink_timer(unsigned long data)
+void mesh_plink_timer(struct timer_list *t)
 {
+   struct mesh_sta *mesh = from_timer(mesh, t, plink_timer);
struct sta_info *sta;
u16 reason = 0;
struct ieee80211_sub_if_data *sdata;
@@ -617,7 +618,7 @@ static void mesh_plink_timer(unsigned long data)
 * del_timer_sync() this timer after having made sure
 * it cannot be readded (by deleting the plink.)
 */
-   sta = (struct sta_info *) data;
+   sta = mesh->plink_sta;
 
if (sta->sdata->local->quiescing)
return;
@@ -697,11 +698,8 @@ static void mesh_plink_timer(unsigned long data)
 
 static inline void mesh_plink_timer_set(struct sta_info *sta, u32 timeout)
 {
-   sta->mesh->plink_timer.expires = jiffies + msecs_to_jiffies(timeout);
-   sta->mesh->plink_timer.data = (unsigned long) sta;
-   sta->mesh->plink_timer.function = mesh_plink_timer;
sta->mesh->plink_timeout = timeout;
-   add_timer(>mesh->plink_timer);
+   mod_timer(>mesh->plink_timer, jiffies + msecs_to_jiffies(timeout));
 }
 
 static bool llid_in_use(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 69615016d5bf..6c254a9d5f11 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -329,10 +329,12 @@ struct sta_info *sta_info_alloc(struct 
ieee80211_sub_if_data *sdata,
sta->mesh = kzalloc(sizeof(*sta->mesh), gfp);
if (!sta->mesh)
goto free;
+   sta->mesh->plink_sta = sta;
spin_lock_init(>mesh->plink_lock);
if (ieee80211_vif_is_mesh(>vif) &&
!sdata->u.mesh.user_mpm)
-   init_timer(>mesh->plink_timer);
+   timer_setup(>mesh->plink_timer, mesh_plink_timer,
+   0);
sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
}
 #endif
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 3acbdfa9f649..21d9760ce5c3 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -344,6 +344,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
  * @plink_state: peer link state
  * @plink_timeout: timeout of peer link
  * @plink_timer: peer link watch timer
+ * @plink_sta: peer link watch timer's sta_info
  * @t_offset: timing offset relative to this host
  * @t_offset_setpoint: reference timing offset of this sta to be used when
  * calculating clockdrift
@@ -356,6 +357,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
  */
 struct mesh_sta {
struct timer_list plink_timer;
+   struct sta_info *plink_sta;
 
s64 t_offset;
s64 t_offset_setpoint;
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [08/11] ath10k_sdio: common read write

[Alagu Sankar]

Hi Gary,


On 2017-10-05 15:39, Gary Bisson wrote:

Hi Alagu,

On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcom...@gmail.com wrote:

From: Alagu Sankar 

convert different read write functions in sdio hif to bring it under a
single read-write path. This helps in having a common dma bounce 
buffer

implementation. Also helps in address modification that is required
specific to change in certain mbox addresses of sdio_write.

Signed-off-by: Alagu Sankar 
---
 drivers/net/wireless/ath/ath10k/sdio.c | 131 
-

 1 file changed, 64 insertions(+), 67 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c 
b/drivers/net/wireless/ath/ath10k/sdio.c

index 77d4fa4..bb6fa67 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -36,6 +36,11 @@

 #define ATH10K_SDIO_DMA_BUF_SIZE   (32 * 1024)

+static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
+   u32 len, bool incr);
+static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void 
*buf,

+u32 len, bool incr);
+


As mentioned by Kalle, u32 needs to be size_t.
Yes, the compiler I used is probably a step older and did not catch 
this.



 /* inlined helper functions */

 /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
@@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
struct sdio_func *func = ar_sdio->func;
unsigned char byte, asyncintdelay = 2;
int ret;
+   u32 addr;

ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");

@@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);

-   ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
- 
CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
- byte);
+   addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
+   ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);


Not sure this part is needed.
This is to overcome checkpatch warning. Too big a names for the function 
and macro

not helping in there. Will have to move it as a separate patch.



if (ret) {
ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
goto out;
@@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)

 static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 {
-   struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-   struct sdio_func *func = ar_sdio->func;
+   __le32 *buf;
int ret;

-   sdio_claim_host(func);
+   buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;

-   sdio_writel(func, val, addr, );
+   *buf = cpu_to_le32(val);
+
+   ret = ath10k_sdio_write(ar, addr, , sizeof(val), true);


Shouldn't we use buf instead of val? buf seems pretty useless 
otherwise.

Yes, thanks for pointing this out. will be corrected in v2.


Regards,
Gary

___
ath10k mailing list
ath...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

On Thu, 2017-10-05 at 18:28 +0200, Toke Høiland-Jørgensen wrote:

> I'm been thinking about how to move the airtime fairness scheduler
> out of ath9k and into mac80211 (so more drivers can take advantage of
> it). This will require some changes to the TXQ API that drivers speak
> to, so wanted to add my thoughts here to make sure it's compatible
> with your thinking.
> 
> I think the easiest way to have mac80211 handle airtime fairness is
> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
> signal which semantically means "there is no packet for this queue
> right now, but please keep scheduling it" (which would be the result
> of a TXQ belonging to a station that has used its airtime quota but
> still has packets pending). This is different from the current
> meaning of NULL, which will make the driver stop scheduling that TXQ
> until it gets a new wake signal.

I think that's reasonable. I'm not really sure it's *necessary* though?
Couldn't mac80211 return NULL, and then simply call wake_tx_queue again
when the TXQ becomes eligible for scheduling again? Otherwise the
driver might end up doing a lot of polling for it to become eligible
again?

I've mostly glossed over a mac80211 scheduler, which is obviously
needed as part of a complete conversion, and it'd just have to
integrate with this new return value.

> The alternative would be to change the API so the driver asks
> mac80211 which TXQ it should pull from next, instead of doing its own
> scheduling as it does now. But I'm not sure that's doable with a sane
> API.
> 
> Now, I believe this is related to this point of yours:
> 
> > 5) handle non-data frames for vifs
> > 
> > Similarly, we need a vif->nondata_txq where we can put probe
> > responses and the (few) other frames that we send before we have a
> > station entry.
> > 
> > According to my earlier analysis (previous email) after these steps
> > we have a TXQ for all frames. All of these steps will require
> > certain adjustments in the drivers currently using TXQs (ath9k &
> > ath10k) since they'll be relying on some amount of buffering and
> > queue stop/wake in mac80211 for these frames. We might just have to
> > add some API to ask "is queue stopped" to make the transition here
> > easy.
> 
> And I'm wondering if this "is queue stopped" API could be the same
> one used for the airtime DEFER case?

I think these are two completely different cases.

The "is queue stopped" I was thinking of would be basically checking
the variable local->queue_stop_reasons, so that the driver could still
use the stop_queue API(s). Yeah, this would be very roundabout, but as
a conversion step I think that'd not be a bad thing.

A more complete conversion likely wouldn't need this, but would instead
have the driver record its own stop reasons and just stop scheduling
those TXQs that belong to a stopped "class" (it's no longer really a
queue).

(and for mac80211 stop reasons, it would just return NULL and re-
schedule the TXQ when it becomes eligible again)


> Oh, and BTW: I see this is on the list of topics for the wireless
> summit in Seoul. How do I go about signing up for that? I'll be at
> netdev talking about some of this stuff anyway[1] :)

Just show up there, or you can add yourself to the list on the wiki
page :)

johannes

Re: converting mac80211 to TXQs entirely

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> Part 2 - where can we go from here
>
>
> Of course - as mentioned in the subject - my goal is to just convert
> over to TXQs entirely in mac80211. That would get rid of a LOT of
> special case code, like queueing for aggregation setup, powersave
> buffering (both unicast and multicast), etc.


At first glance this looks like a decent way forward. I'll think about
it some more and comment again later, but for now I just wanted to add
this:

I'm been thinking about how to move the airtime fairness scheduler out
of ath9k and into mac80211 (so more drivers can take advantage of it).
This will require some changes to the TXQ API that drivers speak to, so
wanted to add my thoughts here to make sure it's compatible with your
thinking.

I think the easiest way to have mac80211 handle airtime fairness is to
add a way for ieee80211_tx_dequeue() to return some sort of DEFER signal
which semantically means "there is no packet for this queue right now,
but please keep scheduling it" (which would be the result of a TXQ
belonging to a station that has used its airtime quota but still has
packets pending). This is different from the current meaning of NULL,
which will make the driver stop scheduling that TXQ until it gets a new
wake signal.

The alternative would be to change the API so the driver asks mac80211
which TXQ it should pull from next, instead of doing its own scheduling
as it does now. But I'm not sure that's doable with a sane API.

Now, I believe this is related to this point of yours:

> 5) handle non-data frames for vifs
>
> Similarly, we need a vif->nondata_txq where we can put probe responses
> and the (few) other frames that we send before we have a station entry.
>
> According to my earlier analysis (previous email) after these steps we
> have a TXQ for all frames. All of these steps will require certain
> adjustments in the drivers currently using TXQs (ath9k & ath10k) since
> they'll be relying on some amount of buffering and queue stop/wake in
> mac80211 for these frames. We might just have to add some API to ask
> "is queue stopped" to make the transition here easy.

And I'm wondering if this "is queue stopped" API could be the same one
used for the airtime DEFER case?


Oh, and BTW: I see this is on the list of topics for the wireless summit
in Seoul. How do I go about signing up for that? I'll be at netdev
talking about some of this stuff anyway[1] :)

-Toke

[1] https://www.netdevconf.org/2.2/session.html?jorgensen-wifistack-talk

Re: [PATCH] rsi: fix integer overflow warning

[Joe Perches]

On Thu, 2017-10-05 at 15:12 +, David Laight wrote:
> From: Joe Perches
> > Sent: 05 October 2017 13:19
> > On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> > > gcc produces a harmless warning about a recently introduced
> > > signed integer overflow:
> > > 
> > > drivers/net/wireless/rsi/rsi_91x_hal.c: In function 
> > > 'rsi_prepare_mgmt_desc':
> > > include/uapi/linux/swab.h:13:15: error: integer overflow in expression 
> > > [-Werror=overflow]
> > >   (((__u16)(x) & (__u16)0x00ffU) << 8) |   \
> > >^
> > > include/uapi/linux/swab.h:104:2: note: in expansion of macro 
> > > '___constant_swab16'
> > >   ___constant_swab16(x) :   \
> > >   ^~
> > > include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of 
> > > macro '__swab16'
> > >  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> > 
> > []
> > 
> > > The problem is that the 'mask' value is a signed integer that gets
> > > turned into a negative number when truncated to 16 bits. Making it
> > > an unsigned constant avoids this.
> > 
> > I would expect there are more of these.
> > 
> > Perhaps this define in include/uapi/linux/swab.h:
> > 
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16(x) : \
> > __fswab16(x))
> > 
> > should be
> > 
> > #define __swab16(x) \
> > (__builtin_constant_p((__u16)(x)) ? \
> > ___constant_swab16((__u16)(x)) :\
> > __fswab16((__u16)(x)))
> 
> You probably don't want the cast in the call to __fswab16() since
> that is likely to generate an explicit and with 0x.
> You will likely also get one if the argument is _u16 (not unsigned int).

It would just an explicit vs implicit cast as __fswab16 is
a static inline with a __u16 argument

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

Part 2 - where can we go from here


Of course - as mentioned in the subject - my goal is to just convert
over to TXQs entirely in mac80211. That would get rid of a LOT of
special case code, like queueing for aggregation setup, powersave
buffering (both unicast and multicast), etc.


I think the following would be appropriate steps to take

1) convert multicast PS buffering

This is a bit strange today - when multicast PS buffering *isn't*
needed, frames go to TXQ drivers via the vif->txq, but when it *is*
done, then frames are tagged with IEEE80211_TX_CTL_SEND_AFTER_DTIM and
are buffered on ps->bc_buf (if HOST_BROADCAST_PS_BUFFERING is set) and
then handed to the driver through the legacy ->tx() path.

This should be reasonably simple to change - get rid of ps->bc_buf, and
keep the frames on the TXQ, making ieee80211_get_buffered_bc() retrieve
them from there instead. We'd have to tell the driver (it needs to know
in the wake callback) that it has sleeping clients and needs to buffer,
so it can decide whether or not to retrieve immediately (basically, for
TXQ drivers, implementing HOST_BROADCAST_PS_BUFFERING in driver logic:
retrieve immediately if it wants to buffer itself, or keep them there
for a later ieee80211_get_buffered_bc() call if it doesn't).

2) use TXQ for offchannel frames

I'm a bit unsure about this - we never really want to queue offchannel
packets, and if they don't go out immediately then we can basically
drop them. Nevertheless, having everything deal with the TXQ API will
be simpler, and so I think this also should. Perhaps for this we should
have a TXQ but only ever use txqi->frags, so that we never really have
to deal with the FQ stuff for these frames. Then the wake would
basically just pull down the packets and send them immediately.

3) handle monitor mode

I thought this was more complicated, but I think the easiest way to
solve this is to actually just use the local->monitor_sdata->vif.txq.
This requires that mac80211 be changed to always allocate local-
>monitor_sdata, even if WANT_MONITOR_VIF isn't set, and ath9k/ath10k do
something special for this TXQ (and perhaps ath9k should set
WANT_MONITOR_VIF), but that seems reasonable.

4) handle non-data frames for stations

This is probably the trickiest part. I have a patch to add a non-data
TID to the STA TXQs, and that's perhaps the right thing to do - though
I guess those frames should again always go onto txqi->frags so they
don't compete with data frames for resources.

For powersave reasons we'll discuss later, this should probably only
apply to bufferable MMPDUs, with others going to the vif->nondata_txq
(next item).

5) handle non-data frames for vifs

Similarly, we need a vif->nondata_txq where we can put probe responses
and the (few) other frames that we send before we have a station entry.



According to my earlier analysis (previous email) after these steps we
have a TXQ for all frames. All of these steps will require certain
adjustments in the drivers currently using TXQs (ath9k & ath10k) since
they'll be relying on some amount of buffering and queue stop/wake in
mac80211 for these frames. We might just have to add some API to ask
"is queue stopped" to make the transition here easy.


After this, we can start thinking about doing internal cleanups in
mac80211.


6) First thing to do is probably to introduce a compat layer, so that
all drivers go through the TXQs, regardless of whether they handle
that. I have the beginnings of a patch that does that, it basically
requires drivers to set the wake_tx_queue method to a new mac80211
function ieee80211_wake_tx_queue() [so the ops can remain const] when
they don't actually want to deal with TXQs themselves.

This method can then check the queue stop reasons etc. and only service
TXQs that don't have their corresponding queue stopped. We'd also hook
into when the queues get re-enabled, and call the servicing function
from there for such drivers.

My current prototype just calls the existing __ieee80211_tx() but I no
longer think that's a good idea - the idea is that this would
eventually allow us to get rid of tx_pending.

So it'd be better to have ieee80211_wake_tx_queue() just check all the
required things, and once a frame is pulled from a TXQ it's committed
to be given to the driver.

For off-channel, a special case will be needed - dropping the frame
when there's no way to transmit it at the time.

7) Remove all the now-dead code

A lot of code is now dead - we'll never have multiple netdev queues,
all IFF_NOQUEUE etc - remove all the code associated with that


8) convert more infrastructure to TXQs

It gets more vague (starting from 6), but eventually I want to
 * get rid of tx_pending (why do we even use both TXQ and tx_pending
   for aggregation?)
 * do all powersave buffering on TXQs
   [this may be tricky for filtered frames, perhaps disallow filtered
   for TXQ drivers like ath9k/ath10k, and have a separate per-TXQI list
   in the private txq data for 

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Himanshu Jha]

On Thu, Oct 05, 2017 at 11:41:38AM +0300, Kalle Valo wrote:
> Himanshu Jha  writes:
> 
> >> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > @@ -17,6 +17,7 @@
> >> >   * this warranty disclaimer.
> >> >   */
> >> >  
> >> > +#include 
> >> 
> >> I don't think this is correct. Should it be asm/unaligned.h?
> >
> > Would mind explainig me as to why it is incorrect! Also, it defined in
> > both the header files but, why is asm/unaligned.h preferred ?
> 
> asm/unaligned.h seems to be the toplevel header file which includes
> header files based on arch configuration. Also grepping the sources
> support that, nobody from drivers/ include access_ok.h directly.
>
Yes, you are correct!

I will send v2 patch with asm/unaligned.h

> But I can't say that I fully understand how the header files work so
> please do correct me if I have mistaken.
>
It is confusing to me as well.
There are various instances where a function used in file say for eg
int func_align (void* a)
is used and it is defined in align.h
But many files don't *directly* include align.h and rather include
any other header which includes align.h

Is compiling the file the only way to check if apppropriate header is
included or is there some other way to check for it.

Thanks

RE: [PATCH] rsi: fix integer overflow warning

[David Laight]

From: Joe Perches
> Sent: 05 October 2017 13:19
> On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> > gcc produces a harmless warning about a recently introduced
> > signed integer overflow:
> >
> > drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> > include/uapi/linux/swab.h:13:15: error: integer overflow in expression 
> > [-Werror=overflow]
> >   (((__u16)(x) & (__u16)0x00ffU) << 8) |   \
> >^
> > include/uapi/linux/swab.h:104:2: note: in expansion of macro 
> > '___constant_swab16'
> >   ___constant_swab16(x) :   \
> >   ^~
> > include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of 
> > macro '__swab16'
> >  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> 
> []
> 
> > The problem is that the 'mask' value is a signed integer that gets
> > turned into a negative number when truncated to 16 bits. Making it
> > an unsigned constant avoids this.
> 
> I would expect there are more of these.
> 
> Perhaps this define in include/uapi/linux/swab.h:
> 
> #define __swab16(x)   \
>   (__builtin_constant_p((__u16)(x)) ? \
>   ___constant_swab16(x) : \
>   __fswab16(x))
> 
> should be
> 
> #define __swab16(x)   \
>   (__builtin_constant_p((__u16)(x)) ? \
>   ___constant_swab16((__u16)(x)) :\
>   __fswab16((__u16)(x)))

You probably don't want the cast in the call to __fswab16() since
that is likely to generate an explicit and with 0x.
You will likely also get one if the argument is _u16 (not unsigned int).

David

Re: [PATCH 00/11] SDIO support for ath10k

[Gary Bisson]

Hi Alagu,

First of all, thank you for sharing your patches, this will be a very
nice improvement to have SDIO QCA9377 working with ath10k.

I've tried your series with Nitrogen7 [1] platform which is supported in
mainline already. It uses BD-SDMAC [2] which uses the same module as the
SX-SDMAC.

Below are some questions/remarks I have after the testing.

On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
> From: Alagu Sankar 
> 
> This patchset, generated against master-pending branch, enables a fully
> functional SDIO interface driver for ath10k.  Patches have been verified on
> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access 
> Point
> and P2P modes.
> 
> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1

Quick question on the firmware, is it the one from Kalle's repository?[3]

If so, where does this firmware comes from? Is 00061 the firmware
version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1

> with the board data from respective SDIO card vendors.

About the board-sdio.bin, is it just a copy of your bdwlan30.bin?

> Receive performance
> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s

Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
and 50Mbits/s in RX:
# iperf -c 192.168.1.1

Client connecting to 192.168.1.1, TCP port 5001
TCP window size: 43.8 KByte (default)

[  3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
5001
[ ID] Interval   Transfer Bandwidth
[  3]  0.0-10.0 sec  34.9 MBytes  29.2 Mbits/sec
# iperf -s

Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)

[  4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
50646
[ ID] Interval   Transfer Bandwidth
[  4]  0.0-10.0 sec  63.1 MBytes  52.7 Mbits/sec

Do you have any idea why? Note that qcacld-2.0 driver on that same
platform (same OS) gives the performances you advertize (150Mbits/s).

> This patchset differs from the previous high latency patches, specific to 
> SDIO.
> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs 
> the
> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
> this flag, the management frames are not sent out by the firmware. Possibility
> of management frames being sent via WMI and data frames through the reduced Tx
> completion needs to be probed further.
> 
> Further improvements can be done on the transmit path by implementing packet
> bundle. Scatter Gather is another area of improvement for both Transmit and
> Receive, but may not work on all platforms
> 
> Known issues: Surprise removal of the card, when the device is in connected
> state, delays sdio function remove due to delayed WMI command failures.
> Existing ath10k framework can not differentiate between a kernel module
> removae and the surprise removal of teh card.

Here are some questions:
- Is it normal to see a warning about board-2.bin, shouldn't it look for
  board-sdio.bin only?
[   14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/QCA9377/hw1.0/board-2.bin failed with error -2

- Did you have pre-cal and/or cal binaries for your testing?
[   14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2
[   14.149257] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/cal-sdio-mmc1:0001:1.bin failed with error -2

Also noticed that the "tx bitrate" output of 'iw link' is wrong in my
case:
# iw wlan0 link
Connected to 00:00:00:00:00:b0 (on wlan0)
SSID: TPLINK_AC_5G
freq: 5180
RX: 72072365 bytes (67934 packets)
TX: 79084128 bytes (73649 packets)
signal: -35 dBm
tx bitrate: 6.0 MBit/s

bss flags:  short-slot-time
dtim period:2
beacon int: 100

When connecting using qcacld driver it shows 433MBit/s as expected. Is
it working properly in your case?

As a FYI, I've built Erik's tree[4] for this testing, should I use
another tree instead?

Let me know your thoughts.

Regards,
Gary

[1] https://boundarydevices.com/product/nitrogen7/
[2] https://boundarydevices.com/product/bd_sdmac_wifi/
[3] https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested
[4] https://github.com/erstrom/linux-ath

Re: [PATCH] NFC: fdp: make struct nci_ops static

[Stephen Hemminger]

On Thu,  5 Oct 2017 10:47:12 +0100
Colin King  wrote:

> From: Colin Ian King 
> 
> The structure nci_ops is local to the source and does not need to
> be in global scope, so make it static.
> 
> Cleans up sparse warning:
> symbol 'nci_ops' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/nfc/fdp/fdp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
> index ec50027b0d8b..d5784a47fc13 100644
> --- a/drivers/nfc/fdp/fdp.c
> +++ b/drivers/nfc/fdp/fdp.c
> @@ -726,7 +726,7 @@ static struct nci_driver_ops fdp_prop_ops[] = {
>   },
>  };
>  
> -struct nci_ops nci_ops = {
> +static struct nci_ops nci_ops = {
>   .open = fdp_nci_open,
>   .close = fdp_nci_close,
>   .send = fdp_nci_send,

Why not static const?

Yes this goes deeper. NFC needs to make all nfc ops const.

Re: [RFC v2 1/2] fq: support filtering a given tin

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> From: Johannes Berg 
>
> Add to the FQ API a way to filter a given tin, in order to
> remove frames that fulfil certain criteria according to a
> filter function.
>
> This will be used by mac80211 to remove frames belonging to
> an AP VLAN interface that's being removed.
>
> Signed-off-by: Johannes Berg 

Yup, this version looks reasonable to me :)

-Toke

[RFC v2 1/2] fq: support filtering a given tin

[Johannes Berg]

From: Johannes Berg 

Add to the FQ API a way to filter a given tin, in order to
remove frames that fulfil certain criteria according to a
filter function.

This will be used by mac80211 to remove frames belonging to
an AP VLAN interface that's being removed.

Signed-off-by: Johannes Berg 
---
 include/net/fq.h  |  7 +
 include/net/fq_impl.h | 72 ---
 2 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/include/net/fq.h b/include/net/fq.h
index 6d8521a30c5c..ac944a686840 100644
--- a/include/net/fq.h
+++ b/include/net/fq.h
@@ -90,6 +90,13 @@ typedef void fq_skb_free_t(struct fq *,
   struct fq_flow *,
   struct sk_buff *);
 
+/* Return %true to filter (drop) the frame. */
+typedef bool fq_skb_filter_t(struct fq *,
+struct fq_tin *,
+struct fq_flow *,
+struct sk_buff *,
+void *);
+
 typedef struct fq_flow *fq_flow_get_default_t(struct fq *,
  struct fq_tin *,
  int idx,
diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h
index 4e6131cd3f43..8b237e4afee6 100644
--- a/include/net/fq_impl.h
+++ b/include/net/fq_impl.h
@@ -12,24 +12,22 @@
 
 /* functions that are embedded into includer */
 
-static struct sk_buff *fq_flow_dequeue(struct fq *fq,
-  struct fq_flow *flow)
+static void fq_adjust_removal(struct fq *fq,
+ struct fq_flow *flow,
+ struct sk_buff *skb)
 {
struct fq_tin *tin = flow->tin;
-   struct fq_flow *i;
-   struct sk_buff *skb;
-
-   lockdep_assert_held(>lock);
-
-   skb = __skb_dequeue(>queue);
-   if (!skb)
-   return NULL;
 
tin->backlog_bytes -= skb->len;
tin->backlog_packets--;
flow->backlog -= skb->len;
fq->backlog--;
fq->memory_usage -= skb->truesize;
+}
+
+static void fq_rejigger_backlog(struct fq *fq, struct fq_flow *flow)
+{
+   struct fq_flow *i;
 
if (flow->backlog == 0) {
list_del_init(>backlogchain);
@@ -43,6 +41,21 @@ static struct sk_buff *fq_flow_dequeue(struct fq *fq,
list_move_tail(>backlogchain,
   >backlogchain);
}
+}
+
+static struct sk_buff *fq_flow_dequeue(struct fq *fq,
+  struct fq_flow *flow)
+{
+   struct sk_buff *skb;
+
+   lockdep_assert_held(>lock);
+
+   skb = __skb_dequeue(>queue);
+   if (!skb)
+   return NULL;
+
+   fq_adjust_removal(fq, flow, skb);
+   fq_rejigger_backlog(fq, flow);
 
return skb;
 }
@@ -188,6 +201,45 @@ static void fq_tin_enqueue(struct fq *fq,
}
 }
 
+static void fq_flow_filter(struct fq *fq,
+  struct fq_flow *flow,
+  fq_skb_filter_t filter_func,
+  void *filter_data,
+  fq_skb_free_t free_func)
+{
+   struct fq_tin *tin = flow->tin;
+   struct sk_buff *skb, *tmp;
+
+   lockdep_assert_held(>lock);
+
+   skb_queue_walk_safe(>queue, skb, tmp) {
+   if (!filter_func(fq, tin, flow, skb, filter_data))
+   continue;
+
+   __skb_unlink(skb, >queue);
+   fq_adjust_removal(fq, flow, skb);
+   free_func(fq, tin, flow, skb);
+   }
+
+   fq_rejigger_backlog(fq, flow);
+}
+
+static void fq_tin_filter(struct fq *fq,
+ struct fq_tin *tin,
+ fq_skb_filter_t filter_func,
+ void *filter_data,
+ fq_skb_free_t free_func)
+{
+   struct fq_flow *flow;
+
+   lockdep_assert_held(>lock);
+
+   list_for_each_entry(flow, >new_flows, flowchain)
+   fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+   list_for_each_entry(flow, >old_flows, flowchain)
+   fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+}
+
 static void fq_flow_reset(struct fq *fq,
  struct fq_flow *flow,
  fq_skb_free_t free_func)
-- 
2.14.2

[RFC v2 2/2] mac80211: only remove AP VLAN frames from TXQ

[Johannes Berg]

From: Johannes Berg 

When removing an AP VLAN interface, mac80211 currently purges
the entire TXQ for the AP interface. Fix this by using the FQ
API introduced in the previous patch to filter frames.

Signed-off-by: Johannes Berg 
---
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/iface.c   | 25 +++--
 net/mac80211/tx.c  | 34 ++
 3 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..68f874e73561 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2009,6 +2009,8 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data 
*sdata,
struct txq_info *txq, int tid);
 void ieee80211_txq_purge(struct ieee80211_local *local,
 struct txq_info *txqi);
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+  struct ieee80211_sub_if_data *sdata);
 void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 u16 transaction, u16 auth_alg, u16 status,
 const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2619daa29961..13b16f90e1cf 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -793,9 +793,7 @@ static int ieee80211_open(struct net_device *dev)
 static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
  bool going_down)
 {
-   struct ieee80211_sub_if_data *txq_sdata = sdata;
struct ieee80211_local *local = sdata->local;
-   struct fq *fq = >fq;
unsigned long flags;
struct sk_buff *skb, *tmp;
u32 hw_reconf_flags = 0;
@@ -939,9 +937,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
 
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP_VLAN:
-   txq_sdata = container_of(sdata->bss,
-struct ieee80211_sub_if_data, u.ap);
-
mutex_lock(>mtx);
list_del(>u.vlan.list);
mutex_unlock(>mtx);
@@ -998,8 +993,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
skb_queue_purge(>skb_queue);
}
 
-   sdata->bss = NULL;
-
spin_lock_irqsave(>queue_stop_reason_lock, flags);
for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
skb_queue_walk_safe(>pending[i], skb, tmp) {
@@ -1012,22 +1005,10 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
}
spin_unlock_irqrestore(>queue_stop_reason_lock, flags);
 
-   if (txq_sdata->vif.txq) {
-   struct txq_info *txqi = to_txq_info(txq_sdata->vif.txq);
+   if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+   ieee80211_txq_remove_vlan(local, sdata);
 
-   /*
-* FIXME FIXME
-*
-* We really shouldn't purge the *entire* txqi since that
-* contains frames for the other AP_VLANs (and possibly
-* the AP itself) as well, but there's no API in FQ now
-* to be able to filter.
-*/
-
-   spin_lock_bh(>lock);
-   ieee80211_txq_purge(local, txqi);
-   spin_unlock_bh(>lock);
-   }
+   sdata->bss = NULL;
 
if (local->open_count == 0)
ieee80211_clear_tx_pending(local);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..7b8154474b9e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1396,6 +1396,40 @@ static void ieee80211_txq_enqueue(struct ieee80211_local 
*local,
   fq_flow_get_default_func);
 }
 
+static bool fq_vlan_filter_func(struct fq *fq, struct fq_tin *tin,
+   struct fq_flow *flow, struct sk_buff *skb,
+   void *data)
+{
+   struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+   return info->control.vif == data;
+}
+
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+  struct ieee80211_sub_if_data *sdata)
+{
+   struct fq *fq = >fq;
+   struct txq_info *txqi;
+   struct fq_tin *tin;
+   struct ieee80211_sub_if_data *ap;
+
+   if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_AP_VLAN))
+   return;
+
+   ap = container_of(sdata->bss, struct ieee80211_sub_if_data, u.ap);
+
+   if (!ap->vif.txq)
+   return;
+
+   txqi = to_txq_info(ap->vif.txq);
+   tin = >tin;
+
+   spin_lock_bh(>lock);
+   fq_tin_filter(fq, tin, fq_vlan_filter_func, >vif,
+ fq_skb_free_func);
+   spin_unlock_bh(>lock);
+}
+
 void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
struct sta_info *sta,
   

Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned

[Herbert Xu]

On Thu, Oct 05, 2017 at 06:16:20PM +0800, Herbert Xu wrote:
>
> That was my point.  Functions like sctp_pack_cookie shouldn't be
> setting the key in the first place.  The setkey should happen at
> the point when the key is generated.  That's sctp_endpoint_init
> which AFAICS only gets called in GFP_KERNEL context.
> 
> Or is there a code-path where sctp_endpoint_init is called in
> softirq context?

OK, there are indeed code paths where the key is derived in softirq
context.  Notably sctp_auth_calculate_hmac.

So I think this patch is the correct fix and I will push it upstream
as well as back to stable.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC 1/2] fq: support filtering a given tin

[Johannes Berg]

On Thu, 2017-10-05 at 14:24 +0200, Toke Høiland-Jørgensen wrote:

> > +   for (;;) {
> > +   head = >new_flows;
> > +   if (list_empty(head)) {
> > +   head = >old_flows;
> > +   if (list_empty(head))
> > +   break;
> > +   }
> > +
> > +   flow = list_first_entry(head, struct fq_flow, flowchain);
> > +   fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
> > +   }
> 
> Isn't this going to loop forever?

Good question, I'll admit that I copied this without understanding it
from fq_tin_reset(), and didn't think about it much.

I think you're right though - I guess this needs to iterate the
new_flows and old_flows.

johannes

Re: [RFC 1/2] fq: support filtering a given tin

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> +static void fq_tin_filter(struct fq *fq,
> +   struct fq_tin *tin,
> +   fq_skb_filter_t filter_func,
> +   void *filter_data,
> +   fq_skb_free_t free_func)
> +{
> + struct list_head *head;
> + struct fq_flow *flow;
> +
> + lockdep_assert_held(>lock);
> +
> + for (;;) {
> + head = >new_flows;
> + if (list_empty(head)) {
> + head = >old_flows;
> + if (list_empty(head))
> + break;
> + }
> +
> + flow = list_first_entry(head, struct fq_flow, flowchain);
> + fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
> + }

Isn't this going to loop forever?

-Toke

Re: [PATCH] rsi: fix integer overflow warning

[Joe Perches]

On Thu, 2017-10-05 at 14:05 +0200, Arnd Bergmann wrote:
> gcc produces a harmless warning about a recently introduced
> signed integer overflow:
> 
> drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
> include/uapi/linux/swab.h:13:15: error: integer overflow in expression 
> [-Werror=overflow]
>   (((__u16)(x) & (__u16)0x00ffU) << 8) |   \
>^
> include/uapi/linux/swab.h:104:2: note: in expansion of macro 
> '___constant_swab16'
>   ___constant_swab16(x) :   \
>   ^~
> include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro 
> '__swab16'
>  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))

[]

> The problem is that the 'mask' value is a signed integer that gets
> turned into a negative number when truncated to 16 bits. Making it
> an unsigned constant avoids this.

I would expect there are more of these.

Perhaps this define in include/uapi/linux/swab.h:

#define __swab16(x) \
(__builtin_constant_p((__u16)(x)) ? \
___constant_swab16(x) : \
__fswab16(x))

should be

#define __swab16(x) \
(__builtin_c
onstant_p((__u16)(x)) ? \
___constant_swab16((__u16)(x)) :
\
__fswab16((__u16)(x)))

converting mac80211 to TXQs entirely

[Johannes Berg]

Hi,

Part 1 is just a dump of my notes analysing the current TX scheme.


driver setup


non-QUEUE_CONTROL drivers
 * have >= 4 queues
   - per-AC queues [0-3]
 * have < 4 queues
   - all goes to queue 0

QUEUE_CONTROL drivers
 * hwsim (doesn't handle CAB correctly)
   - each vif: 0...3
   - each cab: 0
   - offchannel: 4
 * TI (doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (separate)
   - each cab: separate queue
   - offchannel: separate from all others
 * iwldvm/iwlmvm (doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (separate)
   - each cab: separate queue
   - offchannel: separate from all others (AUX)
 * ath9k (uses TXQ, sets HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (shared based on chanctx)
   - each cab: all the same (# queues - 2)
   - offchannel: separate from all others
 * ath10k (may use TXQ, doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 1 queue for all ACs
   - each cab: same queue as for ACs
   - offchannel: separate from all others


current TX scheme
=
HOST_BROADCAST_PS_BUFFERING && IEEE80211_TX_CTL_SEND_AFTER_DTIM
 --> queue for ieee80211_get_buffered_bc()

!AP_LINK_PS && sta sleeping
 --> queue on sta->ps_tx_buf[ac] for wakeup/poll
 --> send on poll with IEEE80211_TX_CTRL_PS_RESPONSE
 --> send on wakeup as normal frame (with or without TXQ)
 [NB: with TXQs, this is buggy due to waking old TXQ before tx_filtered]

finally
 --> send directly (with or without TXQ)

if filtered TX status
 --> append to tx_filtered[ac] and use that before ps_tx_buf[ac]

TXQ scheme (where used)
===

MONITOR || IEEE80211_TX_CTL_SEND_AFTER_DTIM || IEEE80211_TX_CTRL_PS_RESPONSE || 
non-data
 --> send directly (to TX queue number as given above)
have STA
 --> per-STA/TID TXQ
otherwise
 --> per-VIF TXQ (for VLAN use AP instead)

[PATCH] rsi: fix integer overflow warning

[Arnd Bergmann]

gcc produces a harmless warning about a recently introduced
signed integer overflow:

drivers/net/wireless/rsi/rsi_91x_hal.c: In function 'rsi_prepare_mgmt_desc':
include/uapi/linux/swab.h:13:15: error: integer overflow in expression 
[-Werror=overflow]
  (((__u16)(x) & (__u16)0x00ffU) << 8) |   \
   ^
include/uapi/linux/swab.h:104:2: note: in expansion of macro 
'___constant_swab16'
  ___constant_swab16(x) :   \
  ^~
include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro 
'__swab16'
 #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
   ^~~~
include/linux/byteorder/generic.h:89:21: note: in expansion of macro 
'__cpu_to_le16'
 #define cpu_to_le16 __cpu_to_le16
 ^
drivers/net/wireless/rsi/rsi_91x_hal.c:136:3: note: in expansion of macro 
'cpu_to_le16'
   cpu_to_le16((tx_params->vap_id << RSI_DESC_VAP_ID_OFST) &
   ^~~

The problem is that the 'mask' value is a signed integer that gets
turned into a negative number when truncated to 16 bits. Making it
an unsigned constant avoids this.

Fixes: eac4eed3224b ("rsi: tx and rx path enhancements for p2p mode")
Signed-off-by: Arnd Bergmann 
---
 drivers/net/wireless/rsi/rsi_mgmt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_mgmt.h 
b/drivers/net/wireless/rsi/rsi_mgmt.h
index b9d0802c1b0f..e21723013f8d 100644
--- a/drivers/net/wireless/rsi/rsi_mgmt.h
+++ b/drivers/net/wireless/rsi/rsi_mgmt.h
@@ -189,7 +189,7 @@
 IEEE80211_WMM_IE_STA_QOSINFO_AC_BE | \
 IEEE80211_WMM_IE_STA_QOSINFO_AC_BK)
 
-#define RSI_DESC_VAP_ID_MASK   0xC000
+#define RSI_DESC_VAP_ID_MASK   0xC000u
 #define RSI_DESC_VAP_ID_OFST   14
 #define RSI_DATA_DESC_MAC_BBP_INFO BIT(0)
 #define RSI_DATA_DESC_NO_ACK_IND   BIT(9)
-- 
2.9.0

[RFC 1/2] fq: support filtering a given tin

[Johannes Berg]

From: Johannes Berg 

Add to the FQ API a way to filter a given tin, in order to
remove frames that fulfil certain criteria according to a
filter function.

This will be used by mac80211 to remove frames belonging to
an AP VLAN interface that's being removed.

Signed-off-by: Johannes Berg 
---
 include/net/fq.h  |  7 
 include/net/fq_impl.h | 92 ++-
 2 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/include/net/fq.h b/include/net/fq.h
index 6d8521a30c5c..ac944a686840 100644
--- a/include/net/fq.h
+++ b/include/net/fq.h
@@ -90,6 +90,13 @@ typedef void fq_skb_free_t(struct fq *,
   struct fq_flow *,
   struct sk_buff *);
 
+/* Return %true to filter (drop) the frame. */
+typedef bool fq_skb_filter_t(struct fq *,
+struct fq_tin *,
+struct fq_flow *,
+struct sk_buff *,
+void *);
+
 typedef struct fq_flow *fq_flow_get_default_t(struct fq *,
  struct fq_tin *,
  int idx,
diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h
index 4e6131cd3f43..b27f13d22a90 100644
--- a/include/net/fq_impl.h
+++ b/include/net/fq_impl.h
@@ -12,24 +12,9 @@
 
 /* functions that are embedded into includer */
 
-static struct sk_buff *fq_flow_dequeue(struct fq *fq,
-  struct fq_flow *flow)
+static void fq_rejigger_backlog(struct fq *fq, struct fq_flow *flow)
 {
-   struct fq_tin *tin = flow->tin;
struct fq_flow *i;
-   struct sk_buff *skb;
-
-   lockdep_assert_held(>lock);
-
-   skb = __skb_dequeue(>queue);
-   if (!skb)
-   return NULL;
-
-   tin->backlog_bytes -= skb->len;
-   tin->backlog_packets--;
-   flow->backlog -= skb->len;
-   fq->backlog--;
-   fq->memory_usage -= skb->truesize;
 
if (flow->backlog == 0) {
list_del_init(>backlogchain);
@@ -43,6 +28,34 @@ static struct sk_buff *fq_flow_dequeue(struct fq *fq,
list_move_tail(>backlogchain,
   >backlogchain);
}
+}
+
+static void fq_adjust_removal(struct fq *fq,
+ struct fq_flow *flow,
+ struct sk_buff *skb)
+{
+   struct fq_tin *tin = flow->tin;
+
+   tin->backlog_bytes -= skb->len;
+   tin->backlog_packets--;
+   flow->backlog -= skb->len;
+   fq->backlog--;
+   fq->memory_usage -= skb->truesize;
+}
+
+static struct sk_buff *fq_flow_dequeue(struct fq *fq,
+  struct fq_flow *flow)
+{
+   struct sk_buff *skb;
+
+   lockdep_assert_held(>lock);
+
+   skb = __skb_dequeue(>queue);
+   if (!skb)
+   return NULL;
+
+   fq_adjust_removal(fq, flow, skb);
+   fq_rejigger_backlog(fq, flow);
 
return skb;
 }
@@ -188,6 +201,53 @@ static void fq_tin_enqueue(struct fq *fq,
}
 }
 
+static void fq_flow_filter(struct fq *fq,
+  struct fq_flow *flow,
+  fq_skb_filter_t filter_func,
+  void *filter_data,
+  fq_skb_free_t free_func)
+{
+   struct fq_tin *tin = flow->tin;
+   struct sk_buff *skb, *tmp;
+
+   lockdep_assert_held(>lock);
+
+   skb_queue_walk_safe(>queue, skb, tmp) {
+   if (!filter_func(fq, tin, flow, skb, filter_data))
+   continue;
+
+   __skb_unlink(skb, >queue);
+   fq_adjust_removal(fq, flow, skb);
+   free_func(fq, tin, flow, skb);
+   }
+
+   fq_rejigger_backlog(fq, flow);
+}
+
+static void fq_tin_filter(struct fq *fq,
+ struct fq_tin *tin,
+ fq_skb_filter_t filter_func,
+ void *filter_data,
+ fq_skb_free_t free_func)
+{
+   struct list_head *head;
+   struct fq_flow *flow;
+
+   lockdep_assert_held(>lock);
+
+   for (;;) {
+   head = >new_flows;
+   if (list_empty(head)) {
+   head = >old_flows;
+   if (list_empty(head))
+   break;
+   }
+
+   flow = list_first_entry(head, struct fq_flow, flowchain);
+   fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+   }
+}
+
 static void fq_flow_reset(struct fq *fq,
  struct fq_flow *flow,
  fq_skb_free_t free_func)
-- 
2.14.2

[RFC 2/2] mac80211: only remove AP VLAN frames from TXQ

[Johannes Berg]

From: Johannes Berg 

When removing an AP VLAN interface, mac80211 currently purges
the entire TXQ for the AP interface. Fix this by using the FQ
API introduced in the previous patch to filter frames.

Signed-off-by: Johannes Berg 
---
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/iface.c   | 25 +++--
 net/mac80211/tx.c  | 34 ++
 3 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..68f874e73561 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2009,6 +2009,8 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data 
*sdata,
struct txq_info *txq, int tid);
 void ieee80211_txq_purge(struct ieee80211_local *local,
 struct txq_info *txqi);
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+  struct ieee80211_sub_if_data *sdata);
 void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 u16 transaction, u16 auth_alg, u16 status,
 const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2619daa29961..13b16f90e1cf 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -793,9 +793,7 @@ static int ieee80211_open(struct net_device *dev)
 static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
  bool going_down)
 {
-   struct ieee80211_sub_if_data *txq_sdata = sdata;
struct ieee80211_local *local = sdata->local;
-   struct fq *fq = >fq;
unsigned long flags;
struct sk_buff *skb, *tmp;
u32 hw_reconf_flags = 0;
@@ -939,9 +937,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
 
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP_VLAN:
-   txq_sdata = container_of(sdata->bss,
-struct ieee80211_sub_if_data, u.ap);
-
mutex_lock(>mtx);
list_del(>u.vlan.list);
mutex_unlock(>mtx);
@@ -998,8 +993,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
skb_queue_purge(>skb_queue);
}
 
-   sdata->bss = NULL;
-
spin_lock_irqsave(>queue_stop_reason_lock, flags);
for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
skb_queue_walk_safe(>pending[i], skb, tmp) {
@@ -1012,22 +1005,10 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
}
spin_unlock_irqrestore(>queue_stop_reason_lock, flags);
 
-   if (txq_sdata->vif.txq) {
-   struct txq_info *txqi = to_txq_info(txq_sdata->vif.txq);
+   if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+   ieee80211_txq_remove_vlan(local, sdata);
 
-   /*
-* FIXME FIXME
-*
-* We really shouldn't purge the *entire* txqi since that
-* contains frames for the other AP_VLANs (and possibly
-* the AP itself) as well, but there's no API in FQ now
-* to be able to filter.
-*/
-
-   spin_lock_bh(>lock);
-   ieee80211_txq_purge(local, txqi);
-   spin_unlock_bh(>lock);
-   }
+   sdata->bss = NULL;
 
if (local->open_count == 0)
ieee80211_clear_tx_pending(local);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..7b8154474b9e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1396,6 +1396,40 @@ static void ieee80211_txq_enqueue(struct ieee80211_local 
*local,
   fq_flow_get_default_func);
 }
 
+static bool fq_vlan_filter_func(struct fq *fq, struct fq_tin *tin,
+   struct fq_flow *flow, struct sk_buff *skb,
+   void *data)
+{
+   struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+   return info->control.vif == data;
+}
+
+void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
+  struct ieee80211_sub_if_data *sdata)
+{
+   struct fq *fq = >fq;
+   struct txq_info *txqi;
+   struct fq_tin *tin;
+   struct ieee80211_sub_if_data *ap;
+
+   if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_AP_VLAN))
+   return;
+
+   ap = container_of(sdata->bss, struct ieee80211_sub_if_data, u.ap);
+
+   if (!ap->vif.txq)
+   return;
+
+   txqi = to_txq_info(ap->vif.txq);
+   tin = >tin;
+
+   spin_lock_bh(>lock);
+   fq_tin_filter(fq, tin, fq_vlan_filter_func, >vif,
+ fq_skb_free_func);
+   spin_unlock_bh(>lock);
+}
+
 void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
struct sta_info *sta,
   

Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned

[Herbert Xu]

On Wed, Oct 04, 2017 at 09:37:58PM -0700, David Miller wrote:
>
> > I'm not talking about the code-path in question.  I'm talking
> > about the function which generates the secret key in the first
> > place.  AFAICS that's only called in GFP_KERNEL context.  What
> > am I missing?
> 
> The setkey happens in functions like sctp_pack_cookie() and
> sctp_unpack_cookie(), which seems to run from software interrupts.

That was my point.  Functions like sctp_pack_cookie shouldn't be
setting the key in the first place.  The setkey should happen at
the point when the key is generated.  That's sctp_endpoint_init
which AFAICS only gets called in GFP_KERNEL context.

Or is there a code-path where sctp_endpoint_init is called in
softirq context?

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation

[Thomas Gleixner]

On Thu, 5 Oct 2017, Daniel Drake wrote:
> On Mon, Oct 2, 2017 at 10:38 PM, Thomas Gleixner  wrote:
> > What's wrong with just using the legacy INTx emulation if you cannot
> > allocate 4 MSI vectors?
> 
> The Legacy interrupt simply doesn't work for the wifi on at least 8 new Acer
> laptop products based on Intel Apollo Lake.
> Plus 4 Dell systems included in the patches in this thread:
> https://lkml.org/lkml/2017/9/26/55
> (the 2 which I can find specs for are also Apollo Lake)
>
> We have tried taking the mini-PCIe wifi module out of one of the affected
> Acer products and moved it to another computer, where it is working fine
> with legacy interrupts. So this suggests that the wifi module itself is OK,
> but we are facing a hardware limitation or BIOS limitation on the affected
> products. In the Dell thread it says "Some platform(BIOS) blocks legacy
> interrupts (INTx)".
>
> If you have any suggestions for how we might solve this without getting into
> the MSI mess then that would be much appreciated. If the BIOS blocks the
> interrupts, can Linux unblock them?

I'm pretty sure we can. Cc'ed Rafael and Andy. They might know, if not they
certainly know whom to ask @Intel.

> Just for reference I'm attaching my latest attempt at enabling MULTI_PCI_MSI.
> It would definitely need further work if we proceed here - so far I've
> ignored the affinity considerations that you explained, and it's not
> particularly clean.

Yeah, I know how that looks. When I rewrote the allocator I initialy had
that multi-vector mode implemented and then ditched it due to the affinity
setting mess and because its hard vs. the global best effort reservation
mode, which is way more important to have than multi MSI.

>  int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
> -  bool reserved, unsigned int *mapped_cpu);
> +  bool reserved, unsigned int *mapped_cpu, unsigned int num,
> +  unsigned int align_mask);

That's not needed. We can assume that multivector allocations must be
aligned in the following way:

count = __roundup_pow_of_two(count);
mask = ilog2(count);

That's a sane assumption in general.

Thanks,

tglx

Re: [08/11] ath10k_sdio: common read write

[Gary Bisson]

Hi Alagu,

On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcom...@gmail.com wrote:
> From: Alagu Sankar 
> 
> convert different read write functions in sdio hif to bring it under a
> single read-write path. This helps in having a common dma bounce buffer
> implementation. Also helps in address modification that is required
> specific to change in certain mbox addresses of sdio_write.
> 
> Signed-off-by: Alagu Sankar 
> ---
>  drivers/net/wireless/ath/ath10k/sdio.c | 131 
> -
>  1 file changed, 64 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c 
> b/drivers/net/wireless/ath/ath10k/sdio.c
> index 77d4fa4..bb6fa67 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -36,6 +36,11 @@
>  
>  #define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024)
>  
> +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
> + u32 len, bool incr);
> +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
> +  u32 len, bool incr);
> +

As mentioned by Kalle, u32 needs to be size_t.

>  /* inlined helper functions */
>  
>  /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
> @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
>   struct sdio_func *func = ar_sdio->func;
>   unsigned char byte, asyncintdelay = 2;
>   int ret;
> + u32 addr;
>  
>   ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
>  
> @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
>CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
>CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
>  
> - ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
> -   
> CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> -   byte);
> + addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> + ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);

Not sure this part is needed.

>   if (ret) {
>   ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
>   goto out;
> @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
>  
>  static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
>  {
> - struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> - struct sdio_func *func = ar_sdio->func;
> + __le32 *buf;
>   int ret;
>  
> - sdio_claim_host(func);
> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
>  
> - sdio_writel(func, val, addr, );
> + *buf = cpu_to_le32(val);
> +
> + ret = ath10k_sdio_write(ar, addr, , sizeof(val), true);

Shouldn't we use buf instead of val? buf seems pretty useless otherwise.

Regards,
Gary

[PATCH] NFC: fdp: make struct nci_ops static

[Colin King]

From: Colin Ian King 

The structure nci_ops is local to the source and does not need to
be in global scope, so make it static.

Cleans up sparse warning:
symbol 'nci_ops' was not declared. Should it be static?

Signed-off-by: Colin Ian King 
---
 drivers/nfc/fdp/fdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
index ec50027b0d8b..d5784a47fc13 100644
--- a/drivers/nfc/fdp/fdp.c
+++ b/drivers/nfc/fdp/fdp.c
@@ -726,7 +726,7 @@ static struct nci_driver_ops fdp_prop_ops[] = {
},
 };
 
-struct nci_ops nci_ops = {
+static struct nci_ops nci_ops = {
.open = fdp_nci_open,
.close = fdp_nci_close,
.send = fdp_nci_send,
-- 
2.14.1

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Kalle Valo]

Himanshu Jha  writes:

>> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
>> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
>> > @@ -17,6 +17,7 @@
>> >   * this warranty disclaimer.
>> >   */
>> >  
>> > +#include 
>> 
>> I don't think this is correct. Should it be asm/unaligned.h?
>
> Would mind explainig me as to why it is incorrect! Also, it defined in
> both the header files but, why is asm/unaligned.h preferred ?

asm/unaligned.h seems to be the toplevel header file which includes
header files based on arch configuration. Also grepping the sources
support that, nobody from drivers/ include access_ok.h directly.

But I can't say that I fully understand how the header files work so
please do correct me if I have mistaken.

-- 
Kalle Valo

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Himanshu Jha]

On Thu, Oct 05, 2017 at 10:23:37AM +0300, Kalle Valo wrote:
> Himanshu Jha  writes:
> 
> > Use put_unaligned_le32 rather than using byte ordering function and
> > memcpy which makes code clear.
> > Also, add the header file where it is declared.
> >
> > Done using Coccinelle and semantic patch used is :
> >
> > @ rule1 @
> > identifier tmp; expression ptr,x; type T;
> > @@
> >
> > - tmp = cpu_to_le32(x);
> >
> >   <+... when != tmp
> > - memcpy(ptr, (T), ...);
> > + put_unaligned_le32(x,ptr);
> >   ...+>
> >
> > @ depends on rule1 @
> > type j; identifier tmp;
> > @@
> >
> > - j tmp;
> >   ...when != tmp
> >
> > Signed-off-by: Himanshu Jha 
> > ---
> >  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 10 --
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c 
> > b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > index 0edc5d6..e28e119 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > @@ -17,6 +17,7 @@
> >   * this warranty disclaimer.
> >   */
> >  
> > +#include 
> 
> I don't think this is correct. Should it be asm/unaligned.h?

Would mind explainig me as to why it is incorrect! Also, it defined in
both the header files but, why is asm/unaligned.h preferred ?

Thanks

> -- 
> Kalle Valo

[PATCH v6] brcmfmac: add CLM download support

[Wright Feng]

From: Chung-Hsien Hsu 

The firmware for brcmfmac devices includes information regarding
regulatory constraints. For certain devices this information is kept
separately in a binary form that needs to be downloaded to the device.
This patch adds support to download this so-called CLM blob file. It
uses the same naming scheme as the other firmware files with extension
of .clm_blob.

The CLM blob file is optional. If the file does not exist, the download
process will be bypassed. It will not affect the driver loading.

Signed-off-by: Chung-Hsien Hsu 
---
v2: Revise commit message to describe in more detail
v3: Add error handling in brcmf_c_get_clm_name function
v4: Correct the length of dload_buf in brcmf_c_download function
v5: Remove unnecessary cast and alignment
v6: Add debug log for the case of no CLM file present
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h |  10 ++
 .../wireless/broadcom/brcm80211/brcmfmac/common.c  | 162 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.c|   2 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.h|   2 +
 .../broadcom/brcm80211/brcmfmac/fwil_types.h   |  31 
 .../wireless/broadcom/brcm80211/brcmfmac/pcie.c|  19 +++
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c|  19 +++
 .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c |  18 +++
 8 files changed, 263 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
index b55c329..df42e09 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
@@ -71,6 +71,7 @@ struct brcmf_bus_dcmd {
  * @wowl_config: specify if dongle is configured for wowl when going to suspend
  * @get_ramsize: obtain size of device memory.
  * @get_memdump: obtain device memory dump in provided buffer.
+ * @get_fwname: obtain firmware name.
  *
  * This structure provides an abstract interface towards the
  * bus specific driver. For control messages to common driver
@@ -87,6 +88,8 @@ struct brcmf_bus_ops {
void (*wowl_config)(struct device *dev, bool enabled);
size_t (*get_ramsize)(struct device *dev);
int (*get_memdump)(struct device *dev, void *data, size_t len);
+   int (*get_fwname)(struct device *dev, uint chip, uint chiprev,
+ unsigned char *fw_name);
 };
 
 
@@ -214,6 +217,13 @@ int brcmf_bus_get_memdump(struct brcmf_bus *bus, void 
*data, size_t len)
return bus->ops->get_memdump(bus->dev, data, len);
 }
 
+static inline
+int brcmf_bus_get_fwname(struct brcmf_bus *bus, uint chip, uint chiprev,
+unsigned char *fw_name)
+{
+   return bus->ops->get_fwname(bus->dev, chip, chiprev, fw_name);
+}
+
 /*
  * interface functions from common layer
  */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 7a2b495..5397727 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "core.h"
@@ -28,6 +29,7 @@
 #include "tracepoint.h"
 #include "common.h"
 #include "of.h"
+#include "firmware.h"
 
 MODULE_AUTHOR("Broadcom Corporation");
 MODULE_DESCRIPTION("Broadcom 802.11 wireless LAN fullmac driver.");
@@ -104,12 +106,140 @@ void brcmf_c_set_joinpref_default(struct brcmf_if *ifp)
brcmf_err("Set join_pref error (%d)\n", err);
 }
 
+static int brcmf_c_download(struct brcmf_if *ifp, u16 flag,
+   struct brcmf_dload_data_le *dload_buf,
+   u32 len)
+{
+   s32 err;
+
+   flag |= (DLOAD_HANDLER_VER << DLOAD_FLAG_VER_SHIFT);
+   dload_buf->flag = cpu_to_le16(flag);
+   dload_buf->dload_type = cpu_to_le16(DL_TYPE_CLM);
+   dload_buf->len = cpu_to_le32(len);
+   dload_buf->crc = cpu_to_le32(0);
+   len = sizeof(*dload_buf) + len - 1;
+
+   err = brcmf_fil_iovar_data_set(ifp, "clmload", dload_buf, len);
+
+   return err;
+}
+
+static int brcmf_c_get_clm_name(struct brcmf_if *ifp, u8 *clm_name)
+{
+   struct brcmf_bus *bus = ifp->drvr->bus_if;
+   struct brcmf_rev_info *ri = >drvr->revinfo;
+   u8 fw_name[BRCMF_FW_NAME_LEN];
+   u8 *ptr;
+   size_t len;
+   s32 err;
+
+   memset(fw_name, 0, BRCMF_FW_NAME_LEN);
+   err = brcmf_bus_get_fwname(bus, ri->chipnum, ri->chiprev, fw_name);
+   if (err) {
+   brcmf_err("get firmware name failed (%d)\n", err);
+   goto done;
+   }
+
+   /* generate CLM blob file name */
+   ptr = strrchr(fw_name, '.');
+   if (!ptr) {
+   err = -ENOENT;
+   goto done;
+   }
+
+   len = ptr - fw_name + 1;
+   if (len + strlen(".clm_blob") > 

Re: [PATCH] mwifiex: Use put_unaligned_le32

[Kalle Valo]

Himanshu Jha  writes:

> Use put_unaligned_le32 rather than using byte ordering function and
> memcpy which makes code clear.
> Also, add the header file where it is declared.
>
> Done using Coccinelle and semantic patch used is :
>
> @ rule1 @
> identifier tmp; expression ptr,x; type T;
> @@
>
> - tmp = cpu_to_le32(x);
>
>   <+... when != tmp
> - memcpy(ptr, (T), ...);
> + put_unaligned_le32(x,ptr);
>   ...+>
>
> @ depends on rule1 @
> type j; identifier tmp;
> @@
>
> - j tmp;
>   ...when != tmp
>
> Signed-off-by: Himanshu Jha 
> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c 
> b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 0edc5d6..e28e119 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -17,6 +17,7 @@
>   * this warranty disclaimer.
>   */
>  
> +#include 

I don't think this is correct. Should it be asm/unaligned.h?

-- 
Kalle Valo

Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi

[Kalle Valo]

Icenowy Zheng  writes:

> 于 2017年10月4日 GMT+08:00 下午6:11:45, Maxime Ripard
>  写到:
>>On Wed, Oct 04, 2017 at 10:02:48AM +, Arend van Spriel wrote:
>>> On 10/4/2017 11:03 AM, Icenowy Zheng wrote:
>>> > 
>>> > 
>>> > 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo 
>>写到:
>>> > > Icenowy Zheng  writes:
>>> > > 
>>> > > > Allwinner XR819 is a SDIO Wi-Fi chip, which has the
>>functionality to
>>> > > use
>>> > > > an out-of-band interrupt pin instead of SDIO in-band interrupt.
>>> > > > 
>>> > > > Add the device tree binding of this chip, in order to make it
>>> > > possible
>>> > > > to add this interrupt pin to device trees.
>>> > > > 
>>> > > > Signed-off-by: Icenowy Zheng 
>>> > > > Acked-by: Rob Herring 
>>> > > > ---
>>> > > > Changes in v3:
>>> > > > - Renames the node name.
>>> > > > - Adds ACK from Rob.
>>> > > > Changes in v2:
>>> > > > - Removed status property in example.
>>> > > > - Added required property reg.
>>> > > > 
>>> > > >   .../bindings/net/wireless/allwinner,xr819.txt  | 38
>>> > > ++
>>> > > >   1 file changed, 38 insertions(+)
>>> > > >   create mode 100644
>>> > >
>>Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
>>> > > 
>>> > > Like I asked already last time, AFAICS there is no upstream xr819
>>> > > wireless driver in drivers/net/wireless directory. Do we still
>>accept
>>> > > bindings like this for out-of-tree drivers?
>>> > 
>>> > See esp8089.
>>> > 
>>> > There's also no in-tree driver for it.
>>> 
>>> The question is whether we should. The above might be a precedent,
>>but it
>>> may not necessarily be the way to go. The commit message for esp8089
>>seems
>>> to hint that there is intent to have an in-tree driver:
>>> 
>>> """
>>> Note that at this point there only is an out of tree driver for
>>this
>>> hardware, there is no clear timeline / path for merging this.
>>Still
>>> I believe it would be good to specify the binding for this in
>>tree
>>> now, so that any future migration to an in tree driver will not
>>cause
>>> compatiblity issues.
>>> 
>>> Cc: Icenowy Zheng 
>>> Signed-off-by: Hans de Goede 
>>> Signed-off-by: Rob Herring 
>>> """
>>> 
>>> Regardless the bindings are in principle independent of the kernel
>>and just
>>> describing hardware. I think there have been discussions to move the
>>> bindings to their own repository, but apparently it was decided
>>otherwise.
>>
>>Yeah, I guess especially how it could be merged with the cw1200 driver
>>would be very relevant to that commit log.
>
> The cw1200 driver seems to still have some legacy platform
> data. Maybe they should also be convert to DT.
> (Or maybe compatible = "allwinner,xr819" is enough, as
> xr819 is a specified variant of cw1200 family)

Ah, so the upstream cw1200 driver supports xr819? Has anyone tested
that? Or does cw1200 more changes than just adding the DT support?

-- 
Kalle Valo

Re: [PATCH] net/mac80211/mesh_plink: Convert timers to use

[Johannes Berg]

On Wed, 2017-10-04 at 17:49 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to all timer callbacks, switch to using the new timer_setup()
> and from_timer() to pass the timer pointer explicitly. This requires
> adding a pointer back to the sta_info since container_of() can't
> resolve the sta_info.

The subject seems to be lacking something ... :-)

> This requires commit 686fef928bba ("timer: Prepare to change timer
> callback argument type") in v4.14-rc3, but should be otherwise
> stand-alone.

I still can't apply that because that's not in net-next right now.

>  static inline void mesh_plink_timer_set(struct sta_info *sta, u32
> timeout)
>  {
>   sta->mesh->plink_timer.expires = jiffies +
> msecs_to_jiffies(timeout);
> - sta->mesh->plink_timer.data = (unsigned long) sta;
> - sta->mesh->plink_timer.function = mesh_plink_timer;
> + sta->mesh->plink_sta = sta;
> + sta->mesh->plink_timer.function =
> (TIMER_FUNC_TYPE)mesh_plink_timer;
>   sta->mesh->plink_timeout = timeout;
>   add_timer(>mesh->plink_timer);

Wouldn't it be better to convert this to timer_setup() now?

That add_timer() should probably also be mod_timer() anyway?

> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index 69615016d5bf..5e5de9455e4e 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -332,7 +332,7 @@ struct sta_info *sta_info_alloc(struct
> ieee80211_sub_if_data *sdata,
>   spin_lock_init(>mesh->plink_lock);
>   if (ieee80211_vif_is_mesh(>vif) &&
>   !sdata->u.mesh.user_mpm)
> - init_timer(>mesh->plink_timer);
> + timer_setup(>mesh->plink_timer, NULL,
> 0);
>   sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
>   }

You just have to make mesh_plink_timer() non-static, put a prototype
into mesh.h and then you can use the proper timer_setup() here with the
function?

Also, the sta->mesh->plink_sta assignment should be here I'd say, no
point rewriting it all the time.

I guess you were shooting for minimal, and I suppose we can do the
cleanups later too ...

johannes

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi all,

Please drop my patches, Qualcomm is working internally and will submit
the MSI patch by themselves.
Thanks.

Hi Daniel,

I'll try your patches tomorrow.

Best regards,
AceLan Kao.

2017-10-02 12:21 GMT+08:00 Daniel Drake :
> Hi AceLan,
>
> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao  wrote:
>> Hi Daniel,
>>
>> I've tried your patch, but it doesn't work for me.
>> Wifi can scan AP, but can't get connected.
>
> Can you please clarify which patch(es) you have tried?
>
> This is the base patch which adds the infrastructure to request
> specific MSI IRQ vectors:
> https://marc.info/?l=linux-wireless=150631274108016=2
>
> This is the ath9k MSI patch which makes use of that:
> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>
> If you were already able to use ath9k MSI interrupts without specific
> consideration for which MSI vector numbers were used, these are the
> possible explanations that spring to mind:
>
> 1. You got lucky and it picked a vector number that is 4-aligned. You
> can check this in the "lspci -vvv" output. You'll see something like:
> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
> Address: fee0300c  Data: 4142
> The lower number is the vector number. In my example here 0x42 (66) is
> not 4-aligned so the failure condition will be hit.
>
> 2. You are using interrupt remapping, which I suspect may provide a
> high likelihood of MSI interrupt vectors being 4-aligned. See if
> /proc/interrupts shows the IRQ type as IR-PCI-MSI
> Unfortunately interrupt remapping is not available here,
> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>
> 3. My assumption that all ath9k hardware corrupts the MSI vector
> number could wrong. However we've seen this on different wifi modules
> in laptops produced by different OEMs and ODMs, so it seems to be a
> somewhat widespread problem at least.
>
> 4. My assumption that ath9k hardware is corrupting the MSI vector
> number could be wrong; maybe another component is to blame, could it
> be a BIOS issue? Admittedly I don't really know how I can debug the
> layers inbetween seeing the MSI Message Data value disagree with the
> vector number being handled inside do_IRQ().
>
> Daniel

[PATCH] ath10k: fix sending wmi cmd during the tdls teardown

[akolli]

From: Anilkumar Kolli 

The current firmware 10.4-3.5.1-00035 on QCA9888 supports
TDLS explicit mode, it expects WMI_TDLS_ENABLE_PASSIVE
for tdls setup and WMI_TDLS_DISABLE for tdls teardown.

Signed-off-by: Anilkumar Kolli 
---
 drivers/net/wireless/ath/ath10k/wmi.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
b/drivers/net/wireless/ath/ath10k/wmi.c
index 38a97086708b..cad2e42dcef6 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -7870,7 +7870,8 @@ static int ath10k_wmi_10_4_op_get_vdev_subtype(struct 
ath10k *ar,
if (!skb)
return ERR_PTR(-ENOMEM);
 
-   if (test_bit(WMI_SERVICE_TDLS_EXPLICIT_MODE_ONLY, ar->wmi.svc_map))
+   if (test_bit(WMI_SERVICE_TDLS_EXPLICIT_MODE_ONLY, ar->wmi.svc_map) &&
+   state == WMI_TDLS_ENABLE_ACTIVE)
state = WMI_TDLS_ENABLE_PASSIVE;
 
if (test_bit(WMI_SERVICE_TDLS_UAPSD_BUFFER_STA, ar->wmi.svc_map))
-- 
1.7.9.5

[PATCH 2/2] ath10k: block offchannel operations if TDLS session is active

[akolli]

From: Anilkumar Kolli 

Do not allow off channel operations like scans/roc when
there are active TDLS sessions.

The Current firmware 10.4-3.5.1-00035 on QCA9888 does not
supports any offchannel operations on active TDLS sessions,
either driver needs to block the offchannel operation requests
or should teardown the TDLS connection.

Signed-off-by: Anilkumar Kolli 
---
 drivers/net/wireless/ath/ath10k/mac.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 0f14da73cf64..2f3851e688f4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5641,6 +5641,11 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
 
mutex_lock(>conf_mutex);
 
+   if (ath10k_mac_tdls_vif_stations_count(hw, vif) > 0) {
+   ret = -EBUSY;
+   goto exit;
+   }
+
spin_lock_bh(>data_lock);
switch (ar->scan.state) {
case ATH10K_SCAN_IDLE:
@@ -6477,6 +6482,11 @@ static int ath10k_remain_on_channel(struct ieee80211_hw 
*hw,
 
mutex_lock(>conf_mutex);
 
+   if (ath10k_mac_tdls_vif_stations_count(hw, vif) > 0) {
+   ret = -EBUSY;
+   goto exit;
+   }
+
spin_lock_bh(>data_lock);
switch (ar->scan.state) {
case ATH10K_SCAN_IDLE:
-- 
1.7.9.5

[PATCH 1/2] ath10k: move ath10k_mac_tdls_vif*() functions

[akolli]

From: Anilkumar Kolli 

To be able to use ath10k_mac_tdls_vif_stations_count() in
ath10k_hw_scan() in the following patch, move the functions
earlier in the file.

This commit is pure code move, no functional changes.

Signed-off-by: Anilkumar Kolli 
---
 drivers/net/wireless/ath/ath10k/mac.c |  106 -
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 5683f1a5330e..0f14da73cf64 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5575,6 +5575,59 @@ static void ath10k_mac_op_set_coverage_class(struct 
ieee80211_hw *hw, s16 value)
ar->hw_params.hw_ops->set_coverage_class(ar, value);
 }
 
+struct ath10k_mac_tdls_iter_data {
+   u32 num_tdls_stations;
+   struct ieee80211_vif *curr_vif;
+};
+
+static void ath10k_mac_tdls_vif_stations_count_iter(void *data,
+   struct ieee80211_sta *sta)
+{
+   struct ath10k_mac_tdls_iter_data *iter_data = data;
+   struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+   struct ieee80211_vif *sta_vif = arsta->arvif->vif;
+
+   if (sta->tdls && sta_vif == iter_data->curr_vif)
+   iter_data->num_tdls_stations++;
+}
+
+static int ath10k_mac_tdls_vif_stations_count(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif)
+{
+   struct ath10k_mac_tdls_iter_data data = {};
+
+   data.curr_vif = vif;
+
+   ieee80211_iterate_stations_atomic(hw,
+ 
ath10k_mac_tdls_vif_stations_count_iter,
+ );
+   return data.num_tdls_stations;
+}
+
+static void ath10k_mac_tdls_vifs_count_iter(void *data, u8 *mac,
+   struct ieee80211_vif *vif)
+{
+   struct ath10k_vif *arvif = (void *)vif->drv_priv;
+   int *num_tdls_vifs = data;
+
+   if (vif->type != NL80211_IFTYPE_STATION)
+   return;
+
+   if (ath10k_mac_tdls_vif_stations_count(arvif->ar->hw, vif) > 0)
+   (*num_tdls_vifs)++;
+}
+
+static int ath10k_mac_tdls_vifs_count(struct ieee80211_hw *hw)
+{
+   int num_tdls_vifs = 0;
+
+   ieee80211_iterate_active_interfaces_atomic(hw,
+  IEEE80211_IFACE_ITER_NORMAL,
+  
ath10k_mac_tdls_vifs_count_iter,
+  _tdls_vifs);
+   return num_tdls_vifs;
+}
+
 static int ath10k_hw_scan(struct ieee80211_hw *hw,
  struct ieee80211_vif *vif,
  struct ieee80211_scan_request *hw_req)
@@ -6000,59 +6053,6 @@ static void ath10k_mac_dec_num_stations(struct 
ath10k_vif *arvif,
ar->num_stations--;
 }
 
-struct ath10k_mac_tdls_iter_data {
-   u32 num_tdls_stations;
-   struct ieee80211_vif *curr_vif;
-};
-
-static void ath10k_mac_tdls_vif_stations_count_iter(void *data,
-   struct ieee80211_sta *sta)
-{
-   struct ath10k_mac_tdls_iter_data *iter_data = data;
-   struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
-   struct ieee80211_vif *sta_vif = arsta->arvif->vif;
-
-   if (sta->tdls && sta_vif == iter_data->curr_vif)
-   iter_data->num_tdls_stations++;
-}
-
-static int ath10k_mac_tdls_vif_stations_count(struct ieee80211_hw *hw,
- struct ieee80211_vif *vif)
-{
-   struct ath10k_mac_tdls_iter_data data = {};
-
-   data.curr_vif = vif;
-
-   ieee80211_iterate_stations_atomic(hw,
- 
ath10k_mac_tdls_vif_stations_count_iter,
- );
-   return data.num_tdls_stations;
-}
-
-static void ath10k_mac_tdls_vifs_count_iter(void *data, u8 *mac,
-   struct ieee80211_vif *vif)
-{
-   struct ath10k_vif *arvif = (void *)vif->drv_priv;
-   int *num_tdls_vifs = data;
-
-   if (vif->type != NL80211_IFTYPE_STATION)
-   return;
-
-   if (ath10k_mac_tdls_vif_stations_count(arvif->ar->hw, vif) > 0)
-   (*num_tdls_vifs)++;
-}
-
-static int ath10k_mac_tdls_vifs_count(struct ieee80211_hw *hw)
-{
-   int num_tdls_vifs = 0;
-
-   ieee80211_iterate_active_interfaces_atomic(hw,
-  IEEE80211_IFACE_ITER_NORMAL,
-  
ath10k_mac_tdls_vifs_count_iter,
-  _tdls_vifs);
-   return num_tdls_vifs;
-}
-
 static int ath10k_sta_state(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta,