Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Himanshu Jha @ 2017-10-05 15:22 UTC (permalink / raw)
  To: Kalle Valo
  Cc: amitkarwar, nishants, gbhat, huxm, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <87zi963s1p.fsf@codeaurora.org>

On Thu, Oct 05, 2017 at 11:41:38AM +0300, Kalle Valo wrote:
> Himanshu Jha <himanshujha199640@gmail.com> 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 <linux/unaligned/access_ok.h>
> >> 
> >> 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

^ permalink raw reply

* Re: [PATCH 00/11] SDIO support for ath10k
From: Gary Bisson @ 2017-10-05 15:12 UTC (permalink / raw)
  To: silexcommon; +Cc: ath10k, alagusankar, linux-wireless, erik.stromdahl
In-Reply-To: <1506793068-27445-1-git-send-email-alagusankar@silex-india.com>

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 <alagusankar at silex-india.com>
> 
> 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

^ permalink raw reply

* RE: [PATCH] rsi: fix integer overflow warning
From: David Laight @ 2017-10-05 15:12 UTC (permalink / raw)
  To: 'Joe Perches', Arnd Bergmann, Kalle Valo,
	Prameela Rani Garnepudi, Amitkumar Karwar
  Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1507205947.4434.23.camel@perches.com>

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 0xffff.
You will likely also get one if the argument is _u16 (not unsigned int).

	David

^ permalink raw reply

* Re: [PATCH] NFC: fdp: make struct nci_ops static
From: Stephen Hemminger @ 2017-10-05 15:09 UTC (permalink / raw)
  To: Colin King
  Cc: Samuel Ortiz, Andy Shevchenko, David S . Miller, Johannes Berg,
	linux-wireless, kernel-janitors, linux-kernel
In-Reply-To: <20171005094712.28627-1-colin.king@canonical.com>

On Thu,  5 Oct 2017 10:47:12 +0100
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> 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 <colin.king@canonical.com>
> ---
>  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.

^ permalink raw reply

* Re: [RFC v2 1/2] fq: support filtering a given tin
From: Toke Høiland-Jørgensen @ 2017-10-05 14:39 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless; +Cc: Johannes Berg
In-Reply-To: <20171005131803.12328-1-johannes@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> From: Johannes Berg <johannes.berg@intel.com>
>
> 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 <johannes.berg@intel.com>

Yup, this version looks reasonable to me :)

-Toke

^ permalink raw reply

* [RFC v2 2/2] mac80211: only remove AP VLAN frames from TXQ
From: Johannes Berg @ 2017-10-05 13:18 UTC (permalink / raw)
  To: linux-wireless; +Cc: Toke Høiland-Jørgensen, Johannes Berg
In-Reply-To: <20171005131803.12328-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
---
 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 = &local->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(&local->mtx);
 		list_del(&sdata->u.vlan.list);
 		mutex_unlock(&local->mtx);
@@ -998,8 +993,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		skb_queue_purge(&sdata->skb_queue);
 	}
 
-	sdata->bss = NULL;
-
 	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 	for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
 		skb_queue_walk_safe(&local->pending[i], skb, tmp) {
@@ -1012,22 +1005,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	}
 	spin_unlock_irqrestore(&local->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(&fq->lock);
-		ieee80211_txq_purge(local, txqi);
-		spin_unlock_bh(&fq->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 = &local->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 = &txqi->tin;
+
+	spin_lock_bh(&fq->lock);
+	fq_tin_filter(fq, tin, fq_vlan_filter_func, &sdata->vif,
+		      fq_skb_free_func);
+	spin_unlock_bh(&fq->lock);
+}
+
 void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
 			struct sta_info *sta,
 			struct txq_info *txqi, int tid)
-- 
2.14.2

^ permalink raw reply related

* [RFC v2 1/2] fq: support filtering a given tin
From: Johannes Berg @ 2017-10-05 13:18 UTC (permalink / raw)
  To: linux-wireless; +Cc: Toke Høiland-Jørgensen, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
---
 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(&fq->lock);
-
-	skb = __skb_dequeue(&flow->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(&flow->backlogchain);
@@ -43,6 +41,21 @@ static struct sk_buff *fq_flow_dequeue(struct fq *fq,
 		list_move_tail(&flow->backlogchain,
 			       &i->backlogchain);
 	}
+}
+
+static struct sk_buff *fq_flow_dequeue(struct fq *fq,
+				       struct fq_flow *flow)
+{
+	struct sk_buff *skb;
+
+	lockdep_assert_held(&fq->lock);
+
+	skb = __skb_dequeue(&flow->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(&fq->lock);
+
+	skb_queue_walk_safe(&flow->queue, skb, tmp) {
+		if (!filter_func(fq, tin, flow, skb, filter_data))
+			continue;
+
+		__skb_unlink(skb, &flow->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(&fq->lock);
+
+	list_for_each_entry(flow, &tin->new_flows, flowchain)
+		fq_flow_filter(fq, flow, filter_func, filter_data, free_func);
+	list_for_each_entry(flow, &tin->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

^ permalink raw reply related

* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Herbert Xu @ 2017-10-05 13:16 UTC (permalink / raw)
  To: David Miller
  Cc: marcelo.leitner, luto, baijiaju1990, nhorman, vyasevich, kvalo,
	linux-crypto, netdev, linux-sctp, linux-wireless
In-Reply-To: <20171005101620.GA1246@gondor.apana.org.au>

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [RFC 1/2] fq: support filtering a given tin
From: Johannes Berg @ 2017-10-05 13:13 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, linux-wireless; +Cc: Johannes Berg
In-Reply-To: <87k209sryx.fsf@toke.dk>

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

> > +	for (;;) {
> > +		head = &tin->new_flows;
> > +		if (list_empty(head)) {
> > +			head = &tin->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

^ permalink raw reply

* Re: [RFC 1/2] fq: support filtering a given tin
From: Toke Høiland-Jørgensen @ 2017-10-05 12:24 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless; +Cc: Johannes Berg
In-Reply-To: <20171005113808.3889-1-johannes@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> 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(&fq->lock);
> +
> +	for (;;) {
> +		head = &tin->new_flows;
> +		if (list_empty(head)) {
> +			head = &tin->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

^ permalink raw reply

* Re: [PATCH] rsi: fix integer overflow warning
From: Joe Perches @ 2017-10-05 12:19 UTC (permalink / raw)
  To: Arnd Bergmann, Kalle Valo, Prameela Rani Garnepudi,
	Amitkumar Karwar
  Cc: Pavani Muthyala, Karun Eagalapati, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20171005120547.328687-1-arnd@arndb.de>

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

^ permalink raw reply

* converting mac80211 to TXQs entirely
From: Johannes Berg @ 2017-10-05 12:13 UTC (permalink / raw)
  To: linux-wireless

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)

^ permalink raw reply

* [PATCH] rsi: fix integer overflow warning
From: Arnd Bergmann @ 2017-10-05 12:05 UTC (permalink / raw)
  To: Kalle Valo, Prameela Rani Garnepudi, Amitkumar Karwar
  Cc: Arnd Bergmann, Pavani Muthyala, Karun Eagalapati, linux-wireless,
	netdev, linux-kernel

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 <arnd@arndb.de>
---
 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

^ permalink raw reply related

* [RFC 1/2] fq: support filtering a given tin
From: Johannes Berg @ 2017-10-05 11:38 UTC (permalink / raw)
  To: linux-wireless; +Cc: Toke Høiland-Jørgensen, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
---
 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(&fq->lock);
-
-	skb = __skb_dequeue(&flow->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(&flow->backlogchain);
@@ -43,6 +28,34 @@ static struct sk_buff *fq_flow_dequeue(struct fq *fq,
 		list_move_tail(&flow->backlogchain,
 			       &i->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(&fq->lock);
+
+	skb = __skb_dequeue(&flow->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(&fq->lock);
+
+	skb_queue_walk_safe(&flow->queue, skb, tmp) {
+		if (!filter_func(fq, tin, flow, skb, filter_data))
+			continue;
+
+		__skb_unlink(skb, &flow->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(&fq->lock);
+
+	for (;;) {
+		head = &tin->new_flows;
+		if (list_empty(head)) {
+			head = &tin->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

^ permalink raw reply related

* [RFC 2/2] mac80211: only remove AP VLAN frames from TXQ
From: Johannes Berg @ 2017-10-05 11:38 UTC (permalink / raw)
  To: linux-wireless; +Cc: Toke Høiland-Jørgensen, Johannes Berg
In-Reply-To: <20171005113808.3889-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
---
 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 = &local->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(&local->mtx);
 		list_del(&sdata->u.vlan.list);
 		mutex_unlock(&local->mtx);
@@ -998,8 +993,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		skb_queue_purge(&sdata->skb_queue);
 	}
 
-	sdata->bss = NULL;
-
 	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 	for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
 		skb_queue_walk_safe(&local->pending[i], skb, tmp) {
@@ -1012,22 +1005,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	}
 	spin_unlock_irqrestore(&local->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(&fq->lock);
-		ieee80211_txq_purge(local, txqi);
-		spin_unlock_bh(&fq->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 = &local->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 = &txqi->tin;
+
+	spin_lock_bh(&fq->lock);
+	fq_tin_filter(fq, tin, fq_vlan_filter_func, &sdata->vif,
+		      fq_skb_free_func);
+	spin_unlock_bh(&fq->lock);
+}
+
 void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
 			struct sta_info *sta,
 			struct txq_info *txqi, int tid)
-- 
2.14.2

^ permalink raw reply related

* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Herbert Xu @ 2017-10-05 10:16 UTC (permalink / raw)
  To: David Miller
  Cc: marcelo.leitner, luto, baijiaju1990, nhorman, vyasevich, kvalo,
	linux-crypto, netdev, linux-sctp, linux-wireless
In-Reply-To: <20171004.213758.2210486785503998906.davem@davemloft.net>

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
From: Thomas Gleixner @ 2017-10-05 10:13 UTC (permalink / raw)
  To: Daniel Drake
  Cc: LKML, linux-pci, x86, linux-wireless, ath9k-devel, linux,
	Rafael J. Wysocki, Andy Shevchenko
In-Reply-To: <20171005042316.12261-1-drake@endlessm.com>

On Thu, 5 Oct 2017, Daniel Drake wrote:
> On Mon, Oct 2, 2017 at 10:38 PM, Thomas Gleixner <tglx@linutronix.de> 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

^ permalink raw reply

* Re: [08/11] ath10k_sdio: common read write
From: Gary Bisson @ 2017-10-05 10:09 UTC (permalink / raw)
  To: silexcommon; +Cc: ath10k, Alagu Sankar, linux-wireless
In-Reply-To: <1506793068-27445-9-git-send-email-alagusankar@silex-india.com>

Hi Alagu,

On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@gmail.com wrote:
> From: Alagu Sankar <alagusankar@silex-india.com>
> 
> 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 <alagusankar@silex-india.com>
> ---
>  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, &ret);
> +	*buf = cpu_to_le32(val);
> +
> +	ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);

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

Regards,
Gary

^ permalink raw reply

* [PATCH] NFC: fdp: make struct nci_ops static
From: Colin King @ 2017-10-05  9:47 UTC (permalink / raw)
  To: Samuel Ortiz, Andy Shevchenko, David S . Miller,
	Stephen Hemminger, Johannes Berg, linux-wireless
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

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 <colin.king@canonical.com>
---
 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

^ permalink raw reply related

* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Kalle Valo @ 2017-10-05  8:41 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: amitkarwar, nishants, gbhat, huxm, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20171005083433.GA11485@himanshu-Vostro-3559>

Himanshu Jha <himanshujha199640@gmail.com> 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 <linux/unaligned/access_ok.h>
>> 
>> 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

^ permalink raw reply

* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Himanshu Jha @ 2017-10-05  8:34 UTC (permalink / raw)
  To: Kalle Valo
  Cc: amitkarwar, nishants, gbhat, huxm, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <874lre5a86.fsf@codeaurora.org>

On Thu, Oct 05, 2017 at 10:23:37AM +0300, Kalle Valo wrote:
> Himanshu Jha <himanshujha199640@gmail.com> 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)&tmp, ...);
> > + put_unaligned_le32(x,ptr);
> >   ...+>
> >
> > @ depends on rule1 @
> > type j; identifier tmp;
> > @@
> >
> > - j tmp;
> >   ...when != tmp
> >
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > ---
> >  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 <linux/unaligned/access_ok.h>
> 
> 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

^ permalink raw reply

* [PATCH v6] brcmfmac: add CLM download support
From: Wright Feng @ 2017-10-05  7:31 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, kvalo, chi-hsien.lin
  Cc: wright.feng, linux-wireless, brcm80211-dev-list.pdl,
	Chung-Hsien Hsu

From: Chung-Hsien Hsu <stanley.hsu@cypress.com>

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 <stanley.hsu@cypress.com>
---
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 <linux/string.h>
 #include <linux/netdevice.h>
 #include <linux/module.h>
+#include <linux/firmware.h>
 #include <brcmu_wifi.h>
 #include <brcmu_utils.h>
 #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 = &ifp->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") > BRCMF_FW_NAME_LEN) {
+		err = -E2BIG;
+	} else {
+		strlcpy(clm_name, fw_name, len);
+		strlcat(clm_name, ".clm_blob", BRCMF_FW_NAME_LEN);
+	}
+done:
+	return err;
+}
+
+static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
+{
+	struct device *dev = ifp->drvr->bus_if->dev;
+	struct brcmf_dload_data_le *chunk_buf;
+	const struct firmware *clm = NULL;
+	u8 clm_name[BRCMF_FW_NAME_LEN];
+	u32 chunk_len;
+	u32 datalen;
+	u32 cumulative_len;
+	u16 dl_flag = DL_BEGIN;
+	u32 status;
+	s32 err;
+
+	brcmf_dbg(TRACE, "Enter\n");
+
+	memset(clm_name, 0, BRCMF_FW_NAME_LEN);
+	err = brcmf_c_get_clm_name(ifp, clm_name);
+	if (err) {
+		brcmf_err("get CLM blob file name failed (%d)\n", err);
+		return err;
+	}
+
+	err = request_firmware(&clm, clm_name, dev);
+	if (err) {
+		if (err == -ENOENT) {
+			brcmf_dbg(INFO, "continue with CLM data currently present in firmware\n");
+			return 0;
+		}
+		brcmf_err("request CLM blob file failed (%d)\n", err);
+		return err;
+	}
+
+	chunk_buf = kzalloc(sizeof(*chunk_buf) + MAX_CHUNK_LEN - 1, GFP_KERNEL);
+	if (!chunk_buf) {
+		err = -ENOMEM;
+		goto done;
+	}
+
+	datalen = clm->size;
+	cumulative_len = 0;
+	do {
+		if (datalen > MAX_CHUNK_LEN) {
+			chunk_len = MAX_CHUNK_LEN;
+		} else {
+			chunk_len = datalen;
+			dl_flag |= DL_END;
+		}
+		memcpy(chunk_buf->data, clm->data + cumulative_len, chunk_len);
+
+		err = brcmf_c_download(ifp, dl_flag, chunk_buf, chunk_len);
+
+		dl_flag &= ~DL_BEGIN;
+
+		cumulative_len += chunk_len;
+		datalen -= chunk_len;
+	} while ((datalen > 0) && (err == 0));
+
+	if (err) {
+		brcmf_err("clmload (%zu byte file) failed (%d); ",
+			  clm->size, err);
+		/* Retrieve clmload_status and print */
+		err = brcmf_fil_iovar_int_get(ifp, "clmload_status", &status);
+		if (err)
+			brcmf_err("get clmload_status failed (%d)\n", err);
+		else
+			brcmf_dbg(INFO, "clmload_status=%d\n", status);
+		err = -EIO;
+	}
+
+	kfree(chunk_buf);
+done:
+	release_firmware(clm);
+	return err;
+}
+
 int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 {
 	s8 eventmask[BRCMF_EVENTING_MASK_LEN];
 	u8 buf[BRCMF_DCMD_SMLEN];
 	struct brcmf_rev_info_le revinfo;
 	struct brcmf_rev_info *ri;
+	char *clmver;
 	char *ptr;
 	s32 err;
 
@@ -148,6 +278,13 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 	}
 	ri->result = err;
 
+	/* Do any CLM downloading */
+	err = brcmf_c_process_clm_blob(ifp);
+	if (err < 0) {
+		brcmf_err("download CLM blob file failed, %d\n", err);
+		goto done;
+	}
+
 	/* query for 'ver' to get version info from firmware */
 	memset(buf, 0, sizeof(buf));
 	strcpy(buf, "ver");
@@ -167,6 +304,31 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 	ptr = strrchr(buf, ' ') + 1;
 	strlcpy(ifp->drvr->fwver, ptr, sizeof(ifp->drvr->fwver));
 
+	/* Query for 'clmver' to get CLM version info from firmware */
+	memset(buf, 0, sizeof(buf));
+	err = brcmf_fil_iovar_data_get(ifp, "clmver", buf, sizeof(buf));
+	if (err) {
+		if (err == -23) {
+			brcmf_dbg(INFO, "clmver iovar unsupported, %d\n", err);
+		} else {
+			brcmf_err("retrieving clmver failed, %d\n", err);
+			goto done;
+		}
+	} else {
+		clmver = (char *)buf;
+		/* store CLM version for adding it to revinfo debugfs file */
+		memcpy(ifp->drvr->clmver, clmver, sizeof(ifp->drvr->clmver));
+
+		/* Replace all newline/linefeed characters with space
+		 * character
+		 */
+		ptr = clmver;
+		while ((ptr = strnchr(ptr, '\n', sizeof(buf))) != NULL)
+			*ptr = ' ';
+
+		brcmf_dbg(INFO, "CLM version = %s\n", clmver);
+	}
+
 	/* set mpc */
 	err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
 	if (err) {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 511d190..7414dff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -941,6 +941,8 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data)
 	seq_printf(s, "anarev: %u\n", ri->anarev);
 	seq_printf(s, "nvramrev: %08x\n", ri->nvramrev);
 
+	seq_printf(s, "clmrev: %s\n", bus_if->drvr->clmver);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index a4dd313..ae39128 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -141,6 +141,8 @@ struct brcmf_pub {
 	struct notifier_block inetaddr_notifier;
 	struct notifier_block inet6addr_notifier;
 	struct brcmf_mp_device *settings;
+
+	u8 clmver[BRCMF_DCMD_SMLEN];
 };
 
 /* forward declarations */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 9a1eb5a..fffe347 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -147,6 +147,21 @@
 #define BRCMF_MFP_CAPABLE		1
 #define BRCMF_MFP_REQUIRED		2
 
+/* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
+ * ioctl. It is relatively small because firmware has small maximum size input
+ * playload restriction for ioctls.
+ */
+#define MAX_CHUNK_LEN			1400
+
+#define DLOAD_HANDLER_VER		1	/* Downloader version */
+#define DLOAD_FLAG_VER_MASK		0xf000	/* Downloader version mask */
+#define DLOAD_FLAG_VER_SHIFT		12	/* Downloader version shift */
+
+#define DL_BEGIN			0x0002
+#define DL_END				0x0004
+
+#define DL_TYPE_CLM			2
+
 /* join preference types for join_pref iovar */
 enum brcmf_join_pref_types {
 	BRCMF_JOIN_PREF_RSSI = 1,
@@ -835,4 +850,20 @@ struct brcmf_gtk_keyinfo_le {
 	u8 replay_counter[BRCMF_RSN_REPLAY_LEN];
 };
 
+/**
+ * struct brcmf_dload_data_le - data passing to firmware for downloading
+ * @flag: flags related to download data.
+ * @dload_type: type of download data.
+ * @len: length in bytes of download data.
+ * @crc: crc of download data.
+ * @data: download data.
+ */
+struct brcmf_dload_data_le {
+	__le16 flag;
+	__le16 dload_type;
+	__le32 len;
+	__le32 crc;
+	u8 data[1];
+};
+
 #endif /* FWIL_TYPES_H_ */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f878706..b370ee7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1350,6 +1350,24 @@ static int brcmf_pcie_get_memdump(struct device *dev, void *data, size_t len)
 	return 0;
 }
 
+static int brcmf_pcie_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+				 u8 *fw_name)
+{
+	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+	struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+	struct brcmf_pciedev_info *devinfo = buspub->devinfo;
+	int ret = 0;
+
+	if (devinfo->fw_name[0] != '\0')
+		strlcpy(fw_name, devinfo->fw_name, BRCMF_FW_NAME_LEN);
+	else
+		ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+						brcmf_pcie_fwnames,
+						ARRAY_SIZE(brcmf_pcie_fwnames),
+						fw_name, NULL);
+
+	return ret;
+}
 
 static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
 	.txdata = brcmf_pcie_tx,
@@ -1359,6 +1377,7 @@ static int brcmf_pcie_get_memdump(struct device *dev, void *data, size_t len)
 	.wowl_config = brcmf_pcie_wowl_config,
 	.get_ramsize = brcmf_pcie_get_ramsize,
 	.get_memdump = brcmf_pcie_get_memdump,
+	.get_fwname = brcmf_pcie_get_fwname,
 };
 
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b1789b1..3a71ca0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3972,6 +3972,24 @@ static void brcmf_sdio_buscore_write32(void *ctx, u32 addr, u32 val)
 	}
 }
 
+static int brcmf_sdio_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+				 u8 *fw_name)
+{
+	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	int ret = 0;
+
+	if (sdiodev->fw_name[0] != '\0')
+		strlcpy(fw_name, sdiodev->fw_name, BRCMF_FW_NAME_LEN);
+	else
+		ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+						brcmf_sdio_fwnames,
+						ARRAY_SIZE(brcmf_sdio_fwnames),
+						fw_name, NULL);
+
+	return ret;
+}
+
 static const struct brcmf_bus_ops brcmf_sdio_bus_ops = {
 	.stop = brcmf_sdio_bus_stop,
 	.preinit = brcmf_sdio_bus_preinit,
@@ -3982,6 +4000,7 @@ static void brcmf_sdio_buscore_write32(void *ctx, u32 addr, u32 val)
 	.wowl_config = brcmf_sdio_wowl_config,
 	.get_ramsize = brcmf_sdio_bus_get_ramsize,
 	.get_memdump = brcmf_sdio_bus_get_memdump,
+	.get_fwname = brcmf_sdio_get_fwname,
 };
 
 static void brcmf_sdio_firmware_callback(struct device *dev, int err,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 8f20a4b..9622dd9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -1128,12 +1128,30 @@ static void brcmf_usb_wowl_config(struct device *dev, bool enabled)
 		device_set_wakeup_enable(devinfo->dev, false);
 }
 
+static int brcmf_usb_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+				u8 *fw_name)
+{
+	struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
+	int ret = 0;
+
+	if (devinfo->fw_name[0] != '\0')
+		strlcpy(fw_name, devinfo->fw_name, BRCMF_FW_NAME_LEN);
+	else
+		ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+						brcmf_usb_fwnames,
+						ARRAY_SIZE(brcmf_usb_fwnames),
+						fw_name, NULL);
+
+	return ret;
+}
+
 static const struct brcmf_bus_ops brcmf_usb_bus_ops = {
 	.txdata = brcmf_usb_tx,
 	.stop = brcmf_usb_down,
 	.txctl = brcmf_usb_tx_ctlpkt,
 	.rxctl = brcmf_usb_rx_ctlpkt,
 	.wowl_config = brcmf_usb_wowl_config,
+	.get_fwname = brcmf_usb_get_fwname,
 };
 
 static int brcmf_usb_bus_setup(struct brcmf_usbdev_info *devinfo)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] mwifiex: Use put_unaligned_le32
From: Kalle Valo @ 2017-10-05  7:23 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: amitkarwar, nishants, gbhat, huxm, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1507141686-5178-1-git-send-email-himanshujha199640@gmail.com>

Himanshu Jha <himanshujha199640@gmail.com> 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)&tmp, ...);
> + put_unaligned_le32(x,ptr);
>   ...+>
>
> @ depends on rule1 @
> type j; identifier tmp;
> @@
>
> - j tmp;
>   ...when != tmp
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
>  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 <linux/unaligned/access_ok.h>

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

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Kalle Valo @ 2017-10-05  6:58 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: linux-arm-kernel, Maxime Ripard, Arend van Spriel, devicetree,
	netdev, linux-sunxi, linux-wireless, linux-kernel, Chen-Yu Tsai,
	Rob Herring
In-Reply-To: <C4895259-FCDE-4B21-BB15-8F150FC53BE3@aosc.io>

Icenowy Zheng <icenowy@aosc.io> writes:

> =E4=BA=8E 2017=E5=B9=B410=E6=9C=884=E6=97=A5 GMT+08:00 =E4=B8=8B=E5=8D=88=
6:11:45, Maxime Ripard
> <maxime.ripard@free-electrons.com> =E5=86=99=E5=88=B0:
>>On Wed, Oct 04, 2017 at 10:02:48AM +0000, Arend van Spriel wrote:
>>> On 10/4/2017 11:03 AM, Icenowy Zheng wrote:
>>> >=20
>>> >=20
>>> > =E4=BA=8E 2017=E5=B9=B410=E6=9C=884=E6=97=A5 GMT+08:00 =E4=B8=8B=E5=
=8D=885:02:17, Kalle Valo <kvalo@codeaurora.org>
>>=E5=86=99=E5=88=B0:
>>> > > Icenowy Zheng <icenowy@aosc.io> writes:
>>> > >=20
>>> > > > 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.
>>> > > >=20
>>> > > > Add the device tree binding of this chip, in order to make it
>>> > > possible
>>> > > > to add this interrupt pin to device trees.
>>> > > >=20
>>> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> > > > Acked-by: Rob Herring <robh@kernel.org>
>>> > > > ---
>>> > > > Changes in v3:
>>> > > > - Renames the node name.
>>> > > > - Adds ACK from Rob.
>>> > > > Changes in v2:
>>> > > > - Removed status property in example.
>>> > > > - Added required property reg.
>>> > > >=20
>>> > > >   .../bindings/net/wireless/allwinner,xr819.txt      | 38
>>> > > ++++++++++++++++++++++
>>> > > >   1 file changed, 38 insertions(+)
>>> > > >   create mode 100644
>>> > >
>>Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
>>> > >=20
>>> > > 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?
>>> >=20
>>> > See esp8089.
>>> >=20
>>> > There's also no in-tree driver for it.
>>>=20
>>> 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:
>>>=20
>>> """
>>>     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.
>>>=20
>>>     Cc: Icenowy Zheng <icenowy@aosc.xyz>
>>>     Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>     Signed-off-by: Rob Herring <robh@kernel.org>
>>> """
>>>=20
>>> 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 =3D "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?

--=20
Kalle Valo

^ permalink raw reply

* Re: [PATCH] net/mac80211/mesh_plink: Convert timers to use
From: Johannes Berg @ 2017-10-05  6:47 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: David S. Miller, linux-wireless, netdev, Thomas Gleixner
In-Reply-To: <20171005004952.GA23133@beast>

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(&sta->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(&sta->mesh->plink_lock);
>  		if (ieee80211_vif_is_mesh(&sdata->vif) &&
>  		    !sdata->u.mesh.user_mpm)
> -			init_timer(&sta->mesh->plink_timer);
> +			timer_setup(&sta->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

^ 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