Re: [v3] b43: fix transmit failure when VT is switched

2018-05-14 Thread Kalle Valo
k...@sra-tohoku.co.jp wrote:

> From: Taketo Kabe 
> 
> Setup:
> Using BCM4306 rev.03 chip based CardBus wireless card.
> IRQ is shared with yenta (cardbus bridge) and i915 (display) driver.
> For firmware, installed latest but dated openfwwf 5.2
> (http://netweb.ing.unibs.it/~openfwwf/)
> 
> How-to-reproduce:
> Do "ssh ", then "ls -lR /" to generate traffic, then
> repeatedly switch VTs by Alt-F1<>Alt-F2.
> Eventually (within a minute) the card stops working.
> You can receive traffic but no transmission.
> For unknown reason it doesn't occur when just generating traffic by
> "ssh  ls -lR /".
> 
> With CONFIG_B43_DEBUG=y kernel config, when it stops,
> the debug message shows
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. 
> Expected 148, but got 180
> The slot offset I observed so far was always 32.
> 
> When err_out2 is not set to make error messages successive,
> the debug output will be like this:
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 148
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 150
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 120
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 152
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 122
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 154
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 124
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 156
> kernel: b43-phy1 debug: Out of order TX status report on DMA ring 1. Expected 
> 116, but got 126
> The TX ring alternates between 2 sequences; the ring seems
> to be completely confused. Controller restart is needed.
> 
> Workaround(1):
> This problem doesn't occur when using propriatory firmware
> you will extract by b43-fwcutter, so it may be a bug in
> openfwwf firmware, as the comment in the b43_dma_handle_txstatus() suggests.
> I wasn't able to find a bug in the terse openfwwf code though.
> 
> Workaround(2):
> Using "pio=1" option to not use DMA makes this problem to
> not occur.
> 
> Description of the patch:
> This patch will forcibly reset the controller to make it
> work again. Very kludgy and doesn't look right, but
> the traffic will continue to flow.
> 
> Signed-off-by: Taketo Kabe 
> Reviewed-by: Michael Buesch 

Patch applied to wireless-drivers-next.git, thanks.

66cffd6daab7 b43: fix transmit failure when VT is switched

-- 
https://patchwork.kernel.org/patch/10396133/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: [1/3] rsi: Add null check for virtual interfaces in wowlan config

2018-05-14 Thread Kalle Valo
Sushant Kumar Mishra  wrote:

> From: Sanjay Kumar Konduri 
> 
> When the "poweroff" command is executed after wowlan enabled, we have
> observed a system crash. In the system "poweroff" sequence, network-manager
> is sent to inactive state by cleaning up the network interfaces, using
> rsi_mac80211_remove_interface() and when driver tries to access those
> network interfaces in rsi_wowlan_config() which was invoked by SDIO
> shutdown, results in a crash. Added a NULL check before accessing the
> network interfaces in rsi_wowlan_config().
> 
> Signed-off-by: Sanjay Kumar Konduri 
> Signed-off-by: Siva Rebbagondla 
> Signed-off-by: Sushant Kumar Mishra 

3 patches applied to wireless-drivers-next.git, thanks.

54b5172087ae rsi: Add null check for virtual interfaces in wowlan config
d76f8513fa90 rsi: reset hibernate_resume flag to work hibernate resume in coex 
mode.
1d18c5584c05 rsi: Set wowlan flag while writing wowlan config parameters

-- 
https://patchwork.kernel.org/patch/10394551/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



Re: brcmfmac: set WIPHY_FLAG_HAVE_AP_SME flag

2018-05-14 Thread Kalle Valo
Rafał Miłecki wrote:

> From: Rafał Miłecki 
> 
> brcmfmac is a FullMAC driver and it implements/uses cfg80211 interface
> for stations management. At the same time it doesn't receive or pass up
> management frames.
> 
> This flag indicates that authenticator doesn't have to subscribe to or
> handle management frames. Some authenticators (e.g. hostapd) were
> working with brcmfmac thanks to some extra assumptions. This commit
> clears up the situation.
> 
> Signed-off-by: Rafał Miłecki 
> Acked-by: Arend van Spriel 

Patch applied to wireless-drivers-next.git, thanks.

1204aa17f3b4 brcmfmac: set WIPHY_FLAG_HAVE_AP_SME flag

-- 
https://patchwork.kernel.org/patch/10391817/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches



[RFC 2/2] mac80211: Add support for per-rate rx statistics

2018-05-14 Thread Sriram R
This patch adds support for the mac80211 to collect
rx statistics (Packet count and total bytes received)
per-rate, per-station when enabled by a user space application.

Note that the rate field passed on to the userspace from mac80211
is an encoded value where different rate attributes such as
type(VHT/HT/Legacy), MCS, BW, NSS, GI are collectively used to encode to
this rate and the userspace has to take care of decoding it.This is
done so as to allow scalability in future.

Once statistics collection is enabled, each packet received is recorded
per rate and stored as an entry(per rate) in a hashtable with the encoded rate 
being
the key for the hashtable.As the rate changes, new entries gets added
into this table and this information will be populated and sent back
to userspace when requested. The lifetime of this table is from the enabling
of the per-rate stats feature to disabling it or when the sta gets disconnected.

Memory usage(on a 32-bit machine) by this feature would be,
 96 bytes (static value, per STA for hash table usage)
+28 bytes (increases per rate entry)
+ 4 bytes (increases per hash bucket and rate is used as key.
   Default is 16 buckets)

Signed-off-by: Sriram R 
---
 net/mac80211/cfg.c  |  47 ++
 net/mac80211/rx.c   |  10 ++-
 net/mac80211/sta_info.c | 165 
 net/mac80211/sta_info.h |  17 +
 4 files changed, 237 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5ce9d12..abee3ff 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -703,6 +703,30 @@ static int ieee80211_dump_station(struct wiphy *wiphy, 
struct net_device *dev,
return ret;
 }
 
+static int
+ieee80211_dump_sta_rate_stats(struct wiphy *wiphy, struct net_device *dev,
+ int idx, u8 *mac,
+ struct sta_rate_stats **rate_stats_buf, int *len)
+{
+   struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+   struct ieee80211_local *local = sdata->local;
+   struct sta_info *sta;
+   int ret = -ENOENT;
+
+   mutex_lock(&local->sta_mtx);
+
+   sta = sta_info_get_by_idx(sdata, idx);
+   if (sta) {
+   memcpy(mac, sta->sta.addr, ETH_ALEN);
+   ret = ieee80211_sta_get_rate_stats_report(sta, rate_stats_buf,
+ len);
+   }
+
+   mutex_unlock(&local->sta_mtx);
+
+   return ret;
+}
+
 static int ieee80211_dump_survey(struct wiphy *wiphy, struct net_device *dev,
 int idx, struct survey_info *survey)
 {
@@ -732,6 +756,27 @@ static int ieee80211_get_station(struct wiphy *wiphy, 
struct net_device *dev,
return ret;
 }
 
+static int
+ieee80211_get_sta_rate_stats(struct wiphy *wiphy, struct net_device *dev,
+const u8 *mac,
+struct sta_rate_stats **rate_stats_buf, int *len)
+{
+   struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+   struct ieee80211_local *local = sdata->local;
+   struct sta_info *sta;
+   int ret = -ENOENT;
+
+   mutex_lock(&local->sta_mtx);
+
+   sta = sta_info_get_bss(sdata, mac);
+   if (sta)
+   ret = ieee80211_sta_get_rate_stats_report(sta, rate_stats_buf,
+ len);
+   mutex_unlock(&local->sta_mtx);
+
+   return ret;
+}
+
 static int ieee80211_set_monitor_channel(struct wiphy *wiphy,
 struct cfg80211_chan_def *chandef)
 {
@@ -3822,6 +3867,8 @@ const struct cfg80211_ops mac80211_config_ops = {
.change_station = ieee80211_change_station,
.get_station = ieee80211_get_station,
.dump_station = ieee80211_dump_station,
+   .get_sta_rate_stats = ieee80211_get_sta_rate_stats,
+   .dump_sta_rate_stats = ieee80211_dump_sta_rate_stats,
.dump_survey = ieee80211_dump_survey,
 #ifdef CONFIG_MAC80211_MESH
.add_mpath = ieee80211_add_mpath,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0a38cc1..0f63b18 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1561,9 +1561,11 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
test_sta_flag(sta, WLAN_STA_AUTHORIZED)) {
sta->rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control) &&
-   !is_multicast_ether_addr(hdr->addr1))
+   !is_multicast_ether_addr(hdr->addr1)) {
sta->rx_stats.last_rate =
sta_stats_encode_rate(status);
+   ieee80211_sta_update_rate_stats(&sta);
+   }
}
} else if (rx->sdata->vif.type == NL80211_IFTYPE_OCB) {
sta->rx_st

[RFC 1/2] nl80211: Add support for collection of per rate rx statistics

2018-05-14 Thread Sriram R
RX Statistics per rate, per station can be useful in
understanding the quality of communication with our peers
and can be helpful in evaluating the consistency at which a
specific peer has been communicating in different MCS/BW/NSS
after association at different time periods and in varied environments.

This patch adds support for collecting the rx statistics
from the kernel and providing it to the userspace.

Signed-off-by: Sriram R 
---
 include/net/cfg80211.h   |  25 +++
 include/uapi/linux/nl80211.h |  50 +
 net/wireless/nl80211.c   | 172 +++
 net/wireless/rdev-ops.h  |  30 
 net/wireless/trace.h |  26 +++
 5 files changed, 303 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8db6071..8bd818f 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1235,6 +1235,19 @@ struct station_info {
s8 avg_ack_signal;
 };
 
+/**
+ * struct sta_rate_stats - per rate statistics of station
+ *
+ * @rate: encoded rate value
+ * @packets: Packet count received at this @rate
+ * @bytes: Total number of bytes received at this @rate
+ */
+struct sta_rate_stats {
+   u32 rate;
+   u32 packets;
+   u64 bytes;
+};
+
 #if IS_ENABLED(CONFIG_CFG80211)
 /**
  * cfg80211_get_station - retrieve information about a given station
@@ -3018,6 +3031,9 @@ struct cfg80211_external_auth_params {
  *
  * @tx_control_port: TX a control port frame (EAPoL).  The noencrypt parameter
  * tells the driver that the frame should not be encrypted.
+ *
+ * @get_sta_rate_stats: get per-rate stats of the station identified by @mac
+ * @dump_sta_rate_stats: get per-rate stats dump of stations from index @idx
  */
 struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3323,6 +3339,15 @@ struct cfg80211_ops {
   const u8 *buf, size_t len,
   const u8 *dest, const __be16 proto,
   const bool noencrypt);
+
+   int (*get_sta_rate_stats)(struct wiphy *wiphy,
+ struct net_device *dev, const u8 *mac,
+ struct sta_rate_stats **rate_stats_buf,
+ int *len);
+   int (*dump_sta_rate_stats)(struct wiphy *wiphy,
+   struct net_device *dev, int idx, u8 *mac,
+   struct sta_rate_stats **rate_stats_buf,
+   int *len);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 83ed1dd..beeca81 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1031,6 +1031,10 @@
  * &NL80211_ATTR_CHANNEL_WIDTH,&NL80211_ATTR_NSS attributes with its
  * address(specified in &NL80211_ATTR_MAC).
  *
+ * @NL80211_CMD_GET_RATE_STATS: Get Per rate statistics (rx packets and bytes)
+ * for a station identified by %NL80211_ATTR_MAC on the interface
+ * identified by %NL80211_ATTR_IFINDEX.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1243,6 +1247,8 @@ enum nl80211_commands {
 
NL80211_CMD_CONTROL_PORT_FRAME,
 
+   NL80211_CMD_GET_RATE_STATS,
+
/* add new commands above here */
 
/* used to define NL80211_CMD_MAX below */
@@ -2236,6 +2242,10 @@ enum nl80211_commands {
  * @NL80211_ATTR_TXQ_QUANTUM: TXQ scheduler quantum (bytes). Number of bytes
  *  a flow is assigned on each round of the DRR scheduler.
  *
+ * @NL80211_ATTR_RATE_STATS: Indicates the presence of per-rate stats holding
+ * the info of packets and bytes received per rate identified by
+ * the attributes in  @nl80211_rate_stats
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2675,6 +2685,8 @@ enum nl80211_attrs {
NL80211_ATTR_TXQ_MEMORY_LIMIT,
NL80211_ATTR_TXQ_QUANTUM,
 
+   NL80211_ATTR_RATE_STATS,
+
/* add attributes here, update the policy in nl80211.c */
 
__NL80211_ATTR_AFTER_LAST,
@@ -3114,6 +3126,44 @@ enum nl80211_txq_stats {
 };
 
 /**
+ * DOC: per-rate per-station statistics
+ * The nl80211 %NL80211_CMD_GET_RATE_STATS command  provides an interface to
+ * get the statistics of number of packets and bytes received per rate
+ * per station.
+ * The kernel will start collecting this statistics only when it is required
+ * by the userspace application and rate statistics needs to be enabled for
+ * this purpose(TODO: Add comments on interface used to enable rate-stats)
+ * Once enabled, the packet (and corresponding bytes) count received at the
+ * station are recorded per rate on each rx and the userspace application
+ * can get the current stats information using the %NL80211_CMD_GET

[RFC 0/2] nl80211/mac80211 Add support for per-rate rx statistics

2018-05-14 Thread Sriram R
This patchset adds support for the collection and propagating of
per-rate, per-station rx statistics when enabled by a userspace application.

These statistics can be useful in understanding the quality of 
communication with our peers and in evaluating how different peers
are communicating in different MCS/BW/NSS during different time periods and 
environment.

Sriram R (2):
  nl80211: Add support for collection of per rate rx statistics
  mac80211: Add support for per-rate rx statistics

 include/net/cfg80211.h   |  25 +++
 include/uapi/linux/nl80211.h |  50 +
 net/mac80211/cfg.c   |  47 
 net/mac80211/rx.c|  10 ++-
 net/mac80211/sta_info.c  | 165 +
 net/mac80211/sta_info.h  |  17 +
 net/wireless/nl80211.c   | 172 +++
 net/wireless/rdev-ops.h  |  30 
 net/wireless/trace.h |  26 +++
 9 files changed, 540 insertions(+), 2 deletions(-)

-- 
2.7.4



Re: [PATCH 2/2] ath10k: DFS Host Confirmation

2018-05-14 Thread Kalle Valo
Adrian Chadd  writes:

> On Mon, 14 May 2018 at 11:25, Kalle Valo  wrote:
>
>> Adrian Chadd  writes:
>
>> > May we have a little more information about how this is supposed to
> work?
>> >
>> > It looks like we're supposed to send the information about the matched
>> > radar pattern back to the firmware for confirmation? What's the intended
>> > behaviour from the firmware? Will the firmware have a hard-coded set of
>> > patterns we have to answer in/by?
>
>> That's really an implementation detail inside the firmware and from
>> ath10k point of view we don't care what checks the firmware has, we just
>> provide all the necessary information. The checks in firmware might even
>> change in later releases.
>
>> > I ask (like Peter, we work together) because we've had to tweak this
>> > behaviour a little to actually pass FCC / ETSI DFS certification. My
>> > general concern is that this'll cause a lot of false detects on boards
> that
>> > haven't had things tweaked for the given board. As far as I'm aware the
> DFS
>> > parameters are still hard-coded into the firmware image so if you have
> to
>> > change those you're SOL without the relevant NDAs - this makes running
> the
>> > open source DFS stuff a little tricksy on vendor boards.
>
>> This shouldn't cause more false detections, the pattern detection from
>> ath.ko is still used as before. The firmware will just disable DFS
>> altogether if it thinks ath10k is not compliant.
>
>
> Heh, well the fun one for production for us is "ok, so what's
> non-compliant" ?
>
> Eg - if it's 1 out of 100 that we don't hit the explicit timing
> requirements because of the rest of the linux kernel (eg someone holds a
> spinlock more than they should) then I'd prefer that we got a notification
> that something happened so we can fix it. Otherwise in the field it'll just
> be "hey, our stuff stopped working" because whatever the firmware
> expectations are aren't being met.
>
> Again, we're OK because we can at least inspect what's going on, but not
> everyone doing ath10k development/deployment will be so lucky :(

Sure, every software change can cause regressions. But the thing is that
this isn't an optional, ath10k has to have this to be able to continue
using DFS channels.

-- 
Kalle Valo


Re: [PATCH 3/6] firmware: differentiate between signed regulatory.db and other firmware

2018-05-14 Thread Luis R. Rodriguez
On Mon, May 14, 2018 at 10:02:31PM -0400, Mimi Zohar wrote:
> On Mon, 2018-05-14 at 19:28 +, Luis R. Rodriguez wrote:
> > > - CONFIG_IMA_APPRAISE is not fine enough grained.
> > > 
> > > The CONFIG_IMA_APPRAISE_FIRMWARE will be a Kconfig option.  Similar
> > > Kconfig options will require kernel modules, kexec'ed image, and the
> > > IMA policy to be signed.
> > 
> > Sure, it is still unclear to me if CONFIG_IMA_APPRAISE_FIRMWARE will be
> > doing firmware verification in userspace or in the kernel.
> 
> The kernel is verifying signatures.
> 
> > > There are a number of reasons that the kernel should be verifying
> > > firmware signatures (eg. requiring a specific version of the firmware,
> > > that was locally signed).
> > 
> > Oh I agree, Linux enterprise distributions also have a strong reason to
> > have this, so that for instance we only trust and run vendor-approved
> > signed firmware. Otherwise the driver should reject the firmware. Every
> > now and then enterprise distros may run into cases were certain customers
> > may run oddball firmwares, and its unclear if we expect proper functionality
> > with that firmware. Having some form of firmware signing would help with
> > this pipeline, but this is currently dealt with at the packaging, and
> > noting other than logs ensures the driver is using an intended firmware.
> > But these needs *IMHO* have not been enough to push to generalize a kernel
> > firmware signing facility.
> 
> In order for IMA-appraisal to verify firmware signatures, the
> signatures need to be distributed with the firmware.  Perhaps this
> will be enough of an incentive for distros to start including firmware
> signatures in the packages.

Best to poke the maintainers about that... We have been sending mixed messages
about firmware signing over years now. Josh, heads up the new one is we can
do firmware signing through IMA future CONFIG_IMA_APPRAISE_FIRMWARE. I'll
bounce you a few emails related to this.

> > If CONFIG_IMA_APPRAISE_FIRMWARE is going to provide this functionality 
> > somehow
> > I'm happy to hear it.
> 
> The functionality has been there since commit 5a9196d ("ima: add
> support for measuring and appraising firmware").  The
> security_kernel_fw_from_file() hook was later replaced with the
> generic security_kernel_read_file() hook.

Groovy, its unclear from the code on that commit how this is done, so I
suppose I need to study this a bit more. Josh, do you grok it?

  Luis


[PATCH] cfg80211: further limit wiphy names to 64 bytes

2018-05-14 Thread Eric Biggers
From: Eric Biggers 

wiphy names were recently limited to 128 bytes by commit a7cfebcb7594
("cfg80211: limit wiphy names to 128 bytes").  As it turns out though,
this isn't sufficient because dev_vprintk_emit() needs the syslog header
string "SUBSYSTEM=ieee80211\0DEVICE=+ieee80211:$devname" to fit into 128
bytes.  This triggered the "device/subsystem name too long" WARN when
the device name was >= 90 bytes.  As before, this was reproduced by
syzbot by sending an HWSIM_CMD_NEW_RADIO command to the MAC80211_HWSIM
generic netlink family.

Fix it by further limiting wiphy names to 64 bytes.

Reported-by: syzbot+e64565577af34b376...@syzkaller.appspotmail.com
Fixes: a7cfebcb7594 ("cfg80211: limit wiphy names to 128 bytes")
Signed-off-by: Eric Biggers 
---
 include/uapi/linux/nl80211.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9c3630146cec0..271b93783d282 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2698,7 +2698,7 @@ enum nl80211_attrs {
 #define NL80211_ATTR_KEYS NL80211_ATTR_KEYS
 #define NL80211_ATTR_FEATURE_FLAGS NL80211_ATTR_FEATURE_FLAGS
 
-#define NL80211_WIPHY_NAME_MAXLEN  128
+#define NL80211_WIPHY_NAME_MAXLEN  64
 
 #define NL80211_MAX_SUPP_RATES 32
 #define NL80211_MAX_SUPP_HT_RATES  77
-- 
2.17.0



Re: [PATCH 3/6] firmware: differentiate between signed regulatory.db and other firmware

2018-05-14 Thread Mimi Zohar
On Mon, 2018-05-14 at 19:28 +, Luis R. Rodriguez wrote:

[...] 

> > At runtime, in the case
> > that regdb is enabled and a custom policy requires IMA-appraisal
> > firmware signature verification, then both signature verification
> > methods will verify the signatures.  If either fails, then the
> > signature verification will fail.
> 
> OK so you're saying that if CONFIG_IMA_APPRAISE_FIRMWARE is disabled you can
> still end up with CONFIG_CFG80211_REQUIRE_SIGNED_REGDB as enabled *and* a
> custom policy which requires IMA-appraisal for the certain firmware signature
> verifications?

Right



> > There are two problems:
> > - there's no way of configuring a builtin policy to verify firmware
> > signatures.
> 
> I'm not too familiar with IMA however it sounds like you can extend the IMA
> built-in policy on the boot command line.

No, there are a couple of policies predefined in the kernel that can
be loaded by specifying them on the boot command line.  A custom
policy can be loaded later.  Only after specifying a policy on the
boot command line or loading a custom policy, does IMA do anything.


> > - CONFIG_IMA_APPRAISE is not fine enough grained.
> > 
> > The CONFIG_IMA_APPRAISE_FIRMWARE will be a Kconfig option.  Similar
> > Kconfig options will require kernel modules, kexec'ed image, and the
> > IMA policy to be signed.
> 
> Sure, it is still unclear to me if CONFIG_IMA_APPRAISE_FIRMWARE will be
> doing firmware verification in userspace or in the kernel.

The kernel is verifying signatures.



> > There are a number of reasons that the kernel should be verifying
> > firmware signatures (eg. requiring a specific version of the firmware,
> > that was locally signed).
> 
> Oh I agree, Linux enterprise distributions also have a strong reason to
> have this, so that for instance we only trust and run vendor-approved
> signed firmware. Otherwise the driver should reject the firmware. Every
> now and then enterprise distros may run into cases were certain customers
> may run oddball firmwares, and its unclear if we expect proper functionality
> with that firmware. Having some form of firmware signing would help with
> this pipeline, but this is currently dealt with at the packaging, and
> noting other than logs ensures the driver is using an intended firmware.
> But these needs *IMHO* have not been enough to push to generalize a kernel
> firmware signing facility.

In order for IMA-appraisal to verify firmware signatures, the
signatures need to be distributed with the firmware.  Perhaps this
will be enough of an incentive for distros to start including firmware
signatures in the packages.

> If CONFIG_IMA_APPRAISE_FIRMWARE is going to provide this functionality somehow
> I'm happy to hear it.

The functionality has been there since commit 5a9196d ("ima: add
support for measuring and appraising firmware").  The
security_kernel_fw_from_file() hook was later replaced with the
generic security_kernel_read_file() hook.

Mimi



[PATCH] mac80211: TX aggregation stop race can break TX

2018-05-14 Thread Alexander Wetzel
The mac80211 tear down code is not waiting for the driver call back.
This can bring down the the TX path (TID) till the user manually
reconnects. (Observed with iwldvm and enabled TX aggregation.)

The race can be prevented when the ampdu_mlme worker handles the tear
down.

The race:
 * ieee80211_sta_tear_down_BA_sessions calls
   ___ieee80211_stop_tx_ba_session for all TIDs,

 * then cancels the ampdu_mlme worker

 * and cleanups the TIDs the driver already has called back for.

 * ieee80211_stop_tx_ba_cb will never be called for a TID if the callback
   came after the the check in ieee80211_sta_tear_down_BA_sessions.

Signed-off-by: Alexander Wetzel 
---
 net/mac80211/ht.c | 43 +--
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index c78036a0ac94..073258e0cee1 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -301,26 +301,27 @@ void ieee80211_sta_tear_down_BA_sessions(struct sta_info 
*sta,
___ieee80211_stop_tx_ba_session(sta, i, reason);
mutex_unlock(&sta->ampdu_mlme.mtx);
 
-   /* stopping might queue the work again - so cancel only afterwards */
-   cancel_work_sync(&sta->ampdu_mlme.work);
-
/*
 * In case the tear down is part of a reconfigure due to HW restart
 * request, it is possible that the low level driver requested to stop
 * the BA session, so handle it to properly clean tid_tx data.
 */
-   mutex_lock(&sta->ampdu_mlme.mtx);
-   for (i = 0; i < IEEE80211_NUM_TIDS; i++) {
-   struct tid_ampdu_tx *tid_tx =
-   rcu_dereference_protected_tid_tx(sta, i);
+   if(reason == AGG_STOP_DESTROY_STA) {
+   cancel_work_sync(&sta->ampdu_mlme.work);
 
-   if (!tid_tx)
-   continue;
+   mutex_lock(&sta->ampdu_mlme.mtx);
+   for (i = 0; i < IEEE80211_NUM_TIDS; i++) {
+   struct tid_ampdu_tx *tid_tx =
+   rcu_dereference_protected_tid_tx(sta, i);
 
-   if (test_and_clear_bit(HT_AGG_STATE_STOP_CB, &tid_tx->state))
-   ieee80211_stop_tx_ba_cb(sta, i, tid_tx);
+   if (!tid_tx)
+   continue;
+
+   if (test_and_clear_bit(HT_AGG_STATE_STOP_CB, 
&tid_tx->state))
+   ieee80211_stop_tx_ba_cb(sta, i, tid_tx);
+   }
+   mutex_unlock(&sta->ampdu_mlme.mtx);
}
-   mutex_unlock(&sta->ampdu_mlme.mtx);
 }
 
 void ieee80211_ba_session_work(struct work_struct *work)
@@ -328,16 +329,12 @@ void ieee80211_ba_session_work(struct work_struct *work)
struct sta_info *sta =
container_of(work, struct sta_info, ampdu_mlme.work);
struct tid_ampdu_tx *tid_tx;
+   bool enabled = true;
int tid;
 
-   /*
-* When this flag is set, new sessions should be
-* blocked, and existing sessions will be torn
-* down by the code that set the flag, so this
-* need not run.
-*/
+   /* When this flag is set, new sessions should be blocked. */
if (test_sta_flag(sta, WLAN_STA_BLOCK_BA))
-   return;
+   enabled = false;
 
mutex_lock(&sta->ampdu_mlme.mtx);
for (tid = 0; tid < IEEE80211_NUM_TIDS; tid++) {
@@ -352,7 +349,8 @@ void ieee80211_ba_session_work(struct work_struct *work)
sta, tid, WLAN_BACK_RECIPIENT,
WLAN_REASON_UNSPECIFIED, true);
 
-   if (test_and_clear_bit(tid,
+   if (enabled &&
+   test_and_clear_bit(tid,
   sta->ampdu_mlme.tid_rx_manage_offl))
___ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid,
 
IEEE80211_MAX_AMPDU_BUF,
@@ -367,7 +365,7 @@ void ieee80211_ba_session_work(struct work_struct *work)
spin_lock_bh(&sta->lock);
 
tid_tx = sta->ampdu_mlme.tid_start_tx[tid];
-   if (tid_tx) {
+   if (enabled && tid_tx) {
/*
 * Assign it over to the normal tid_tx array
 * where it "goes live".
@@ -390,7 +388,8 @@ void ieee80211_ba_session_work(struct work_struct *work)
if (!tid_tx)
continue;
 
-   if (test_and_clear_bit(HT_AGG_STATE_START_CB, &tid_tx->state))
+   if (enabled &&
+   test_and_clear_bit(HT_AGG_STATE_START_CB, &tid_tx->state))
ieee80211_start_tx_ba_cb(sta, tid, tid_tx);
if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state))
___ieee80211_stop_tx_ba_session(sta, tid,
-- 
2.17.0



Re: [PATCH 2/2] ath10k: DFS Host Confirmation

2018-05-14 Thread Adrian Chadd
On Mon, 14 May 2018 at 11:25, Kalle Valo  wrote:

> Adrian Chadd  writes:

> > May we have a little more information about how this is supposed to
work?
> >
> > It looks like we're supposed to send the information about the matched
> > radar pattern back to the firmware for confirmation? What's the intended
> > behaviour from the firmware? Will the firmware have a hard-coded set of
> > patterns we have to answer in/by?

> That's really an implementation detail inside the firmware and from
> ath10k point of view we don't care what checks the firmware has, we just
> provide all the necessary information. The checks in firmware might even
> change in later releases.

> > I ask (like Peter, we work together) because we've had to tweak this
> > behaviour a little to actually pass FCC / ETSI DFS certification. My
> > general concern is that this'll cause a lot of false detects on boards
that
> > haven't had things tweaked for the given board. As far as I'm aware the
DFS
> > parameters are still hard-coded into the firmware image so if you have
to
> > change those you're SOL without the relevant NDAs - this makes running
the
> > open source DFS stuff a little tricksy on vendor boards.

> This shouldn't cause more false detections, the pattern detection from
> ath.ko is still used as before. The firmware will just disable DFS
> altogether if it thinks ath10k is not compliant.


Heh, well the fun one for production for us is "ok, so what's
non-compliant" ?

Eg - if it's 1 out of 100 that we don't hit the explicit timing
requirements because of the rest of the linux kernel (eg someone holds a
spinlock more than they should) then I'd prefer that we got a notification
that something happened so we can fix it. Otherwise in the field it'll just
be "hey, our stuff stopped working" because whatever the firmware
expectations are aren't being met.

Again, we're OK because we can at least inspect what's going on, but not
everyone doing ath10k development/deployment will be so lucky :(




-adrian


Re: [PATCH 2/2] ath10k: DFS Host Confirmation

2018-05-14 Thread Peter Oh



Which regulatory enforcement are you talking to? Are you talking about
such "prevent users from changing regulatory domain" or "don't provide
a manner to change regulatory domain or channel list" ? If not, can
you share the section of document?

Sorry, I don't have any references about that.

This question is for "what the patch is trying to solve" and "to comply 
FCC regulatory rule" is not the right answer. It should be answered by 
"FCC regulatory rule section xx.xx.xx".
So we all can double check if this patch addresses "the root cause of 
the requirement" and "if it's the best approach or not"


Thanks,
Peter


Linux Plumbers Networking Track CFP

2018-05-14 Thread David Miller

Linux Plumbers Networking Track CFP

This is a call for proposals for the networking track at the 2018
edition of the Linux Plumbers Conference which will be held in
Vancouver on November 13th and November 14th.

The LPC Networking Track is a community event, open to everyone, and
does not require an invitation.

We are seeking talks of 40 minutes in length, accompanied by papers of
2 to 10 pages in length.

Although proposals on finished work are perfectly acceptable, there is
even more value for talks on problems, proposals, and proof-of-concept
solutions that require face-to-face discussions and debate.

Please submit your proposals to the LPC Networking Technical Committee
at:

lpc-net...@vger.kernel.org

Proposals must be submitted by July 11th, and submitters will be
notified of acceptance by August 15th.

The format of the submission and other details can be found at:

http://vger.kernel.org/lpc-networking.html

We are looking forward to seeing everyone in November!


Re: [PATCH 3/6] firmware: differentiate between signed regulatory.db and other firmware

2018-05-14 Thread Luis R. Rodriguez
On Mon, May 14, 2018 at 08:58:12AM -0400, Mimi Zohar wrote:
> On Fri, 2018-05-11 at 21:52 +, Luis R. Rodriguez wrote:
> > diff --git a/drivers/base/firmware_loader/main.c 
> > b/drivers/base/firmware_loader/main.c
> > index eb34089e4299..d7cdf04a8681 100644
> > --- a/drivers/base/firmware_loader/main.c
> > +++ b/drivers/base/firmware_loader/main.c
> > @@ -318,6 +318,11 @@ fw_get_filesystem_firmware(struct device *device, 
> > struct fw_priv *fw_priv)
> > break;
> > }
> > 
> > +#ifdef CONFIG_CFG80211_REQUIRE_SIGNED_REGDB
> > +   if ((strcmp(fw_priv->fw_name, "regulatory.db") == 0) ||
> > +   (strcmp(fw_priv->fw_name, "regulatory.db.p7s") == 0))
> > +   id = READING_FIRMWARE_REGULATORY_DB;
> > +#endif
> > fw_priv->size = 0;
> > rc = kernel_read_file_from_path(path, &fw_priv->data, &size,
> > msize, id);
> > 
> > This is eye-soring, and in turn would mean adding yet more #ifdefs for any
> > code on the kernel which open codes other firmware signing efforts with
> > its own kconfig...
> 
> Agreed, adding ifdef's is ugly.  As previously discussed, this code
> will be removed.
> 
> To coordinate the signature verification, at build time either regdb
> or IMA-appraisal firmware will be enabled. 

But not both, right?

> At runtime, in the case
> that regdb is enabled and a custom policy requires IMA-appraisal
> firmware signature verification, then both signature verification
> methods will verify the signatures.  If either fails, then the
> signature verification will fail.

OK so you're saying that if CONFIG_IMA_APPRAISE_FIRMWARE is disabled you can
still end up with CONFIG_CFG80211_REQUIRE_SIGNED_REGDB as enabled *and* a
custom policy which requires IMA-appraisal for the certain firmware signature
verifications?

> > Then as for my concern on extending and adding new READING_* ID tags
> > for other future open-coded firmware calls, since:
> > 
> > a) You seem to be working on  a CONFIG_IMA_APPRAISE_FIRMWARE which would
> > enable similar functionality as firmware signing but in userspace.
> 
> There are a number of different builtin policies.  The "secure_boot"
> builtin policy enables file signature verification for firmware,
> kernel modules, kexec'ed image, and the IMA policy, but builtin
> policies are enabled on the boot command line.
> 
> There are two problems:
> - there's no way of configuring a builtin policy to verify firmware
> signatures.

I'm not too familiar with IMA however it sounds like you can extend the IMA
built-in policy on the boot command line. If so I'll note MODULE_FIRMWARE()
macro is typically used to annotate firmware description -- not all drivers use
this though for a few reasons, for instance once is that some names are
constructed dynamically at run time. Consider how some drivers rev firmware
with versions, and as they do this they want to keep certain features only for
certain firmware versions. Despite this lack of a direct 1-1 mapping for all
firmwares needed, I *believe* one current use case for this macro is to extract
required firmwares needed on early boot so they can be stashed into the
initramfs if you have these modules enabled on the initramfs. Dracut folks
can confirm but -- dracut *seems* broken then given the semantic gap I
mentioned above.

dracut-init.sh:for _fw in $(modinfo -k $kernel -F firmware $1 2>/dev/null); 
do

If I read this correctly this just complains if the firmware file is
missing if the module is installed on initramfs and the firmware file
is not present. If so we have a current semantic gap and modules with
dynamic names are not handled. And its unclear which modules would be
affected. This is a not a big issue for dracut though since not all drivers
strive to be included on initramfs, unless their storage I suppose -- not
sure how common these storage drivers are for initramfs with dynamic firmware
names which do *not* use MODULE_FIRMWARE().

While the idea of MODULE_FIRMWARE() may need to be given some love or adjusted
to incorporate globs / regexps to fix this existing gap, or we come up with
something more reliable, if fixed, it in theory could end up being re-used to
enable you to extract and build policies for firmware signing at build time...

> - CONFIG_IMA_APPRAISE is not fine enough grained.
> 
> The CONFIG_IMA_APPRAISE_FIRMWARE will be a Kconfig option.  Similar
> Kconfig options will require kernel modules, kexec'ed image, and the
> IMA policy to be signed.

Sure, it is still unclear to me if CONFIG_IMA_APPRAISE_FIRMWARE will be
doing firmware verification in userspace or in the kernel.

> With this, option "d", below, will be possible.

nit: To be clear I was not stating options, I was stating premises to
justify my recommendations.

> > b) CONFIG_CFG80211_REQUIRE_SIGNED_REGDB was added to replace needing
> > CRDA, with an in-kernel interface. CRDA just did a s

Re: Regression: bcm4312 lp-phy device not working in 4.17-rc5 kernel

2018-05-14 Thread Chris Vine
On Mon, 14 May 2018 20:02:05 +0200
Michael Büsch  wrote:
> On Mon, 14 May 2018 19:55:39 +0200
> Michael Büsch  wrote:
> 
> > On Mon, 14 May 2018 18:49:53 +0100
> > Chris Vine  wrote:
> > 
> > > I have an old netbook with a Broadcom bcm4312 802.11b/g lp-phy
> > > [14e4:4315] (rev 01) wifi device.  This works up to and including the
> > > 4.16 kernel, but not with 4.17-rc5.
> > > 
> > > The b43, mac80211, cfg80211, ssb and mmc_core modules are loaded OK, as
> > > is the firmware, and the wlan0 interface will come up, but any attempt
> > > to use the interface fails and it cannot (for example) scan.
> > > 
> > > No useful error messages are given.  The interface (wlan0) is just
> > > reported as not being ready.  
> > 
> > Hi,
> > 
> > thanks for your report.
> > 
> > Can you please provide all kernel log messages anyway?
> > And what does "not ready" mean exactly?
> > 
> > It would be extremely helpful if you'd do a git bisect to find the
> > commit that broke it.
> > 
> 
> Ok, I just noticed that mainline still contains the ssb breakage.
> 
> So you are most likely hitting this:
> https://patchwork.kernel.org/patch/10393729/

Yes thanks, applying that patch resolves the issue fine.  There was a
similar problem with disabled PCI support for b43 which found its way
into the 4.15.0/4.15.1 kernels:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=a9e6d44ddeccd3522670e641f1ed9b068e746ff7
At least this bug should avoid getting into the mainline release.

Chris


Re: [PATCH 2/2] ath10k: DFS Host Confirmation

2018-05-14 Thread Kalle Valo
Adrian Chadd  writes:

> May we have a little more information about how this is supposed to work?
>
> It looks like we're supposed to send the information about the matched
> radar pattern back to the firmware for confirmation? What's the intended
> behaviour from the firmware? Will the firmware have a hard-coded set of
> patterns we have to answer in/by?

That's really an implementation detail inside the firmware and from
ath10k point of view we don't care what checks the firmware has, we just
provide all the necessary information. The checks in firmware might even
change in later releases.

> I ask (like Peter, we work together) because we've had to tweak this
> behaviour a little to actually pass FCC / ETSI DFS certification. My
> general concern is that this'll cause a lot of false detects on boards that
> haven't had things tweaked for the given board. As far as I'm aware the DFS
> parameters are still hard-coded into the firmware image so if you have to
> change those you're SOL without the relevant NDAs - this makes running the
> open source DFS stuff a little tricksy on vendor boards.

This shouldn't cause more false detections, the pattern detection from
ath.ko is still used as before. The firmware will just disable DFS
altogether if it thinks ath10k is not compliant.

-- 
Kalle Valo


Re: [PATCH 2/2] ath10k: DFS Host Confirmation

2018-05-14 Thread Kalle Valo
Peter Oh  writes:

> On 05/02/2018 04:27 AM, Kalle Valo wrote:
>> Peter Oh  writes:
>>
>>> On 04/30/2018 10:45 AM, Sriram R wrote:
 In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
 feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.

 This new features enables the ath10k host to send information to the
 firmware on the specifications of detected radar type. This allows the
 firmware to validate if the host's radar pattern detector unit is
 operational and check if the radar information shared by host matches
 the radar pulses sent as phy error events from firmware. If the check
 fails the firmware won't allow use of DFS channels on AP mode when using
 FCC regulatory region.
>>> What's the main reason you introduce this feature?
>>> What are you trying to solve with this change?
>> Otherwise one cannot use DFS channels on FCC regions with a firmware
>> from 10.4-3.6 branch.
>
> It's a big change and none of us knows until I asked and you answered.

I was hoping that would be clear from the commit log but I guess Sriram
could include my sentence above to enhance it?

 @@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k 
 *ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
-   if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL))
 {
 +  if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
   "dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
 -  ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
 -  ATH10K_DFS_STAT_INC(ar, radar_detected);
 +  if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
 +  ar->dfs_detector->region == NL80211_DFS_FCC) {
>>>
>>> I feel risky that host drivers have no way to control this new feature
>>> and totally rely on FW feature mask. We should have a host drivers'
>>> feature mask such as module param and set it false (don't use) by
>>> default until it proves safe to use.
>>
>> This is for regulatory enforcement so it's not possible to disable the
>> feature from host.
>
> Which regulatory enforcement are you talking to? Are you talking about
> such "prevent users from changing regulatory domain" or "don't provide
> a manner to change regulatory domain or channel list" ? If not, can
> you share the section of document?

Sorry, I don't have any references about that.

> In addition to that, How do you make sure if FW side DFS radar
> detection algorithm has no defects and it always categorizes phy
> errors to correct radar type? I'm asking it, because it's known that
> DFS detector in ath module does sometimes detect radar patterns as
> wrong radar type in both of ath10k and ath9k. I couldn't think FW side
> detector has no such defects at all. It's like blind patch to open
> source community, since no one can review the implementation in FW
> even though no one can guarantee its level of integrity.

It's not about replacing the DFS detector in ath.ko, it's about adding
extra checks in firmware for regulatory compliance. So at least in
theory DFS detection should still work same as before, just with an
extra roundtrip via firmware.

-- 
Kalle Valo


Re: Regression: bcm4312 lp-phy device not working in 4.17-rc5 kernel

2018-05-14 Thread Michael Büsch
On Mon, 14 May 2018 19:55:39 +0200
Michael Büsch  wrote:

> On Mon, 14 May 2018 18:49:53 +0100
> Chris Vine  wrote:
> 
> > I have an old netbook with a Broadcom bcm4312 802.11b/g lp-phy
> > [14e4:4315] (rev 01) wifi device.  This works up to and including the
> > 4.16 kernel, but not with 4.17-rc5.
> > 
> > The b43, mac80211, cfg80211, ssb and mmc_core modules are loaded OK, as
> > is the firmware, and the wlan0 interface will come up, but any attempt
> > to use the interface fails and it cannot (for example) scan.
> > 
> > No useful error messages are given.  The interface (wlan0) is just
> > reported as not being ready.  
> 
> Hi,
> 
> thanks for your report.
> 
> Can you please provide all kernel log messages anyway?
> And what does "not ready" mean exactly?
> 
> It would be extremely helpful if you'd do a git bisect to find the
> commit that broke it.
> 

Ok, I just noticed that mainline still contains the ssb breakage.

So you are most likely hitting this:
https://patchwork.kernel.org/patch/10393729/


-- 
Michael


pgpFvz8XhdUlG.pgp
Description: OpenPGP digital signature


Re: Regression: bcm4312 lp-phy device not working in 4.17-rc5 kernel

2018-05-14 Thread Michael Büsch
On Mon, 14 May 2018 18:49:53 +0100
Chris Vine  wrote:

> I have an old netbook with a Broadcom bcm4312 802.11b/g lp-phy
> [14e4:4315] (rev 01) wifi device.  This works up to and including the
> 4.16 kernel, but not with 4.17-rc5.
> 
> The b43, mac80211, cfg80211, ssb and mmc_core modules are loaded OK, as
> is the firmware, and the wlan0 interface will come up, but any attempt
> to use the interface fails and it cannot (for example) scan.
> 
> No useful error messages are given.  The interface (wlan0) is just
> reported as not being ready.

Hi,

thanks for your report.

Can you please provide all kernel log messages anyway?
And what does "not ready" mean exactly?

It would be extremely helpful if you'd do a git bisect to find the
commit that broke it.

-- 
Michael


pgpSMDNxSWKvg.pgp
Description: OpenPGP digital signature


Regression: bcm4312 lp-phy device not working in 4.17-rc5 kernel

2018-05-14 Thread Chris Vine
I have an old netbook with a Broadcom bcm4312 802.11b/g lp-phy
[14e4:4315] (rev 01) wifi device.  This works up to and including the
4.16 kernel, but not with 4.17-rc5.

The b43, mac80211, cfg80211, ssb and mmc_core modules are loaded OK, as
is the firmware, and the wlan0 interface will come up, but any attempt
to use the interface fails and it cannot (for example) scan.

No useful error messages are given.  The interface (wlan0) is just
reported as not being ready.

Chris


[PATCH v2 3/3] ath10k: Support survey dump for ath10k-ct 10.1 firmware.

2018-05-14 Thread greearb
From: Ben Greear 

Recent ath10k-ct 10.1 firmware supports survey results, and
advertises the appropriate service flags.  This confuses
the ath10k driver because the 10.1 wmi commands are not set up
for survey information.  So, this patch adds support for
handling survey information.  Example output:

Survey data from wlan0
frequency:  5180 MHz [in use]
noise:  -97 dBm
channel active time:44 ms
channel busy time:  15 ms
channel receive time:   7 ms
channel transmit time:  7 ms
Survey data from wlan0
frequency:  5200 MHz
noise:  -98 dBm
channel active time:46 ms
channel busy time:  2 ms
...

Signed-off-by: Ben Greear 
---
 drivers/net/wireless/ath/ath10k/wmi.c | 54 +++
 drivers/net/wireless/ath/ath10k/wmi.h |  8 ++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
b/drivers/net/wireless/ath/ath10k/wmi.c
index df2e92a..3497873 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -365,7 +365,8 @@ static struct wmi_cmd_map wmi_10x_cmd_map = {
.vdev_filter_neighbor_rx_packets_cmdid = WMI_CMD_UNSUPPORTED,
.mu_cal_start_cmdid = WMI_CMD_UNSUPPORTED,
.set_cca_params_cmdid = WMI_CMD_UNSUPPORTED,
-   .pdev_bss_chan_info_request_cmdid = WMI_CMD_UNSUPPORTED,
+   .pdev_bss_chan_info_request_cmdid =
+   WMI_10_2_PDEV_BSS_CHAN_INFO_REQUEST_CMDID,
.pdev_get_tpc_table_cmdid = WMI_CMD_UNSUPPORTED,
 };
 
@@ -2830,9 +2831,15 @@ static int ath10k_wmi_10x_op_pull_fw_stats(struct ath10k 
*ar,
for (i = 0; i < num_peer_stats; i++) {
const struct wmi_10x_peer_stats *src;
struct ath10k_fw_stats_peer *dst;
+   int stats_len;
+
+   if (test_bit(WMI_SERVICE_PEER_STATS, ar->wmi.svc_map))
+   stats_len = sizeof(struct wmi_10x_peer_stats_ct_ext);
+   else
+   stats_len = sizeof(*src);
 
src = (void *)skb->data;
-   if (!skb_pull(skb, sizeof(*src)))
+   if (!skb_pull(skb, stats_len))
return -EPROTO;
 
dst = kzalloc(sizeof(*dst), GFP_ATOMIC);
@@ -2843,6 +2850,12 @@ static int ath10k_wmi_10x_op_pull_fw_stats(struct ath10k 
*ar,
 
dst->peer_rx_rate = __le32_to_cpu(src->peer_rx_rate);
 
+   if (ath10k_peer_stats_enabled(ar)) {
+   struct wmi_10x_peer_stats_ct_ext *src2 = (void *)(src);
+
+   dst->rx_duration = __le32_to_cpu(src2->rx_duration);
+   }
+
list_add_tail(&dst->list, &stats->peers);
}
 
@@ -5488,7 +5501,7 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct 
sk_buff *skb)
ath10k_wmi_event_service_available(ar, skb);
break;
default:
-   ath10k_warn(ar, "Unknown eventid: %d\n", id);
+   ath10k_warn(ar, "Unknown (main) eventid: %d\n", id);
break;
}
 
@@ -5618,8 +5631,11 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, 
struct sk_buff *skb)
case WMI_10X_PDEV_UTF_EVENTID:
/* ignore utf events */
break;
+   case WMI_10_1_PDEV_BSS_CHAN_INFO_EVENTID: /* Newer CT 10.1 firmware */
+   ath10k_wmi_event_pdev_bss_chan_info(ar, skb);
+   break;
default:
-   ath10k_warn(ar, "Unknown eventid: %d\n", id);
+   ath10k_warn(ar, "Unknown (10.1) eventid: %d\n", id);
break;
}
 
@@ -5765,7 +5781,7 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, 
struct sk_buff *skb)
   "received event id %d not implemented\n", id);
break;
default:
-   ath10k_warn(ar, "Unknown eventid: %d\n", id);
+   ath10k_warn(ar, "Unknown (10.2) eventid: %d\n", id);
break;
}
 
@@ -5879,7 +5895,7 @@ static void ath10k_wmi_10_4_op_rx(struct ath10k *ar, 
struct sk_buff *skb)
ath10k_wmi_event_tpc_final_table(ar, skb);
break;
default:
-   ath10k_warn(ar, "Unknown eventid: %d\n", id);
+   ath10k_warn(ar, "Unknown (10.4) eventid: %d\n", id);
break;
}
 
@@ -6174,6 +6190,26 @@ static struct sk_buff 
*ath10k_wmi_10_1_op_gen_init(struct ath10k *ar)
config.num_msdu_desc = __cpu_to_le32(TARGET_10X_NUM_MSDU_DESC);
config.max_frag_entries = __cpu_to_le32(TARGET_10X_MAX_FRAG_ENTRIES);
 
+   if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT,
+ar->running_fw->fw_file.fw_features)) {
+   u32 features = 0;
+
+   if (test_bit(ATH10K_FLAG_BTC

[PATCH v2 1/3] ath10k: Add ath10k-ct firmware feature flags and descriptions.

2018-05-14 Thread greearb
From: Ben Greear 

These feature flags are used by ath10k-ct firmware.  Add these
so we can use them in future patches and so there are no warnings
about feature flags on loading ath10k-ct firmware.

Signed-off-by: Ben Greear 
---
v2:  Fix warings typo in description.

 drivers/net/wireless/ath/ath10k/core.c | 17 +
 drivers/net/wireless/ath/ath10k/core.h | 67 ++
 2 files changed, 84 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 4cf54a7..f2f8f61 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -552,6 +552,23 @@ static const char *const ath10k_core_fw_feature_str[] = {
[ATH10K_FW_FEATURE_NO_PS] = "no-ps",
[ATH10K_FW_FEATURE_MGMT_TX_BY_REF] = "mgmt-tx-by-reference",
[ATH10K_FW_FEATURE_NON_BMI] = "non-bmi",
+   [ATH10K_FW_FEATURE_WMI_10X_CT] = "wmi-10.x-CT",
+   [ATH10K_FW_FEATURE_CT_RXSWCRYPT] = "rxswcrypt-CT",
+   [ATH10K_FW_FEATURE_HAS_TXSTATUS_NOACK] = "txstatus-noack",
+   [ATH10K_FW_FEATURE_CT_RATEMASK] = "ratemask-CT",
+   [ATH10K_FW_FEATURE_HAS_SAFE_BURST] = "safe-burst",
+   [ATH10K_FW_FEATURE_REGDUMP_CT] = "regdump-CT",
+   [ATH10K_FW_FEATURE_TXRATE_CT] = "txrate-CT",
+   [ATH10K_FW_FEATURE_FLUSH_ALL_CT] = "flush-all-CT",
+   [ATH10K_FW_FEATURE_PINGPONG_READ_CT] = "pingpong-CT",
+   [ATH10K_FW_FEATURE_SKIP_CH_RES_CT] = "ch-regs-CT",
+   [ATH10K_FW_FEATURE_NOP_CT] = "nop-CT",
+   [ATH10K_FW_FEATURE_HTT_MGT_CT] = "htt-mgt-CT",
+   [ATH10K_FW_FEATURE_SET_SPECIAL_CT] = "set-special-CT",
+   [ATH10K_FW_FEATURE_NO_BMISS_CT] = "no-bmiss-CT",
+   [ATH10K_FW_FEATURE_HAS_GET_TEMP_CT] = "get-temp-CT",
+   [ATH10K_FW_FEATURE_HAS_TX_RC_CT] = "tx-rc-CT",
+   [ATH10K_FW_FEATURE_CUST_STATS_CT] = "cust-stats-CT",
 };
 
 static unsigned int ath10k_core_get_fw_feature_str(char *buf,
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index e4ac8f2..c1342c5 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -711,6 +711,73 @@ enum ath10k_fw_features {
/* Firmware load is done externally, not by bmi */
ATH10K_FW_FEATURE_NON_BMI = 19,
 
+   /* tx-status has the noack bits (CT firmware version 14 and higher ) */
+   ATH10K_FW_FEATURE_HAS_TXSTATUS_NOACK = 30,
+
+   /* Firmware from Candela Technologies, enables more VIFs, etc */
+   ATH10K_FW_FEATURE_WMI_10X_CT = 31,
+
+   /* Firmware from Candela Technologies with rx-software-crypt.
+* Required for multiple stations connected to same AP when using
+* encryption (ie, commercial version of CT firmware)
+*/
+   ATH10K_FW_FEATURE_CT_RXSWCRYPT = 32,
+
+   /* Firmware supports extended wmi_common_peer_assoc_complete_cmd that
+* contains an array of rate-disable masks.  This allows the host to
+* have better control over what rates the firmware will use.  CT
+* Firmware only (v15 and higher)
+*/
+   ATH10K_FW_FEATURE_CT_RATEMASK = 33,
+
+   /* Versions of firmware before approximately 10.2.4.72 would corrupt
+* txop fields during burst.  Since this is fixed now, add a flag to
+* denote this.
+*/
+   ATH10K_FW_FEATURE_HAS_SAFE_BURST = 34,
+
+   /* Register-dump is supported. */
+   ATH10K_FW_FEATURE_REGDUMP_CT = 35,
+
+   /* TX-Rate is reported. */
+   ATH10K_FW_FEATURE_TXRATE_CT = 36,
+
+   /* Firmware can flush all peers. */
+   ATH10K_FW_FEATURE_FLUSH_ALL_CT = 37,
+
+   /* Firmware can read memory with ping-pong protocol. */
+   ATH10K_FW_FEATURE_PINGPONG_READ_CT = 38,
+
+   /* Firmware can skip channel reservation. */
+   ATH10K_FW_FEATURE_SKIP_CH_RES_CT = 39,
+
+   /* Firmware supports NOP keep-alive message. */
+   ATH10K_FW_FEATURE_NOP_CT = 40,
+
+   /* Firmware supports CT HTT MGT feature. */
+   ATH10K_FW_FEATURE_HTT_MGT_CT = 41,
+
+   /* Set-special cmd-id is supported. */
+   ATH10K_FW_FEATURE_SET_SPECIAL_CT = 42,
+
+   /* SW Beacon Miss is disabled in this kernel, so you have to
+* let mac80211 manage the connection.
+*/
+   ATH10K_FW_FEATURE_NO_BMISS_CT = 43,
+
+   /* 10.1 firmware that supports getting temperature.  Stock
+* 10.1 cannot.
+*/
+   ATH10K_FW_FEATURE_HAS_GET_TEMP_CT = 44,
+
+   /* Can peer-id be over-ridden to provide rix + retries for raw pkts?
+*  CT only option.
+*/
+   ATH10K_FW_FEATURE_HAS_TX_RC_CT = 45,
+
+   /* Do we support requesting custom stats */
+   ATH10K_FW_FEATURE_CUST_STATS_CT = 46,
+
/* keep last */
ATH10K_FW_FEATURE_COUNT,
 };
-- 
2.4.11



[PATCH v2 2/3] ath10k: Don't try un-supported idle_ps_config command.

2018-05-14 Thread greearb
From: Ben Greear 

The warning the the logs does not give user a clue as to what
command is failing, so it is worth it to check for un-supported
command before trying the call.

And add return-code to survey error message.

Signed-off-by: Ben Greear 
---
 drivers/net/wireless/ath/ath10k/mac.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 3d7119a..1185d0c2 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4680,10 +4680,13 @@ static int ath10k_start(struct ieee80211_hw *hw)
}
 
param = ar->wmi.pdev_param->idle_ps_config;
-   ret = ath10k_wmi_pdev_set_param(ar, param, 1);
-   if (ret && ret != -EOPNOTSUPP) {
-   ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
-   goto err_core_stop;
+   if (param != WMI_PDEV_PARAM_UNSUPPORTED) {
+   ret = ath10k_wmi_pdev_set_param(ar, param, 1);
+   if (ret) {
+   ath10k_warn(ar, "failed to enable idle_ps_config: %d\n",
+   ret);
+   goto err_core_stop;
+   }
}
 
__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask);
@@ -6770,7 +6773,8 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
 
ret = ath10k_wmi_pdev_bss_chan_info_request(ar, type);
if (ret) {
-   ath10k_warn(ar, "failed to send pdev bss chan info request\n");
+   ath10k_warn(ar, "failed to send pdev bss chan info request: 
%d\n",
+   ret);
return;
}
 
-- 
2.4.11



Re: [PATCH 1/2] ath: Add support to get the detected radar specifications

2018-05-14 Thread Kalle Valo
Peter Oh  writes:

> On 04/30/2018 10:45 AM, Sriram R wrote:
>> This enables ath10k/ath9k drivers to collect the specifications of the
>> radar type once it is detected by the dfs pattern detector unit.
>> Usage of the collected info is specific to driver implementation.
>> For example, collected radar info could be used by the host driver
>> to send to co-processors for additional processing/validation.
>
> What kind of additional processing/validation does it do?
> Please give us more details for that.

This is handled in patch 2.

-- 
Kalle Valo


Re: [PATCH 07/12] ath10k: Add MSA handshake QMI mgs support

2018-05-14 Thread Govind Singh

Hi Bjorn,
Thanks for the review.


On 2018-05-12 00:13, Bjorn Andersson wrote:

On Sun 25 Mar 22:40 PDT 2018, Govind Singh wrote:


HOST allocates 2mb of region for modem and WCN3990
secure access and provides the address and access
control to target for secure access.
Add MSA handshake request/response messages.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/qmi.c | 288 
+-

 drivers/net/wireless/ath/ath10k/qmi.h |  14 ++
 2 files changed, 300 insertions(+), 2 deletions(-)

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

index bc80b8f..763b812 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -26,6 +26,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "qmi.h"
 #include "qmi_svc_v01.h"

@@ -35,6 +37,240 @@
 static struct ath10k_qmi *qmi;

 static int
+ath10k_qmi_map_msa_permissions(struct ath10k_msa_mem_region_info 
*mem_region)

+{
+   struct qcom_scm_vmperm dst_perms[3];
+   unsigned int src_perms;
+   phys_addr_t addr;
+   u32 perm_count;
+   u32 size;
+   int ret;
+
+   addr = mem_region->reg_addr;
+   size = mem_region->size;


Skip the local variables.



Sure, will do in next version.


+
+   src_perms = BIT(QCOM_SCM_VMID_HLOS);
+
+   dst_perms[0].vmid = QCOM_SCM_VMID_MSS_MSA;
+   dst_perms[0].perm = QCOM_SCM_PERM_RW;
+   dst_perms[1].vmid = QCOM_SCM_VMID_WLAN;
+   dst_perms[1].perm = QCOM_SCM_PERM_RW;
+
+   if (!mem_region->secure_flag) {


So with secure_flag equal to 0 we give less subsystems access to the
data? Is this logic inverted?



Yes with secure flag mean less privileges i,e: Copy engine hardware
can not access the region.


+   dst_perms[2].vmid = QCOM_SCM_VMID_WLAN_CE;
+   dst_perms[2].perm = QCOM_SCM_PERM_RW;
+   perm_count = 3;
+   } else {
+   dst_perms[2].vmid = 0;
+   dst_perms[2].perm = 0;
+   perm_count = 2;


If you set perm_count to 2 you don't need to clear vmid and perm of
dst_perms[2].



ok, will remove.


+   }
+
+   ret = qcom_scm_assign_mem(addr, size, &src_perms,
+ dst_perms, perm_count);
+   if (ret < 0)
+   pr_err("msa map permission failed=%d\n", ret);


Use dev_err()


+
+   return ret;
+}
+
+static int
+ath10k_qmi_unmap_msa_permissions(struct ath10k_msa_mem_region_info 
*mem_region)

+{
+   struct qcom_scm_vmperm dst_perms;
+   unsigned int src_perms;
+   phys_addr_t addr;
+   u32 size;
+   int ret;
+
+   addr = mem_region->reg_addr;
+   size = mem_region->size;


Skip the local variables.



Sure, will do in next version.


+
+   src_perms = BIT(QCOM_SCM_VMID_MSS_MSA) | BIT(QCOM_SCM_VMID_WLAN);
+
+   if (!mem_region->secure_flag)
+   src_perms |= BIT(QCOM_SCM_VMID_WLAN_CE);
+
+   dst_perms.vmid = QCOM_SCM_VMID_HLOS;
+   dst_perms.perm = QCOM_SCM_PERM_RW;
+
+   ret = qcom_scm_assign_mem(addr, size, &src_perms, &dst_perms, 1);
+   if (ret < 0)
+   pr_err("msa unmap permission failed=%d\n", ret);
+
+   return ret;
+}
+



+static int
+   ath10k_qmi_msa_mem_info_send_sync_msg(struct ath10k_qmi *qmi)
+{
+   struct wlfw_msa_info_resp_msg_v01 *resp;


This is 40 bytes,


Sure, will do in next version for all instances.


+   struct wlfw_msa_info_req_msg_v01 *req;


This is 6 bytes.

So just put them on the stack and skip the memory management.



Sure, will do in next version.


+   struct qmi_txn txn;
+   int ret;
+   int i;
+
+   req = kzalloc(sizeof(*req), GFP_KERNEL);
+   if (!req)
+   return -ENOMEM;
+
+   resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+   if (!resp) {
+   kfree(req);
+   return -ENOMEM;
+   }
+
+   req->msa_addr = qmi->msa_pa;
+   req->size = qmi->msa_mem_size;
+
+   ret = qmi_txn_init(&qmi->qmi_hdl, &txn,
+  wlfw_msa_info_resp_msg_v01_ei, resp);
+   if (ret < 0) {
+   pr_err("fail to init txn for MSA mem info resp %d\n",
+  ret);


Unless we have 2 billion outstanding transactions "ret" is going to be
ENOMEM here, in which case there is already a printout telling you 
about

the problem.



Sure, will remove this redundant logging.


+   goto out;
+   }
+
+   ret = qmi_send_request(&qmi->qmi_hdl, NULL, &txn,
+  QMI_WLFW_MSA_INFO_REQ_V01,
+  WLFW_MSA_INFO_REQ_MSG_V01_MAX_MSG_LEN,
+  wlfw_msa_info_req_msg_v01_ei, req);
+   if (ret < 0) {
+   qmi_txn_cancel(&txn);
+   pr_err("fail to send MSA mem info req %d\n", ret);


Use dev_err().


+   goto out;
+   }
+
+   ret = qmi_txn_wait(&txn, WLFW_TIMEOUT * HZ);


The timeout is in

Re: [wireless-drivers-next:master] BUILD REGRESSION b41c393676696c20456d5fd8cbe71453560ee06e

2018-05-14 Thread Kalle Valo
kbuild test robot  writes:

> tree/branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git
> master
> branch HEAD: b41c393676696c20456d5fd8cbe71453560ee06e rsi: fix
> spelling mistake: "thead" -> "thread"
>
> Regressions in current branch:
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:2740:49:
> sparse: Using plain integer as NULL pointer
>
> Error ids grouped by kconfigs:
>
> recent_errors
> └── x86_64-allmodconfig
> └──
> drivers-net-wireless-broadcom-brcm80211-brcmfmac-cfg80211.c:sparse:Using-plain-integer-as-NULL-pointer

Could someone (Franky?) please fix this regression in brcmfmac? (Or if
it's a false positive report it to the kbuilt bot maintainers.) The
kbuild bot is complaining about this in every build report now.

I think the offending commit is:

7742fce4c007 brcmfmac: reports boottime_ns while informing bss

Earlier report:

https://lkml.kernel.org/r/878t94278l@codeaurora.org

-- 
Kalle Valo


Re: [PATCH v3 00/12] *** Add support for wifi QMI client driver ***

2018-05-14 Thread Kalle Valo
Govind Singh  writes:

> Add QMI client driver for Q6 integrated WLAN connectivity subsystem.
> This module is responsible for communicating WLAN control messages to FW
> over QMI interface.
>
> “QUALCOMM Messaging Interface”(QMI) provides the control interface between
> components running b/w remote processors with underlying transport layer
> based on integrated chipset(shared memory) or discrete 
> chipset(PCI/USB/SDIO/UART).
>
> QMI client driver implementation is based on qmi frmework 
> https://lwn.net/Articles/729924/.
>
> Below is the sequence of qmi handshake.
>
>QMI CLIENT(APPS) QMI SERVER(FW 
> in Q6)
>
>  <--wlan service discoverd
>
>-connect to wlam qmi service->
>
>wlan info request->
>
>
>msa info req>
>
>  
>  msa ready req>
>
>  
>  
>  capability req--->
>
> 
> qmi bdf req->
>
>  
>   qmi cal trigger--->
>
>   < QMI FW ready indication---
>
> Govind Singh (12):
>   ath10k: Add qmi service for wlan qmi client
>   dt: bindings: add bindings for ath10k qmi client
>   ath10k: Add WCN3990 QMI client driver
>   ath10k: add support to start and stop qmi service
>   ath10k: Add support of QMI indication message
>   firmware: qcom: scm: Add WLAN VMID for Qualcomm SCM interface
>   ath10k: Add MSA handshake QMI mgs support
>   ath10k: Add QMI CAP request support
>   ath10k: Add QMI HOST CAP request support
>   ath10k: add bdf/cal indication support
>   ath10k: Add wlan mode on/off qmi message
>   ath10k: Add qmi wlan enable/disable support for WCN3990

BTW, I noticed that you had v3 only in the cover letter but the version
number should be in every patch part of the set:

https://patchwork.kernel.org/patch/10307161/

-- 
Kalle Valo


Re: [PATCH 04/12] ath10k: add support to start and stop qmi service

2018-05-14 Thread Govind Singh

On 2018-05-11 23:13, Bjorn Andersson wrote:

On Sun 25 Mar 22:39 PDT 2018, Govind Singh wrote:


Add support to start qmi service to configure the wlan
firmware component and register event notifier to communicate
with the WLAN firmware over qmi communication interface.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/qmi.c | 155 
--

 drivers/net/wireless/ath/ath10k/qmi.h |  13 +++
 2 files changed, 160 insertions(+), 8 deletions(-)

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

index 2235182..3a7fcc6 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -31,15 +31,115 @@

 static struct ath10k_qmi *qmi;

+static int ath10k_qmi_event_fw_ready_ind(struct ath10k_qmi *qmi)
+{
+   pr_debug("fw ready event received\n");


Please use dev_dbg.



I have migrated from pr_ to dev_ logging for the whole set, will share 
in next version.



+   spin_lock(&qmi->event_lock);
+   qmi->fw_ready = true;
+   spin_unlock(&qmi->event_lock);


I see no reason for not just putting this code in
ath10k_qmi_fw_ready_ind().


+


fw_ready isn't used for anything, what purpose does this code have?



fw_ready will be used in later set of changes for qmi client debugfs and
for synchronization.I will decouple this and add as bit mask in 
appropriate change.




+   return 0;
+}
+
+static void ath10k_qmi_fw_ready_ind(struct qmi_handle *qmi_hdl,
+   struct sockaddr_qrtr *sq,
+   struct qmi_txn *txn, const void *data)
+{
+	struct ath10k_qmi *qmi = container_of(qmi_hdl, struct ath10k_qmi, 
qmi_hdl);

+
+   ath10k_qmi_event_fw_ready_ind(qmi);
+}
+
+static void ath10k_qmi_msa_ready_ind(struct qmi_handle *qmi_hdl,
+struct sockaddr_qrtr *sq,
+struct qmi_txn *txn, const void *data)
+{
+	struct ath10k_qmi *qmi = container_of(qmi_hdl, struct ath10k_qmi, 
qmi_hdl);

+
+   qmi->msa_ready = true;


msa_ready is not only unused in this patch, it's unused after all 11
patches. Can you drop it?



Sure, will remove in next version.



+static void ath10k_qmi_event_server_arrive(struct work_struct *work)
+{
+   struct ath10k_qmi *qmi = container_of(work, struct ath10k_qmi,
+ work_svc_arrive);
+   int ret;
+
+   ret = ath10k_qmi_connect_to_fw_server(qmi);
+   if (ret)
+   return;
+
+   pr_debug("qmi server arrive\n");
+}
+



 static int ath10k_qmi_new_server(struct qmi_handle *qmi_hdl,
 struct qmi_service *service)
 {
+	struct ath10k_qmi *qmi = container_of(qmi_hdl, struct ath10k_qmi, 
qmi_hdl);

+   struct sockaddr_qrtr *sq = &qmi->sq;
+
+   sq->sq_family = AF_QIPCRTR;
+   sq->sq_node = service->node;
+   sq->sq_port = service->port;
+
+   queue_work(qmi->event_wq, &qmi->work_svc_arrive);


This is being called in a sleepable context and kernel_connect() will
not block, so I see no reason for queue work here to invoke
ath10k_qmi_event_server_arrive() just to call
ath10k_qmi_connect_to_fw_server().

Just put the kernel_connect() call here.




In this change it just calls ath10k_qmi_connect_to_fw_server, but most 
of the handshaking is done in server arrive callback.

Successive changes adds the request/response of each handshake.
Is it ok to block new_server callback till all client handshaking gets 
completed. I thought it might block other client
notification overlapped on the same time slot(ex: IPA). Do you see any 
concern here or should be ok.

I have not profiled yet, i will share the approx time it can take.

This gives the added benefit that you don't need to use qmi->sq as a 
way

to pass parameters to ath10k_qmi_connect_to_fw_server().


+
return 0;
 }

 static void ath10k_qmi_del_server(struct qmi_handle *qmi_hdl,
  struct qmi_service *service)
 {
+   struct ath10k_qmi *qmi =
+   container_of(qmi_hdl, struct ath10k_qmi, qmi_hdl);
+
+   queue_work(qmi->event_wq, &qmi->work_svc_exit);


I see no reason to queue work to set a boolean to false, just inline 
the

code here.



sure, this i can remove.



 static struct qmi_ops ath10k_qmi_ops = {
@@ -47,6 +147,51 @@ static void ath10k_qmi_del_server(struct 
qmi_handle *qmi_hdl,

.del_server = ath10k_qmi_del_server,
 };

+static int ath10k_alloc_qmi_resources(struct ath10k_qmi *qmi)
+{
+   int ret;
+
+   ret = qmi_handle_init(&qmi->qmi_hdl,
+ WLFW_BDF_DOWNLOAD_REQ_MSG_V01_MAX_MSG_LEN,
+ &ath10k_qmi_ops, qmi_msg_handler);
+   if (ret)
+   goto err;
+
+   qmi->event_wq = alloc_workqueue("qmi_driver_event",
+   WQ_UNBOUND, 1);
+   if (!qmi->event_wq) {
+   pr_err("workqueu

Re: [PATCH 01/12] ath10k: Add qmi service for wlan qmi client

2018-05-14 Thread Kalle Valo
Bjorn Andersson  writes:

>> new file mode 100644
>> index 000..5857c0c
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
>> @@ -0,0 +1,2323 @@
>> +/*
>> + * Copyright (c) 2018 The Linux Foundation. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software for any
>> + * purpose with or without fee is hereby granted, provided that the above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>
> Please use SPDX license header.

As ath10k does not use SPDX yet so for consistency I would like to use
the old style license text still and later switch the whole ath10k to
use SPDX in one go.

-- 
Kalle Valo


Re: [PATCH 3/6] firmware: differentiate between signed regulatory.db and other firmware

2018-05-14 Thread Mimi Zohar
On Fri, 2018-05-11 at 21:52 +, Luis R. Rodriguez wrote:
> On Fri, May 11, 2018 at 01:00:26AM -0400, Mimi Zohar wrote:
> > On Thu, 2018-05-10 at 23:26 +, Luis R. Rodriguez wrote:
> > > On Wed, May 09, 2018 at 10:00:58PM -0400, Mimi Zohar wrote:
> > > > On Wed, 2018-05-09 at 23:48 +, Luis R. Rodriguez wrote:
> > > > > On Wed, May 09, 2018 at 06:06:57PM -0400, Mimi Zohar wrote:
> > > > 
> > > > > > > > Yes, writing regdb as a micro/mini LSM sounds reasonable.  The 
> > > > > > > > LSM
> > > > > > > > would differentiate between other firmware and the 
> > > > > > > > regulatory.db based
> > > > > > > > on the firmware's pathname.
> > > > > > > 
> > > > > > > If that is the only way then it would be silly to do the mini LSM 
> > > > > > > as all
> > > > > > > calls would have to have the check. A special LSM hook for just 
> > > > > > > the
> > > > > > > regulatory db also doesn't make much sense.
> > > > > > 
> > > > > > All calls to request_firmware() are already going through this LSM
> > > > > > hook.  I should have said, it would be based on both 
> > > > > > READING_FIRMWARE
> > > > > > and the firmware's pathname.
> > > > > 
> > > > > Yes, but it would still be a strcmp() computation added for all
> > > > > READING_FIRMWARE. In that sense, the current arrangement is only open 
> > > > > coding the
> > > > > signature verification for the regulatory.db file.  One way to avoid 
> > > > > this would
> > > > > be to add an LSM specific to the regulatory db
> > > > 
> > > > Casey already commented on this suggestion.
> > > 
> > > Sorry but I must have missed this, can you send me the email or URL where 
> > > he did that?
> > > I never got a copy of that email I think.
> > 
> > My mistake.  I've posted similar patches for kexec_load and for the
> > firmware sysfs fallback, both call security_kernel_read_file().
> > Casey's comment was in regards to kexec_load[1], not for the sysfs
> > fallback mode.  Here's the link to the most recent version of the
> > kexec_load patches.[2]
> > 
> > [1] 
> > http://kernsec.org/pipermail/linux-security-module-archive/2018-May/006690.html
> > [2] 
> > http://kernsec.org/pipermail/linux-security-module-archive/2018-May/006854.html
> 
> It seems I share Eric's concern on these threads are over general 
> architecture,
> below some notes which I think may help for the long term on that regards.
> 
> In the firmware_loader case we have *one* subsystem which as open coded 
> firmware
> signing -- the wireless subsystem open codes firmware verification by doing 
> two
> request_firmware() calls, one for the regulatory.bin and one for 
> regulatory.bin.p7s,
> and then it does its own check. In this patch set you suggested adding
> a new READING_FIRMWARE_REGULATORY_DB. But your first patch in the series also
> adds READING_FIRMWARE_FALLBACK for the fallback case where we enable use of
> the old syfs loading facility.
> 
> My concerns are two fold for this case:
> 
> a) This would mean adding a new READING_* ID tag per any kernel mechanism 
> which open
> codes its own signature verification scheme.

Yes, that's true.  In order to differentiate between different
methods, there needs to be some way of differentiating between them.
> 
> b) The way it was implemented was to do (just showing
> READING_FIRMWARE_REGULATORY_DB here):

The purpose for READING_FIRMWARE_REGULATORY_DB is different than for
adding enumerations for other firmware verification methods (eg.
fallback sysfs).  In this case, both IMA-appraisal and REGDB are being
called to verify the firmware signature.  Adding
READING_FIRMWARE_REGULATORY_DB was in order to coordinate between
them.

continued below ...

> 
> diff --git a/drivers/base/firmware_loader/main.c 
> b/drivers/base/firmware_loader/main.c
> index eb34089e4299..d7cdf04a8681 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -318,6 +318,11 @@ fw_get_filesystem_firmware(struct device *device, struct 
> fw_priv *fw_priv)
> break;
> }
> 
> +#ifdef CONFIG_CFG80211_REQUIRE_SIGNED_REGDB
> +   if ((strcmp(fw_priv->fw_name, "regulatory.db") == 0) ||
> +   (strcmp(fw_priv->fw_name, "regulatory.db.p7s") == 0))
> +   id = READING_FIRMWARE_REGULATORY_DB;
> +#endif
> fw_priv->size = 0;
> rc = kernel_read_file_from_path(path, &fw_priv->data, &size,
> msize, id);
> 
> This is eye-soring, and in turn would mean adding yet more #ifdefs for any
> code on the kernel which open codes other firmware signing efforts with
> its own kconfig...

Agreed, adding ifdef's is ugly.  As previously discussed, this code
will be removed.

To coordinate the signature verification, at build time either regdb
or IMA-appraisal firmware will be enabled.  At runtime, in the case
that regdb is enabled and a custom policy requires IMA-appraisal
firmware signature verification, t

Re: WARNING in dev_vprintk_emit

2018-05-14 Thread Johannes Berg
On Sun, 2018-05-13 at 11:47 -0700, Eric Biggers wrote:

> The bug is that mac80211_hwsim allows creating device names that are too long,
> via HWSIM_CMD_NEW_RADIO.  Commit a7cfebcb7594a2 ("cfg80211: limit wiphy names 
> to
> 128 bytes") limited them to 128 bytes, but it's not enough because
> dev_vprintk_emit() needs the device name plus some other stuff to not exceed 
> 128
> bytes.  Here's a reproducer that works on Linus' tree (commit ccda3c4b777),
> provided that CONFIG_MAC80211_HWSIM=y.  Note that you'll probably need to 
> change
> the hardcoded MAC80211_HWSIM generic netlink family ID for it to work.
> Johannes, what would you say about limiting the name length to 64 bytes 
> instead?

Don't really have any objections - care to send a patch?

johannes


Re: [PATCH V3 0/5] Update brcm firmware files

2018-05-14 Thread Josh Boyer
n Mon, May 14, 2018 at 6:11 AM Arend van Spriel <
arend.vanspr...@broadcom.com> wrote:

> On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:
> > Update brcm firmware files and WHENCE accordingly.

> Hi firmware-maintainers,

> It seems this series somehow got lost. Can these still be applied. They
> can be found in the linux-wireless patchwork database. I provided links
> below.

All 5 of these move the respective firmware files under the Cypress
license.  It has been pointed out that the Cypress license has some
questionable language in it and that people have been in touch to try and
get this resolved.  I'm personally waiting on applying them until the
licence issue is sorted out.

josh


> Regards,
> Arend
> ---
> > Chi-Hsien Lin (5):
> >brcm: update firmware for bcm43430

> https://patchwork.kernel.org/patch/10287495/

> >brcm: update firmware for bcm43340

> https://patchwork.kernel.org/patch/10287497/

> >brcm: update firmware for bcm43362

> https://patchwork.kernel.org/patch/10287493/

> >brcm: update firmware for bcm4354

> https://patchwork.kernel.org/patch/10287499/

> >brcm: update firmware for bcm4356 pcie

> https://patchwork.kernel.org/patch/10287501/


Re: [PATCH 03/12] ath10k: Add ath10k QMI client driver

2018-05-14 Thread Govind Singh

Hi Bjorn,
Thanks for the review.

On 2018-05-11 22:55, Bjorn Andersson wrote:

On Sun 25 Mar 22:39 PDT 2018, Govind Singh wrote:


Add QMI client driver for Q6 integrated WLAN connectivity
subsystem. This module is responsible for communicating WLAN
control messages to FW over QUALCOMM MSM Interface (QMI).

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/Kconfig  |   2 +-
 drivers/net/wireless/ath/ath10k/Makefile |   4 +
 drivers/net/wireless/ath/ath10k/qmi.c| 121 
+++

 drivers/net/wireless/ath/ath10k/qmi.h|  24 ++
 4 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/qmi.c
 create mode 100644 drivers/net/wireless/ath/ath10k/qmi.h

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig 
b/drivers/net/wireless/ath/ath10k/Kconfig

index 84f071a..9978ad5e 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -42,7 +42,7 @@ config ATH10K_USB

 config ATH10K_SNOC
 tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
-depends on ATH10K && ARCH_QCOM
+depends on ATH10K && ARCH_QCOM && QCOM_QMI_HELPERS


QCOM_QMI_HELPERS is expected to be selected by the clients, so this
would be:

select QCOM_QMI_HELPERS


Sure, will do in next version.


+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE 
LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY 
DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN 
AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING 
OUT OF

+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */


SPDX headers for new files please.



Sure, will do in next version.



+static struct ath10k_qmi *qmi;


Please design your code so that you don't depend on a global state
pointer.



Actually i thought about this, i can save this in platform drvdata and 
get this at

run time by saving the pdev in qmi_service->priv.
But qmi_service is available only in new_server/del_server, but not in 
the qmi indication callbacks.

Qmi handle also does not have the reference to the qmi_service.
Can you pls suggest something here.


+
+static int ath10k_qmi_new_server(struct qmi_handle *qmi_hdl,
+struct qmi_service *service)
+{
+   return 0;
+}
+
+static void ath10k_qmi_del_server(struct qmi_handle *qmi_hdl,
+ struct qmi_service *service)
+{
+}
+
+static struct qmi_ops ath10k_qmi_ops = {
+   .new_server = ath10k_qmi_new_server,
+   .del_server = ath10k_qmi_del_server,
+};
+
+static int ath10k_qmi_probe(struct platform_device *pdev)
+{
+   int ret;
+
+   qmi = devm_kzalloc(&pdev->dev, sizeof(*qmi),
+  GFP_KERNEL);


This doesn't need to be line wrapped.



Sure, will do in next version.


+   if (!qmi)
+   return -ENOMEM;
+
+   qmi->pdev = pdev;


The only place you use this is to access pdev->dev, so carry the struct
device * instead.



Sure, will do in next version.


+   platform_set_drvdata(pdev, qmi);
+   ret = qmi_handle_init(&qmi->qmi_hdl,
+ WLFW_BDF_DOWNLOAD_REQ_MSG_V01_MAX_MSG_LEN,
+ &ath10k_qmi_ops, NULL);
+   if (ret < 0)
+   goto err;
+
+   ret = qmi_add_lookup(&qmi->qmi_hdl, WLFW_SERVICE_ID_V01,
+WLFW_SERVICE_VERS_V01, 0);
+   if (ret < 0)
+   goto err;
+
+   pr_debug("qmi client driver probed successfully\n");


Rather than printing a line to indicate that your driver probed you can
check /sys/bus/platform/drivers/ath10k_QMI_client for for devices,
regardless of debug level.



Sure, will do in next version.


+
+   return 0;


qmi_add_lookup() returns 0 on success, so you can have a common return,
preferably after renaming "err" to "out" or something that indicates
that it covers both cases.


+
+err:
+   return ret;
+}
+
+static int ath10k_qmi_remove(struct platform_device *pdev)
+{
+   struct ath10k_qmi *qmi = platform_get_drvdata(pdev);
+
+   qmi_handle_release(&qmi->qmi_hdl);
+
+   return 0;
+}
+
+static const struct of_device_id ath10k_qmi_dt_match[] = {
+   {.compatible = "qcom,ath10k-qmi"},
+   {}
+};
+
+MODULE_DEVICE_TABLE(of, ath10k_qmi_dt_match);
+
+static struct platform_driver ath10k_qmi_clinet = {


Spelling of "client".



Sure, will do in next version.


+   .probe  = ath10k_qmi_probe,
+   .remove = ath10k_qmi_remove,
+   .driver = {
+   .name = "ath10k QMI client",
+   .owner = THIS_MODULE,


platform_driver_register() will fill out .owner for you, so skip this.


+   .of_match_table = ath10k_qmi_dt_match,
+   },
+};
+
+static int __init ath10k_qmi_init(void)
+{
+   return platform_driver_register(&ath10k_qmi_clinet);
+}
+
+static void __exit ath10k_qmi_exit(void)
+{

Re: brcmfmac: Updating Broadcom/Cypress Wifi/BT firmware

2018-05-14 Thread James Hughes
On 14 May 2018 at 10:52, Stefan Wahren  wrote:
> Hi Arend,
>
> Am 14.05.2018 um 10:15 schrieb Arend van Spriel:
>>
>> On 5/2/2018 8:05 PM, Stefan Wahren wrote:
>>>
>>> Hi,
>>> i wanted to know, if there are any plans to update the Broadcom/Cypress
>>> firmware under:
>>>
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
>>>
>>> As BCM2835 maintainer i'm interested in the following files:
>>>
>>> brcm/brcmfmac43430-sdio.bin
>>> brcm/brcmfmac43455-sdio.bin
>>
>>
>> To my knowledge (just checked ;-) ) these are already in the repository.
>
>
> i know, but the subject was about updating. These files are from 2015/2016
> and contains a lot of issues (including CVE-2016-0801 CVE-2017-0561
> CVE-2017-9417).
>
> AFAIK the latest version of brcmfmac43430-sdio.bin was from October 2017
> [1].
>
>>
>>> brcm/brcmfmac43430-sdio.txt
>>
>>
>> We do not distribute these .txt files aka nvram files as they are board
>> specific and no good solution to express that in the filename.
>
>
> Thanks for clarification
>
>>
>>> brcm/BCM43430A1.hcd
>>
>>
>> The BT side is not handled by us. Might be different within Cypress
>> though.
>
>
> Okay
>
>>
>> Regards,
>> Arend
>
>
> [1] - https://github.com/raspberrypi/linux/issues/1342
>
> Best regards
> Stefan


Just an an indicator, the Raspberry Pi supplied firmware, which I
believe is now a standard build (no customisations) from the Cypress
firmware source tree, is dated Oct 2017, so somewhat newer than that
in firmware. It also has a some bug fixes in, Stefan has flagged
some of the security ones, but there are others.


Re: [PATCH 14/30] staging: wilc1000: fix line over 80 chars in add_network_to_shadow()

2018-05-14 Thread Ajay Singh
Hi Claudiu,

On Mon, 14 May 2018 11:57:24 +0300
Claudiu Beznea  wrote:

> Hi Ajay,
> 
> On 10.05.2018 08:27, Claudiu Beznea wrote:
> > 
> > 
> > On 09.05.2018 21:42, Ajay Singh wrote:  
> >> On Wed, 9 May 2018 16:43:14 +0300
> >> Claudiu Beznea  wrote:
> >>  
> >>> On 07.05.2018 11:43, Ajay Singh wrote:  
>  Fix line over 80 characters issue reported by checkpatch in
>  add_network_to_shadow() by using temporary variable.
> >>>
> >>> I, personally, don't like this way of fixing line over 80. From my
> >>> point of view this introduces a new future patch. Maybe, in
> >>> future, somebody will remove this temporary variable stating that
> >>> there is no need for it.
> >>>  
> >>
> >> In my opinion, just by removing this temporary variable the patch
> >> might not go through because it will definitely have line over
> >> 80 character issue. As per guideline its recommended to run the
> >> checkpatch before submitting the patch.
> >>
> >> Only using short variables names might help to resolve that issue
> >> but using short variable names will not give clear meaning for the
> >> code. I  don't want to shorten the variable name as they don't
> >> convey the complete meaning.
> >>
> >> Do you have any suggestion/code which can help to understand how to
> >> resolve this without using temp/variables name changes.  
> > 
> > No, for this one I have not. Maybe further refactoring...
> >   
> 
> Looking over the v2 of this series you send, and over
> wilc_wfi_cfgoperations.c, and remembering your last question on this
> patch, I was thinking that one suggestion for this would be to
> replace last_scanned_shadow with just scanned_shadow or nw_info or
> scanned_nw_info. Just an idea.
> 

I avoided use of short name for 'last_scanned_shadow' because it might
not give clear meaning as there are variables like 'last_scanned_cnt',
which also uses same prefix 'last_' and its helpful to know its related
data.


> >>  
> 
>  Signed-off-by: Ajay Singh 
>  ---
>   drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52
>  +++ 1 file changed, 25 insertions(+), 27
>  deletions(-)
> 
>  diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>  b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
>  f1ebaea..0ae2065 100644 ---
>  a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
>  b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6
>  +300,7 @@ static void add_network_to_shadow(struct network_info
>  *nw_info, int ap_found = is_network_in_shadow(nw_info,
>  user_void); u32 ap_index = 0; u8 rssi_index = 0;
>  +struct network_info *shadow_nw_info;
>   
>   if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW)
>   return;
>  @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct
>  network_info *nw_info, } else {
>   ap_index = ap_found;
>   }
>  -rssi_index =
>  last_scanned_shadow[ap_index].rssi_history.index;
>  -
>  last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++]
>  = nw_info->rssi;
>  +shadow_nw_info = &last_scanned_shadow[ap_index];
>  +rssi_index = shadow_nw_info->rssi_history.index;
>  +shadow_nw_info->rssi_history.samples[rssi_index++] =
>  nw_info->rssi; if (rssi_index == NUM_RSSI) {
>   rssi_index = 0;
>  -last_scanned_shadow[ap_index].rssi_history.full
>  = true;
>  -}
>  -last_scanned_shadow[ap_index].rssi_history.index =
>  rssi_index;
>  -last_scanned_shadow[ap_index].rssi = nw_info->rssi;
>  -last_scanned_shadow[ap_index].cap_info =
>  nw_info->cap_info;
>  -last_scanned_shadow[ap_index].ssid_len =
>  nw_info->ssid_len;
>  -memcpy(last_scanned_shadow[ap_index].ssid,
>  -   nw_info->ssid, nw_info->ssid_len);
>  -memcpy(last_scanned_shadow[ap_index].bssid,
>  -   nw_info->bssid, ETH_ALEN);
>  -last_scanned_shadow[ap_index].beacon_period =
>  nw_info->beacon_period;
>  -last_scanned_shadow[ap_index].dtim_period =
>  nw_info->dtim_period;
>  -last_scanned_shadow[ap_index].ch = nw_info->ch;
>  -last_scanned_shadow[ap_index].ies_len =
>  nw_info->ies_len;
>  -last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi;
>  +shadow_nw_info->rssi_history.full = true;
>  +}
>  +shadow_nw_info->rssi_history.index = rssi_index;
>  +shadow_nw_info->rssi = nw_info->rssi;
>  +shadow_nw_info->cap_info = nw_info->cap_info;
>  +shadow_nw_info->ssid_len = nw_info->ssid_len;
>  +memcpy(shadow_nw_info->ssid, nw_info->ssid,
>  nw_info->ssid_len);
>  +memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH

Re: [PATCH V3 0/5] Update brcm firmware files

2018-05-14 Thread Arend van Spriel

On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:

Update brcm firmware files and WHENCE accordingly.


Hi firmware-maintainers,

It seems this series somehow got lost. Can these still be applied. They 
can be found in the linux-wireless patchwork database. I provided links 
below.


Regards,
Arend
---

Chi-Hsien Lin (5):
   brcm: update firmware for bcm43430


https://patchwork.kernel.org/patch/10287495/


   brcm: update firmware for bcm43340


https://patchwork.kernel.org/patch/10287497/


   brcm: update firmware for bcm43362


https://patchwork.kernel.org/patch/10287493/


   brcm: update firmware for bcm4354


https://patchwork.kernel.org/patch/10287499/


   brcm: update firmware for bcm4356 pcie


https://patchwork.kernel.org/patch/10287501/





Re: brcmfmac: Updating Broadcom/Cypress Wifi/BT firmware

2018-05-14 Thread Stefan Wahren

Hi Arend,

Am 14.05.2018 um 10:15 schrieb Arend van Spriel:

On 5/2/2018 8:05 PM, Stefan Wahren wrote:

Hi,
i wanted to know, if there are any plans to update the 
Broadcom/Cypress firmware under:


https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git 



As BCM2835 maintainer i'm interested in the following files:

brcm/brcmfmac43430-sdio.bin
brcm/brcmfmac43455-sdio.bin


To my knowledge (just checked ;-) ) these are already in the repository.


i know, but the subject was about updating. These files are from 
2015/2016 and contains a lot of issues (including CVE-2016-0801 
CVE-2017-0561 CVE-2017-9417).


AFAIK the latest version of brcmfmac43430-sdio.bin was from October 2017 
[1].





brcm/brcmfmac43430-sdio.txt


We do not distribute these .txt files aka nvram files as they are 
board specific and no good solution to express that in the filename.


Thanks for clarification




brcm/BCM43430A1.hcd


The BT side is not handled by us. Might be different within Cypress 
though.


Okay



Regards,
Arend


[1] - https://github.com/raspberrypi/linux/issues/1342

Best regards
Stefan


Re: [PATCH 14/30] staging: wilc1000: fix line over 80 chars in add_network_to_shadow()

2018-05-14 Thread Claudiu Beznea
Hi Ajay,

On 10.05.2018 08:27, Claudiu Beznea wrote:
> 
> 
> On 09.05.2018 21:42, Ajay Singh wrote:
>> On Wed, 9 May 2018 16:43:14 +0300
>> Claudiu Beznea  wrote:
>>
>>> On 07.05.2018 11:43, Ajay Singh wrote:
 Fix line over 80 characters issue reported by checkpatch in
 add_network_to_shadow() by using temporary variable.  
>>>
>>> I, personally, don't like this way of fixing line over 80. From my
>>> point of view this introduces a new future patch. Maybe, in future,
>>> somebody will remove this temporary variable stating that there is
>>> no need for it.
>>>
>>
>> In my opinion, just by removing this temporary variable the patch
>> might not go through because it will definitely have line over
>> 80 character issue. As per guideline its recommended to run the
>> checkpatch before submitting the patch.
>>
>> Only using short variables names might help to resolve that issue but
>> using short variable names will not give clear meaning for the code. 
>> I  don't want to shorten the variable name as they don't convey the
>> complete meaning.
>>
>> Do you have any suggestion/code which can help to understand how to
>> resolve this without using temp/variables name changes.
> 
> No, for this one I have not. Maybe further refactoring...
> 

Looking over the v2 of this series you send, and over wilc_wfi_cfgoperations.c,
and remembering your last question on this patch, I was thinking that
one suggestion for this would be to replace last_scanned_shadow with
just scanned_shadow or nw_info or scanned_nw_info. Just an idea.

Claudiu

>>

 Signed-off-by: Ajay Singh 
 ---
  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52
 +++ 1 file changed, 25 insertions(+), 27
 deletions(-)

 diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
 b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
 f1ebaea..0ae2065 100644 ---
 a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
 b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6
 +300,7 @@ static void add_network_to_shadow(struct network_info
 *nw_info, int ap_found = is_network_in_shadow(nw_info, user_void);
 u32 ap_index = 0; u8 rssi_index = 0;
 +  struct network_info *shadow_nw_info;
  
if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW)
return;
 @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct
 network_info *nw_info, } else {
ap_index = ap_found;
}
 -  rssi_index =
 last_scanned_shadow[ap_index].rssi_history.index;
 -
 last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] =
 nw_info->rssi;
 +  shadow_nw_info = &last_scanned_shadow[ap_index];
 +  rssi_index = shadow_nw_info->rssi_history.index;
 +  shadow_nw_info->rssi_history.samples[rssi_index++] =
 nw_info->rssi; if (rssi_index == NUM_RSSI) {
rssi_index = 0;
 -  last_scanned_shadow[ap_index].rssi_history.full =
 true;
 -  }
 -  last_scanned_shadow[ap_index].rssi_history.index =
 rssi_index;
 -  last_scanned_shadow[ap_index].rssi = nw_info->rssi;
 -  last_scanned_shadow[ap_index].cap_info = nw_info->cap_info;
 -  last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len;
 -  memcpy(last_scanned_shadow[ap_index].ssid,
 - nw_info->ssid, nw_info->ssid_len);
 -  memcpy(last_scanned_shadow[ap_index].bssid,
 - nw_info->bssid, ETH_ALEN);
 -  last_scanned_shadow[ap_index].beacon_period =
 nw_info->beacon_period;
 -  last_scanned_shadow[ap_index].dtim_period =
 nw_info->dtim_period;
 -  last_scanned_shadow[ap_index].ch = nw_info->ch;
 -  last_scanned_shadow[ap_index].ies_len = nw_info->ies_len;
 -  last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi;
 +  shadow_nw_info->rssi_history.full = true;
 +  }
 +  shadow_nw_info->rssi_history.index = rssi_index;
 +  shadow_nw_info->rssi = nw_info->rssi;
 +  shadow_nw_info->cap_info = nw_info->cap_info;
 +  shadow_nw_info->ssid_len = nw_info->ssid_len;
 +  memcpy(shadow_nw_info->ssid, nw_info->ssid,
 nw_info->ssid_len);
 +  memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN);
 +  shadow_nw_info->beacon_period = nw_info->beacon_period;
 +  shadow_nw_info->dtim_period = nw_info->dtim_period;
 +  shadow_nw_info->ch = nw_info->ch;
 +  shadow_nw_info->ies_len = nw_info->ies_len;
 +  shadow_nw_info->tsf_hi = nw_info->tsf_hi;
if (ap_found != -1)
 -  kfree(last_scanned_shadow[ap_index].ies);
 -  last_scanned_shadow[ap_index].ies =
 kmalloc(nw_info->ies_len,
 -  GFP_KERNEL);
 -  memcpy(last_scanned_shadow[ap_index].ies,
 - nw_info->ies, nw_info->ies_len);
 -  last_scanned_shadow[ap_index].time_scan = jiffies;
 -  last_scanned_shadow[ap_index].time_scan_cached = jiffies;
>

Re: brcmfmac: Updating Broadcom/Cypress Wifi/BT firmware

2018-05-14 Thread Arend van Spriel

On 5/2/2018 8:05 PM, Stefan Wahren wrote:

Hi,
i wanted to know, if there are any plans to update the Broadcom/Cypress 
firmware under:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git

As BCM2835 maintainer i'm interested in the following files:

brcm/brcmfmac43430-sdio.bin
brcm/brcmfmac43455-sdio.bin


To my knowledge (just checked ;-) ) these are already in the repository.


brcm/brcmfmac43430-sdio.txt


We do not distribute these .txt files aka nvram files as they are board 
specific and no good solution to express that in the filename.



brcm/BCM43430A1.hcd


The BT side is not handled by us. Might be different within Cypress though.

Regards,
Arend


Re: rtl8188eu: link is not ready error on lm816

2018-05-14 Thread Mylène Josserand
Hello Larry,

Sorry about the delay, I was on holiday last week.

On Sat, 5 May 2018 16:11:13 -0500
Larry Finger  wrote:

> On 05/04/2018 02:20 PM, Mylène Josserand wrote:
> > Hello everyone,
> > 
> > I am currently testing a USB Wifi dongle LM816 [1] on a custom platform
> > based on an IMX6.
> > 
> > My configuration:
> > kernel from mainline: v4.14
> > driver: staging R8188EU
> > firmware: rtl8188eu from linux-firmware, last commit
> > a3a26af24e29c818ef9b5661856018e21a5c49fb
> > 
> > When I tried to set the wlan interface up, it says that "link is not
> > ready" and that's all. Because of that, it fails to scan available
> > networks.
> > 
> > Here are commands used and their output:
> > 
> > # dmesg
> > [...]
> > [   14.690152] r8188eu: module is from the staging directory, the
> > quality is unknown, you have been warned.
> > [   14.703105] Chip Version
> > Info: CHIP_8188E_Normal_Chip_TSMC_D_CUT_1T1R_RomVer(0)
> > [...]
> > 
> > # lsusb
> > Bus 002 Device 004: ID 0bda:8179  <-- LM816
> > Bus 002 Device 003: ID 0424:9e00
> > Bus 002 Device 002: ID 0424:2517
> > Bus 002 Device 001: ID 1d6b:0002
> > Bus 001 Device 001: ID 1d6b:0002
> > 
> > # lsmod
> > Module  Size  Used byTainted: G
> > dw_hdmi_ahb_audio  16384  0
> > r8188eu   401408  0
> > coda   61440  0
> > imx_vdoa   16384  1 coda
> > v4l2_mem2mem   24576  1 coda
> > videobuf2_vmalloc  16384  1 coda
> > evbug  16384  0
> > 
> > # ifconfig wlan0 up
> > [  447.639524] MAC Address = 5c:f3:70:3d:5c:71
> > [  447.653636] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> > #
> > #
> > # iw wlan0 scan
> > command failed: No such device (-19)
> > 
> > 
> > Does anyone already have this issue with this dongle?
> > If it is not the case, what is your setup? Which firmware? Which driver?
> > Any help will be very appreciated.
> > 
> > Thanks in advance,
> > Best regards,
> > 
> > [1]:
> > https://www.lm-technologies.com/product/wifi-usb-nano-adapter-150mbps-lm816/
> >   
> 
> I am running kernel 4.17.0-rc3 under openSUSE Tumbleweed KDE on a Toshiba 
> laptop, and controlling networks with NetworkManager. 

I am running a kernel v4.14.0. The last commit I have for this driver
is:
b24413180f56 'License cleanup: add SPDX GPL-2.0 license identifier to
files with no license'.

There is many new commits on top of it. I will try to update the driver
or my kernel to have the last version.

> When I plug my dongle in, 
> the device connects automatically. The firmware I am using has the following 
> md5sum:
> 
> aaef52a47852e599cbff63a3e7f96a94  /lib/firmware/rtlwifi/rtl8188eufw.bin

Okay, I have the same md5sum so we are using the same firmware:
aaef52a47852e599cbff63a3e7f96a94  /lib/firmware/rtlwifi/rtl8188eufw.bin

> 
> My dongle shows the following with lsusb:
> ID 0bda:8179 Realtek Semiconductor Corp. RTL8188EUS 802.11n Wireless Network 
> Adapter

Same here:
ID 0bda:8179 Realtek Semiconductor Corp. RTL8188EUS 802.11n Wireless
Network Adapter

> 
> Mine shows as an A-cut:
> Chip Version Info: CHIP_8188E_Normal_Chip_TSMC_A_CUT_1T1R_RomVer(0)
> 
> Yours is a D-cut, but I would not expect any difference from that. The only 
> place that information is used is in the log entry shown above.
> 
> If you want to try a different driver, there is one at
> http://github.com/lwfinger/rtl8188eu.git
> 
> Larry
> 
> 

I will try to update my kernel and I will let you know if it is now
working (or not).

Anyway, thanks for your help!

Best regards,

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com


Re: [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities

2018-05-14 Thread Arend van Spriel

On 5/14/2018 7:11 AM, Rafał Miłecki wrote:

+/**
+ * brcmf_cap_read() - expose firmware capabilities to debugfs.


Please stick to naming convention brcmf__foo(), ie.
brcmf_feat_cap_read().


Will do, thanks! I missed that naming convention because of
brcmf_debugfs_fws_stats_read() / brcmf_debugfs_sdio_count_read() which
slightly confused me. I see your point now!


Yeah. Sorry for the confusion. There is always room to improve.

Gr. AvS