Re: [PATCH 15/30] staging: wilc1000: use kmemdup instead of kmalloc in add_network_to_shadow()

[Claudiu Beznea]

On 09.05.2018 22:17, Ajay Singh wrote:
> On Wed, 9 May 2018 16:42:59 +0300
> Claudiu Beznea  wrote:
> 
>> On 07.05.2018 11:43, Ajay Singh wrote:
>>> Use kmemdup instead of kmalloc & memcpy in add_network_to_shadow().
>>>
>>> Signed-off-by: Ajay Singh 
>>> ---
>>>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
>>> 0ae2065..ca221f1 100644 ---
>>> a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -331,8
>>> +331,8 @@ static void add_network_to_shadow(struct network_info
>>> *nw_info, shadow_nw_info->tsf_hi = nw_info->tsf_hi; if (ap_found !=
>>> -1) kfree(shadow_nw_info->ies);
>>> -   shadow_nw_info->ies = kmalloc(nw_info->ies_len,
>>> GFP_KERNEL);
>>> -   memcpy(shadow_nw_info->ies, nw_info->ies,
>>> nw_info->ies_len);
>>> +   shadow_nw_info->ies = kmemdup(nw_info->ies,
>>> nw_info->ies_len,
>>> + GFP_KERNEL);  
>>
>> Maybe, in case of NULL, you will want to set ies_len = 0 ?
> 
> 
> I couldn't find code where 'ies_len' is check to validity of data.
> Mostly we use NULL check for "ies" pointer for data
> validity.So in my opinion setting it to zero would be
> irrelevant.

I'm seeing this in refresh_scan():
network_info = _scanned_shadow[i]; 

if (!memcmp("DIRECT-", network_info->ssid, 7) && !direct_scan)  
continue;   

freq = ieee80211_channel_to_frequency((s32)network_info->ch,
  NL80211_BAND_2GHZ);   
channel = ieee80211_get_channel(wiphy, freq);   
rssi = get_rssi_avg(network_info);  
bss = cfg80211_inform_bss(wiphy,
  channel,  
  CFG80211_BSS_FTYPE_UNKNOWN,   
  network_info->bssid,  
  network_info->tsf_hi, 
  network_info->cap_info,   
  network_info->beacon_period,  
  (const u8 *)network_info->ies,
  (size_t)network_info->ies_len,
  (s32)rssi * 100,  
  GFP_KERNEL);  

Looking further into cfg80211_inform_bss():
-> cfg80211_inform_bss_data()
-> cfg80211_get_bss_channel()
-> cfg80211_find_ie()
-> cfg80211_find_ie_match():
while (len >= 2 && len >= ies[1] + 2) { 
if ((ies[0] == eid) &&  
(ies[1] + 2 >= match_offset + match_len) && 
!memcmp(ies + match_offset, match, match_len))  
return ies; 

len -= ies[1] + 2;  
ies += ies[1] + 2;  
}   


> 
> 
>>
>>> shadow_nw_info->time_scan = jiffies;
>>> shadow_nw_info->time_scan_cached = jiffies;
>>> shadow_nw_info->found = 1;
>>>   
> 
> 
> Regards,
> Ajay
> 
> 

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

[Claudiu Beznea]

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

> 
>>>
>>> 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 = _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;
>>> -   last_scanned_shadow[ap_index].found = 1;
>>> +   kfree(shadow_nw_info->ies);
>>> +   shadow_nw_info->ies = kmalloc(nw_info->ies_len,
>>> GFP_KERNEL);
>>> +   memcpy(shadow_nw_info->ies, nw_info->ies,
>>> nw_info->ies_len);
>>> +   shadow_nw_info->time_scan = jiffies;
>>> +   shadow_nw_info->time_scan_cached = jiffies;
>>> +   

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

[Claudiu Beznea]

On 09.05.2018 21:36, Ajay Singh wrote:
> On Wed, 9 May 2018 16:44:47 +0300
> Claudiu Beznea  wrote:
> 
>> On 07.05.2018 11:43, Ajay Singh wrote:
>>> Fix checkpatch reported issue of line over 80 char in handle_key().
>>> Introduced new functions by spliting existing function to address
>>> the checkpatch issue.
>>>
>>> Signed-off-by: Ajay Singh 
>>> ---
>>>  drivers/staging/wilc1000/host_interface.c | 59
>>> +++ 1 file changed, 37 insertions(+),
>>> 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/host_interface.c
>>> b/drivers/staging/wilc1000/host_interface.c index 4ba868c..29f9907
>>> 100644 --- a/drivers/staging/wilc1000/host_interface.c
>>> +++ b/drivers/staging/wilc1000/host_interface.c
>>> @@ -1498,12 +1498,45 @@ static s32
>>> handle_rcvd_gnrl_async_info(struct wilc_vif *vif, return result;
>>>  }
>>>  
>>> +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct
>>> key_attr *hif_key) +{
>>> +   int i;
>>> +   int ret;
>>> +   struct wid wid;
>>> +   u8 *key_buf;
>>> +
>>> +   key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1,
>>> + GFP_KERNEL);
>>> +   if (!key_buf)
>>> +   return -ENOMEM;
>>> +
>>> +   key_buf[0] = hif_key->attr.pmkid.numpmkid;
>>> +
>>> +   for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
>>> +   memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
>>> +  hif_key->attr.pmkid.pmkidlist[i].bssid,
>>> ETH_ALEN);
>>> +   memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN +
>>> 1),
>>> +  hif_key->attr.pmkid.pmkidlist[i].pmkid,
>>> PMKID_LEN);
>>> +   }
>>> +
>>> +   wid.id = (u16)WID_PMKID_INFO;
>>> +   wid.type = WID_STR;
>>> +   wid.val = (s8 *)key_buf;  
>>
>> Is this cast really needed?
>>
> 
> This patch is only to address line over 80 chars checkpatch issue.
> It is not good to club these change with type cast related
> modification. For removing unnecessary cast we can submit
> a separate patch series.
> These are my views. What do you think?
> 

I'm ok with this. I was thinking that since you introduced this new function
you may want to also address this, if any.

> 
>>> +   wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN)
>>> + 1; +
>>> +   ret = wilc_send_config_pkt(vif, SET_CFG, , 1,
>>> +  wilc_get_vif_idx(vif));
>>> +
>>> +   kfree(key_buf);
>>> +
>>> +   return ret;
>>> +}
>>> +
>>>  static int handle_key(struct wilc_vif *vif, struct key_attr
>>> *hif_key) {
>>> int result = 0;
>>> struct wid wid;
>>> struct wid wid_list[5];
>>> -   u8 i;
>>> u8 *key_buf;
>>> s8 s8idxarray[1];
>>> struct host_if_drv *hif_drv = vif->hif_drv;
>>> @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif,
>>> struct key_attr *hif_key) wilc_get_vif_idx(vif));
>>> kfree(key_buf);
>>> } else if (hif_key->action & ADDKEY) {
>>> -   key_buf =
>>> kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
>>> +   key_buf =
>>> kmalloc(hif_key->attr.wep.key_len + 2,
>>> + GFP_KERNEL);
>>> if (!key_buf) {
>>> result = -ENOMEM;
>>> goto out_wep;
>>> @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif,
>>> struct key_attr *hif_key) break;
>>>  
>>> case PMKSA:
>>> -   key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1, GFP_KERNEL);
>>> -   if (!key_buf)
>>> -   return -ENOMEM;
>>> -
>>> -   key_buf[0] = hif_key->attr.pmkid.numpmkid;
>>> -
>>> -   for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++)
>>> {
>>> -   memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
>>> 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
>>> -   memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
>>> ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
>>> -   }
>>> -
>>> -   wid.id = (u16)WID_PMKID_INFO;
>>> -   wid.type = WID_STR;
>>> -   wid.val = (s8 *)key_buf;
>>> -   wid.size = (hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1; -
>>> -   result = wilc_send_config_pkt(vif, SET_CFG, ,
>>> 1,
>>> -
>>> wilc_get_vif_idx(vif)); -
>>> -   kfree(key_buf);
>>> +   result = wilc_pmksa_key_copy(vif, hif_key);
>>> break;
>>> }
>>>  
>>>   
> 
> 
> 

Re: [PATCH v2] ath10k: support for multicast rate control

[pradeepc]

On 2018-05-09 07:31, Ben Greear wrote:

On 05/08/2018 11:57 PM, Sebastian Gottschall wrote:



Am 09.05.2018 um 08:15 schrieb Sven Eckelmann:

On Montag, 7. Mai 2018 18:34:51 CEST Pradeep Kumar Chitrapu wrote:

Issues wmi command to firmwre when multicast rate change is received
with the new BSS_CHANGED_MCAST_RATE flag.
Also fixes the incorrect fixed_rate setting for CCK rates which got
introduced with addition of ath10k_rates_rev2 enum.

Tested on QCA9984 with firmware ver 10.4-3.6-00104
Signed-off-by: Pradeep Kumar Chitrapu 
---
v2:
  - fixed the CCK rates setting for mcast_rate and fixed_rate paths.
  - set broadcast rate along with multicast rate setting.
These things are only modified in ath10k_bss_info_changed and not 
saved/
restored. What happens when the HW needs to be reset (there are are 
couple of
firmware crashes which can be seen in the wild and are handled by 
ath10k)? I
would guess that not all of them automatically cause an 
.bss_info_changed.


Yes, that sounds like a good idea to me.


Hi Sven, Thanks for the review.
In case of HW reset, I see ieee80211_reconfig triggers bss_info changed 
and

BSS_CHANGED_MCAST_RATE is already being set in this function.
(https://patchwork.kernel.org/patch/10302175/). isn't this sufficient 
for the
re-configuring the mcast rate? Please let me know if I am missing 
something.


Thanks
Pradeep

i have never seen a card properly recovering after a firmware crash, 
all firmware crashes i have seen
are caused by bugs in firmware handling or the firmware itself. so if 
it recovers it crashes immediatly again
ending in a endless crashloop since the cause for the crash hasnt been 
fixed. they are often triggered by extern clients for instance which 
still remain in the field.
i think firmware crashes should not be hidden by recovering. this 
would also force users to report problems more frequently, than they 
do

since they dont even take notice of such problems


We see recovery work often.  If you see repeatable crashes, post them
to the list.

Do you know of any particular external clients that cause these 
crashes?


Thanks,
Ben

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

[Mimi Zohar]

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:
> > On Wed, 2018-05-09 at 21:22 +, Luis R. Rodriguez wrote:
> > > 
> > > OK, its still not clear to what it will do. If it does not touch the 
> > > firmware
> > > loader code, and it just sets and configures IMA to do file signature 
> > > checking
> > > on its own, then yes I think both mechanisms would be doing the similar 
> > > work.
> > > 
> > > Wouldn't IMA do file signature checks then for all files? Or it would just
> > > enable this for whatever files userspace wishes to cover?
> > 
> > Enabling CONFIG_IMA_APPRAISE_FIRMWARE would enforce firmware
> > signatures on all directly loaded firmware and fail any method of
> > loading firmware that the signature couldn't be verified.
> 
> Ah, so a generic firmware signing mechanism via IMA. Sounds very sensible to 
> me.
> Specially in light of the fact that its what we recommend folks to consider
> if they need to address firmware signing for devices which do not have the
> ability to do hardware firmware signing verification on their own.
> 
> > > One of the things with READING_FIRMWARE_REGULATORY_DB is to also use and 
> > > trust
> > > the wireless-regdgb maintainer's key for this file, could IMA be 
> > > configured to
> > > do that?
> > 
> > IMA has its own trusted keyring.  So either the maintainer's key would
> > need to be added to the IMA keyring,
> 
> I see so we'd need this documented somehow.
> 
> > or IMA-appraisal would need to use the regdb keyring.
> 
> Can you describe this a bit more, for those not too familiar with IMA, in 
> terms
> of what would be involved in the kernel? Or is this all userspace 
> configuration
> stuff?

I think it's a bit premature to be discussing how IMA could add the
builtin regulatory key to its keyring or use the regdb keyring, as
IMA-appraisal doesn't (yet) support detached signatures.

The other option would be to include the regulatory.db signature in
the package.  For rpm, the file signature is included in the RPM
header.  Multiple attempts have been made to have Debian packages
include file signatures.  This is the most recent attempt - https://li
sts.debian.org/debian-dpkg/2018/05/msg5.html

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

Mimi

> and have the
> security_check_regulatory_db() do what it needs per LSM, but that would mean
> setting a precedent for open possibly open coded future firmware verification
> call. Its not too crazy to consider if an end goal may be avoid further open
> coded firmware signature verification hacks.
> 
> > > > Making regdb an LSM would have the same issues as currently - deciding
> > > > if regdb, IMA-appraisal, or both verify the regdb's signature.
> > > 
> > > Its unclear to me why they can't co-exist yet and not have to touch
> > > the firmware_loader code at all.
> > 
> > With the changes discussed above, they will co-exist.  Other than the
> > Kconfig changes, I don't think it will touch the firmware_loader code.
> 
> Great.
> 
>   Luis
> 

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[kbuild test robot]

Hi Toke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.17-rc4 next-20180509]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireless-drivers-Dynamically-allocate-struct-station_info/20180510-034416
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
master
config: i386-randconfig-x0-05100327 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   net//batman-adv/bat_v_elp.c: In function 'batadv_v_elp_get_throughput':
>> net//batman-adv/bat_v_elp.c:113:21: error: request for member 
>> 'expected_throughput' in something not a structure or union
  throughput = sinfo.expected_throughput / 100;
^
>> net//batman-adv/bat_v_elp.c:114:20: error: request for member 'filled' in 
>> something not a structure or union
  filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   ^

vim +/expected_throughput +113 net//batman-adv/bat_v_elp.c

69  
70  /**
71   * batadv_v_elp_get_throughput() - get the throughput towards a 
neighbour
72   * @neigh: the neighbour for which the throughput has to be obtained
73   *
74   * Return: The throughput towards the given neighbour in multiples of 
100kpbs
75   * (a value of '1' equals to 0.1Mbps, '10' equals 1Mbps, etc).
76   */
77  static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node 
*neigh)
78  {
79  struct batadv_hard_iface *hard_iface = neigh->if_incoming;
80  struct ethtool_link_ksettings link_settings;
81  struct net_device *real_netdev;
82  struct station_info *sinfo;
83  u32 throughput;
84  bool filled;
85  int ret;
86  
87  /* if the user specified a customised value for this interface, 
then
88   * return it directly
89   */
90  throughput =  
atomic_read(_iface->bat_v.throughput_override);
91  if (throughput != 0)
92  return throughput;
93  
94  /* if this is a wireless device, then ask its throughput through
95   * cfg80211 API
96   */
97  if (batadv_is_wifi_hardif(hard_iface)) {
98  if (!batadv_is_cfg80211_hardif(hard_iface))
99  /* unsupported WiFi driver version */
   100  goto default_throughput;
   101  
   102  real_netdev = 
batadv_get_real_netdev(hard_iface->net_dev);
   103  if (!real_netdev)
   104  goto default_throughput;
   105  
   106  sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
   107  if (!sinfo)
   108  goto default_throughput;
   109  
   110  ret = cfg80211_get_station(real_netdev, neigh->addr, 
sinfo);
   111  
   112  /* just save these here instead of having complex free 
logic below */
 > 113  throughput = sinfo.expected_throughput / 100;
 > 114  filled = !!(sinfo.filled & 
 > BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   115  
   116  kfree(sinfo);
   117  
   118  dev_put(real_netdev);
   119  if (ret == -ENOENT) {
   120  /* Node is not associated anymore! It would be
   121   * possible to delete this neighbor. For now set
   122   * the throughput metric to 0.
   123   */
   124  return 0;
   125  }
   126  if (ret || !filled)
   127  goto default_throughput;
   128  
   129  return throughput;
   130  }
   131  
   132  /* if not a wifi interface, check if this device provides data 
via
   133   * ethtool (e.g. an Ethernet adapter)
   134   */
   135  memset(_settings, 0, sizeof(link_settings));
   136  rtnl_lock();
   137  ret = __ethtool_get_link_ksettings(hard_iface->net_dev, 
_settings);
   138  rtnl_unlock();
   139  if (ret == 0) {
   140  /* link characteristics might change over time */
   141  if (link_settings.base.duplex == DUPLEX_FULL)
   142  hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
   143  else
   144  hard_iface->bat_v.flags &= ~BATADV_FULL_DUPLEX;
   145

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

[Luis R. Rodriguez]

On Wed, May 09, 2018 at 06:06:57PM -0400, Mimi Zohar wrote:
> On Wed, 2018-05-09 at 21:22 +, Luis R. Rodriguez wrote:
> > 
> > OK, its still not clear to what it will do. If it does not touch the 
> > firmware
> > loader code, and it just sets and configures IMA to do file signature 
> > checking
> > on its own, then yes I think both mechanisms would be doing the similar 
> > work.
> > 
> > Wouldn't IMA do file signature checks then for all files? Or it would just
> > enable this for whatever files userspace wishes to cover?
> 
> Enabling CONFIG_IMA_APPRAISE_FIRMWARE would enforce firmware
> signatures on all directly loaded firmware and fail any method of
> loading firmware that the signature couldn't be verified.

Ah, so a generic firmware signing mechanism via IMA. Sounds very sensible to me.
Specially in light of the fact that its what we recommend folks to consider
if they need to address firmware signing for devices which do not have the
ability to do hardware firmware signing verification on their own.

> > One of the things with READING_FIRMWARE_REGULATORY_DB is to also use and 
> > trust
> > the wireless-regdgb maintainer's key for this file, could IMA be configured 
> > to
> > do that?
> 
> IMA has its own trusted keyring.  So either the maintainer's key would
> need to be added to the IMA keyring,

I see so we'd need this documented somehow.

> or IMA-appraisal would need to use the regdb keyring.

Can you describe this a bit more, for those not too familiar with IMA, in terms
of what would be involved in the kernel? Or is this all userspace configuration
stuff?

> > Because that would be one difference here. The whole point of adding
> > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB was to replace CRDA which is a 
> > userspace
> > component which checks the signature of regulatory.db before reading it and
> > passing data to the kernel from it.
> > 
> > Now, however silly it may be to have CONFIG_IMA_APPRAISE_FIRMWARE *and*
> > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB, is your intent in this new patch set
> > you are mentioning, to still enable both to co-exist?
> 
> At build, either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
> CONFIG_IMA_APPRAISE_FIRMWARE, where IMA is appraising all firwmare,
> would be enabled, not both.

OK.

> > > 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 and have the
security_check_regulatory_db() do what it needs per LSM, but that would mean
setting a precedent for open possibly open coded future firmware verification
call. Its not too crazy to consider if an end goal may be avoid further open
coded firmware signature verification hacks.

> > > Making regdb an LSM would have the same issues as currently - deciding
> > > if regdb, IMA-appraisal, or both verify the regdb's signature.
> > 
> > Its unclear to me why they can't co-exist yet and not have to touch
> > the firmware_loader code at all.
> 
> With the changes discussed above, they will co-exist.  Other than the
> Kconfig changes, I don't think it will touch the firmware_loader code.

Great.

  Luis

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

On 10 May 2018 00:55:01 CEST, kbuild test robot <l...@intel.com> wrote:
>Hi Toke,
>
>Thank you for the patch! Yet something to improve:
>
>[auto build test ERROR on wireless-drivers-next/master]
>[also build test ERROR on v4.17-rc4 next-20180509]
>[if your patch is applied to the wrong git tree, please drop us a note
>to help improve the system]
>
>url:   
>https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireless-drivers-Dynamically-allocate-struct-station_info/20180510-034416
>base:  
>https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git
>master
>config: i386-randconfig-x000-201818 (attached as .config)
>compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
>reproduce:
># save the attached .config to linux build tree
>make ARCH=i386 
>
>All errors (new ones prefixed by >>):
>
> net/batman-adv/bat_v_elp.c: In function 'batadv_v_elp_get_throughput':
>>> net/batman-adv/bat_v_elp.c:113:21: error: 'sinfo' is a pointer; did
>you mean to use '->'?
>  throughput = sinfo.expected_throughput / 100;
>^
>->
>net/batman-adv/bat_v_elp.c:114:20: error: 'sinfo' is a pointer; did you
>mean to use '->'?
> filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
>   ^
>   ->

Huh, I could swear I compile tested this. Will send another version tomorrow, 
sorry about that :/

-Toke

>
>vim +113 net/batman-adv/bat_v_elp.c
>
>69 
>70 /**
>71  * batadv_v_elp_get_throughput() - get the throughput towards a
>neighbour
>72  * @neigh: the neighbour for which the throughput has to be obtained
>73  *
>74  * Return: The throughput towards the given neighbour in multiples
>of 100kpbs
>75  * (a value of '1' equals to 0.1Mbps, '10' equals 1Mbps,
>etc).
>76  */
>77 static u32 batadv_v_elp_get_throughput(struct
>batadv_hardif_neigh_node *neigh)
>78 {
>79 struct batadv_hard_iface *hard_iface = neigh->if_incoming;
>80 struct ethtool_link_ksettings link_settings;
>81 struct net_device *real_netdev;
>82 struct station_info *sinfo;
>83 u32 throughput;
>84 bool filled;
>85 int ret;
>86 
>87 /* if the user specified a customised value for this interface,
>then
>88  * return it directly
>89  */
> 90throughput =  
> atomic_read(_iface->bat_v.throughput_override);
>91 if (throughput != 0)
>92 return throughput;
>93 
>   94  /* if this is a wireless device, then ask its throughput through
>95  * cfg80211 API
>96  */
>97 if (batadv_is_wifi_hardif(hard_iface)) {
>98 if (!batadv_is_cfg80211_hardif(hard_iface))
>99 /* unsupported WiFi driver version */
>   100 goto default_throughput;
>   101 
>   102 real_netdev = 
> batadv_get_real_netdev(hard_iface->net_dev);
>   103 if (!real_netdev)
>   104 goto default_throughput;
>   105 
>   106 sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
>   107 if (!sinfo)
>   108 goto default_throughput;
>   109 
>   110 ret = cfg80211_get_station(real_netdev, neigh->addr, 
> sinfo);
>   111 
>112/* just save these here instead of having complex free 
>logic
>below */
> > 113 throughput = sinfo.expected_throughput / 100;
>114filled = !!(sinfo.filled &
>BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
>   115 
>   116 kfree(sinfo);
>   117 
>   118 dev_put(real_netdev);
>   119 if (ret == -ENOENT) {
>   120 /* Node is not associated anymore! It would be
>   121  * possible to delete this neighbor. For now set
>   122  * the throughput metric to 0.
>   123  */
>   124 return 0;
>   125 }
>   126 if (ret || !filled)
>   127 goto default_throughput;
>   128 
>   129 return throughput;
>   130 }
>   131 
>132/* if not a wifi interface, check if this device provides data 
>via
>   133  * ethtool (e.g. an Ethernet adapter)
>   134  */
>   135 memset(_settings, 0, sizeof(link_settings));
>   13

Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER

[Kees Cook]

On Wed, May 9, 2018 at 1:55 PM, Luis R. Rodriguez  wrote:
> On Tue, May 08, 2018 at 03:42:33PM -0700, Kees Cook wrote:
>> On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez  wrote:
>> > + This used to be the default firmware loading facility, and udev 
>> > used
>> > + to listen for uvents to load firmware for the kernel. The 
>> > firmware
>> > + loading facility functionality in udev has been removed, as such 
>> > it
>> > + can no longer be relied upon as a fallback mechanism. Linux no 
>> > longer
>> > + relies on or uses a fallback mechanism in userspace. If you need 
>> > to
>> > + rely on one refer to the permissively licensed firmwared:
>>
>> Typo: firmware
>
> Thanks fixed all typos except this one, this one is meant to be firmwared as
> that is the name of the project, the url is below.
>
>>
>> > +
>> > + https://github.com/teg/firmwared

Oh! Yes, hah. :) Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[kbuild test robot]

Hi Toke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.17-rc4 next-20180509]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireless-drivers-Dynamically-allocate-struct-station_info/20180510-034416
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
master
config: i386-randconfig-x000-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   net/batman-adv/bat_v_elp.c: In function 'batadv_v_elp_get_throughput':
>> net/batman-adv/bat_v_elp.c:113:21: error: 'sinfo' is a pointer; did you mean 
>> to use '->'?
  throughput = sinfo.expected_throughput / 100;
^
->
   net/batman-adv/bat_v_elp.c:114:20: error: 'sinfo' is a pointer; did you mean 
to use '->'?
  filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   ^
   ->

vim +113 net/batman-adv/bat_v_elp.c

69  
70  /**
71   * batadv_v_elp_get_throughput() - get the throughput towards a 
neighbour
72   * @neigh: the neighbour for which the throughput has to be obtained
73   *
74   * Return: The throughput towards the given neighbour in multiples of 
100kpbs
75   * (a value of '1' equals to 0.1Mbps, '10' equals 1Mbps, etc).
76   */
77  static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node 
*neigh)
78  {
79  struct batadv_hard_iface *hard_iface = neigh->if_incoming;
80  struct ethtool_link_ksettings link_settings;
81  struct net_device *real_netdev;
82  struct station_info *sinfo;
83  u32 throughput;
84  bool filled;
85  int ret;
86  
87  /* if the user specified a customised value for this interface, 
then
88   * return it directly
89   */
90  throughput =  
atomic_read(_iface->bat_v.throughput_override);
91  if (throughput != 0)
92  return throughput;
93  
94  /* if this is a wireless device, then ask its throughput through
95   * cfg80211 API
96   */
97  if (batadv_is_wifi_hardif(hard_iface)) {
98  if (!batadv_is_cfg80211_hardif(hard_iface))
99  /* unsupported WiFi driver version */
   100  goto default_throughput;
   101  
   102  real_netdev = 
batadv_get_real_netdev(hard_iface->net_dev);
   103  if (!real_netdev)
   104  goto default_throughput;
   105  
   106  sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
   107  if (!sinfo)
   108  goto default_throughput;
   109  
   110  ret = cfg80211_get_station(real_netdev, neigh->addr, 
sinfo);
   111  
   112  /* just save these here instead of having complex free 
logic below */
 > 113  throughput = sinfo.expected_throughput / 100;
   114  filled = !!(sinfo.filled & 
BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   115  
   116  kfree(sinfo);
   117  
   118  dev_put(real_netdev);
   119  if (ret == -ENOENT) {
   120  /* Node is not associated anymore! It would be
   121   * possible to delete this neighbor. For now set
   122   * the throughput metric to 0.
   123   */
   124  return 0;
   125  }
   126  if (ret || !filled)
   127  goto default_throughput;
   128  
   129  return throughput;
   130  }
   131  
   132  /* if not a wifi interface, check if this device provides data 
via
   133   * ethtool (e.g. an Ethernet adapter)
   134   */
   135  memset(_settings, 0, sizeof(link_settings));
   136  rtnl_lock();
   137  ret = __ethtool_get_link_ksettings(hard_iface->net_dev, 
_settings);
   138  rtnl_unlock();
   139  if (ret == 0) {
   140  /* link characteristics might change over time */
   141  if (link_settings.base.duplex == DUPLEX_FULL)
   142  hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
   143  else
   144  hard_iface->bat_v.flags &= ~BATADV_FULL_DUPLEX;
   145  
   146  throughput 

Proposal

[Zeliha Omer Faruk]

--
Hello

Greetings to you please i have a business proposal for you contact me
for more detailes asap thanks.

Best Regards,
Miss.Zeliha ömer faruk
Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
Sisli - Istanbul, Turkey

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

[Mimi Zohar]

On Wed, 2018-05-09 at 21:22 +, Luis R. Rodriguez wrote:
> On Wed, May 09, 2018 at 03:57:18PM -0400, Mimi Zohar wrote:
> > On Wed, 2018-05-09 at 19:15 +, Luis R. Rodriguez wrote:
> > 
> > > > > > If both are enabled, do we require both signatures or is one enough.
> > > > > 
> > > > > Good question. Considering it as a stacked LSM (although not 
> > > > > implemented
> > > > > as one), I'd say its up to who enabled the Kconfig entries. If IMA and
> > > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are enabled then both. If 
> > > > > someone enabled
> > > > > IMA though, then surely I agree that enabling
> > > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is stupid and redundant, but its 
> > > > > up to the
> > > > > system integrator to decide.
> > > > 
> > > > Just because IMA-appraisal is enabled in the kernel doesn't mean that
> > > > firmware signatures will be verified.  That is a run time policy
> > > > decision.
> > > 
> > > Sure, I accept this if IMA does not do signature verification. However
> > > signature verification seems like a stackable LSM decision, no?
> > 
> > IMA-appraisal can be configured to enforce file signatures.  Refer to
> > discussion below as to how.
> > 
> > > > > If we however want to make it clear that such things as
> > > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are not required when IMA is 
> > > > > enabled we
> > > > > could just make the kconfig depend on !IMA or something?  Or perhaps 
> > > > > a new
> > > > > kconfig for IMA which if selected it means that drivers can opt to 
> > > > > open code
> > > > > *further* kernel signature verification, even though IMA already is 
> > > > > sufficient.
> > > > > Perhaps CONFIG_ENABLE_IMA_OVERLAPPING, and the driver depends on it?
> > > > 
> > > > The existing CONFIG_IMA_APPRAISE is not enough.  If there was a build
> > > > time IMA config that translated into an IMA policy requiring firmware
> > > > signature verification (eg. CONFIG_IMA_APPRAISE_FIRMWARE), this could
> > > > be sorted out at build time.
> > > 
> > > I see makes sense.
> > 
> > Ok, so instead of introducing READING_FIRMWARE_REGULATORY_DB, I'll
> > post patches introducing CONFIG_IMA_APPRAISE_FIRMWARE, as described
> > above.
> 
> OK, its still not clear to what it will do. If it does not touch the firmware
> loader code, and it just sets and configures IMA to do file signature checking
> on its own, then yes I think both mechanisms would be doing the similar work.
> 
> Wouldn't IMA do file signature checks then for all files? Or it would just
> enable this for whatever files userspace wishes to cover?

Enabling CONFIG_IMA_APPRAISE_FIRMWARE would enforce firmware
signatures on all directly loaded firmware and fail any method of
loading firmware that the signature couldn't be verified.

> One of the things with READING_FIRMWARE_REGULATORY_DB is to also use and trust
> the wireless-regdgb maintainer's key for this file, could IMA be configured to
> do that?

IMA has its own trusted keyring.  So either the maintainer's key would
need to be added to the IMA keyring, or IMA-appraisal would need to
use the regdb keyring.
  
> Because that would be one difference here. The whole point of adding
> CONFIG_CFG80211_REQUIRE_SIGNED_REGDB was to replace CRDA which is a userspace
> component which checks the signature of regulatory.db before reading it and
> passing data to the kernel from it.
> 
> Now, however silly it may be to have CONFIG_IMA_APPRAISE_FIRMWARE *and*
> CONFIG_CFG80211_REQUIRE_SIGNED_REGDB, is your intent in this new patch set
> you are mentioning, to still enable both to co-exist?

At build, either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
CONFIG_IMA_APPRAISE_FIRMWARE, where IMA is appraising all firwmare,
would be enabled, not both.

The builtin IMA-policies could be replaced with a custom policy,
requiring firmware signature verification.  In that case, the regdb
signature would be verified twice.

> 
> > > > > > Assigning a different id for regdb signed firmware allows LSMs and 
> > > > > > IMA
> > > > > > to handle regdb files differently.
> > > > > 
> > > > > That's not the main concern here, its the precedent we are setting 
> > > > > here for
> > > > > any new kernel interface which open codes firmware signing on its 
> > > > > own. What
> > > > > you are doing means other kernel users who open codes their own 
> > > > > firmware
> > > > > signing may need to add yet-another reading ID. That doesn't either 
> > > > > look
> > > > > well on code, and seems kind of silly from a coding perspective given
> > > > > the above, in which I clarify IMA still is doing its own appraisal on 
> > > > > it.
> > > > 
> > > > Suppose,
> > > > 
> > > > 1. Either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
> > > > "CONFIG_IMA_APPRAISE_FIRMWARE" would be configured at build.
> > > > 
> > > > 2. If CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is configured, not
> > > > "CONFIG_IMA_APPRAISE_FIRMWARE", a custom IMA-policy rule that
> > > > appraises the firmware signature could be 

Re: [PATCH] ssb: Fix regression caused by disabling PCI cores on non-MIPS architecture

[Rafał Miłecki]

On 9 May 2018 at 18:42, Larry Finger  wrote:
> Some MPIS-based SoCs use chips driven by b43 for wireless capability.
> When ssb is configured as a module, build errors happen on these
> platforms as described in the commit 882164a4a928 ("ssb: Prevent build
> of PCI host features in module"). Unfortunately that change leads to
> the removal of code needed for correct operation on platforms that use
> PCI cores on the chip bus. The fix allows ssb to be build as a module
> for all architectures other than MIPS. This approach is ad-hoc, but it
> is the same as done in commit a9e6d44ddecc ("ssb: Do not disable PCI
> host on non-Mips").
>
> This problem was reported and discussed in
> https://bugzilla.redhat.com/show_bug.cgi?id=1572349.

I'll try to look at that tomorrow

[PATCH v2 1/3] wireless-drivers: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.

Fixes: 52539ca89f36 ("cfg80211: Expose TXQ stats and parameters to userspace")
Signed-off-by: Toke Høiland-Jørgensen 
---
 drivers/net/wireless/ath/ath6kl/main.c |   14 ++
 drivers/net/wireless/ath/wil6210/debugfs.c |   22 ++-
 drivers/net/wireless/ath/wil6210/wmi.c |   19 -
 .../broadcom/brcm80211/brcmfmac/cfg80211.c |   18 
 drivers/net/wireless/marvell/mwifiex/uap_event.c   |   25 +++--
 drivers/net/wireless/quantenna/qtnfmac/event.c |   29 +---
 6 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c 
b/drivers/net/wireless/ath/ath6kl/main.c
index db95f85751e3..808fb30be9ad 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -426,7 +426,7 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, u16 
aid, u8 *mac_addr,
 {
u8 *ies = NULL, *wpa_ie = NULL, *pos;
size_t ies_len = 0;
-   struct station_info sinfo;
+   struct station_info *sinfo;
 
ath6kl_dbg(ATH6KL_DBG_TRC, "new station %pM aid=%d\n", mac_addr, aid);
 
@@ -482,16 +482,20 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, 
u16 aid, u8 *mac_addr,
   keymgmt, ucipher, auth, apsd_info);
 
/* send event to application */
-   memset(, 0, sizeof(sinfo));
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return;
 
/* TODO: sinfo.generation */
 
-   sinfo.assoc_req_ies = ies;
-   sinfo.assoc_req_ies_len = ies_len;
+   sinfo->assoc_req_ies = ies;
+   sinfo->assoc_req_ies_len = ies_len;
 
-   cfg80211_new_sta(vif->ndev, mac_addr, , GFP_KERNEL);
+   cfg80211_new_sta(vif->ndev, mac_addr, sinfo, GFP_KERNEL);
 
netif_wake_queue(vif->ndev);
+
+   kfree(sinfo);
 }
 
 void disconnect_timer_handler(struct timer_list *t)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c 
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 8c90b3111f0b..11e46e44381e 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1200,8 +1200,12 @@ static const struct file_operations fops_freq = {
 static int wil_link_debugfs_show(struct seq_file *s, void *data)
 {
struct wil6210_priv *wil = s->private;
-   struct station_info sinfo;
-   int i, rc;
+   struct station_info *sinfo;
+   int i, rc = 0;
+
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return -ENOMEM;
 
for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
struct wil_sta_info *p = >sta[i];
@@ -1229,19 +1233,21 @@ static int wil_link_debugfs_show(struct seq_file *s, 
void *data)
 
vif = (mid < wil->max_vifs) ? wil->vifs[mid] : NULL;
if (vif) {
-   rc = wil_cid_fill_sinfo(vif, i, );
+   rc = wil_cid_fill_sinfo(vif, i, sinfo);
if (rc)
-   return rc;
+   goto out;
 
-   seq_printf(s, "  Tx_mcs = %d\n", sinfo.txrate.mcs);
-   seq_printf(s, "  Rx_mcs = %d\n", sinfo.rxrate.mcs);
-   seq_printf(s, "  SQ = %d\n", sinfo.signal);
+   seq_printf(s, "  Tx_mcs = %d\n", sinfo->txrate.mcs);
+   seq_printf(s, "  Rx_mcs = %d\n", sinfo->rxrate.mcs);
+   seq_printf(s, "  SQ = %d\n", sinfo->signal);
} else {
seq_puts(s, "  INVALID MID\n");
}
}
 
-   return 0;
+out:
+   kfree(sinfo);
+   return rc;
 }
 
 static int wil_link_seq_open(struct inode *inode, struct file *file)
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c 
b/drivers/net/wireless/ath/wil6210/wmi.c
index a3dda9a97c1f..21124af06bdd 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -824,7 +824,7 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int 
id, void *d, int len)
struct wireless_dev *wdev = vif_to_wdev(vif);
struct wmi_connect_event *evt = d;
int ch; /* channel number */
-   struct station_info sinfo;
+   struct station_info *sinfo;
u8 *assoc_req_ie, *assoc_resp_ie;
size_t assoc_req_ielen, assoc_resp_ielen;
/* capinfo(u16) + listen_interval(u16) + IEs */
@@ -940,6 +940,11 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int 
id, void *d, int len)
vif->bss = NULL;
} else if ((wdev->iftype == NL80211_IFTYPE_AP) ||
   (wdev->iftype == 

[PATCH v2 2/3] staging: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.

This patch applies the fix to the rtl8723bs driver in staging while a
separate patch fixes the drivers in the main tree.

Fixes: 52539ca89f36 ("cfg80211: Expose TXQ stats and parameters to userspace")
Signed-off-by: Toke Høiland-Jørgensen 
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 46bc2e512557..1ffc8c9ada52 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -2431,17 +2431,23 @@ void rtw_cfg80211_indicate_sta_assoc(struct adapter 
*padapter, u8 *pmgmt_frame,
DBG_871X(FUNC_ADPT_FMT"\n", FUNC_ADPT_ARG(padapter));
 
{
-   struct station_info sinfo;
+   struct station_info *sinfo;
u8 ie_offset;
if (GetFrameSubType(pmgmt_frame) == WIFI_ASSOCREQ)
ie_offset = _ASOCREQ_IE_OFFSET_;
else /*  WIFI_REASSOCREQ */
ie_offset = _REASOCREQ_IE_OFFSET_;
 
-   sinfo.filled = 0;
-   sinfo.assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + ie_offset;
-   sinfo.assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - 
ie_offset;
-   cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), , 
GFP_ATOMIC);
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return;
+
+   sinfo->filled = 0;
+   sinfo->assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + 
ie_offset;
+   sinfo->assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - 
ie_offset;
+   cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), sinfo, 
GFP_ATOMIC);
+
+   kfree(sinfo);
}
 }
 

[PATCH v2 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.

This patch applies the fix to batman-adv and wext-compat, while a
separate patch fixes up the drivers.

Fixes: 52539ca89f36 ("cfg80211: Expose TXQ stats and parameters to userspace")
Signed-off-by: Toke Høiland-Jørgensen 
---
 net/batman-adv/bat_v_elp.c |   21 +++--
 net/wireless/wext-compat.c |   29 +
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 28687493599f..442b3ebb54f0 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -79,8 +79,9 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
struct batadv_hard_iface *hard_iface = neigh->if_incoming;
struct ethtool_link_ksettings link_settings;
struct net_device *real_netdev;
-   struct station_info sinfo;
+   struct station_info *sinfo;
u32 throughput;
+   bool filled;
int ret;
 
/* if the user specified a customised value for this interface, then
@@ -102,7 +103,17 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
if (!real_netdev)
goto default_throughput;
 
-   ret = cfg80211_get_station(real_netdev, neigh->addr, );
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   goto default_throughput;
+
+   ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
+
+   /* just save these here instead of having complex free logic 
below */
+   throughput = sinfo.expected_throughput / 100;
+   filled = (sinfo.filled & 
BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
+
+   kfree(sinfo);
 
dev_put(real_netdev);
if (ret == -ENOENT) {
@@ -112,12 +123,10 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
 */
return 0;
}
-   if (ret)
-   goto default_throughput;
-   if (!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT)))
+   if (ret || !filled)
goto default_throughput;
 
-   return sinfo.expected_throughput / 100;
+   return throughput;
}
 
/* if not a wifi interface, check if this device provides data via
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 9e002df0f8d8..2038e3fb25fa 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -1300,7 +1300,7 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
/* we are under RTNL - globally locked - so can use static structs */
static struct iw_statistics wstats;
-   static struct station_info sinfo;
+   static struct station_info *sinfo;
u8 bssid[ETH_ALEN];
 
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION)
@@ -1318,17 +1318,21 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
memcpy(bssid, wdev->current_bss->pub.bssid, ETH_ALEN);
wdev_unlock(wdev);
 
-   memset(, 0, sizeof(sinfo));
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return NULL;
 
-   if (rdev_get_station(rdev, dev, bssid, ))
+   if (rdev_get_station(rdev, dev, bssid, sinfo)) {
+   kfree(sinfo);
return NULL;
+   }
 
memset(, 0, sizeof(wstats));
 
switch (rdev->wiphy.signal_type) {
case CFG80211_SIGNAL_TYPE_MBM:
-   if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
-   int sig = sinfo.signal;
+   if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+   int sig = sinfo->signal;
wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
wstats.qual.updated |= IW_QUAL_DBM;
@@ -1341,11 +1345,11 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
break;
}
case CFG80211_SIGNAL_TYPE_UNSPEC:
-   if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+   if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
-   wstats.qual.level = sinfo.signal;
-   wstats.qual.qual = 

Re: [wl-testing/master] build regression in brcmfmac

[Johannes Berg]

On Wed, 2018-05-09 at 23:36 +0200, Toke Høiland-Jørgensen wrote:
> Johannes Berg  writes:
> 
> > On Wed, 2018-05-09 at 23:27 +0200, Toke Høiland-Jørgensen wrote:
> > 
> > > That would be these:
> > > 
> > > https://patchwork.kernel.org/patch/10389333/
> > > https://patchwork.kernel.org/patch/10389331/
> > > https://patchwork.kernel.org/patch/10389335/
> > > 
> > 
> > D'oh, how did I miss that?!
> > 
> > Your fixes line is slightly wrong, it should be
> > 
> > Fixes: 12-digitid ("subject")
> 
> You mean it's missing the parenthesis?

Right, and the quotes.

johannes

Re: [wl-testing/master] build regression in brcmfmac

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> On Wed, 2018-05-09 at 23:27 +0200, Toke Høiland-Jørgensen wrote:
>
>> That would be these:
>> 
>> https://patchwork.kernel.org/patch/10389333/
>> https://patchwork.kernel.org/patch/10389331/
>> https://patchwork.kernel.org/patch/10389335/
>> 
>
> D'oh, how did I miss that?!
>
> Your fixes line is slightly wrong, it should be
>
> Fixes: 12-digitid ("subject")

You mean it's missing the parenthesis?

-Toke

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

>> WARN_ON is used where the function has no way to signal errors to the
>> caller.
>
> There isn't really much point in that - failing allocations are already
> *very* noisy. Please respin with that removed (and then I guess you can
> fix the Fixes: too)

OK, will do.

> You didn't actually do that in this patch though :-)

Well, I may have copy-pasted that from the commit message of the
previous patch ;)

> It's bool so you don't need the !!(...) :-)

But what if I want it to be extra super duper bool? ;)

-Toke

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Johannes Berg]

> WARN_ON is used where the function has no way to signal errors to the
> caller.

There isn't really much point in that - failing allocations are already
*very* noisy. Please respin with that removed (and then I guess you can
fix the Fixes: too)

You didn't actually do that in this patch though :-)

johannes

> + bool filled;
>   int ret;
>  
>   /* if the user specified a customised value for this interface, then
> @@ -102,7 +103,17 @@ static u32 batadv_v_elp_get_throughput(struct 
> batadv_hardif_neigh_node *neigh)
>   if (!real_netdev)
>   goto default_throughput;
>  
> - ret = cfg80211_get_station(real_netdev, neigh->addr, );
> + sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
> + if (!sinfo)
> + goto default_throughput;
> +
> + ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
> +
> + /* just save these here instead of having complex free logic 
> below */
> + throughput = sinfo.expected_throughput / 100;
> + filled = !!(sinfo.filled & 
> BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));

It's bool so you don't need the !!(...) :-)

johannes

Re: [wl-testing/master] build regression in brcmfmac

[Johannes Berg]

On Wed, 2018-05-09 at 23:27 +0200, Toke Høiland-Jørgensen wrote:

> That would be these:
> 
> https://patchwork.kernel.org/patch/10389333/
> https://patchwork.kernel.org/patch/10389331/
> https://patchwork.kernel.org/patch/10389335/
> 

D'oh, how did I miss that?!

Your fixes line is slightly wrong, it should be

Fixes: 12-digitid ("subject")

but I'm happy to fix that when I apply it.

johannes

Re: pull-request: mac80211-next 2018-05-09

[Johannes Berg]

Hi,

Sorry, scratch that.

I forgot that this commit:

> Toke Høiland-Jørgensen (3):

>   cfg80211: Expose TXQ stats and parameters to userspace

caused a bunch of "too much stack" warnings - I should put in at least
the non-driver fix for that first, and then coordinate with Kalle to
send the driver fixes in too.

johannes

Re: [wl-testing/master] build regression in brcmfmac

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> On Wed, 2018-05-09 at 23:12 +0200, Arend van Spriel wrote:
>> Since yesterday I get the following build error on wl-testing/master:
>> 
>>CC [M]  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.o
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 
>> ‘brcmf_notify_connect_status_ap’:
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5530:1: 
>> error: the frame size of 1592 bytes is larger than 1024 bytes 
>> [-Werror=frame-larger-than=]
>>   }
>>   ^
>> 
>> In the function brcmf_notify_connect_status_ap() I do have following on 
>> the stack:
>> 
>> static s32
>> brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
>> struct net_device *ndev,
>> const struct brcmf_event_msg *e, void *data)
>> {
>>  static int generation;
>>  u32 event = e->event_code;
>>  u32 reason = e->reason;
>>  struct station_info sinfo;
>> 
>> I guess the struct station_info grew bigger by these commits:
>> 
>> 52539ca cfg80211: Expose TXQ stats and parameters to userspace   +816
>> 81d5439 cfg80211: average ack rssi support for data frames   +1
>> 
>> There are a number of other (cfg80211-based) drivers doing the same 
>> thing, ie. mwifiex, ath6kl, qtnfmac.
>
> Yeah. Toke said this morning he was working on fixes.

That would be these:

https://patchwork.kernel.org/patch/10389333/
https://patchwork.kernel.org/patch/10389331/
https://patchwork.kernel.org/patch/10389335/

-Toke

Re: [wl-testing/master] build regression in brcmfmac

[Johannes Berg]

On Wed, 2018-05-09 at 23:12 +0200, Arend van Spriel wrote:
> Since yesterday I get the following build error on wl-testing/master:
> 
>CC [M]  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.o
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 
> ‘brcmf_notify_connect_status_ap’:
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5530:1: 
> error: the frame size of 1592 bytes is larger than 1024 bytes 
> [-Werror=frame-larger-than=]
>   }
>   ^
> 
> In the function brcmf_notify_connect_status_ap() I do have following on 
> the stack:
> 
> static s32
> brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
> struct net_device *ndev,
> const struct brcmf_event_msg *e, void *data)
> {
>  static int generation;
>  u32 event = e->event_code;
>  u32 reason = e->reason;
>  struct station_info sinfo;
> 
> I guess the struct station_info grew bigger by these commits:
> 
> 52539ca cfg80211: Expose TXQ stats and parameters to userspace+816
> 81d5439 cfg80211: average ack rssi support for data frames+1
> 
> There are a number of other (cfg80211-based) drivers doing the same 
> thing, ie. mwifiex, ath6kl, qtnfmac.

Yeah. Toke said this morning he was working on fixes.

johannes

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

[Luis R. Rodriguez]

On Wed, May 09, 2018 at 03:57:18PM -0400, Mimi Zohar wrote:
> On Wed, 2018-05-09 at 19:15 +, Luis R. Rodriguez wrote:
> 
> > > > > If both are enabled, do we require both signatures or is one enough.
> > > > 
> > > > Good question. Considering it as a stacked LSM (although not implemented
> > > > as one), I'd say its up to who enabled the Kconfig entries. If IMA and
> > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are enabled then both. If someone 
> > > > enabled
> > > > IMA though, then surely I agree that enabling
> > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is stupid and redundant, but its 
> > > > up to the
> > > > system integrator to decide.
> > > 
> > > Just because IMA-appraisal is enabled in the kernel doesn't mean that
> > > firmware signatures will be verified.  That is a run time policy
> > > decision.
> > 
> > Sure, I accept this if IMA does not do signature verification. However
> > signature verification seems like a stackable LSM decision, no?
> 
> IMA-appraisal can be configured to enforce file signatures.  Refer to
> discussion below as to how.
> 
> > > > If we however want to make it clear that such things as
> > > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are not required when IMA is 
> > > > enabled we
> > > > could just make the kconfig depend on !IMA or something?  Or perhaps a 
> > > > new
> > > > kconfig for IMA which if selected it means that drivers can opt to open 
> > > > code
> > > > *further* kernel signature verification, even though IMA already is 
> > > > sufficient.
> > > > Perhaps CONFIG_ENABLE_IMA_OVERLAPPING, and the driver depends on it?
> > > 
> > > The existing CONFIG_IMA_APPRAISE is not enough.  If there was a build
> > > time IMA config that translated into an IMA policy requiring firmware
> > > signature verification (eg. CONFIG_IMA_APPRAISE_FIRMWARE), this could
> > > be sorted out at build time.
> > 
> > I see makes sense.
> 
> Ok, so instead of introducing READING_FIRMWARE_REGULATORY_DB, I'll
> post patches introducing CONFIG_IMA_APPRAISE_FIRMWARE, as described
> above.

OK, its still not clear to what it will do. If it does not touch the firmware
loader code, and it just sets and configures IMA to do file signature checking
on its own, then yes I think both mechanisms would be doing the similar work.

Wouldn't IMA do file signature checks then for all files? Or it would just
enable this for whatever files userspace wishes to cover?

One of the things with READING_FIRMWARE_REGULATORY_DB is to also use and trust
the wireless-regdgb maintainer's key for this file, could IMA be configured to
do that?

Because that would be one difference here. The whole point of adding
CONFIG_CFG80211_REQUIRE_SIGNED_REGDB was to replace CRDA which is a userspace
component which checks the signature of regulatory.db before reading it and
passing data to the kernel from it.

Now, however silly it may be to have CONFIG_IMA_APPRAISE_FIRMWARE *and*
CONFIG_CFG80211_REQUIRE_SIGNED_REGDB, is your intent in this new patch set
you are mentioning, to still enable both to co-exist?

> > > > > Assigning a different id for regdb signed firmware allows LSMs and IMA
> > > > > to handle regdb files differently.
> > > > 
> > > > That's not the main concern here, its the precedent we are setting here 
> > > > for
> > > > any new kernel interface which open codes firmware signing on its own. 
> > > > What
> > > > you are doing means other kernel users who open codes their own firmware
> > > > signing may need to add yet-another reading ID. That doesn't either look
> > > > well on code, and seems kind of silly from a coding perspective given
> > > > the above, in which I clarify IMA still is doing its own appraisal on 
> > > > it.
> > > 
> > > Suppose,
> > > 
> > > 1. Either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
> > > "CONFIG_IMA_APPRAISE_FIRMWARE" would be configured at build.
> > > 
> > > 2. If CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is configured, not
> > > "CONFIG_IMA_APPRAISE_FIRMWARE", a custom IMA-policy rule that
> > > appraises the firmware signature could be defined.  In this case, both
> > > signature verification methods would be enforced.
> > > 
> > > then READING_FIRMWARE_REGULATORY_DB would not be needed.
> > 
> > True, however I'm suggesting that CONFIG_CFG80211_REQUIRE_SIGNED_REGDB
> > could just be a mini subsystem stackable LSM.
> 
> 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.

> Making regdb an LSM would have the same issues as currently - deciding
> if regdb, IMA-appraisal, or both verify the regdb's signature.

Its unclear to me why they can't co-exist yet and not have to touch
the firmware_loader code at all.

  Luis

[wl-testing/master] build regression in brcmfmac

[Arend van Spriel]

Since yesterday I get the following build error on wl-testing/master:

  CC [M]  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.o
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 
‘brcmf_notify_connect_status_ap’:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5530:1: 
error: the frame size of 1592 bytes is larger than 1024 bytes 
[-Werror=frame-larger-than=]

 }
 ^

In the function brcmf_notify_connect_status_ap() I do have following on 
the stack:


static s32
brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
   struct net_device *ndev,
   const struct brcmf_event_msg *e, void *data)
{
static int generation;
u32 event = e->event_code;
u32 reason = e->reason;
struct station_info sinfo;

I guess the struct station_info grew bigger by these commits:

52539ca cfg80211: Expose TXQ stats and parameters to userspace  +816
81d5439 cfg80211: average ack rssi support for data frames  +1

There are a number of other (cfg80211-based) drivers doing the same 
thing, ie. mwifiex, ath6kl, qtnfmac.


Regards,
Arend

Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER

[Luis R. Rodriguez]

On Tue, May 08, 2018 at 03:42:33PM -0700, Kees Cook wrote:
> On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez  wrote:
> > + This used to be the default firmware loading facility, and udev 
> > used
> > + to listen for uvents to load firmware for the kernel. The firmware
> > + loading facility functionality in udev has been removed, as such 
> > it
> > + can no longer be relied upon as a fallback mechanism. Linux no 
> > longer
> > + relies on or uses a fallback mechanism in userspace. If you need 
> > to
> > + rely on one refer to the permissively licensed firmwared:
> 
> Typo: firmware

Thanks fixed all typos except this one, this one is meant to be firmwared as
that is the name of the project, the url is below.

> 
> > +
> > + https://github.com/teg/firmwared

  Luis

Re: RTL8723BE performance regression

[João Paulo Rechi Vita]

On Tue, May 8, 2018 at 1:37 AM, Pkshih  wrote:
> On Mon, 2018-05-07 at 14:49 -0700, João Paulo Rechi Vita wrote:
>> On Tue, May 1, 2018 at 10:58 PM, Pkshih  wrote:
>> > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote:
>> >>
>> >> > -Original Message-
>> >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com]
>> >> > Sent: Wednesday, May 02, 2018 6:41 AM
>> >> > To: Larry Finger
>> >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; 
>> >> > Chaoming_Li; Kalle Valo;
>> >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo 
>> >> > Rechi Vita; linux@endless
>> m.c
>> >> om
>> >> > Subject: Re: RTL8723BE performance regression
>> >> >
>> >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger 
>> >> >  wrote:
>> >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote:
>> >> > >>
>> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger 
>> >> > >> 
>> >> > >> wrote:
>> >> > >>
>> >> > >> (...)
>> >> > >>
>> >> > >>> As the antenna selection code changes affected your first 
>> >> > >>> bisection, do
>> >> > >>> you
>> >> > >>> have one of those HP laptops with only one antenna and the incorrect
>> >> > >>> coding
>> >> > >>> in the FUSE?
>> >> > >>
>> >> > >>
>> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this
>> >> > >> was needed to achieve a good performance in the past, before this
>> >> > >> regression. I've also opened the laptop chassis and confirmed the
>> >> > >> antenna cable is plugged to the connector labeled with "1" on the
>> >> > >> card.
>> >> > >>
>> >> > >>> If so, please make sure that you still have the same signal
>> >> > >>> strength for good and bad cases. I have tried to keep the driver 
>> >> > >>> and the
>> >> > >>> btcoex code in sync, but there may be some combinations of antenna
>> >> > >>> configuration and FUSE contents that cause the code to fail.
>> >> > >>>
>> >> > >>
>> >> > >> What is the recommended way to monitor the signal strength?
>> >> > >
>> >> > >
>> >> > > The btcoex code is developed for multiple platforms by a different 
>> >> > > group
>> >> > > than the Linux driver. I think they made a change that caused ant_sel 
>> >> > > to
>> >> > > switch from 1 to 2. At least numerous comments at
>> >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that 
>> >> > > change.
>> >> > >
>> >> > > Mhy recommended method is to verify the wifi device name with "iw 
>> >> > > dev". Then
>> >> > > using that device
>> >> > >
>> >> > > sudo iw dev  scan | egrep "SSID|signal"
>> >> > >
>> >> >
>> >> > I have confirmed that the performance regression is indeed tied to
>> >> > signal strength: on the good cases signal was between -16 and -8 dBm,
>> >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've
>> >> > also switched to testing bandwidth in controlled LAN environment using
>> >> > iperf3, as suggested by Steve deRosier, with the DUT being the only
>> >> > machine connected to the 2.4 GHz radio and the machine running the
>> >> > iperf3 server connected via ethernet.
>> >> >
>> >>
>> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: 
>> >> cleanup
>> >> 8723be ant_sel definition"). You can use the above commit and do the same
>> >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your 
>> >> results.
>> >> Since performance is tied to signal strength, you can only share signal 
>> >> strength.
>> >>
>> >
>> > Please pay attention to cold reboot once ant_sel is changed.
>> >
>>
>> I've tested the commit mentioned above and it fixes the problem on top
>> of v4.16 (in addition to the latest wireless-drivers-next also been
>> fixed as it already contains such commit). On v4.15, we also need the
>> following commits before "af8a41cccf8f rtlwifi: cleanup 8723be ant_sel
>> definition" to have a good performance again:
>>
>>   874e837d67d0 rtlwifi: fill FW version and subversion
>>   a44709bba70f rtlwifi: btcoex: Add power_on_setting routine
>>   40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex
>
> v4.15 isn't longterm version and had been EOL.
>

Right, but this is a performace regression in comparison to v4.11, so
if "af8a41cccf8f rtlwifi: cleanup 8723be ant_sel definition" is marked
for stable, shouldn't these other patches be brought as well? All
releases since v4.11 are probably affected, but honestly I don't have
a strong understanding of how the stable trees operate in situations
like this.

>>
>> Surprisingly, it seems forcing ant_sel=1 is not needed anymore on
>> these machines, as the shown by the numbers bellow (ant_sel=0 means
>> that actually no parameter was passed to the module). I have powered
>> off the machine and done a cold boot for every test. It seems
>> something have changed in the antenna auto-selection code since v4.11,
>> the latest point where I could confirm we definitely need to force
>> ant_sel=1. 

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

[Mimi Zohar]

On Wed, 2018-05-09 at 19:15 +, Luis R. Rodriguez wrote:

> > > > If both are enabled, do we require both signatures or is one enough.
> > > 
> > > Good question. Considering it as a stacked LSM (although not implemented
> > > as one), I'd say its up to who enabled the Kconfig entries. If IMA and
> > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are enabled then both. If someone 
> > > enabled
> > > IMA though, then surely I agree that enabling
> > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is stupid and redundant, but its up 
> > > to the
> > > system integrator to decide.
> > 
> > Just because IMA-appraisal is enabled in the kernel doesn't mean that
> > firmware signatures will be verified.  That is a run time policy
> > decision.
> 
> Sure, I accept this if IMA does not do signature verification. However
> signature verification seems like a stackable LSM decision, no?

IMA-appraisal can be configured to enforce file signatures.  Refer to
discussion below as to how.

> > > If we however want to make it clear that such things as
> > > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are not required when IMA is enabled 
> > > we
> > > could just make the kconfig depend on !IMA or something?  Or perhaps a new
> > > kconfig for IMA which if selected it means that drivers can opt to open 
> > > code
> > > *further* kernel signature verification, even though IMA already is 
> > > sufficient.
> > > Perhaps CONFIG_ENABLE_IMA_OVERLAPPING, and the driver depends on it?
> > 
> > The existing CONFIG_IMA_APPRAISE is not enough.  If there was a build
> > time IMA config that translated into an IMA policy requiring firmware
> > signature verification (eg. CONFIG_IMA_APPRAISE_FIRMWARE), this could
> > be sorted out at build time.
> 
> I see makes sense.

Ok, so instead of introducing READING_FIRMWARE_REGULATORY_DB, I'll
post patches introducing CONFIG_IMA_APPRAISE_FIRMWARE, as described
above.

> 
> > > > Assigning a different id for regdb signed firmware allows LSMs and IMA
> > > > to handle regdb files differently.
> > > 
> > > That's not the main concern here, its the precedent we are setting here 
> > > for
> > > any new kernel interface which open codes firmware signing on its own. 
> > > What
> > > you are doing means other kernel users who open codes their own firmware
> > > signing may need to add yet-another reading ID. That doesn't either look
> > > well on code, and seems kind of silly from a coding perspective given
> > > the above, in which I clarify IMA still is doing its own appraisal on it.
> > 
> > Suppose,
> > 
> > 1. Either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
> > "CONFIG_IMA_APPRAISE_FIRMWARE" would be configured at build.
> > 
> > 2. If CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is configured, not
> > "CONFIG_IMA_APPRAISE_FIRMWARE", a custom IMA-policy rule that
> > appraises the firmware signature could be defined.  In this case, both
> > signature verification methods would be enforced.
> > 
> > then READING_FIRMWARE_REGULATORY_DB would not be needed.
> 
> True, however I'm suggesting that CONFIG_CFG80211_REQUIRE_SIGNED_REGDB
> could just be a mini subsystem stackable LSM.

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.

Making regdb an LSM would have the same issues as currently - deciding
if regdb, IMA-appraisal, or both verify the regdb's signature.

Mimi

Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference

[Luis R. Rodriguez]

On Wed, May 09, 2018 at 12:12:09PM -0300, Mauro Carvalho Chehab wrote:
> Em Tue,  8 May 2018 11:12:46 -0700
> "Luis R. Rodriguez"  escreveu:
> 
> > It refers to a pending patch, but this was merged eons ago.
> 
> Didn't know that such patch was already merged. Great!
> 
> Regards,
> Mauro
> 
> > 
> > Signed-off-by: Luis R. Rodriguez 
> > ---
> >  Documentation/dell_rbu.txt | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> > index 0fdb6aa2704c..077fdc29a0d0 100644
> > --- a/Documentation/dell_rbu.txt
> > +++ b/Documentation/dell_rbu.txt
> > @@ -121,9 +121,6 @@ read back the image downloaded.
> >  
> >  .. note::
> >  
> > -   This driver requires a patch for firmware_class.c which has the modified
> > -   request_firmware_nowait function.
> > -
> 
> > Also after updating the BIOS image a user mode application needs to 
> > execute
> > code which sends the BIOS update request to the BIOS. So on the next 
> > reboot
> > the BIOS knows about the new image downloaded and it updates itself.
> 
> You should likely remove the "Also" here.

Good catch. Will adjust.

> 
> With that,
> 
> Reviewed-by: Mauro Carvalho Chehab 

Great, thanks.

  Luis

pull-request: mac80211-next 2018-05-09

[Johannes Berg]

Hi Dave,

Nothing major here either. I'm still waiting for the HE/802.11ax
stuff, but even when we do get that it'll just add rates and not
be very exciting at this point :-)

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 415787d7799f4fccbe8d49cb0b8e5811be6b0389:

  ipv6: frags: fix a lockdep false positive (2018-04-18 23:19:39 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
tags/mac80211-next-for-davem-2018-05-09

for you to fetch changes up to 57c6cb81717f957fb741f2e4c79bd0e2f4f55910:

  mac80211: ethtool: avoid 32 bit multiplication overflow (2018-05-08 15:02:03 
+0200)


While we're still waiting for HE/802.11ax code, we just
have a few things:
 * some new statistics (ACK RSSI, TXQ)
 * TXQ configuration
 * infrastructure to handle CSA in firmware
 * and some various cleanups/improvements


Amar Singhal (1):
  cfg80211: Call reg_notifier for self managed hints conditionally

Balaji Pothunoori (2):
  cfg80211: average ack rssi support for data frames
  mac80211: average ack rssi support for data frames

Bjoern Johansson (1):
  mac80211_hwsim: indicate support for powersave.

Colin Ian King (1):
  mac80211: ethtool: avoid 32 bit multiplication overflow

Gregory Greenman (1):
  mac80211: add api to set CSA counter in mac80211

Haim Dreyfuss (1):
  nl80211: Add wmm rule attribute to NL80211_CMD_GET_WIPHY dump command

Johannes Berg (4):
  mac80211: rename rtap_vendor_space to rtap_space
  mac80211: clean up rate info bandwidth setting
  mac80211: ethtool: memset the whole sinfo struct to 0
  mac80211: remove pointless flags=0 assignment

Toke Høiland-Jørgensen (3):
  regulatory: Rename confusing 'country IE' in log output
  cfg80211: Expose TXQ stats and parameters to userspace
  mac80211: Support the new cfg80211 TXQ stats API

 drivers/net/wireless/mac80211_hwsim.c |   1 +
 include/net/cfg80211.h|  53 +++
 include/net/mac80211.h|  13 ++
 include/uapi/linux/nl80211.h  |  93 
 net/mac80211/cfg.c|  99 +
 net/mac80211/ethtool.c|  37 +++--
 net/mac80211/ieee80211_i.h|   3 +
 net/mac80211/main.c   |   3 +
 net/mac80211/rx.c |  40 +++---
 net/mac80211/sta_info.c   |  24 +++-
 net/mac80211/sta_info.h   |   2 +
 net/mac80211/status.c |   2 +
 net/mac80211/tx.c |  45 ++
 net/mac80211/util.c   |   6 +-
 net/wireless/nl80211.c| 262 ++
 net/wireless/rdev-ops.h   |  12 ++
 net/wireless/reg.c|  37 -
 net/wireless/trace.h  |  14 ++
 net/wireless/wext-compat.c|  23 +--
 19 files changed, 683 insertions(+), 86 deletions(-)

pull-request: mac80211 2018-05-09

[Johannes Berg]

Hi Dave,

We just have a few fixes this time around.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 64e86fec54069266ba32be551d7b7f75e88ab60c:

  net: qualcomm: rmnet: Fix warning seen with fill_info (2018-04-18 21:23:06 
-0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2018-05-09

for you to fetch changes up to 914eac248d876f9c00cd1792ffec3d182c863f13:

  mac80211: use timeout from the AddBA response instead of the request 
(2018-05-07 20:35:15 +0200)


We only have a few fixes this time:
 * WMM element validation
 * SAE timeout
 * add-BA timeout
 * docbook parsing
 * a few memory leaks in error paths


Ilan Peer (2):
  mac80211: Fix condition validating WMM IE
  mac80211: Adjust SAE authentication timeout

Johan Hovold (1):
  rfkill: gpio: fix memory leak in probe error path

Johannes Berg (1):
  cfg80211: limit wiphy names to 128 bytes

Randy Dunlap (1):
  mac80211: fix kernel-doc "bad line" warning

Sara Sharon (1):
  mac80211: use timeout from the AddBA response instead of the request

Srinivas Dasari (1):
  nl80211: Free connkeys on external authentication failure

YueHaibing (1):
  mac80211_hwsim: fix a possible memory leak in hwsim_new_radio_nl()

weiyongjun (A) (1):
  cfg80211: fix possible memory leak in regdb_query_country()

 drivers/net/wireless/mac80211_hwsim.c |  1 +
 include/net/mac80211.h|  2 +-
 include/uapi/linux/nl80211.h  |  2 ++
 net/mac80211/agg-tx.c |  4 
 net/mac80211/mlme.c   | 27 +++
 net/mac80211/tx.c |  3 ++-
 net/rfkill/rfkill-gpio.c  |  7 ++-
 net/wireless/core.c   |  3 +++
 net/wireless/nl80211.c|  1 +
 net/wireless/reg.c|  1 +
 10 files changed, 40 insertions(+), 11 deletions(-)

Re: [PATCH 15/30] staging: wilc1000: use kmemdup instead of kmalloc in add_network_to_shadow()

[Ajay Singh]

On Wed, 9 May 2018 16:42:59 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Use kmemdup instead of kmalloc & memcpy in add_network_to_shadow().
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
> > 0ae2065..ca221f1 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -331,8
> > +331,8 @@ static void add_network_to_shadow(struct network_info
> > *nw_info, shadow_nw_info->tsf_hi = nw_info->tsf_hi; if (ap_found !=
> > -1) kfree(shadow_nw_info->ies);
> > -   shadow_nw_info->ies = kmalloc(nw_info->ies_len,
> > GFP_KERNEL);
> > -   memcpy(shadow_nw_info->ies, nw_info->ies,
> > nw_info->ies_len);
> > +   shadow_nw_info->ies = kmemdup(nw_info->ies,
> > nw_info->ies_len,
> > + GFP_KERNEL);  
> 
> Maybe, in case of NULL, you will want to set ies_len = 0 ?


I couldn't find code where 'ies_len' is check to validity of data.
Mostly we use NULL check for "ies" pointer for data
validity.So in my opinion setting it to zero would be
irrelevant.


> 
> > shadow_nw_info->time_scan = jiffies;
> > shadow_nw_info->time_scan_cached = jiffies;
> > shadow_nw_info->found = 1;
> >   


Regards,
Ajay

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

[Luis R. Rodriguez]

On Wed, May 09, 2018 at 07:30:28AM -0400, Mimi Zohar wrote:
> On Tue, 2018-05-08 at 17:34 +, Luis R. Rodriguez wrote:
> > On Thu, May 03, 2018 at 08:24:26PM -0400, Mimi Zohar wrote:
> > > On Fri, 2018-05-04 at 00:07 +, Luis R. Rodriguez wrote:
> > > > On Tue, May 01, 2018 at 09:48:20AM -0400, Mimi Zohar wrote:
> > > > > Allow LSMs and IMA to differentiate between signed regulatory.db and
> > > > > other firmware.
> > > > > 
> > > > > Signed-off-by: Mimi Zohar 
> > > > > Cc: Luis R. Rodriguez 
> > > > > Cc: David Howells 
> > > > > Cc: Kees Cook 
> > > > > Cc: Seth Forshee 
> > > > > Cc: Johannes Berg 
> > > > > ---
> > > > >  drivers/base/firmware_loader/main.c | 5 +
> > > > >  include/linux/fs.h  | 1 +
> > > > >  2 files changed, 6 insertions(+)
> > > > > 
> > > > > 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
> > > > 
> > > > Whoa, no way.
> > > 
> > > There are two methods for the kernel to verify firmware signatures.
> > 
> > Yes, but although CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is its own kernel
> > mechanism to verify firmware it uses the request_firmware*() API for
> > regulatory.db and regulatory.db.p7s, and IMA already can appraise these two
> > files since the firmware API is used.
> 
> IMA-appraisal can verify a signature stored as an xattr, but not a
> detached signature.  That support could be added, but isn't there
> today.  Today, a regulatory.db signature would have to be stored as an
> xattr. 

Right, my point was that if someone has IMA installed:

a) they would add those xattr to files in /lib/firmware/ already
b) upon request_firmware*() calls a security hook would trigger
   which would enable IMA to appraise those files. So not only
   would the kernel in turn do double checks on regulatory.db,
   but also a check on regulatory.db.p7s as well.

The difference I suppose is IMA would use a hash function instead of signature
check, correct?

> > As such I see no reason to add a new ID for them at all.
> > K
> > Its not providing an *alternative*, its providing an *extra* kernel measure.
> > If anything CONFIG_CFG80211_REQUIRE_SIGNED_REGDB perhaps should be its own
> > stacked LSM. I'd be open to see patches which set that out. May be a
> > cleaner interface.
> > 
> > > If both are enabled, do we require both signatures or is one enough.
> > 
> > Good question. Considering it as a stacked LSM (although not implemented
> > as one), I'd say its up to who enabled the Kconfig entries. If IMA and
> > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are enabled then both. If someone 
> > enabled
> > IMA though, then surely I agree that enabling
> > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is stupid and redundant, but its up to 
> > the
> > system integrator to decide.
> 
> Just because IMA-appraisal is enabled in the kernel doesn't mean that
> firmware signatures will be verified.  That is a run time policy
> decision.

Sure, I accept this if IMA does not do signature verification. However
signature verification seems like a stackable LSM decision, no?

> > If we however want to make it clear that such things as
> > CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are not required when IMA is enabled we
> > could just make the kconfig depend on !IMA or something?  Or perhaps a new
> > kconfig for IMA which if selected it means that drivers can opt to open code
> > *further* kernel signature verification, even though IMA already is 
> > sufficient.
> > Perhaps CONFIG_ENABLE_IMA_OVERLAPPING, and the driver depends on it?
> 
> The existing CONFIG_IMA_APPRAISE is not enough.  If there was a build
> time IMA config that translated into an IMA policy requiring firmware
> signature verification (eg. CONFIG_IMA_APPRAISE_FIRMWARE), this could
> be sorted out at build time.

I see makes sense.

> > > Assigning a different id for regdb signed firmware allows LSMs and IMA
> > > to handle regdb files differently.
> > 
> > That's not the main concern here, its the precedent we are setting here for
> > any new kernel interface which open codes firmware signing on its own. What
> > you are doing means other kernel users who open 

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

[Ajay Singh]

On Wed, 9 May 2018 16:44:38 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix line over 80 characters issue reported by checkpatch.pl in
> > host_int_parse_assoc_resp_info().
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 37
> > ++- 1 file changed, 21 insertions(+),
> > 16 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 55a61d5..a341ff1
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1293,6 +1293,23 @@ static inline void
> > host_int_free_user_conn_req(struct host_if_drv *hif_drv)
> > hif_drv->usr_conn_req.ies = NULL; }
> >  
> > +static void host_int_copy_conn_info(struct connect_resp_info
> > *conn_resp_info,
> > +   struct connect_info *conn_info)
> > +{
> > +   conn_info->status = conn_resp_info->status;
> > +
> > +   if (conn_info->status == SUCCESSFUL_STATUSCODE &&
> > conn_resp_info->ies) {
> > +   conn_info->resp_ies = kmemdup(conn_resp_info->ies,
> > +
> > conn_resp_info->ies_len,
> > + GFP_KERNEL);
> > +   if (conn_info->resp_ies)
> > +   conn_info->resp_ies_len =
> > conn_resp_info->ies_len;
> > +   }
> > +
> > +   kfree(conn_resp_info->ies);
> > +   kfree(conn_resp_info);
> > +}
> > +  
> 
> Instead of adding new function why not using
> wilc_parse_assoc_resp_info() to return you the conn_info that you are
> copying it here? The connect_resp_info object that you are passing to
> wilc_parse_assoc_resp_info() is not used anywhere in
> host_int_parse_assoc_resp_info() it is used only to copy again from
> it to conn_info object.

Yes, there are different function because the purpose is different.
Primarily wilc_parse_assoc_resp_info() is to take care of parsing and
filing the response info and later connect_resp_info is used to fill
conn_info values. The elements in conn_info and connect_resp_info are
different, but now most of the fields are not used in connect_resp_info.
Returning 'conn_info' from wilc_parse_assoc_resp_info() might not be
correct. I will check if we can go away with connect_resp_info
structure itself as most of the field are not used in it.

> 
> Also, in wilc_parse_assoc_resp_info() you can get rid of these lines:
> connect_resp_info->capability = get_assoc_resp_cap_info(buffer);
> connect_resp_info->assoc_id = get_asoc_id(buffer);  
> 

With above changes it can be taken care.

> 
> >  static inline void host_int_parse_assoc_resp_info(struct wilc_vif
> > *vif, u8 mac_status)
> >  {
> > @@ -1316,25 +1333,13 @@ static inline void
> > host_int_parse_assoc_resp_info(struct wilc_vif *vif, 
> > err =
> > wilc_parse_assoc_resp_info(rcv_assoc_resp,
> > rcvd_assoc_resp_info_len, _resp_info);
> > -   if (err) {
> > +   if (err)
> > netdev_err(vif->ndev,
> >"wilc_parse_assoc_resp_info()
> > returned error %d\n", err);
> > -   } else {
> > -   conn_info.status =
> > connect_resp_info->status; -
> > -   if (conn_info.status ==
> > SUCCESSFUL_STATUSCODE &&
> > -   connect_resp_info->ies) {
> > -   conn_info.resp_ies =
> > kmemdup(connect_resp_info->ies,
> > -
> > connect_resp_info->ies_len,
> > -
> > GFP_KERNEL);
> > -   if (conn_info.resp_ies)
> > -
> > conn_info.resp_ies_len = connect_resp_info->ies_len;
> > -   }
> > -
> > -   kfree(connect_resp_info->ies);
> > -   kfree(connect_resp_info);
> > -   }
> > +   else
> > +
> > host_int_copy_conn_info(connect_resp_info,
> > +
> > _info); }
> > }
> >  
> >   

Re: [PATCH] ssb: Fix regression caused by disabling PCI cores on non-MIPS architecture

[Larry Finger]

On 05/09/2018 11:57 AM, Jeff Johnson wrote:

On 2018-05-09 09:42, Larry Finger wrote:

Some MPIS-based SoCs use chips driven by b43 for wireless capability.

typo: MPIS=>MIPS


Jeff,

Thanks. I will fix this with V2.

Larry

Re: [PATCH 30/30] staging: wilc1000: rename s8idxarray to avoid datatype in variable name

[Ajay Singh]

On Wed, 9 May 2018 16:42:20 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Cleanup patch to have variable names as per linux coding style.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index ad9270e..e27010c
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1543,7 +1543,7 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) struct wid wid;
> > struct wid wid_list[5];
> > u8 *key_buf;
> > -   s8 s8idxarray[1];
> > +   s8 idxarray[1];
> > struct host_if_drv *hif_drv = vif->hif_drv;
> >  
> > switch (hif_key->type) {
> > @@ -1610,8 +1610,8 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) wid.id = (u16)WID_REMOVE_WEP_KEY;
> > wid.type = WID_STR;
> >  
> > -   s8idxarray[0] =
> > (s8)hif_key->attr.wep.index;
> > -   wid.val = s8idxarray;
> > +   idxarray[0] =
> > (s8)hif_key->attr.wep.index;  
> 
> I think you can get rid of idxarray at all.
> 

Yes, we can remove idxarray variable. I will update and resubmit this
patch.

> > +   wid.val = idxarray;>
> > wid.size = 1; 
> > result = wilc_send_config_pkt(vif, SET_CFG,
> >   

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

[Ajay Singh]

On Wed, 9 May 2018 16:42:45 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix line over 80 characters issues reported by checkpatch.pl script
> > in wilc_wfi_cfg_tx_vendor_spec() by using temporary variable.
> > Simplified 'if else' condition with 'if'.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 14
> > +++--- 1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
> > 4f35178..8dea414 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -1573,14
> > +1573,14 @@ static void wilc_wfi_cfg_tx_vendor_spec(struct
> > p2p_mgmt_data *mgmt_tx, for (i = P2P_PUB_ACTION_SUBTYPE + 2; i <
> > len; i++) { if (buf[i] == P2PELEM_ATTR_ID && !memcmp(p2p_oui,
> > [i + 2], 4)) {
> > +   bool oper_ch = false;
> > +   u8 *tx_buff = _tx->buff[i + 6];
> > +
> > if (subtype == P2P_INV_REQ || subtype ==
> > P2P_INV_RSP)
> > -
> > wilc_wfi_cfg_parse_tx_action(_tx->buff[i + 6],
> > -len -
> > (i + 6),
> > -true,
> > iftype);
> > -   else
> > -
> > wilc_wfi_cfg_parse_tx_action(_tx->buff[i + 6],
> > -len -
> > (i + 6),
> > -
> > false, iftype);
> > +   oper_ch = true;
> > +
> > +   wilc_wfi_cfg_parse_tx_action(tx_buff, len
> > - (i + 6),
> > +oper_ch,
> > iftype);  
> 
> What about:
>   wilc_wfi_cfg_parse_tx_action(_tx->buff[i
> + 6], len - (i + 6),
>(subtype ==
> P2P_INV_REQ || subtype == P2P_INV_RSP),
>iftype);
> 
> 
> instead all the temporary variables?

In my opinion adding one bool variable making the code more readable
then using adding extra logic with parameters for function call.

> 
> >  
> > break;
> > }
> >   

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

[Ajay Singh]

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.

> > 
> > 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 = _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;
> > -   last_scanned_shadow[ap_index].found = 1;
> > +   kfree(shadow_nw_info->ies);
> > +   shadow_nw_info->ies = kmalloc(nw_info->ies_len,
> > GFP_KERNEL);
> > +   memcpy(shadow_nw_info->ies, nw_info->ies,
> > nw_info->ies_len);
> > +   shadow_nw_info->time_scan = jiffies;
> > +   shadow_nw_info->time_scan_cached = jiffies;
> > +   shadow_nw_info->found = 1;
> > if (ap_found != -1)
> > -   kfree(last_scanned_shadow[ap_index].join_params);
> > -   

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

[Ajay Singh]

On Wed, 9 May 2018 16:43:37 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix line over 80 characters issue in
> > host_int_parse_assoc_resp_info() by using shorter name for the
> > local variable.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index c9c5d352..b1f67a7
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1320,18 +1320,19 @@ static inline void
> > host_int_parse_assoc_resp_info(struct wilc_vif *vif,
> > memset(_info, 0, sizeof(struct connect_info)); 
> > if (mac_status == MAC_STATUS_CONNECTED) {
> > -   u32 rcvd_assoc_resp_info_len;
> > +   u32 assoc_resp_info_len;  
> 
> I would simply use len or size. It is just my preference.

I would prefer to keep it same(as submitted patch), as its more
readable.

> 
> >  
> > memset(rcv_assoc_resp, 0,
> > MAX_ASSOC_RESP_FRAME_SIZE); 
> > host_int_get_assoc_res_info(vif, rcv_assoc_resp,
> > MAX_ASSOC_RESP_FRAME_SIZE,
> > -
> > _assoc_resp_info_len);
> > +   _resp_info_len);
> >  
> > -   if (rcvd_assoc_resp_info_len != 0) {
> > +   if (assoc_resp_info_len != 0) {
> > s32 err = 0;
> >  
> > -   err =
> > wilc_parse_assoc_resp_info(rcv_assoc_resp, rcvd_assoc_resp_info_len,
> > +   err =
> > wilc_parse_assoc_resp_info(rcv_assoc_resp,
> > +
> > assoc_resp_info_len, _resp_info);
> > if (err)
> > netdev_err(vif->ndev,
> >   

Re: [PATCH 07/30] staging: wilc1000: fix line over 80 characters in host_int_parse_join_bss_param()

[Ajay Singh]

On Wed, 9 May 2018 16:43:59 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Split host_int_parse_join_bss_param() to avoid the line over 80
> > character issue reported by checkpatch.pl script.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 247
> > -- 1 file changed, 131 insertions(+),
> > 116 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 0d84af9..c9c5d352
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -3856,150 +3856,165 @@ int wilc_setup_multicast_filter(struct
> > wilc_vif *vif, bool enabled, return result;
> >  }
> >  
> > -static void *host_int_parse_join_bss_param(struct network_info
> > *info) +static void host_int_fill_join_bss_param(struct
> > join_bss_param *param, u8 *ies,
> > +u16 *out_index, u8
> > *pcipher_tc,
> > +u8 *auth_total_cnt, u32
> > tsf_lo) {
> > -   struct join_bss_param *param = NULL;
> > -   u8 *ies;
> > -   u16 ies_len;
> > -   u16 index = 0;
> > u8 rates_no = 0;
> > u8 ext_rates_no;
> > u16 offset;
> > u8 pcipher_cnt;
> > u8 auth_cnt;
> > -   u8 pcipher_total_cnt = 0;
> > -   u8 auth_total_cnt = 0;
> > u8 i, j;
> > +   u16 index = *out_index;  
> 
> Why not having a single index, the one passed as argument?
> 

If we use 'index' argument then we have to change all the lines below
with '*index' and it would make code look more complicated. So to
reduce those changes below I have the above logic.

> >  
> > -   ies = info->ies;
> > -   ies_len = info->ies_len;
> > +   if (ies[index] == SUPP_RATES_IE) {
> > +   rates_no = ies[index + 1];
> > +   param->supp_rates[0] = rates_no;
> > +   index += 2;
> >  
> > -   param = kzalloc(sizeof(*param), GFP_KERNEL);
> > -   if (!param)
> > -   return NULL;
> > +   for (i = 0; i < rates_no; i++)
> > +   param->supp_rates[i + 1] = ies[index + i];
> >  
> > -   param->dtim_period = info->dtim_period;
> > -   param->beacon_period = info->beacon_period;
> > -   param->cap_info = info->cap_info;
> > -   memcpy(param->bssid, info->bssid, 6);
> > -   memcpy((u8 *)param->ssid, info->ssid, info->ssid_len + 1);
> > -   param->ssid_len = info->ssid_len;
> > -   memset(param->rsn_pcip_policy, 0xFF, 3);
> > -   memset(param->rsn_auth_policy, 0xFF, 3);
> > +   index += rates_no;
> > +   } else if (ies[index] == EXT_SUPP_RATES_IE) {
> > +   ext_rates_no = ies[index + 1];
> > +   if (ext_rates_no > (MAX_RATES_SUPPORTED -
> > rates_no))
> > +   param->supp_rates[0] = MAX_RATES_SUPPORTED;
> > +   else
> > +   param->supp_rates[0] += ext_rates_no;
> > +   index += 2;
> > +   for (i = 0; i < (param->supp_rates[0] - rates_no);
> > i++)
> > +   param->supp_rates[rates_no + i + 1] =
> > ies[index + i]; +
> > +   index += ext_rates_no;
> > +   } else if (ies[index] == HT_CAPABILITY_IE) {
> > +   param->ht_capable = true;
> > +   index += ies[index + 1] + 2;
> > +   } else if ((ies[index] == WMM_IE) &&
> > +  (ies[index + 2] == 0x00) && (ies[index + 3] ==
> > 0x50) &&
> > +  (ies[index + 4] == 0xF2) && (ies[index + 5] ==
> > 0x02) &&
> > +  ((ies[index + 6] == 0x00) || (ies[index + 6] ==
> > 0x01)) &&
> > +  (ies[index + 7] == 0x01)) {
> > +   param->wmm_cap = true;
> > +
> > +   if (ies[index + 8] & BIT(7))
> > +   param->uapsd_cap = true;
> > +   index += ies[index + 1] + 2;
> > +   } else if ((ies[index] == P2P_IE) &&
> > +(ies[index + 2] == 0x50) && (ies[index + 3] ==
> > 0x6f) &&
> > +(ies[index + 4] == 0x9a) &&
> > +(ies[index + 5] == 0x09) && (ies[index + 6] ==
> > 0x0c)) {
> > +   u16 p2p_cnt;
> > +
> > +   param->tsf = tsf_lo;
> > +   param->noa_enabled = 1;
> > +   param->idx = ies[index + 9];
> > +
> > +   if (ies[index + 10] & BIT(7)) {
> > +   param->opp_enabled = 1;
> > +   param->ct_window = ies[index + 10];
> > +   } else {
> > +   param->opp_enabled = 0;
> > +   }
> >  
> > -   while (index < ies_len) {
> > -   if (ies[index] == SUPP_RATES_IE) {
> > -   rates_no = ies[index + 1];
> > -   param->supp_rates[0] = rates_no;
> > -   index += 2;
> > +   param->cnt = ies[index + 11];
> > +   p2p_cnt = index + 12;
> >  
> > -   for (i = 0; i < rates_no; i++)
> > -   param->supp_rates[i + 1] =
> > ies[index + i];
> > +   

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

[Ajay Singh]

On Wed, 9 May 2018 16:44:47 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix checkpatch reported issue of line over 80 char in handle_key().
> > Introduced new functions by spliting existing function to address
> > the checkpatch issue.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 59
> > +++ 1 file changed, 37 insertions(+),
> > 22 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 4ba868c..29f9907
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1498,12 +1498,45 @@ static s32
> > handle_rcvd_gnrl_async_info(struct wilc_vif *vif, return result;
> >  }
> >  
> > +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct
> > key_attr *hif_key) +{
> > +   int i;
> > +   int ret;
> > +   struct wid wid;
> > +   u8 *key_buf;
> > +
> > +   key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1,
> > + GFP_KERNEL);
> > +   if (!key_buf)
> > +   return -ENOMEM;
> > +
> > +   key_buf[0] = hif_key->attr.pmkid.numpmkid;
> > +
> > +   for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> > +   memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
> > +  hif_key->attr.pmkid.pmkidlist[i].bssid,
> > ETH_ALEN);
> > +   memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN +
> > 1),
> > +  hif_key->attr.pmkid.pmkidlist[i].pmkid,
> > PMKID_LEN);
> > +   }
> > +
> > +   wid.id = (u16)WID_PMKID_INFO;
> > +   wid.type = WID_STR;
> > +   wid.val = (s8 *)key_buf;  
> 
> Is this cast really needed?
> 

This patch is only to address line over 80 chars checkpatch issue.
It is not good to club these change with type cast related
modification. For removing unnecessary cast we can submit
a separate patch series.
These are my views. What do you think?


> > +   wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN)
> > + 1; +
> > +   ret = wilc_send_config_pkt(vif, SET_CFG, , 1,
> > +  wilc_get_vif_idx(vif));
> > +
> > +   kfree(key_buf);
> > +
> > +   return ret;
> > +}
> > +
> >  static int handle_key(struct wilc_vif *vif, struct key_attr
> > *hif_key) {
> > int result = 0;
> > struct wid wid;
> > struct wid wid_list[5];
> > -   u8 i;
> > u8 *key_buf;
> > s8 s8idxarray[1];
> > struct host_if_drv *hif_drv = vif->hif_drv;
> > @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) wilc_get_vif_idx(vif));
> > kfree(key_buf);
> > } else if (hif_key->action & ADDKEY) {
> > -   key_buf =
> > kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
> > +   key_buf =
> > kmalloc(hif_key->attr.wep.key_len + 2,
> > + GFP_KERNEL);
> > if (!key_buf) {
> > result = -ENOMEM;
> > goto out_wep;
> > @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) break;
> >  
> > case PMKSA:
> > -   key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1, GFP_KERNEL);
> > -   if (!key_buf)
> > -   return -ENOMEM;
> > -
> > -   key_buf[0] = hif_key->attr.pmkid.numpmkid;
> > -
> > -   for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++)
> > {
> > -   memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
> > 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> > -   memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
> > ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> > -   }
> > -
> > -   wid.id = (u16)WID_PMKID_INFO;
> > -   wid.type = WID_STR;
> > -   wid.val = (s8 *)key_buf;
> > -   wid.size = (hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1; -
> > -   result = wilc_send_config_pkt(vif, SET_CFG, ,
> > 1,
> > -
> > wilc_get_vif_idx(vif)); -
> > -   kfree(key_buf);
> > +   result = wilc_pmksa_key_copy(vif, hif_key);
> > break;
> > }
> >  
> >   

Re: [PATCH 06/30] staging: wilc1000: fix line over 80 chars issue in host_int_handle_disconnect()

[Ajay Singh]

On Wed, 9 May 2018 16:44:13 +0300
Claudiu Beznea  wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix line over 80 char issue in host_int_handle_disconnect() by using
> > temp variable to hold the 'wilc_connect_result' function pointer.
> > 
> > Signed-off-by: Ajay Singh 
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index a341ff1..0d84af9
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1401,6 +1401,7 @@ static inline void
> > host_int_handle_disconnect(struct wilc_vif *vif) {
> > struct disconnect_info disconn_info;
> > struct host_if_drv *hif_drv = vif->hif_drv;
> > +   wilc_connect_result conn_result =
> > hif_drv->usr_conn_req.conn_result; 
> 
> Is there a scenario you can end up here with a null hif_drv? I'm
> seeing that some of the functions from this layer are checking for
> hif_drv at the beginning, some are not.
>

Before calling host_int_handle_disconnect() there is already a NULL
check in handle_rcvd_gnrl_async_info(), so I think we don't need to add
another check here. In handle_scan_done() there is NULL check
inside the function because its also called from different places 
without NULL check.

> 
> > memset(_info, 0, sizeof(struct disconnect_info));
> >  
> > @@ -1413,13 +1414,12 @@ static inline void
> > host_int_handle_disconnect(struct wilc_vif *vif) disconn_info.ie =
> > NULL; disconn_info.ie_len = 0;
> >  
> > -   if (hif_drv->usr_conn_req.conn_result) {
> > +   if (conn_result) {
> > wilc_optaining_ip = false;
> > wilc_set_power_mgmt(vif, 0, 0);
> >  
> > -
> > hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
> > - NULL, 0,
> > _info,
> > -
> > hif_drv->usr_conn_req.arg);
> > +   conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
> > NULL, 0,
> > +   _info,
> > hif_drv->usr_conn_req.arg); } else {
> > netdev_err(vif->ndev, "Connect result NULL\n");
> > }
> >   

Re: [PATCH] ssb: Fix regression caused by disabling PCI cores on non-MIPS architecture

[Jeff Johnson]

On 2018-05-09 09:42, Larry Finger wrote:

Some MPIS-based SoCs use chips driven by b43 for wireless capability.

typo: MPIS=>MIPS

[PATCH] ssb: Fix regression caused by disabling PCI cores on non-MIPS architecture

[Larry Finger]

Some MPIS-based SoCs use chips driven by b43 for wireless capability.
When ssb is configured as a module, build errors happen on these
platforms as described in the commit 882164a4a928 ("ssb: Prevent build
of PCI host features in module"). Unfortunately that change leads to
the removal of code needed for correct operation on platforms that use
PCI cores on the chip bus. The fix allows ssb to be build as a module
for all architectures other than MIPS. This approach is ad-hoc, but it
is the same as done in commit a9e6d44ddecc ("ssb: Do not disable PCI
host on non-Mips").

This problem was reported and discussed in
https://bugzilla.redhat.com/show_bug.cgi?id=1572349.

Fixes: commit 882164a4a928 ("ssb: Prevent build of PCI host features in module")
Tested-by: Matt Redfearn 
Tested-by: Bruno Wolff III 
Cc: Michael Büsch 
Signed-off-by: Larry Finger 
---

Kalle,

This patch fixes the regression in 4.17 that was discussed on the
wireless mailing list. I'm not really happy about the ad-hoc rejection
of ssb as a module only on MIPS; however, that seems to be a unique
implementation of this hardware.

Larry

Michael,

Although I think your version of the patch is superior, this is the one that
Matt tested, thus I'm submitting this version.

Larry

---
 drivers/ssb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 9371651d8017..3743533c8057 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -117,7 +117,7 @@ config SSB_SERIAL
 
 config SSB_DRIVER_PCICORE_POSSIBLE
bool
-   depends on SSB_PCIHOST && SSB = y
+   depends on SSB_PCIHOST && (SSB = y || !MIPS)
default y
 
 config SSB_DRIVER_PCICORE
-- 
2.16.3

Re: Regression caused by commit 882164a4a928

[Michael Büsch]

On Wed, 9 May 2018 13:55:43 +0100
Matt Redfearn  wrote:

> Hi Larry
> 
> On 07/05/18 16:44, Larry Finger wrote:
> > Matt,
> > 
> > Although commit 882164a4a928 ("ssb: Prevent build of PCI host features 
> > in module") appeared to be harmless, it leads to complete failure of 
> > drivers b43. and b43legacy, and likely affects b44 as well. The problem 
> > is that CONFIG_SSB_PCIHOST is undefined, which prevents the compilation 
> > of the code that controls the PCI cores of the device. See 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.  
> 
> Sorry for the breakage :-/
> 
> > 
> > As the underlying errors ("pcibios_enable_device" undefined, and 
> > "register_pci_controller" undefined) do not appear on the architectures 
> > that I have tested (x86_64, x86, and ppc), I suspect something in the 
> > arch-specific code for your setup (MIPS?). As I have no idea on how to 
> > fix that problem, would the following patch work for you?
> > 
> > diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> > index 9371651d8017..3743533c8057 100644
> > --- a/drivers/ssb/Kconfig
> > +++ b/drivers/ssb/Kconfig
> > @@ -117,7 +117,7 @@ config SSB_SERIAL
> > 
> >   config SSB_DRIVER_PCICORE_POSSIBLE
> >      bool
> > -   depends on SSB_PCIHOST && SSB = y
> > +   depends on SSB_PCIHOST && (SSB = y || !MIPS)
> >      default y
> > 
> >   config SSB_DRIVER_PCICORE  
> 
> I believe that the problem stems from these drivers being used for some 
> wireless AP functionality built into some MIPS based SoCs. The Kconfig 
> rules sort out building this additional functionality when configured 
> for MIPS (in a round about sort of way), but it allowed it even when SSB 
> is a module, leading to build failures. My patch was intended to prevent 
> that.
> 
> There was a similar issue in the same Kconfig file, introduced by 
> c5611df96804 and fixed by a9e6d44ddecc. It was fixed the same way as you 
> suggest. I've tested the above patch and it does work for MIPS 
> (preventing the PCICORE being built into the module).
> 
> Tested-by: Matt Redfearn 


Could you please try this?

config SSB_DRIVER_PCICORE_POSSIBLE
depends on SSB_PCIHOST

config SSB_PCICORE_HOSTMODE
depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && (SSB = y) && 
PCI_DRIVERS_LEGACY


The affected API pcibios_enable_device() and register_pci_controller()
is only used in HOSTMODE. So I think it makes sense to make HOSTMODE
depend on SSB=y and PCI_DRIVERS_LEGACY.

PCICore itself does not use the API, if hostmode is disabled.

-- 
Michael


pgpH8hefzhTTn.pgp
Description: OpenPGP digital signature

[PATCH] iw: support reloading the regulatory database

[Seth Forshee]

Add a "iw reg reload" command, useful for testing new regulatory
databases.

Signed-off-by: Seth Forshee 
---
 reg.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/reg.c b/reg.c
index cee0b5e8cedb..cadff3884c04 100644
--- a/reg.c
+++ b/reg.c
@@ -259,3 +259,13 @@ COMMAND(reg, get, NULL, NL80211_CMD_GET_REG, 0, CIB_PHY, 
handle_reg_get,
"Print out the devices' current regulatory domain information.");
 HIDDEN(reg, dump, NULL, NL80211_CMD_GET_REG, NLM_F_DUMP, CIB_NONE,
handle_reg_dump);
+
+static int handle_reg_reload(struct nl80211_state *state,
+struct nl_msg *msg,
+int argc, char **argv,
+enum id_input id)
+{
+   return 0;
+}
+COMMAND(reg, reload, NULL, NL80211_CMD_RELOAD_REGDB, 0, CIB_NONE,
+   handle_reg_reload, "Reload the kernel's regulatory database.");
-- 
2.17.0

Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference

[Mauro Carvalho Chehab]

Em Tue,  8 May 2018 11:12:46 -0700
"Luis R. Rodriguez"  escreveu:

> It refers to a pending patch, but this was merged eons ago.

Didn't know that such patch was already merged. Great!

Regards,
Mauro

> 
> Signed-off-by: Luis R. Rodriguez 
> ---
>  Documentation/dell_rbu.txt | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> index 0fdb6aa2704c..077fdc29a0d0 100644
> --- a/Documentation/dell_rbu.txt
> +++ b/Documentation/dell_rbu.txt
> @@ -121,9 +121,6 @@ read back the image downloaded.
>  
>  .. note::
>  
> -   This driver requires a patch for firmware_class.c which has the modified
> -   request_firmware_nowait function.
> -

> Also after updating the BIOS image a user mode application needs to 
> execute
> code which sends the BIOS update request to the BIOS. So on the next 
> reboot
> the BIOS knows about the new image downloaded and it updates itself.

You should likely remove the "Also" here.

With that,

Reviewed-by: Mauro Carvalho Chehab 

Regards,
Mauro

Re: [PATCH v2] ath10k: support for multicast rate control

[Ben Greear]

On 05/08/2018 11:57 PM, Sebastian Gottschall wrote:



Am 09.05.2018 um 08:15 schrieb Sven Eckelmann:

On Montag, 7. Mai 2018 18:34:51 CEST Pradeep Kumar Chitrapu wrote:

Issues wmi command to firmwre when multicast rate change is received
with the new BSS_CHANGED_MCAST_RATE flag.
Also fixes the incorrect fixed_rate setting for CCK rates which got
introduced with addition of ath10k_rates_rev2 enum.

Tested on QCA9984 with firmware ver 10.4-3.6-00104
Signed-off-by: Pradeep Kumar Chitrapu 
---
v2:
  - fixed the CCK rates setting for mcast_rate and fixed_rate paths.
  - set broadcast rate along with multicast rate setting.

These things are only modified in ath10k_bss_info_changed and not saved/
restored. What happens when the HW needs to be reset (there are are couple of
firmware crashes which can be seen in the wild and are handled by ath10k)? I
would guess that not all of them automatically cause an .bss_info_changed.


Yes, that sounds like a good idea to me.


i have never seen a card properly recovering after a firmware crash, all 
firmware crashes i have seen
are caused by bugs in firmware handling or the firmware itself. so if it 
recovers it crashes immediatly again
ending in a endless crashloop since the cause for the crash hasnt been fixed. 
they are often triggered by extern clients for instance which still remain in 
the field.
i think firmware crashes should not be hidden by recovering. this would also 
force users to report problems more frequently, than they do
since they dont even take notice of such problems


We see recovery work often.  If you see repeatable crashes, post them
to the list.

Do you know of any particular external clients that cause these crashes?

Thanks,
Ben

--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com

[ANN] wireless-regdb: master-2018-05-09

[Seth Forshee]

A new release of wireless-regdb (master-2018-05-09) is available at:

https://www.kernel.org/pub/software/network/wireless-regdb/wireless-regdb-2018.05.09.tar.gz

The short log of changes since the 2017-12-23 release is below.

Thanks,
Seth

---

Haim Dreyfuss (2):
  wireless-regdb: Add wmm rule for EEA and EFTA countries
  wireless-regdb: Parse wmm rule data

Matthias Schiffer (2):
  wireless-regdb: do not rely on sorting of dict keys in conversion scripts
  wireless-regdb: make scripts compatible with Python 3

Seth Forshee (2):
  wireless-regdb: Update rules for QA and add 5 GHz and 60 GHz rules
  wireless-regdb: update regulatory database based on preceding changes

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

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Fix checkpatch reported issue of line over 80 char in handle_key().
> Introduced new functions by spliting existing function to address the
> checkpatch issue.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 59 
> +++
>  1 file changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index 4ba868c..29f9907 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1498,12 +1498,45 @@ static s32 handle_rcvd_gnrl_async_info(struct 
> wilc_vif *vif,
>   return result;
>  }
>  
> +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct key_attr 
> *hif_key)
> +{
> + int i;
> + int ret;
> + struct wid wid;
> + u8 *key_buf;
> +
> + key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1,
> +   GFP_KERNEL);
> + if (!key_buf)
> + return -ENOMEM;
> +
> + key_buf[0] = hif_key->attr.pmkid.numpmkid;
> +
> + for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> + memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
> +hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> + memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1),
> +hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> + }
> +
> + wid.id = (u16)WID_PMKID_INFO;
> + wid.type = WID_STR;
> + wid.val = (s8 *)key_buf;

Is this cast really needed?

> + wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
> +
> + ret = wilc_send_config_pkt(vif, SET_CFG, , 1,
> +wilc_get_vif_idx(vif));
> +
> + kfree(key_buf);
> +
> + return ret;
> +}
> +
>  static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
>  {
>   int result = 0;
>   struct wid wid;
>   struct wid wid_list[5];
> - u8 i;
>   u8 *key_buf;
>   s8 s8idxarray[1];
>   struct host_if_drv *hif_drv = vif->hif_drv;
> @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif, struct 
> key_attr *hif_key)
> wilc_get_vif_idx(vif));
>   kfree(key_buf);
>   } else if (hif_key->action & ADDKEY) {
> - key_buf = kmalloc(hif_key->attr.wep.key_len + 2, 
> GFP_KERNEL);
> + key_buf = kmalloc(hif_key->attr.wep.key_len + 2,
> +   GFP_KERNEL);
>   if (!key_buf) {
>   result = -ENOMEM;
>   goto out_wep;
> @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif, struct 
> key_attr *hif_key)
>   break;
>  
>   case PMKSA:
> - key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * 
> PMKSA_KEY_LEN) + 1, GFP_KERNEL);
> - if (!key_buf)
> - return -ENOMEM;
> -
> - key_buf[0] = hif_key->attr.pmkid.numpmkid;
> -
> - for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> - memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1), 
> hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> - memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1), 
> hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> - }
> -
> - wid.id = (u16)WID_PMKID_INFO;
> - wid.type = WID_STR;
> - wid.val = (s8 *)key_buf;
> - wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
> -
> - result = wilc_send_config_pkt(vif, SET_CFG, , 1,
> -   wilc_get_vif_idx(vif));
> -
> - kfree(key_buf);
> + result = wilc_pmksa_key_copy(vif, hif_key);
>   break;
>   }
>  
> 

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

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Fix line over 80 characters issue reported by checkpatch.pl in
> host_int_parse_assoc_resp_info().
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 37 
> ++-
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index 55a61d5..a341ff1 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1293,6 +1293,23 @@ static inline void host_int_free_user_conn_req(struct 
> host_if_drv *hif_drv)
>   hif_drv->usr_conn_req.ies = NULL;
>  }
>  
> +static void host_int_copy_conn_info(struct connect_resp_info *conn_resp_info,
> + struct connect_info *conn_info)
> +{
> + conn_info->status = conn_resp_info->status;
> +
> + if (conn_info->status == SUCCESSFUL_STATUSCODE && conn_resp_info->ies) {
> + conn_info->resp_ies = kmemdup(conn_resp_info->ies,
> +   conn_resp_info->ies_len,
> +   GFP_KERNEL);
> + if (conn_info->resp_ies)
> + conn_info->resp_ies_len = conn_resp_info->ies_len;
> + }
> +
> + kfree(conn_resp_info->ies);
> + kfree(conn_resp_info);
> +}
> +

Instead of adding new function why not using wilc_parse_assoc_resp_info() to
return you the conn_info that you are copying it here? The connect_resp_info
object that you are passing to wilc_parse_assoc_resp_info() is not used 
anywhere in host_int_parse_assoc_resp_info() it is used only to copy again
from it to conn_info object.

Also, in wilc_parse_assoc_resp_info() you can get rid of these lines:
connect_resp_info->capability = get_assoc_resp_cap_info(buffer);
connect_resp_info->assoc_id = get_asoc_id(buffer);  


>  static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
> u8 mac_status)
>  {
> @@ -1316,25 +1333,13 @@ static inline void 
> host_int_parse_assoc_resp_info(struct wilc_vif *vif,
>  
>   err = wilc_parse_assoc_resp_info(rcv_assoc_resp, 
> rcvd_assoc_resp_info_len,
>_resp_info);
> - if (err) {
> + if (err)
>   netdev_err(vif->ndev,
>  "wilc_parse_assoc_resp_info() 
> returned error %d\n",
>  err);
> - } else {
> - conn_info.status = connect_resp_info->status;
> -
> - if (conn_info.status == SUCCESSFUL_STATUSCODE &&
> - connect_resp_info->ies) {
> - conn_info.resp_ies = 
> kmemdup(connect_resp_info->ies,
> -  
> connect_resp_info->ies_len,
> -  
> GFP_KERNEL);
> - if (conn_info.resp_ies)
> - conn_info.resp_ies_len = 
> connect_resp_info->ies_len;
> - }
> -
> - kfree(connect_resp_info->ies);
> - kfree(connect_resp_info);
> - }
> + else
> + host_int_copy_conn_info(connect_resp_info,
> + _info);
>   }
>   }
>  
> 

Re: [PATCH 06/30] staging: wilc1000: fix line over 80 chars issue in host_int_handle_disconnect()

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Fix line over 80 char issue in host_int_handle_disconnect() by using
> temp variable to hold the 'wilc_connect_result' function pointer.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index a341ff1..0d84af9 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1401,6 +1401,7 @@ static inline void host_int_handle_disconnect(struct 
> wilc_vif *vif)
>  {
>   struct disconnect_info disconn_info;
>   struct host_if_drv *hif_drv = vif->hif_drv;
> + wilc_connect_result conn_result = hif_drv->usr_conn_req.conn_result;
>  

Is there a scenario you can end up here with a null hif_drv? I'm seeing that 
some
of the functions from this layer are checking for hif_drv at the beginning, some
are not.


>   memset(_info, 0, sizeof(struct disconnect_info));
>  
> @@ -1413,13 +1414,12 @@ static inline void host_int_handle_disconnect(struct 
> wilc_vif *vif)
>   disconn_info.ie = NULL;
>   disconn_info.ie_len = 0;
>  
> - if (hif_drv->usr_conn_req.conn_result) {
> + if (conn_result) {
>   wilc_optaining_ip = false;
>   wilc_set_power_mgmt(vif, 0, 0);
>  
> - 
> hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
> -   NULL, 0, _info,
> -   hif_drv->usr_conn_req.arg);
> + conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF, NULL, 0,
> + _info, hif_drv->usr_conn_req.arg);
>   } else {
>   netdev_err(vif->ndev, "Connect result NULL\n");
>   }
> 

Re: [PATCH 07/30] staging: wilc1000: fix line over 80 characters in host_int_parse_join_bss_param()

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Split host_int_parse_join_bss_param() to avoid the line over 80
> character issue reported by checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 247 
> --
>  1 file changed, 131 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index 0d84af9..c9c5d352 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -3856,150 +3856,165 @@ int wilc_setup_multicast_filter(struct wilc_vif 
> *vif, bool enabled,
>   return result;
>  }
>  
> -static void *host_int_parse_join_bss_param(struct network_info *info)
> +static void host_int_fill_join_bss_param(struct join_bss_param *param, u8 
> *ies,
> +  u16 *out_index, u8 *pcipher_tc,
> +  u8 *auth_total_cnt, u32 tsf_lo)
>  {
> - struct join_bss_param *param = NULL;
> - u8 *ies;
> - u16 ies_len;
> - u16 index = 0;
>   u8 rates_no = 0;
>   u8 ext_rates_no;
>   u16 offset;
>   u8 pcipher_cnt;
>   u8 auth_cnt;
> - u8 pcipher_total_cnt = 0;
> - u8 auth_total_cnt = 0;
>   u8 i, j;
> + u16 index = *out_index;

Why not having a single index, the one passed as argument?

>  
> - ies = info->ies;
> - ies_len = info->ies_len;
> + if (ies[index] == SUPP_RATES_IE) {
> + rates_no = ies[index + 1];
> + param->supp_rates[0] = rates_no;
> + index += 2;
>  
> - param = kzalloc(sizeof(*param), GFP_KERNEL);
> - if (!param)
> - return NULL;
> + for (i = 0; i < rates_no; i++)
> + param->supp_rates[i + 1] = ies[index + i];
>  
> - param->dtim_period = info->dtim_period;
> - param->beacon_period = info->beacon_period;
> - param->cap_info = info->cap_info;
> - memcpy(param->bssid, info->bssid, 6);
> - memcpy((u8 *)param->ssid, info->ssid, info->ssid_len + 1);
> - param->ssid_len = info->ssid_len;
> - memset(param->rsn_pcip_policy, 0xFF, 3);
> - memset(param->rsn_auth_policy, 0xFF, 3);
> + index += rates_no;
> + } else if (ies[index] == EXT_SUPP_RATES_IE) {
> + ext_rates_no = ies[index + 1];
> + if (ext_rates_no > (MAX_RATES_SUPPORTED - rates_no))
> + param->supp_rates[0] = MAX_RATES_SUPPORTED;
> + else
> + param->supp_rates[0] += ext_rates_no;
> + index += 2;
> + for (i = 0; i < (param->supp_rates[0] - rates_no); i++)
> + param->supp_rates[rates_no + i + 1] = ies[index + i];
> +
> + index += ext_rates_no;
> + } else if (ies[index] == HT_CAPABILITY_IE) {
> + param->ht_capable = true;
> + index += ies[index + 1] + 2;
> + } else if ((ies[index] == WMM_IE) &&
> +(ies[index + 2] == 0x00) && (ies[index + 3] == 0x50) &&
> +(ies[index + 4] == 0xF2) && (ies[index + 5] == 0x02) &&
> +((ies[index + 6] == 0x00) || (ies[index + 6] == 0x01)) &&
> +(ies[index + 7] == 0x01)) {
> + param->wmm_cap = true;
> +
> + if (ies[index + 8] & BIT(7))
> + param->uapsd_cap = true;
> + index += ies[index + 1] + 2;
> + } else if ((ies[index] == P2P_IE) &&
> +  (ies[index + 2] == 0x50) && (ies[index + 3] == 0x6f) &&
> +  (ies[index + 4] == 0x9a) &&
> +  (ies[index + 5] == 0x09) && (ies[index + 6] == 0x0c)) {
> + u16 p2p_cnt;
> +
> + param->tsf = tsf_lo;
> + param->noa_enabled = 1;
> + param->idx = ies[index + 9];
> +
> + if (ies[index + 10] & BIT(7)) {
> + param->opp_enabled = 1;
> + param->ct_window = ies[index + 10];
> + } else {
> + param->opp_enabled = 0;
> + }
>  
> - while (index < ies_len) {
> - if (ies[index] == SUPP_RATES_IE) {
> - rates_no = ies[index + 1];
> - param->supp_rates[0] = rates_no;
> - index += 2;
> + param->cnt = ies[index + 11];
> + p2p_cnt = index + 12;
>  
> - for (i = 0; i < rates_no; i++)
> - param->supp_rates[i + 1] = ies[index + i];
> + memcpy(param->duration, ies + p2p_cnt, 4);
> + p2p_cnt += 4;
>  
> - index += rates_no;
> - } else if (ies[index] == EXT_SUPP_RATES_IE) {
> - ext_rates_no = ies[index + 1];
> - if (ext_rates_no > (MAX_RATES_SUPPORTED - rates_no))
> - param->supp_rates[0] = 

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

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Fix line over 80 characters issue in host_int_parse_assoc_resp_info() by
> using shorter name for the local variable.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index c9c5d352..b1f67a7 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1320,18 +1320,19 @@ static inline void 
> host_int_parse_assoc_resp_info(struct wilc_vif *vif,
>   memset(_info, 0, sizeof(struct connect_info));
>  
>   if (mac_status == MAC_STATUS_CONNECTED) {
> - u32 rcvd_assoc_resp_info_len;
> + u32 assoc_resp_info_len;

I would simply use len or size. It is just my preference.

>  
>   memset(rcv_assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
>  
>   host_int_get_assoc_res_info(vif, rcv_assoc_resp,
>   MAX_ASSOC_RESP_FRAME_SIZE,
> - _assoc_resp_info_len);
> + _resp_info_len);
>  
> - if (rcvd_assoc_resp_info_len != 0) {
> + if (assoc_resp_info_len != 0) {
>   s32 err = 0;
>  
> - err = wilc_parse_assoc_resp_info(rcv_assoc_resp, 
> rcvd_assoc_resp_info_len,
> + err = wilc_parse_assoc_resp_info(rcv_assoc_resp,
> +  assoc_resp_info_len,
>_resp_info);
>   if (err)
>   netdev_err(vif->ndev,
> 

Re: [PATCH 13/30] staging: wilc1000: rename clear_duringIP() to avoid camelCase issue

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Rename clear_duringIP() function to avoid camelCase issue reported by
> checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index d0fb31a..f1ebaea 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -269,7 +269,7 @@ static void remove_network_from_shadow(struct timer_list 
> *unused)
>   mod_timer(_timer, jiffies + msecs_to_jiffies(AGING_TIME));
>  }
>  
> -static void clear_duringIP(struct timer_list *unused)
> +static void clear_duringip(struct timer_list *unused)

Maybe add an '_' for better understanding: like clear_during_ip() ?

>  {
>   wilc_optaining_ip = false;
>  }
> @@ -2272,7 +2272,7 @@ int wilc_init_host_int(struct net_device *net)
>   priv = wdev_priv(net->ieee80211_ptr);
>   if (op_ifcs == 0) {
>   timer_setup(_timer, remove_network_from_shadow, 0);
> - timer_setup(_during_ip_timer, clear_duringIP, 0);
> + timer_setup(_during_ip_timer, clear_duringip, 0);
>   }
>   op_ifcs++;
>  
> 

Re: [PATCH 15/30] staging: wilc1000: use kmemdup instead of kmalloc in add_network_to_shadow()

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Use kmemdup instead of kmalloc & memcpy in add_network_to_shadow().
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index 0ae2065..ca221f1 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -331,8 +331,8 @@ static void add_network_to_shadow(struct network_info 
> *nw_info,
>   shadow_nw_info->tsf_hi = nw_info->tsf_hi;
>   if (ap_found != -1)
>   kfree(shadow_nw_info->ies);
> - shadow_nw_info->ies = kmalloc(nw_info->ies_len, GFP_KERNEL);
> - memcpy(shadow_nw_info->ies, nw_info->ies, nw_info->ies_len);
> + shadow_nw_info->ies = kmemdup(nw_info->ies, nw_info->ies_len,
> +   GFP_KERNEL);

Maybe, in case of NULL, you will want to set ies_len = 0 ?

>   shadow_nw_info->time_scan = jiffies;
>   shadow_nw_info->time_scan_cached = jiffies;
>   shadow_nw_info->found = 1;
> 

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

[Claudiu Beznea]

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.

> 
> 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 = _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;
> - last_scanned_shadow[ap_index].found = 1;
> + kfree(shadow_nw_info->ies);
> + shadow_nw_info->ies = kmalloc(nw_info->ies_len, GFP_KERNEL);
> + memcpy(shadow_nw_info->ies, nw_info->ies, nw_info->ies_len);
> + shadow_nw_info->time_scan = jiffies;
> + shadow_nw_info->time_scan_cached = jiffies;
> + shadow_nw_info->found = 1;
>   if (ap_found != -1)
> - kfree(last_scanned_shadow[ap_index].join_params);
> - last_scanned_shadow[ap_index].join_params = join_params;
> + kfree(shadow_nw_info->join_params);
> + shadow_nw_info->join_params = join_params;
>  }
>  
>  static void cfg_scan_result(enum scan_event scan_event,
> 

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

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Fix line over 80 characters issues reported by checkpatch.pl script in
> wilc_wfi_cfg_tx_vendor_spec() by using temporary variable. Simplified
> 'if else' condition with 'if'.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index 4f35178..8dea414 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -1573,14 +1573,14 @@ static void wilc_wfi_cfg_tx_vendor_spec(struct 
> p2p_mgmt_data *mgmt_tx,
>   for (i = P2P_PUB_ACTION_SUBTYPE + 2; i < len; i++) {
>   if (buf[i] == P2PELEM_ATTR_ID &&
>   !memcmp(p2p_oui, [i + 2], 4)) {
> + bool oper_ch = false;
> + u8 *tx_buff = _tx->buff[i + 6];
> +
>   if (subtype == P2P_INV_REQ || subtype == P2P_INV_RSP)
> - wilc_wfi_cfg_parse_tx_action(_tx->buff[i + 
> 6],
> -  len - (i + 6),
> -  true, iftype);
> - else
> - wilc_wfi_cfg_parse_tx_action(_tx->buff[i + 
> 6],
> -  len - (i + 6),
> -  false, iftype);
> + oper_ch = true;
> +
> + wilc_wfi_cfg_parse_tx_action(tx_buff, len - (i + 6),
> +  oper_ch, iftype);

What about:
wilc_wfi_cfg_parse_tx_action(_tx->buff[i + 6],
 len - (i + 6),
 (subtype == P2P_INV_REQ ||
  subtype == P2P_INV_RSP),
 iftype);


instead all the temporary variables?

>  
>   break;
>   }
> 

Re: [PATCH 28/30] staging: wilc1000: added comments for mutex and spinlock_t

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Added comments for mutex and spinlock_t to avoid checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.h | 2 +-
>  drivers/staging/wilc1000/wilc_wfi_netdevice.h | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.h 
> b/drivers/staging/wilc1000/host_interface.h
> index 7a26f34..5e00dde 100644
> --- a/drivers/staging/wilc1000/host_interface.h
> +++ b/drivers/staging/wilc1000/host_interface.h
> @@ -271,7 +271,7 @@ struct host_if_drv {
>  
>   u8 assoc_bssid[ETH_ALEN];
>   struct cfg_param_attr cfg_values;
> -
> +/*lock to protect concurrent setting of cfg params*/
>   struct mutex cfg_values_lock;
>   struct completion comp_test_key_block;
>   struct completion comp_test_disconn_block;
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
> b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index 607dae0..e517768 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -131,10 +131,11 @@ struct wilc {
>   u8 vif_num;
>   struct wilc_vif *vif[NUM_CONCURRENT_IFC];
>   u8 open_ifcs;
> -
> +/*protect head of transmit queue*/

Add an extra tag to align it with the rest of the structure's members. 

>   struct mutex txq_add_to_head_cs;
> +/*protect txq_entry_t transmit queue*/
Ditto

>   spinlock_t txq_spinlock;
> -
> +/*protect rxq_entry_t receiver queue*/

Ditto

>   struct mutex rxq_cs;
>   struct mutex hif_cs;
>  
> 

Re: [PATCH 30/30] staging: wilc1000: rename s8idxarray to avoid datatype in variable name

[Claudiu Beznea]

On 07.05.2018 11:43, Ajay Singh wrote:
> Cleanup patch to have variable names as per linux coding style.
> 
> Signed-off-by: Ajay Singh 
> ---
>  drivers/staging/wilc1000/host_interface.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index ad9270e..e27010c 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1543,7 +1543,7 @@ static int handle_key(struct wilc_vif *vif, struct 
> key_attr *hif_key)
>   struct wid wid;
>   struct wid wid_list[5];
>   u8 *key_buf;
> - s8 s8idxarray[1];
> + s8 idxarray[1];
>   struct host_if_drv *hif_drv = vif->hif_drv;
>  
>   switch (hif_key->type) {
> @@ -1610,8 +1610,8 @@ static int handle_key(struct wilc_vif *vif, struct 
> key_attr *hif_key)
>   wid.id = (u16)WID_REMOVE_WEP_KEY;
>   wid.type = WID_STR;
>  
> - s8idxarray[0] = (s8)hif_key->attr.wep.index;
> - wid.val = s8idxarray;
> + idxarray[0] = (s8)hif_key->attr.wep.index;

I think you can get rid of idxarray at all.

> + wid.val = idxarray;>wid.size = 1;
>  
>   result = wilc_send_config_pkt(vif, SET_CFG,
> 

Re: [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module

[Mauro Carvalho Chehab]

Em Tue,  8 May 2018 11:12:47 -0700
"Luis R. Rodriguez"  escreveu:

> Clarify the provenance of the firmware loader firmware_class module name
> and why we cannot rename the module in the future.
> 
> Signed-off-by: Luis R. Rodriguez 
> ---
>  .../driver-api/firmware/fallback-mechanisms.rst  | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst 
> b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index a39323ef7d29..a8047be4a96e 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device 
> hierarchy by
>  associating the device used to make the request as the device's parent.
>  The sysfs directory's file attributes are defined and controlled through
>  the new device's class (firmware_class) and group (fw_dev_attr_groups).
> -This is actually where the original firmware_class.c file name comes from,
> -as originally the only firmware loading mechanism available was the
> -mechanism we now use as a fallback mechanism.
> +This is actually where the original firmware_class module name came from,
> +given that originally the only firmware loading mechanism available was the
> +mechanism we now use as a fallback mechanism, which which registers a
> +struct class firmware_class. Because the attributes exposed are part of the
> +module name, the module name firmware_class cannot be renamed in the future, 
> to
> +ensure backward compatibilty with old userspace.

Ah, now the explanation makes a lot more sense to me :-)

Reviewed-by: Mauro Carvalho Chehab 

>  
>  To load firmware using the sysfs interface we expose a loading indicator,
>  and a file upload firmware into:



Thanks,
Mauro

Re: Regression caused by commit 882164a4a928

[Matt Redfearn]

Hi Larry

On 07/05/18 16:44, Larry Finger wrote:

Matt,

Although commit 882164a4a928 ("ssb: Prevent build of PCI host features 
in module") appeared to be harmless, it leads to complete failure of 
drivers b43. and b43legacy, and likely affects b44 as well. The problem 
is that CONFIG_SSB_PCIHOST is undefined, which prevents the compilation 
of the code that controls the PCI cores of the device. See 
https://bugzilla.redhat.com/show_bug.cgi?id=1572349 for details.


Sorry for the breakage :-/



As the underlying errors ("pcibios_enable_device" undefined, and 
"register_pci_controller" undefined) do not appear on the architectures 
that I have tested (x86_64, x86, and ppc), I suspect something in the 
arch-specific code for your setup (MIPS?). As I have no idea on how to 
fix that problem, would the following patch work for you?


diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 9371651d8017..3743533c8057 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -117,7 +117,7 @@ config SSB_SERIAL

  config SSB_DRIVER_PCICORE_POSSIBLE
     bool
-   depends on SSB_PCIHOST && SSB = y
+   depends on SSB_PCIHOST && (SSB = y || !MIPS)
     default y

  config SSB_DRIVER_PCICORE


I believe that the problem stems from these drivers being used for some 
wireless AP functionality built into some MIPS based SoCs. The Kconfig 
rules sort out building this additional functionality when configured 
for MIPS (in a round about sort of way), but it allowed it even when SSB 
is a module, leading to build failures. My patch was intended to prevent 
that.


There was a similar issue in the same Kconfig file, introduced by 
c5611df96804 and fixed by a9e6d44ddecc. It was fixed the same way as you 
suggest. I've tested the above patch and it does work for MIPS 
(preventing the PCICORE being built into the module).


Tested-by: Matt Redfearn 

Thanks & sorry again for the breakage,
Matt





Thanks,

Larry

[PATCH 2/3] staging: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

This patch applies the fix to the rtl8723bs driver in staging while a
separate patch fixes the drivers in the main tree.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen 
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c 
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 46bc2e512557..c76c9a8066c4 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -2431,17 +2431,23 @@ void rtw_cfg80211_indicate_sta_assoc(struct adapter 
*padapter, u8 *pmgmt_frame,
DBG_871X(FUNC_ADPT_FMT"\n", FUNC_ADPT_ARG(padapter));
 
{
-   struct station_info sinfo;
+   struct station_info *sinfo;
u8 ie_offset;
if (GetFrameSubType(pmgmt_frame) == WIFI_ASSOCREQ)
ie_offset = _ASOCREQ_IE_OFFSET_;
else /*  WIFI_REASSOCREQ */
ie_offset = _REASOCREQ_IE_OFFSET_;
 
-   sinfo.filled = 0;
-   sinfo.assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + ie_offset;
-   sinfo.assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - 
ie_offset;
-   cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), , 
GFP_ATOMIC);
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (WARN_ON(!sinfo))
+   return;
+
+   sinfo->filled = 0;
+   sinfo->assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + 
ie_offset;
+   sinfo->assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - 
ie_offset;
+   cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), sinfo, 
GFP_ATOMIC);
+
+   kfree(sinfo);
}
 }
 

[PATCH 1/3] wireless-drivers: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen 
---
 drivers/net/wireless/ath/ath6kl/main.c |   14 ++
 drivers/net/wireless/ath/wil6210/debugfs.c |   22 ++-
 drivers/net/wireless/ath/wil6210/wmi.c |   19 -
 .../broadcom/brcm80211/brcmfmac/cfg80211.c |   18 
 drivers/net/wireless/marvell/mwifiex/uap_event.c   |   25 +++--
 drivers/net/wireless/quantenna/qtnfmac/event.c |   29 +---
 6 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c 
b/drivers/net/wireless/ath/ath6kl/main.c
index db95f85751e3..cf5e4ed5c8fc 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -426,7 +426,7 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, u16 
aid, u8 *mac_addr,
 {
u8 *ies = NULL, *wpa_ie = NULL, *pos;
size_t ies_len = 0;
-   struct station_info sinfo;
+   struct station_info *sinfo;
 
ath6kl_dbg(ATH6KL_DBG_TRC, "new station %pM aid=%d\n", mac_addr, aid);
 
@@ -482,16 +482,20 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, 
u16 aid, u8 *mac_addr,
   keymgmt, ucipher, auth, apsd_info);
 
/* send event to application */
-   memset(, 0, sizeof(sinfo));
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (WARN_ON(!sinfo))
+   return;
 
/* TODO: sinfo.generation */
 
-   sinfo.assoc_req_ies = ies;
-   sinfo.assoc_req_ies_len = ies_len;
+   sinfo->assoc_req_ies = ies;
+   sinfo->assoc_req_ies_len = ies_len;
 
-   cfg80211_new_sta(vif->ndev, mac_addr, , GFP_KERNEL);
+   cfg80211_new_sta(vif->ndev, mac_addr, sinfo, GFP_KERNEL);
 
netif_wake_queue(vif->ndev);
+
+   kfree(sinfo);
 }
 
 void disconnect_timer_handler(struct timer_list *t)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c 
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 8c90b3111f0b..11e46e44381e 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1200,8 +1200,12 @@ static const struct file_operations fops_freq = {
 static int wil_link_debugfs_show(struct seq_file *s, void *data)
 {
struct wil6210_priv *wil = s->private;
-   struct station_info sinfo;
-   int i, rc;
+   struct station_info *sinfo;
+   int i, rc = 0;
+
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return -ENOMEM;
 
for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
struct wil_sta_info *p = >sta[i];
@@ -1229,19 +1233,21 @@ static int wil_link_debugfs_show(struct seq_file *s, 
void *data)
 
vif = (mid < wil->max_vifs) ? wil->vifs[mid] : NULL;
if (vif) {
-   rc = wil_cid_fill_sinfo(vif, i, );
+   rc = wil_cid_fill_sinfo(vif, i, sinfo);
if (rc)
-   return rc;
+   goto out;
 
-   seq_printf(s, "  Tx_mcs = %d\n", sinfo.txrate.mcs);
-   seq_printf(s, "  Rx_mcs = %d\n", sinfo.rxrate.mcs);
-   seq_printf(s, "  SQ = %d\n", sinfo.signal);
+   seq_printf(s, "  Tx_mcs = %d\n", sinfo->txrate.mcs);
+   seq_printf(s, "  Rx_mcs = %d\n", sinfo->rxrate.mcs);
+   seq_printf(s, "  SQ = %d\n", sinfo->signal);
} else {
seq_puts(s, "  INVALID MID\n");
}
}
 
-   return 0;
+out:
+   kfree(sinfo);
+   return rc;
 }
 
 static int wil_link_seq_open(struct inode *inode, struct file *file)
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c 
b/drivers/net/wireless/ath/wil6210/wmi.c
index a3dda9a97c1f..21124af06bdd 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -824,7 +824,7 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int 
id, void *d, int len)
struct wireless_dev *wdev = vif_to_wdev(vif);
struct wmi_connect_event *evt = d;
int ch; /* channel number */
-   struct station_info sinfo;
+   struct station_info *sinfo;
u8 *assoc_req_ie, *assoc_resp_ie;
size_t assoc_req_ielen, assoc_resp_ielen;
/* capinfo(u16) + listen_interval(u16) + IEs */
@@ -940,6 +940,11 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int 
id, void *d, int len)
vif->bss = NULL;
} else if 

[PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

This patch applies the fix to batman-adv and wext-compat, while a
separate patch fixes up the drivers.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen 
---
 net/batman-adv/bat_v_elp.c |   21 +++--
 net/wireless/wext-compat.c |   29 +
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 28687493599f..d2393ebc6af4 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -79,8 +79,9 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
struct batadv_hard_iface *hard_iface = neigh->if_incoming;
struct ethtool_link_ksettings link_settings;
struct net_device *real_netdev;
-   struct station_info sinfo;
+   struct station_info *sinfo;
u32 throughput;
+   bool filled;
int ret;
 
/* if the user specified a customised value for this interface, then
@@ -102,7 +103,17 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
if (!real_netdev)
goto default_throughput;
 
-   ret = cfg80211_get_station(real_netdev, neigh->addr, );
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   goto default_throughput;
+
+   ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
+
+   /* just save these here instead of having complex free logic 
below */
+   throughput = sinfo.expected_throughput / 100;
+   filled = !!(sinfo.filled & 
BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
+
+   kfree(sinfo);
 
dev_put(real_netdev);
if (ret == -ENOENT) {
@@ -112,12 +123,10 @@ static u32 batadv_v_elp_get_throughput(struct 
batadv_hardif_neigh_node *neigh)
 */
return 0;
}
-   if (ret)
-   goto default_throughput;
-   if (!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT)))
+   if (ret || !filled)
goto default_throughput;
 
-   return sinfo.expected_throughput / 100;
+   return throughput;
}
 
/* if not a wifi interface, check if this device provides data via
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 9e002df0f8d8..2038e3fb25fa 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -1300,7 +1300,7 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
/* we are under RTNL - globally locked - so can use static structs */
static struct iw_statistics wstats;
-   static struct station_info sinfo;
+   static struct station_info *sinfo;
u8 bssid[ETH_ALEN];
 
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION)
@@ -1318,17 +1318,21 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
memcpy(bssid, wdev->current_bss->pub.bssid, ETH_ALEN);
wdev_unlock(wdev);
 
-   memset(, 0, sizeof(sinfo));
+   sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+   if (!sinfo)
+   return NULL;
 
-   if (rdev_get_station(rdev, dev, bssid, ))
+   if (rdev_get_station(rdev, dev, bssid, sinfo)) {
+   kfree(sinfo);
return NULL;
+   }
 
memset(, 0, sizeof(wstats));
 
switch (rdev->wiphy.signal_type) {
case CFG80211_SIGNAL_TYPE_MBM:
-   if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
-   int sig = sinfo.signal;
+   if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+   int sig = sinfo->signal;
wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
wstats.qual.updated |= IW_QUAL_DBM;
@@ -1341,11 +1345,11 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
break;
}
case CFG80211_SIGNAL_TYPE_UNSPEC:
-   if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+   if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
-   

Re: [PATCH 09/18] net: mac80211.h: fix a bad comment line

[Johannes Berg]

On Wed, 2018-05-09 at 09:04 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 07 May 2018 14:38:26 +0200
> Johannes Berg  escreveu:
> 
> > On Mon, 2018-05-07 at 15:37 +0300, Kalle Valo wrote:
> > > Mauro Carvalho Chehab  writes:
> > >   
> > > > Sphinx produces a lot of errors like this:
> > > > ./include/net/mac80211.h:2083: warning: bad line:  >
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab   
> > > 
> > > Randy already submitted a similar patch:
> > > 
> > > https://patchwork.kernel.org/patch/10367275/
> > > 
> > > But it seems Johannes has not applied that yet.  
> > 
> > Yeah, I've been super busy preparing for the plugfest.
> > 
> > I'll make a pass over all the patches as soon as I can, hopefully today
> > or tomorrow.
> 
> Thanks. I'll drop it from my patchset, assuming that you'll
> be applying Randy's version or mine via your tree.

Right, I did, just need to send a pull request.

johannes

Re: [PATCH 09/18] net: mac80211.h: fix a bad comment line

[Mauro Carvalho Chehab]

Em Mon, 07 May 2018 14:38:26 +0200
Johannes Berg  escreveu:

> On Mon, 2018-05-07 at 15:37 +0300, Kalle Valo wrote:
> > Mauro Carvalho Chehab  writes:
> >   
> > > Sphinx produces a lot of errors like this:
> > >   ./include/net/mac80211.h:2083: warning: bad line:  >
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab   
> > 
> > Randy already submitted a similar patch:
> > 
> > https://patchwork.kernel.org/patch/10367275/
> > 
> > But it seems Johannes has not applied that yet.  
> 
> Yeah, I've been super busy preparing for the plugfest.
> 
> I'll make a pass over all the patches as soon as I can, hopefully today
> or tomorrow.

Thanks. I'll drop it from my patchset, assuming that you'll
be applying Randy's version or mine via your tree.

Thanks,
Mauro

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Toke Høiland-Jørgensen]

Kalle Valo  writes:

> Toke Høiland-Jørgensen  writes:
>
>> Kalle Valo  writes:
>>
>>> Toke Høiland-Jørgensen  writes:
>>>
 Kalle Valo  writes:

> Johannes Berg  writes:
>
>> On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
>>> Johannes Berg  writes:
>>> 
>>> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
>>> > > Johannes Berg  writes:
>>> > > 
>>> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>>> > > > > 
>>> > > > > I guess these warnings come because Toke's patch increased size 
>>> > > > > of
>>> > > > > struct cfg80211_tid_stats (which is included in struct 
>>> > > > > station_info) and
>>> > > > > both wil6210 and qtnfmac allocate a struct station_info from 
>>> > > > > stack? 
>>> > > > 
>>> > > > Yes.
>>> > > > 
>>> > > > > Can
>>> > > > > someone send a fix for the drivers?
>>> > > > 
>>> > > > I guess Toke/I should do that through my tree.
>>> > > 
>>> > > IMHO the fix could go through my tree as well, less risk of 
>>> > > conflicts in
>>> > > drivers. AFAICS the fix (allocating station_info dynamically?) 
>>> > > would not
>>> > > depend on Toke's patch and could be applied separately.
>>> > 
>>> > That's true, if you prefer that it's fine with me.
>>> 
>>> I'll send a patch.
>>> 
>>> What's the right tag to put in the commit for this?
>>> Fixes-but-is-independent-from: ? ;)
>>
>> Heh. You can still put Fixes: I think.
>
> Yeah, I think so too.

 Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
 in staging; that should be fixed as well, right? In the same commit?
>>>
>>> Not in the same commit at least, I don't want to touch staging even with
>>> a ten foot pole :) I guess either Greg or Johannes would take that
>>> patch.
>>
>> What about batman-adv and wext-compat? Should I split those out as well?
>
> So normally I only take patches touching drivers/net/wireless, anything
> else has to go via other trees.

Right, thought so; will send a series as soon as I've verified that it
compiles :)

-Toke

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

[Mimi Zohar]

On Tue, 2018-05-08 at 17:34 +, Luis R. Rodriguez wrote:
> On Thu, May 03, 2018 at 08:24:26PM -0400, Mimi Zohar wrote:
> > On Fri, 2018-05-04 at 00:07 +, Luis R. Rodriguez wrote:
> > > On Tue, May 01, 2018 at 09:48:20AM -0400, Mimi Zohar wrote:
> > > > Allow LSMs and IMA to differentiate between signed regulatory.db and
> > > > other firmware.
> > > > 
> > > > Signed-off-by: Mimi Zohar 
> > > > Cc: Luis R. Rodriguez 
> > > > Cc: David Howells 
> > > > Cc: Kees Cook 
> > > > Cc: Seth Forshee 
> > > > Cc: Johannes Berg 
> > > > ---
> > > >  drivers/base/firmware_loader/main.c | 5 +
> > > >  include/linux/fs.h  | 1 +
> > > >  2 files changed, 6 insertions(+)
> > > > 
> > > > 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
> > > 
> > > Whoa, no way.
> > 
> > There are two methods for the kernel to verify firmware signatures.
> 
> Yes, but although CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is its own kernel
> mechanism to verify firmware it uses the request_firmware*() API for
> regulatory.db and regulatory.db.p7s, and IMA already can appraise these two
> files since the firmware API is used.

IMA-appraisal can verify a signature stored as an xattr, but not a
detached signature.  That support could be added, but isn't there
today.  Today, a regulatory.db signature would have to be stored as an
xattr. 

> 
> As such I see no reason to add a new ID for them at all.
> K
> Its not providing an *alternative*, its providing an *extra* kernel measure.
> If anything CONFIG_CFG80211_REQUIRE_SIGNED_REGDB perhaps should be its own
> stacked LSM. I'd be open to see patches which set that out. May be a
> cleaner interface.
> 
> > If both are enabled, do we require both signatures or is one enough.
> 
> Good question. Considering it as a stacked LSM (although not implemented
> as one), I'd say its up to who enabled the Kconfig entries. If IMA and
> CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are enabled then both. If someone enabled
> IMA though, then surely I agree that enabling
> CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is stupid and redundant, but its up to 
> the
> system integrator to decide.

Just because IMA-appraisal is enabled in the kernel doesn't mean that
firmware signatures will be verified.  That is a run time policy
decision.

> 
> If we however want to make it clear that such things as
> CONFIG_CFG80211_REQUIRE_SIGNED_REGDB are not required when IMA is enabled we
> could just make the kconfig depend on !IMA or something?  Or perhaps a new
> kconfig for IMA which if selected it means that drivers can opt to open code
> *further* kernel signature verification, even though IMA already is 
> sufficient.
> Perhaps CONFIG_ENABLE_IMA_OVERLAPPING, and the driver depends on it?

The existing CONFIG_IMA_APPRAISE is not enough.  If there was a build
time IMA config that translated into an IMA policy requiring firmware
signature verification (eg. CONFIG_IMA_APPRAISE_FIRMWARE), this could
be sorted out at build time.

> 
> > Assigning a different id for regdb signed firmware allows LSMs and IMA
> > to handle regdb files differently.
> 
> That's not the main concern here, its the precedent we are setting here for
> any new kernel interface which open codes firmware signing on its own. What
> you are doing means other kernel users who open codes their own firmware
> signing may need to add yet-another reading ID. That doesn't either look
> well on code, and seems kind of silly from a coding perspective given
> the above, in which I clarify IMA still is doing its own appraisal on it.

Suppose,

1. Either CONFIG_CFG80211_REQUIRE_SIGNED_REGDB or
"CONFIG_IMA_APPRAISE_FIRMWARE" would be configured at build.

2. If CONFIG_CFG80211_REQUIRE_SIGNED_REGDB is configured, not
"CONFIG_IMA_APPRAISE_FIRMWARE", a custom IMA-policy rule that
appraises the firmware signature could be defined.  In this case, both
signature verification methods would be enforced.

then READING_FIRMWARE_REGULATORY_DB would not be needed.

Mimi

> 
> > > > fw_priv->size = 0;
> > > > rc = kernel_read_file_from_path(path, _priv->data, 
> > > > ,
> > > > 

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Kalle Valo]

Toke Høiland-Jørgensen  writes:

> Kalle Valo  writes:
>
>> Toke Høiland-Jørgensen  writes:
>>
>>> Kalle Valo  writes:
>>>
 Johannes Berg  writes:

> On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
>> Johannes Berg  writes:
>> 
>> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
>> > > Johannes Berg  writes:
>> > > 
>> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>> > > > > 
>> > > > > I guess these warnings come because Toke's patch increased size 
>> > > > > of
>> > > > > struct cfg80211_tid_stats (which is included in struct 
>> > > > > station_info) and
>> > > > > both wil6210 and qtnfmac allocate a struct station_info from 
>> > > > > stack? 
>> > > > 
>> > > > Yes.
>> > > > 
>> > > > > Can
>> > > > > someone send a fix for the drivers?
>> > > > 
>> > > > I guess Toke/I should do that through my tree.
>> > > 
>> > > IMHO the fix could go through my tree as well, less risk of 
>> > > conflicts in
>> > > drivers. AFAICS the fix (allocating station_info dynamically?) would 
>> > > not
>> > > depend on Toke's patch and could be applied separately.
>> > 
>> > That's true, if you prefer that it's fine with me.
>> 
>> I'll send a patch.
>> 
>> What's the right tag to put in the commit for this?
>> Fixes-but-is-independent-from: ? ;)
>
> Heh. You can still put Fixes: I think.

 Yeah, I think so too.
>>>
>>> Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
>>> in staging; that should be fixed as well, right? In the same commit?
>>
>> Not in the same commit at least, I don't want to touch staging even with
>> a ten foot pole :) I guess either Greg or Johannes would take that
>> patch.
>
> What about batman-adv and wext-compat? Should I split those out as well?

So normally I only take patches touching drivers/net/wireless, anything
else has to go via other trees.

-- 
Kalle Valo

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

[Sriram R]

On 2018-05-01 01:20, Peter Oh wrote:

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?

+#define ATH10K_WMI_DFS_CONF_TIMEOUT_HZ (HZ / 2)
  +static void ath10k_radar_confirmation_work(struct work_struct 
*work)

+{
+   struct ath10k *ar = container_of(work, struct ath10k,
+radar_confirmation_work);
+   struct ath10k_radar_found_info radar_info;
+   int ret, time_left;
+
+   reinit_completion(>wmi.radar_confirm);
+
+   spin_lock_bh(>data_lock);
+   memcpy(_info, >last_radar_info, sizeof(radar_info));
+   spin_unlock_bh(>data_lock);
+
+   ret = ath10k_wmi_report_radar_found(ar, _info);
+   if (ret) {
+   ath10k_warn(ar, "failed to send radar found %d\n", ret);
+   goto wait_complete;
+   }
+
+   time_left = wait_for_completion_timeout(>wmi.radar_confirm,
+   ATH10K_WMI_DFS_CONF_TIMEOUT_HZ);

It looks wrong to me in terms of timeout value.
Typical channel closing time in FCC domain is 200ms (excluding control
signals), but you're waiting for 500ms for response from FW.
Right Peter, We haven't hit this limit during our testing. On an average 
we
received completion event within few milliseconds ,say less than 
10-15ms. I'll correct

this timeout in my next patch revision.
@@ -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, , NULL)) {
+   if (!ar->dfs_detector->add_pulse(ar->dfs_detector, , )) {
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.

+static void
+ath10k_wmi_event_dfs_status_check(struct ath10k *ar, struct sk_buff 
*skb)

+{
+   struct wmi_dfs_status_ev_arg status_arg = {};
+   int ret;
+
+   ret = ath10k_wmi_pull_dfs_status(ar, skb, _arg);
+
+   if (ret) {
+   ath10k_warn(ar, "failed to parse dfs status event: %d\n", ret);
+   return;
+   }
+
+   ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
+  "dfs status event received from fw: %d\n",
+  status_arg.status);
+
+   /* Even in case of radar detection failure we follow the same
+* behaviour as if radar is detected i.e to switch to a different
+* channel.
+*/
+   if (status_arg.status == WMI_HW_RADAR_DETECTED ||
+   status_arg.status == WMI_RADAR_DETECTION_FAIL)
+   ath10k_radar_detected(ar);
+   complete(>wmi.radar_confirm);
+}

What is typical average duration from
wait_for_completion_timeout(>wmi.radar_confirm) to this completion
called ?

We didn't see this taking more than 20ms to reach this completion.
As mentioned above I'll change the timeout value in the next patch.

Thanks,
Sriram.R


Thanks,
Peter

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Toke Høiland-Jørgensen]

Kalle Valo  writes:

> Toke Høiland-Jørgensen  writes:
>
>> Kalle Valo  writes:
>>
>>> Johannes Berg  writes:
>>>
 On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
> Johannes Berg  writes:
> 
> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
> > > Johannes Berg  writes:
> > > 
> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
> > > > > 
> > > > > I guess these warnings come because Toke's patch increased size of
> > > > > struct cfg80211_tid_stats (which is included in struct 
> > > > > station_info) and
> > > > > both wil6210 and qtnfmac allocate a struct station_info from 
> > > > > stack? 
> > > > 
> > > > Yes.
> > > > 
> > > > > Can
> > > > > someone send a fix for the drivers?
> > > > 
> > > > I guess Toke/I should do that through my tree.
> > > 
> > > IMHO the fix could go through my tree as well, less risk of conflicts 
> > > in
> > > drivers. AFAICS the fix (allocating station_info dynamically?) would 
> > > not
> > > depend on Toke's patch and could be applied separately.
> > 
> > That's true, if you prefer that it's fine with me.
> 
> I'll send a patch.
> 
> What's the right tag to put in the commit for this?
> Fixes-but-is-independent-from: ? ;)

 Heh. You can still put Fixes: I think.
>>>
>>> Yeah, I think so too.
>>
>> Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
>> in staging; that should be fixed as well, right? In the same commit?
>
> Not in the same commit at least, I don't want to touch staging even with
> a ten foot pole :) I guess either Greg or Johannes would take that
> patch.

What about batman-adv and wext-compat? Should I split those out as well?

-Toke

Re: mwifiex: increase TX threashold to avoid TX timeout during ED MAC test

[Kalle Valo]

Ganapathi Bhat  wrote:

> While carrying energy detection(ED) tests, the chip will stop
> transmission upon detecting an energy in the connected channel.
> As a feedback, driver will stop dequeuing TX packets and due to
> which wmm_tx_pending keep incremeting. Once wmm_tx_pending
> reaches 100, driver calls netif_tx_stop_queue(). If TX ques is
> not restarted within 5(watchdog_timeo) seconds, it will result in
> TX timeout.
> 
> The ED test is carried out for one minute and the current
> threshold of 100 is easily overcome by the traffic, cuasing TX
> timeouts. To fix this increase the threshold to 400.
> 
> Signed-off-by: Cathy Luo 
> Signed-off-by: Ganapathi Bhat 

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

4f9fb990013c mwifiex: increase TX threashold to avoid TX timeout during ED MAC 
test

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

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

Re: mwifiex: delete unneeded include

[Kalle Valo]

Julia Lawall  wrote:

> Nothing that is defined in 11ac.h is referenced in cmdevt.c.
> 
> Signed-off-by: Julia Lawall 

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

4793e5a954fa mwifiex: delete unneeded include

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

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

Re: brcmfmac: Add support for bcm43364 wireless chipset

[Kalle Valo]

Sean Lanigan  wrote:

> Add support for the BCM43364 chipset via an SDIO interface, as used in 
> e.g. the Murata 1FX module. 
> 
> The BCM43364 uses the same firmware as the BCM43430 (which is already 
> included), the only difference is the omission of Bluetooth. 
> 
> However, the SDIO_ID for the BCM43364 is 02D0:A9A4, giving it a MODALIAS 
> of sdio:c00v02D0dA9A4, which doesn't get recognised and hence doesn't 
> load the brcmfmac module. Adding the 'A9A4' ID in the appropriate place 
> triggers the brcmfmac driver to load, and then correctly use the 
> firmware file 'brcmfmac43430-sdio.bin'. 
> 
> 
> Signed-off-by: Sean Lanigan 
> Acked-by: Ulf Hansson 

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

9c4a121e8263 brcmfmac: Add support for bcm43364 wireless chipset

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

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

Re: rtlwifi: remove duplicate definition of antenna number for btcoex

[Kalle Valo]

Ping-Ke Shih  wrote:

> From: Ping-Ke Shih 
> 
> Two enumerations bt_total_ant_num and bt_ant_num are identical, so one
> can be removed.
> 
> Signed-off-by: Ping-Ke Shih 

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

bf516e7d8b1c rtlwifi: remove duplicate definition of antenna number for btcoex

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

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

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Toke Høiland-Jørgensen]

Kalle Valo  writes:

> Toke Høiland-Jørgensen  writes:
>
>> Kalle Valo  writes:
>>
>>> Johannes Berg  writes:
>>>
 On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
> Johannes Berg  writes:
> 
> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
> > > Johannes Berg  writes:
> > > 
> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
> > > > > 
> > > > > I guess these warnings come because Toke's patch increased size of
> > > > > struct cfg80211_tid_stats (which is included in struct 
> > > > > station_info) and
> > > > > both wil6210 and qtnfmac allocate a struct station_info from 
> > > > > stack? 
> > > > 
> > > > Yes.
> > > > 
> > > > > Can
> > > > > someone send a fix for the drivers?
> > > > 
> > > > I guess Toke/I should do that through my tree.
> > > 
> > > IMHO the fix could go through my tree as well, less risk of conflicts 
> > > in
> > > drivers. AFAICS the fix (allocating station_info dynamically?) would 
> > > not
> > > depend on Toke's patch and could be applied separately.
> > 
> > That's true, if you prefer that it's fine with me.
> 
> I'll send a patch.
> 
> What's the right tag to put in the commit for this?
> Fixes-but-is-independent-from: ? ;)

 Heh. You can still put Fixes: I think.
>>>
>>> Yeah, I think so too.
>>
>> Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
>> in staging; that should be fixed as well, right? In the same commit?
>
> Not in the same commit at least, I don't want to touch staging even with
> a ten foot pole :) I guess either Greg or Johannes would take that
> patch.

Gotcha, I'll split that out :D

-Toke

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Kalle Valo]

Toke Høiland-Jørgensen  writes:

> Kalle Valo  writes:
>
>> Johannes Berg  writes:
>>
>>> On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
 Johannes Berg  writes:
 
 > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
 > > Johannes Berg  writes:
 > > 
 > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
 > > > > 
 > > > > I guess these warnings come because Toke's patch increased size of
 > > > > struct cfg80211_tid_stats (which is included in struct 
 > > > > station_info) and
 > > > > both wil6210 and qtnfmac allocate a struct station_info from 
 > > > > stack? 
 > > > 
 > > > Yes.
 > > > 
 > > > > Can
 > > > > someone send a fix for the drivers?
 > > > 
 > > > I guess Toke/I should do that through my tree.
 > > 
 > > IMHO the fix could go through my tree as well, less risk of conflicts 
 > > in
 > > drivers. AFAICS the fix (allocating station_info dynamically?) would 
 > > not
 > > depend on Toke's patch and could be applied separately.
 > 
 > That's true, if you prefer that it's fine with me.
 
 I'll send a patch.
 
 What's the right tag to put in the commit for this?
 Fixes-but-is-independent-from: ? ;)
>>>
>>> Heh. You can still put Fixes: I think.
>>
>> Yeah, I think so too.
>
> Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
> in staging; that should be fixed as well, right? In the same commit?

Not in the same commit at least, I don't want to touch staging even with
a ten foot pole :) I guess either Greg or Johannes would take that
patch.

-- 
Kalle Valo

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Toke Høiland-Jørgensen]

Kalle Valo  writes:

> Johannes Berg  writes:
>
>> On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
>>> Johannes Berg  writes:
>>> 
>>> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
>>> > > Johannes Berg  writes:
>>> > > 
>>> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>>> > > > > 
>>> > > > > I guess these warnings come because Toke's patch increased size of
>>> > > > > struct cfg80211_tid_stats (which is included in struct 
>>> > > > > station_info) and
>>> > > > > both wil6210 and qtnfmac allocate a struct station_info from stack? 
>>> > > > 
>>> > > > Yes.
>>> > > > 
>>> > > > > Can
>>> > > > > someone send a fix for the drivers?
>>> > > > 
>>> > > > I guess Toke/I should do that through my tree.
>>> > > 
>>> > > IMHO the fix could go through my tree as well, less risk of conflicts in
>>> > > drivers. AFAICS the fix (allocating station_info dynamically?) would not
>>> > > depend on Toke's patch and could be applied separately.
>>> > 
>>> > That's true, if you prefer that it's fine with me.
>>> 
>>> I'll send a patch.
>>> 
>>> What's the right tag to put in the commit for this?
>>> Fixes-but-is-independent-from: ? ;)
>>
>> Heh. You can still put Fixes: I think.
>
> Yeah, I think so too.

Cool. My "git grep 'struct station_info sinfo'" also shows up a driver
in staging; that should be fixed as well, right? In the same commit?

-Toke

[PATCH v3 7/8] wil6210: remove unused rx_reorder members

[Maya Erez]

From: Dedy Lansky 

Remove unused members from struct wil_tid_ampdu_rx

Signed-off-by: Dedy Lansky 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/debugfs.c|  3 +--
 drivers/net/wireless/ath/wil6210/rx_reorder.c |  7 +--
 drivers/net/wireless/ath/wil6210/wil6210.h| 10 --
 3 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c 
b/drivers/net/wireless/ath/wil6210/debugfs.c
index d3b1069..524a7d6 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1379,8 +1379,7 @@ static void wil_print_rxtid(struct seq_file *s, struct 
wil_tid_ampdu_rx *r)
u16 index = ((r->head_seq_num - r->ssn) & 0xfff) % r->buf_size;
unsigned long long drop_dup = r->drop_dup, drop_old = r->drop_old;
 
-   seq_printf(s, "([%2d] %3d TU) 0x%03x [", r->buf_size, r->timeout,
-  r->head_seq_num);
+   seq_printf(s, "([%2d]) 0x%03x [", r->buf_size, r->head_seq_num);
for (i = 0; i < r->buf_size; i++) {
if (i == index)
seq_printf(s, "%c", r->reorder_buf[i] ? 'O' : '|');
diff --git a/drivers/net/wireless/ath/wil6210/rx_reorder.c 
b/drivers/net/wireless/ath/wil6210/rx_reorder.c
index 14dcb06..76f8084 100644
--- a/drivers/net/wireless/ath/wil6210/rx_reorder.c
+++ b/drivers/net/wireless/ath/wil6210/rx_reorder.c
@@ -206,7 +206,6 @@ void wil_rx_reorder(struct wil6210_priv *wil, struct 
sk_buff *skb)
 
/* put the frame in the reordering buffer */
r->reorder_buf[index] = skb;
-   r->reorder_time[index] = jiffies;
r->stored_mpdu_num++;
wil_reorder_release(ndev, r);
 
@@ -252,11 +251,8 @@ struct wil_tid_ampdu_rx *wil_tid_ampdu_rx_alloc(struct 
wil6210_priv *wil,
 
r->reorder_buf =
kcalloc(size, sizeof(struct sk_buff *), GFP_KERNEL);
-   r->reorder_time =
-   kcalloc(size, sizeof(unsigned long), GFP_KERNEL);
-   if (!r->reorder_buf || !r->reorder_time) {
+   if (!r->reorder_buf) {
kfree(r->reorder_buf);
-   kfree(r->reorder_time);
kfree(r);
return NULL;
}
@@ -286,7 +282,6 @@ void wil_tid_ampdu_rx_free(struct wil6210_priv *wil,
kfree_skb(r->reorder_buf[i]);
 
kfree(r->reorder_buf);
-   kfree(r->reorder_time);
kfree(r);
 }
 
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h 
b/drivers/net/wireless/ath/wil6210/wil6210.h
index c9120dc..b623510c 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -493,38 +493,28 @@ enum { /* for wil6210_priv.status */
  * struct tid_ampdu_rx - TID aggregation information (Rx).
  *
  * @reorder_buf: buffer to reorder incoming aggregated MPDUs
- * @reorder_time: jiffies when skb was added
- * @session_timer: check if peer keeps Tx-ing on the TID (by timeout value)
- * @reorder_timer: releases expired frames from the reorder buffer.
  * @last_rx: jiffies of last rx activity
  * @head_seq_num: head sequence number in reordering buffer.
  * @stored_mpdu_num: number of MPDUs in reordering buffer
  * @ssn: Starting Sequence Number expected to be aggregated.
  * @buf_size: buffer size for incoming A-MPDUs
- * @timeout: reset timer value (in TUs).
  * @ssn_last_drop: SSN of the last dropped frame
  * @total: total number of processed incoming frames
  * @drop_dup: duplicate frames dropped for this reorder buffer
  * @drop_old: old frames dropped for this reorder buffer
- * @dialog_token: dialog token for aggregation session
  * @first_time: true when this buffer used 1-st time
  */
 struct wil_tid_ampdu_rx {
struct sk_buff **reorder_buf;
-   unsigned long *reorder_time;
-   struct timer_list session_timer;
-   struct timer_list reorder_timer;
unsigned long last_rx;
u16 head_seq_num;
u16 stored_mpdu_num;
u16 ssn;
u16 buf_size;
-   u16 timeout;
u16 ssn_last_drop;
unsigned long long total; /* frames processed */
unsigned long long drop_dup;
unsigned long long drop_old;
-   u8 dialog_token;
bool first_time; /* is it 1-st time this buffer used? */
 };
 
-- 
1.9.1

[PATCH v3 4/8] wil6210: change reply_size arg to u16 in wmi_call

[Maya Erez]

From: Lior David 

Change the type of the argument reply_size from u8 to
u16 in order to support reply sizes > 255 bytes.
The driver already supports u16 reply size in all
other places, this was the only missing change.

Signed-off-by: Lior David 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/wil6210.h | 2 +-
 drivers/net/wireless/ath/wil6210/wmi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h 
b/drivers/net/wireless/ath/wil6210/wil6210.h
index f9c5155..9ac9292 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -986,7 +986,7 @@ int wmi_read_hdr(struct wil6210_priv *wil, __le32 ptr,
 int wmi_send(struct wil6210_priv *wil, u16 cmdid, u8 mid, void *buf, u16 len);
 void wmi_recv_cmd(struct wil6210_priv *wil);
 int wmi_call(struct wil6210_priv *wil, u16 cmdid, u8 mid, void *buf, u16 len,
-u16 reply_id, void *reply, u8 reply_size, int to_msec);
+u16 reply_id, void *reply, u16 reply_size, int to_msec);
 void wmi_event_worker(struct work_struct *work);
 void wmi_event_flush(struct wil6210_priv *wil);
 int wmi_set_ssid(struct wil6210_vif *vif, u8 ssid_len, const void *ssid);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c 
b/drivers/net/wireless/ath/wil6210/wmi.c
index 73efa13..fcd9529 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -1416,7 +1416,7 @@ void wmi_recv_cmd(struct wil6210_priv *wil)
 }
 
 int wmi_call(struct wil6210_priv *wil, u16 cmdid, u8 mid, void *buf, u16 len,
-u16 reply_id, void *reply, u8 reply_size, int to_msec)
+u16 reply_id, void *reply, u16 reply_size, int to_msec)
 {
int rc;
unsigned long remain;
-- 
1.9.1

[PATCH v3 1/8] wil6210: disable tracing config option

[Maya Erez]

From: Alexei Avshalom Lazar 

Disable WIL6210_TRACING by default to avoid its performance overhead.

Signed-off-by: Alexei Avshalom Lazar 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wil6210/Kconfig 
b/drivers/net/wireless/ath/wil6210/Kconfig
index b448926..3548e8d 100644
--- a/drivers/net/wireless/ath/wil6210/Kconfig
+++ b/drivers/net/wireless/ath/wil6210/Kconfig
@@ -33,7 +33,7 @@ config WIL6210_TRACING
bool "wil6210 tracing support"
depends on WIL6210
depends on EVENT_TRACING
-   default y
+   default n
---help---
  Say Y here to enable tracepoints for the wil6210 driver
  using the kernel tracing infrastructure.  Select this
-- 
1.9.1

[PATCH v3 3/8] wil6210: fix call to wil6210_disconnect during unload

[Maya Erez]

From: Lior David 

Move the call to wil6210_disconnect so it will be called
before unregister_netdevice. This is because it calls
netif_carrier_off which is forbidden to call on an
unregistered net device. Calling netif_carrier_off can
add a link watch event which might be handled after
net device was freed, causing a kernel oops.

Signed-off-by: Lior David 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/netdev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/netdev.c 
b/drivers/net/wireless/ath/wil6210/netdev.c
index 05e9408..eb6c14ed 100644
--- a/drivers/net/wireless/ath/wil6210/netdev.c
+++ b/drivers/net/wireless/ath/wil6210/netdev.c
@@ -457,16 +457,16 @@ void wil_vif_remove(struct wil6210_priv *wil, u8 mid)
return;
}
 
+   mutex_lock(>mutex);
+   wil6210_disconnect(vif, NULL, WLAN_REASON_DEAUTH_LEAVING, false);
+   mutex_unlock(>mutex);
+
ndev = vif_to_ndev(vif);
/* during unregister_netdevice cfg80211_leave may perform operations
 * such as stop AP, disconnect, so we only clear the VIF afterwards
 */
unregister_netdevice(ndev);
 
-   mutex_lock(>mutex);
-   wil6210_disconnect(vif, NULL, WLAN_REASON_DEAUTH_LEAVING, false);
-   mutex_unlock(>mutex);
-
if (any_active && vif->mid != 0)
wmi_port_delete(wil, vif->mid);
 
-- 
1.9.1

[PATCH v3 2/8] wil6210: align to latest auto generated wmi.h

[Maya Erez]

From: Ahmad Masri 

Align to latest version of the auto generated wmi file
describing the interface with FW

Signed-off-by: Ahmad Masri 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/wmi.c |   6 +-
 drivers/net/wireless/ath/wil6210/wmi.h | 387 +++--
 2 files changed, 371 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/wmi.c 
b/drivers/net/wireless/ath/wil6210/wmi.c
index a3dda9a..73efa13 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -1554,7 +1554,9 @@ int wmi_pcp_start(struct wil6210_vif *vif,
.pcp_max_assoc_sta = max_assoc_sta,
.hidden_ssid = hidden_ssid,
.is_go = is_go,
-   .disable_ap_sme = disable_ap_sme,
+   .ap_sme_offload_mode = disable_ap_sme ?
+  WMI_AP_SME_OFFLOAD_PARTIAL :
+  WMI_AP_SME_OFFLOAD_FULL,
.abft_len = wil->abft_len,
};
struct {
@@ -1574,7 +1576,7 @@ int wmi_pcp_start(struct wil6210_vif *vif,
}
 
if (disable_ap_sme &&
-   !test_bit(WMI_FW_CAPABILITY_DISABLE_AP_SME,
+   !test_bit(WMI_FW_CAPABILITY_AP_SME_OFFLOAD_PARTIAL,
  wil->fw_capabilities)) {
wil_err(wil, "disable_ap_sme not supported by FW\n");
return -EOPNOTSUPP;
diff --git a/drivers/net/wireless/ath/wil6210/wmi.h 
b/drivers/net/wireless/ath/wil6210/wmi.h
index d3e75f0..dc503d9 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.h
+++ b/drivers/net/wireless/ath/wil6210/wmi.h
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
  * Copyright (c) 2012-2017 Qualcomm Atheros, Inc.
  * Copyright (c) 2006-2012 Wilocity
  *
@@ -29,8 +30,6 @@
 #ifndef __WILOCITY_WMI_H__
 #define __WILOCITY_WMI_H__
 
-/* General */
-#define WMI_MAX_ASSOC_STA  (8)
 #define WMI_DEFAULT_ASSOC_STA  (1)
 #define WMI_MAC_LEN(6)
 #define WMI_PROX_RANGE_NUM (3)
@@ -41,6 +40,19 @@
 #define WMI_RF_ETYPE_LENGTH(3)
 #define WMI_RF_RX2TX_LENGTH(3)
 #define WMI_RF_ETYPE_VAL_PER_RANGE (5)
+/* DTYPE configuration array size
+ * must always be kept equal to (WMI_RF_DTYPE_LENGTH+1)
+ */
+#define WMI_RF_DTYPE_CONF_LENGTH   (4)
+/* ETYPE configuration array size
+ * must always be kept equal to
+ * (WMI_RF_ETYPE_LENGTH+WMI_RF_ETYPE_VAL_PER_RANGE)
+ */
+#define WMI_RF_ETYPE_CONF_LENGTH   (8)
+/* RX2TX configuration array size
+ * must always be kept equal to (WMI_RF_RX2TX_LENGTH+1)
+ */
+#define WMI_RF_RX2TX_CONF_LENGTH   (4)
 
 /* Mailbox interface
  * used for commands and events
@@ -61,7 +73,7 @@ enum wmi_fw_capability {
WMI_FW_CAPABILITY_PS_CONFIG = 1,
WMI_FW_CAPABILITY_RF_SECTORS= 2,
WMI_FW_CAPABILITY_MGMT_RETRY_LIMIT  = 3,
-   WMI_FW_CAPABILITY_DISABLE_AP_SME= 4,
+   WMI_FW_CAPABILITY_AP_SME_OFFLOAD_PARTIAL= 4,
WMI_FW_CAPABILITY_WMI_ONLY  = 5,
WMI_FW_CAPABILITY_THERMAL_THROTTLING= 7,
WMI_FW_CAPABILITY_D3_SUSPEND= 8,
@@ -73,6 +85,7 @@ enum wmi_fw_capability {
WMI_FW_CAPABILITY_LO_POWER_CALIB_FROM_OTP   = 14,
WMI_FW_CAPABILITY_PNO   = 15,
WMI_FW_CAPABILITY_REF_CLOCK_CONTROL = 18,
+   WMI_FW_CAPABILITY_AP_SME_OFFLOAD_NONE   = 19,
WMI_FW_CAPABILITY_MAX,
 };
 
@@ -164,12 +177,14 @@ enum wmi_command_id {
WMI_SET_ACTIVE_SILENT_RSSI_TABLE_CMDID  = 0x85C,
WMI_RF_PWR_ON_DELAY_CMDID   = 0x85D,
WMI_SET_HIGH_POWER_TABLE_PARAMS_CMDID   = 0x85E,
+   WMI_FIXED_SCHEDULING_UL_CONFIG_CMDID= 0x85F,
/* Performance monitoring commands */
WMI_BF_CTRL_CMDID   = 0x862,
WMI_NOTIFY_REQ_CMDID= 0x863,
WMI_GET_STATUS_CMDID= 0x864,
WMI_GET_RF_STATUS_CMDID = 0x866,
WMI_GET_BASEBAND_TYPE_CMDID = 0x867,
+   WMI_VRING_SWITCH_TIMING_CONFIG_CMDID= 0x868,
WMI_UNIT_TEST_CMDID = 0x900,
WMI_FLASH_READ_CMDID= 0x902,
WMI_FLASH_WRITE_CMDID   = 0x903,
@@ -202,6 +217,7 @@ enum wmi_command_id {
WMI_GET_THERMAL_THROTTLING_CFG_CMDID= 0x941,
/* Read Power Save profile type */
WMI_PS_DEV_PROFILE_CFG_READ_CMDID   = 0x942,
+   WMI_TSF_SYNC_CMDID  = 0x973,
WMI_TOF_SESSION_START_CMDID = 0x991,

[PATCH v3 8/8] wil6210: rate limit wil_rx_refill error

[Maya Erez]

From: Dedy Lansky 

wil_err inside wil_rx_refill can flood the log buffer.
Replace it with wil_err_ratelimited.

Signed-off-by: Dedy Lansky 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c 
b/drivers/net/wireless/ath/wil6210/txrx.c
index 411130a..b9a9fa8 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -652,8 +652,8 @@ static int wil_rx_refill(struct wil6210_priv *wil, int 
count)
v->swtail = next_tail) {
rc = wil_vring_alloc_skb(wil, v, v->swtail, headroom);
if (unlikely(rc)) {
-   wil_err(wil, "Error %d in wil_rx_refill[%d]\n",
-   rc, v->swtail);
+   wil_err_ratelimited(wil, "Error %d in rx refill[%d]\n",
+   rc, v->swtail);
break;
}
}
-- 
1.9.1

[PATCH v3 0/8] wil6210 patches

[Maya Erez]

Changes from v2:
Removal of the following patches:
- wil6210: use country specific board file upon reg domain change
- wil6210: force EDMG channel through debugfs

Changes from v1:
Updated the commit text of "wil6210: force EDMG channel through debugfs"

The following patches include multiple wil6210 fixes.

Ahmad Masri (1):
  wil6210: align to latest auto generated wmi.h

Alexei Avshalom Lazar (2):
  wil6210: disable tracing config option
  wil6210: Initialize reply struct of the WMI commands

Dedy Lansky (3):
  wil6210: move WMI functionality out of wil_cfg80211_mgmt_tx
  wil6210: remove unused rx_reorder members
  wil6210: rate limit wil_rx_refill error

Lior David (2):
  wil6210: fix call to wil6210_disconnect during unload
  wil6210: change reply_size arg to u16 in wmi_call

 drivers/net/wireless/ath/wil6210/Kconfig  |   2 +-
 drivers/net/wireless/ath/wil6210/cfg80211.c   |  61 ++--
 drivers/net/wireless/ath/wil6210/debugfs.c|   5 +-
 drivers/net/wireless/ath/wil6210/main.c   |   2 +
 drivers/net/wireless/ath/wil6210/netdev.c |   8 +-
 drivers/net/wireless/ath/wil6210/rx_reorder.c |   7 +-
 drivers/net/wireless/ath/wil6210/txrx.c   |  12 +-
 drivers/net/wireless/ath/wil6210/wil6210.h|  13 +-
 drivers/net/wireless/ath/wil6210/wmi.c| 152 +++---
 drivers/net/wireless/ath/wil6210/wmi.h| 387 --
 10 files changed, 519 insertions(+), 130 deletions(-)

-- 
1.9.1

[PATCH v3 6/8] wil6210: Initialize reply struct of the WMI commands

[Maya Erez]

From: Alexei Avshalom Lazar 

WMI command reply saved in uninitialized struct.
In order to avoid accessing unset values from FW initialize
the reply struct.

Signed-off-by: Alexei Avshalom Lazar 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/cfg80211.c | 22 ---
 drivers/net/wireless/ath/wil6210/debugfs.c  |  2 +
 drivers/net/wireless/ath/wil6210/main.c |  2 +
 drivers/net/wireless/ath/wil6210/txrx.c |  8 ++-
 drivers/net/wireless/ath/wil6210/wmi.c  | 97 ++---
 5 files changed, 85 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c 
b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 109bae1..78946f2 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -276,6 +276,8 @@ int wil_cid_fill_sinfo(struct wil6210_vif *vif, int cid,
struct wil_net_stats *stats = >sta[cid].stats;
int rc;
 
+   memset(, 0, sizeof(reply));
+
rc = wmi_call(wil, WMI_NOTIFY_REQ_CMDID, vif->mid, , sizeof(cmd),
  WMI_NOTIFY_REQ_DONE_EVENTID, , sizeof(reply), 20);
if (rc)
@@ -2246,7 +2248,9 @@ static int wil_rf_sector_get_cfg(struct wiphy *wiphy,
struct {
struct wmi_cmd_hdr wmi;
struct wmi_get_rf_sector_params_done_event evt;
-   } __packed reply;
+   } __packed reply = {
+   .evt = {.status = WMI_RF_SECTOR_STATUS_NOT_SUPPORTED_ERROR},
+   };
struct sk_buff *msg;
struct nlattr *nl_cfgs, *nl_cfg;
u32 i;
@@ -2292,7 +2296,6 @@ static int wil_rf_sector_get_cfg(struct wiphy *wiphy,
cmd.sector_idx = cpu_to_le16(sector_index);
cmd.sector_type = sector_type;
cmd.rf_modules_vec = rf_modules_vec & 0xFF;
-   memset(, 0, sizeof(reply));
rc = wmi_call(wil, WMI_GET_RF_SECTOR_PARAMS_CMDID, vif->mid,
  , sizeof(cmd), WMI_GET_RF_SECTOR_PARAMS_DONE_EVENTID,
  , sizeof(reply),
@@ -2367,7 +2370,9 @@ static int wil_rf_sector_set_cfg(struct wiphy *wiphy,
struct {
struct wmi_cmd_hdr wmi;
struct wmi_set_rf_sector_params_done_event evt;
-   } __packed reply;
+   } __packed reply = {
+   .evt = {.status = WMI_RF_SECTOR_STATUS_NOT_SUPPORTED_ERROR},
+   };
struct nlattr *nl_cfg;
struct wmi_rf_sector_info *si;
 
@@ -2450,7 +2455,6 @@ static int wil_rf_sector_set_cfg(struct wiphy *wiphy,
}
 
cmd.rf_modules_vec = rf_modules_vec & 0xFF;
-   memset(, 0, sizeof(reply));
rc = wmi_call(wil, WMI_SET_RF_SECTOR_PARAMS_CMDID, vif->mid,
  , sizeof(cmd), WMI_SET_RF_SECTOR_PARAMS_DONE_EVENTID,
  , sizeof(reply),
@@ -2474,7 +2478,9 @@ static int wil_rf_sector_get_selected(struct wiphy *wiphy,
struct {
struct wmi_cmd_hdr wmi;
struct wmi_get_selected_rf_sector_index_done_event evt;
-   } __packed reply;
+   } __packed reply = {
+   .evt = {.status = WMI_RF_SECTOR_STATUS_NOT_SUPPORTED_ERROR},
+   };
struct sk_buff *msg;
 
if (!test_bit(WMI_FW_CAPABILITY_RF_SECTORS, wil->fw_capabilities))
@@ -2514,7 +2520,6 @@ static int wil_rf_sector_get_selected(struct wiphy *wiphy,
memset(, 0, sizeof(cmd));
cmd.cid = (u8)cid;
cmd.sector_type = sector_type;
-   memset(, 0, sizeof(reply));
rc = wmi_call(wil, WMI_GET_SELECTED_RF_SECTOR_INDEX_CMDID, vif->mid,
  , sizeof(cmd),
  WMI_GET_SELECTED_RF_SECTOR_INDEX_DONE_EVENTID,
@@ -2555,14 +2560,15 @@ static int wil_rf_sector_wmi_set_selected(struct 
wil6210_priv *wil,
struct {
struct wmi_cmd_hdr wmi;
struct wmi_set_selected_rf_sector_index_done_event evt;
-   } __packed reply;
+   } __packed reply = {
+   .evt = {.status = WMI_RF_SECTOR_STATUS_NOT_SUPPORTED_ERROR},
+   };
int rc;
 
memset(, 0, sizeof(cmd));
cmd.sector_idx = cpu_to_le16(sector_index);
cmd.sector_type = sector_type;
cmd.cid = (u8)cid;
-   memset(, 0, sizeof(reply));
rc = wmi_call(wil, WMI_SET_SELECTED_RF_SECTOR_INDEX_CMDID, mid,
  , sizeof(cmd),
  WMI_SET_SELECTED_RF_SECTOR_INDEX_DONE_EVENTID,
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c 
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 8c90b31..d3b1069 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1078,6 +1078,8 @@ static int wil_bf_debugfs_show(struct seq_file *s, void 
*data)
struct wmi_notify_req_done_event evt;
} __packed reply;
 
+   memset(, 0, sizeof(reply));
+
for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
u32 status;
 

[PATCH v3 5/8] wil6210: move WMI functionality out of wil_cfg80211_mgmt_tx

[Maya Erez]

From: Dedy Lansky 

Rearrange the code by having new function wmi_mgmt_tx() to take care
of the WMI part of wil_cfg80211_mgmt_tx().

Signed-off-by: Dedy Lansky 
Signed-off-by: Maya Erez 
---
 drivers/net/wireless/ath/wil6210/cfg80211.c | 39 +++-
 drivers/net/wireless/ath/wil6210/wil6210.h  |  1 +
 drivers/net/wireless/ath/wil6210/wmi.c  | 47 +
 3 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c 
b/drivers/net/wireless/ath/wil6210/cfg80211.c
index cdbb393..109bae1 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -1081,17 +1081,11 @@ int wil_cfg80211_mgmt_tx(struct wiphy *wiphy, struct 
wireless_dev *wdev,
 u64 *cookie)
 {
const u8 *buf = params->buf;
-   size_t len = params->len, total;
+   size_t len = params->len;
struct wil6210_priv *wil = wiphy_to_wil(wiphy);
struct wil6210_vif *vif = wdev_to_vif(wil, wdev);
int rc;
-   bool tx_status = false;
-   struct ieee80211_mgmt *mgmt_frame = (void *)buf;
-   struct wmi_sw_tx_req_cmd *cmd;
-   struct {
-   struct wmi_cmd_hdr wmi;
-   struct wmi_sw_tx_complete_event evt;
-   } __packed evt;
+   bool tx_status;
 
/* Note, currently we do not support the "wait" parameter, user-space
 * must call remain_on_channel before mgmt_tx or listen on a channel
@@ -1100,34 +1094,9 @@ int wil_cfg80211_mgmt_tx(struct wiphy *wiphy, struct 
wireless_dev *wdev,
 * different from currently "listened" channel and fail if it is.
 */
 
-   wil_dbg_misc(wil, "mgmt_tx mid %d\n", vif->mid);
-   wil_hex_dump_misc("mgmt tx frame ", DUMP_PREFIX_OFFSET, 16, 1, buf,
- len, true);
-
-   if (len < sizeof(struct ieee80211_hdr_3addr))
-   return -EINVAL;
-
-   total = sizeof(*cmd) + len;
-   if (total < len)
-   return -EINVAL;
-
-   cmd = kmalloc(total, GFP_KERNEL);
-   if (!cmd) {
-   rc = -ENOMEM;
-   goto out;
-   }
-
-   memcpy(cmd->dst_mac, mgmt_frame->da, WMI_MAC_LEN);
-   cmd->len = cpu_to_le16(len);
-   memcpy(cmd->payload, buf, len);
-
-   rc = wmi_call(wil, WMI_SW_TX_REQ_CMDID, vif->mid, cmd, total,
- WMI_SW_TX_COMPLETE_EVENTID, , sizeof(evt), 2000);
-   if (rc == 0)
-   tx_status = !evt.evt.status;
+   rc = wmi_mgmt_tx(vif, buf, len);
+   tx_status = (rc == 0);
 
-   kfree(cmd);
- out:
cfg80211_mgmt_tx_status(wdev, cookie ? *cookie : 0, buf, len,
tx_status, GFP_KERNEL);
return rc;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h 
b/drivers/net/wireless/ath/wil6210/wil6210.h
index 9ac9292..c9120dc 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -1150,5 +1150,6 @@ int wil_request_firmware(struct wil6210_priv *wil, const 
char *name,
 int wmi_start_sched_scan(struct wil6210_priv *wil,
 struct cfg80211_sched_scan_request *request);
 int wmi_stop_sched_scan(struct wil6210_priv *wil);
+int wmi_mgmt_tx(struct wil6210_vif *vif, const u8 *buf, size_t len);
 
 #endif /* __WIL6210_H__ */
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c 
b/drivers/net/wireless/ath/wil6210/wmi.c
index fcd9529..bf110e3 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -2773,3 +2773,50 @@ int wmi_stop_sched_scan(struct wil6210_priv *wil)
 
return 0;
 }
+
+int wmi_mgmt_tx(struct wil6210_vif *vif, const u8 *buf, size_t len)
+{
+   size_t total;
+   struct wil6210_priv *wil = vif_to_wil(vif);
+   struct ieee80211_mgmt *mgmt_frame = (void *)buf;
+   struct wmi_sw_tx_req_cmd *cmd;
+   struct {
+   struct wmi_cmd_hdr wmi;
+   struct wmi_sw_tx_complete_event evt;
+   } __packed evt = {
+   .evt = {.status = WMI_FW_STATUS_FAILURE},
+   };
+   int rc;
+
+   wil_dbg_misc(wil, "mgmt_tx mid %d\n", vif->mid);
+   wil_hex_dump_misc("mgmt tx frame ", DUMP_PREFIX_OFFSET, 16, 1, buf,
+ len, true);
+
+   if (len < sizeof(struct ieee80211_hdr_3addr))
+   return -EINVAL;
+
+   total = sizeof(*cmd) + len;
+   if (total < len) {
+   wil_err(wil, "mgmt_tx invalid len %zu\n", len);
+   return -EINVAL;
+   }
+
+   cmd = kmalloc(total, GFP_KERNEL);
+   if (!cmd)
+   return -ENOMEM;
+
+   memcpy(cmd->dst_mac, mgmt_frame->da, WMI_MAC_LEN);
+   cmd->len = cpu_to_le16(len);
+   memcpy(cmd->payload, buf, len);
+
+   rc = wmi_call(wil, WMI_SW_TX_REQ_CMDID, vif->mid, cmd, total,
+ 

Re: Regression caused by commit 882164a4a928

[Kalle Valo]

Michael Büsch  writes:

> On Mon, 07 May 2018 22:03:58 +0300
> Kalle Valo  wrote:
>
>> Michael Büsch  writes:
>> 
>> > On Mon, 7 May 2018 10:44:34 -0500
>> > Larry Finger  wrote:
>> >  
>> >> Although commit 882164a4a928 ("ssb: Prevent build of PCI host features in 
>> >> module") appeared to be harmless, it leads to complete failure of drivers 
>> >> b43.   
>> >  
>> >>   config SSB_DRIVER_PCICORE_POSSIBLE
>> >>  bool
>> >> -   depends on SSB_PCIHOST && SSB = y
>> >> +   depends on SSB_PCIHOST && (SSB = y || !MIPS)
>> >>  default y
>> >> 
>> >>   config SSB_DRIVER_PCICORE  
>> >
>> >
>> > https://patchwork.kernel.org/patch/10161131/
>> >
>> > Could we _please_ switch to not applying patches to ssb or b43, if
>> > nobody acked (or better reviewed) a patch?
>> >
>> > We had multiple changes to ssb and b43 in the recent past that did not
>> > have a review at all and broke something. I don't think such software
>> > quality is acceptable at all.
>> > So please revert 882164a4a928.  
>> 
>> Yes, someone please send a revert so that this can be fixed quickly for
>> v4.17.
>
> Uhm, can you just type git revert 882164a4a928? :)

But it needs a proper commit log explaining why it's reverted (links to
bugzilla report etc). And I prefer also reverts to be reviewed on the
list.

> Or do I have to send you a pull request?

A revert is a regular commit, so you can submit it using git
format-patch and git send-email.

>> > I'm sorry that this patch slipped through the cracks of my inbox.
>> > But the reaction to that shall not be to just apply the patch. It
>> > shall be to resubmit it for review.  
>> 
>> The thing is that in general I do not have time to ping people for every
>> patch, I get enough of emails as is. If there are no review comments I
>> have to assume the patch is ok to apply.
>
> Yes, I understand that pinging people can be annoying and time
> consuming. But we have tools like patchwork. Why isn't pinging
> (semi)automated? Patchwork should really track the review status of a
> patch.

That would be awesome but patchwork is nowhere near that kind of
sophistication. I like it but to be honest it's really simple at the
moment. My custom client script has a simple way to ping about patches
but even that is too much work on the long run. Some people do send Acks
to the driver they maintain but not always, I guess because too busy
with real life or something which is totally understandable. But it
would not scale at all if I would start pinging for the 25% of patches
that they have not acked.

> I think the concept of no-comments = everything-ok is
> fundamentally broken. But it has always been that way for wireless and
> lots of other subsystems.

It's a balance between bureaucracy and getting things done. From my POV
the only viable solution is that maintainers actively follow the patches
on the mailing list.

-- 
Kalle Valo

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Kalle Valo]

Johannes Berg  writes:

> On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
>> Johannes Berg  writes:
>> 
>> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
>> > > Johannes Berg  writes:
>> > > 
>> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>> > > > > 
>> > > > > I guess these warnings come because Toke's patch increased size of
>> > > > > struct cfg80211_tid_stats (which is included in struct station_info) 
>> > > > > and
>> > > > > both wil6210 and qtnfmac allocate a struct station_info from stack? 
>> > > > 
>> > > > Yes.
>> > > > 
>> > > > > Can
>> > > > > someone send a fix for the drivers?
>> > > > 
>> > > > I guess Toke/I should do that through my tree.
>> > > 
>> > > IMHO the fix could go through my tree as well, less risk of conflicts in
>> > > drivers. AFAICS the fix (allocating station_info dynamically?) would not
>> > > depend on Toke's patch and could be applied separately.
>> > 
>> > That's true, if you prefer that it's fine with me.
>> 
>> I'll send a patch.
>> 
>> What's the right tag to put in the commit for this?
>> Fixes-but-is-independent-from: ? ;)
>
> Heh. You can still put Fixes: I think.

Yeah, I think so too.

-- 
Kalle Valo

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Johannes Berg]

On Wed, 2018-05-09 at 11:36 +0200, Toke Høiland-Jørgensen wrote:
> Johannes Berg  writes:
> 
> > On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
> > > Johannes Berg  writes:
> > > 
> > > > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
> > > > > 
> > > > > I guess these warnings come because Toke's patch increased size of
> > > > > struct cfg80211_tid_stats (which is included in struct station_info) 
> > > > > and
> > > > > both wil6210 and qtnfmac allocate a struct station_info from stack? 
> > > > 
> > > > Yes.
> > > > 
> > > > > Can
> > > > > someone send a fix for the drivers?
> > > > 
> > > > I guess Toke/I should do that through my tree.
> > > 
> > > IMHO the fix could go through my tree as well, less risk of conflicts in
> > > drivers. AFAICS the fix (allocating station_info dynamically?) would not
> > > depend on Toke's patch and could be applied separately.
> > 
> > That's true, if you prefer that it's fine with me.
> 
> I'll send a patch.
> 
> What's the right tag to put in the commit for this?
> Fixes-but-is-independent-from: ? ;)

Heh. You can still put Fixes: I think.

johannes

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
>> Johannes Berg  writes:
>> 
>> > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>> > > 
>> > > I guess these warnings come because Toke's patch increased size of
>> > > struct cfg80211_tid_stats (which is included in struct station_info) and
>> > > both wil6210 and qtnfmac allocate a struct station_info from stack? 
>> > 
>> > Yes.
>> > 
>> > > Can
>> > > someone send a fix for the drivers?
>> > 
>> > I guess Toke/I should do that through my tree.
>> 
>> IMHO the fix could go through my tree as well, less risk of conflicts in
>> drivers. AFAICS the fix (allocating station_info dynamically?) would not
>> depend on Toke's patch and could be applied separately.
>
> That's true, if you prefer that it's fine with me.

I'll send a patch.

What's the right tag to put in the commit for this?
Fixes-but-is-independent-from: ? ;)

-Toke

Re: [PATCH] ath10k: Replace bit shifts with the BIT() macro for rx desc bits

[Govind Singh]

On 2018-05-09 13:10, Marcus Folkesson wrote:

Hello Govind,

On Wed, May 09, 2018 at 10:18:04AM +0530, Govind Singh wrote:

Use the BIT() macro from 'linux/bitops.h' to define the rx desc
bit flags to have consistency with new definitions.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/rx_desc.h | 134 
+++---

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

diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h 
b/drivers/net/wireless/ath/ath10k/rx_desc.h

index 545deb6d7af1..b3d7c6e7ac90 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h


Looks good, but please include linux/bitops.h as well.

From the documentation [1]:

If you use a facility then #include the file that defines/declares
 that facility.  Don't depend on other header files pulling in ones
 that you use.



Thanks, addressed the comment in v2 patchset.



[1] Documentation/process/submit-checklist.rst


Best regards
Marcus Folkesson

[PATCH v2] ath10k: Replace bit shifts with the BIT() macro for rx desc bits

[Govind Singh]

Use the BIT() macro from 'linux/bitops.h' to define the rx desc
bit flags to have consistency with new definitions.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/rx_desc.h | 136 +++---
 1 file changed, 69 insertions(+), 67 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h 
b/drivers/net/wireless/ath/ath10k/rx_desc.h
index 545deb6d7af1..ea4075d456fa 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -18,39 +18,41 @@
 #ifndef _RX_DESC_H_
 #define _RX_DESC_H_
 
+#include 
+
 enum rx_attention_flags {
-   RX_ATTENTION_FLAGS_FIRST_MPDU  = 1 << 0,
-   RX_ATTENTION_FLAGS_LAST_MPDU   = 1 << 1,
-   RX_ATTENTION_FLAGS_MCAST_BCAST = 1 << 2,
-   RX_ATTENTION_FLAGS_PEER_IDX_INVALID= 1 << 3,
-   RX_ATTENTION_FLAGS_PEER_IDX_TIMEOUT= 1 << 4,
-   RX_ATTENTION_FLAGS_POWER_MGMT  = 1 << 5,
-   RX_ATTENTION_FLAGS_NON_QOS = 1 << 6,
-   RX_ATTENTION_FLAGS_NULL_DATA   = 1 << 7,
-   RX_ATTENTION_FLAGS_MGMT_TYPE   = 1 << 8,
-   RX_ATTENTION_FLAGS_CTRL_TYPE   = 1 << 9,
-   RX_ATTENTION_FLAGS_MORE_DATA   = 1 << 10,
-   RX_ATTENTION_FLAGS_EOSP= 1 << 11,
-   RX_ATTENTION_FLAGS_U_APSD_TRIGGER  = 1 << 12,
-   RX_ATTENTION_FLAGS_FRAGMENT= 1 << 13,
-   RX_ATTENTION_FLAGS_ORDER   = 1 << 14,
-   RX_ATTENTION_FLAGS_CLASSIFICATION  = 1 << 15,
-   RX_ATTENTION_FLAGS_OVERFLOW_ERR= 1 << 16,
-   RX_ATTENTION_FLAGS_MSDU_LENGTH_ERR = 1 << 17,
-   RX_ATTENTION_FLAGS_TCP_UDP_CHKSUM_FAIL = 1 << 18,
-   RX_ATTENTION_FLAGS_IP_CHKSUM_FAIL  = 1 << 19,
-   RX_ATTENTION_FLAGS_SA_IDX_INVALID  = 1 << 20,
-   RX_ATTENTION_FLAGS_DA_IDX_INVALID  = 1 << 21,
-   RX_ATTENTION_FLAGS_SA_IDX_TIMEOUT  = 1 << 22,
-   RX_ATTENTION_FLAGS_DA_IDX_TIMEOUT  = 1 << 23,
-   RX_ATTENTION_FLAGS_ENCRYPT_REQUIRED= 1 << 24,
-   RX_ATTENTION_FLAGS_DIRECTED= 1 << 25,
-   RX_ATTENTION_FLAGS_BUFFER_FRAGMENT = 1 << 26,
-   RX_ATTENTION_FLAGS_MPDU_LENGTH_ERR = 1 << 27,
-   RX_ATTENTION_FLAGS_TKIP_MIC_ERR= 1 << 28,
-   RX_ATTENTION_FLAGS_DECRYPT_ERR = 1 << 29,
-   RX_ATTENTION_FLAGS_FCS_ERR = 1 << 30,
-   RX_ATTENTION_FLAGS_MSDU_DONE   = 1 << 31,
+   RX_ATTENTION_FLAGS_FIRST_MPDU  = BIT(0),
+   RX_ATTENTION_FLAGS_LAST_MPDU   = BIT(1),
+   RX_ATTENTION_FLAGS_MCAST_BCAST = BIT(2),
+   RX_ATTENTION_FLAGS_PEER_IDX_INVALID= BIT(3),
+   RX_ATTENTION_FLAGS_PEER_IDX_TIMEOUT= BIT(4),
+   RX_ATTENTION_FLAGS_POWER_MGMT  = BIT(5),
+   RX_ATTENTION_FLAGS_NON_QOS = BIT(6),
+   RX_ATTENTION_FLAGS_NULL_DATA   = BIT(7),
+   RX_ATTENTION_FLAGS_MGMT_TYPE   = BIT(8),
+   RX_ATTENTION_FLAGS_CTRL_TYPE   = BIT(9),
+   RX_ATTENTION_FLAGS_MORE_DATA   = BIT(10),
+   RX_ATTENTION_FLAGS_EOSP= BIT(11),
+   RX_ATTENTION_FLAGS_U_APSD_TRIGGER  = BIT(12),
+   RX_ATTENTION_FLAGS_FRAGMENT= BIT(13),
+   RX_ATTENTION_FLAGS_ORDER   = BIT(14),
+   RX_ATTENTION_FLAGS_CLASSIFICATION  = BIT(15),
+   RX_ATTENTION_FLAGS_OVERFLOW_ERR= BIT(16),
+   RX_ATTENTION_FLAGS_MSDU_LENGTH_ERR = BIT(17),
+   RX_ATTENTION_FLAGS_TCP_UDP_CHKSUM_FAIL = BIT(18),
+   RX_ATTENTION_FLAGS_IP_CHKSUM_FAIL  = BIT(19),
+   RX_ATTENTION_FLAGS_SA_IDX_INVALID  = BIT(20),
+   RX_ATTENTION_FLAGS_DA_IDX_INVALID  = BIT(21),
+   RX_ATTENTION_FLAGS_SA_IDX_TIMEOUT  = BIT(22),
+   RX_ATTENTION_FLAGS_DA_IDX_TIMEOUT  = BIT(23),
+   RX_ATTENTION_FLAGS_ENCRYPT_REQUIRED= BIT(24),
+   RX_ATTENTION_FLAGS_DIRECTED= BIT(25),
+   RX_ATTENTION_FLAGS_BUFFER_FRAGMENT = BIT(26),
+   RX_ATTENTION_FLAGS_MPDU_LENGTH_ERR = BIT(27),
+   RX_ATTENTION_FLAGS_TKIP_MIC_ERR= BIT(28),
+   RX_ATTENTION_FLAGS_DECRYPT_ERR = BIT(29),
+   RX_ATTENTION_FLAGS_FCS_ERR = BIT(30),
+   RX_ATTENTION_FLAGS_MSDU_DONE   = BIT(31),
 };
 
 struct rx_attention {
@@ -254,15 +256,15 @@ enum htt_rx_mpdu_encrypt_type {
 #define RX_MPDU_START_INFO0_SEQ_NUM_LSB   16
 #define RX_MPDU_START_INFO0_ENCRYPT_TYPE_MASK 0xf000
 #define RX_MPDU_START_INFO0_ENCRYPT_TYPE_LSB  28
-#define RX_MPDU_START_INFO0_FROM_DS   (1 << 11)
-#define RX_MPDU_START_INFO0_TO_DS (1 << 12)
-#define RX_MPDU_START_INFO0_ENCRYPTED (1 << 13)
-#define RX_MPDU_START_INFO0_RETRY (1 << 14)
-#define RX_MPDU_START_INFO0_TXBF_H_INFO   (1 << 15)
+#define RX_MPDU_START_INFO0_FROM_DS   BIT(11)
+#define RX_MPDU_START_INFO0_TO_DS BIT(12)
+#define 

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Johannes Berg]

On Wed, 2018-05-09 at 11:56 +0300, Kalle Valo wrote:
> Johannes Berg  writes:
> 
> > On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
> > > 
> > > I guess these warnings come because Toke's patch increased size of
> > > struct cfg80211_tid_stats (which is included in struct station_info) and
> > > both wil6210 and qtnfmac allocate a struct station_info from stack? 
> > 
> > Yes.
> > 
> > > Can
> > > someone send a fix for the drivers?
> > 
> > I guess Toke/I should do that through my tree.
> 
> IMHO the fix could go through my tree as well, less risk of conflicts in
> drivers. AFAICS the fix (allocating station_info dynamically?) would not
> depend on Toke's patch and could be applied separately.

That's true, if you prefer that it's fine with me.

johannes

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Kalle Valo]

Johannes Berg  writes:

> On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
>> 
>> I guess these warnings come because Toke's patch increased size of
>> struct cfg80211_tid_stats (which is included in struct station_info) and
>> both wil6210 and qtnfmac allocate a struct station_info from stack? 
>
> Yes.
>
>> Can
>> someone send a fix for the drivers?
>
> I guess Toke/I should do that through my tree.

IMHO the fix could go through my tree as well, less risk of conflicts in
drivers. AFAICS the fix (allocating station_info dynamically?) would not
depend on Toke's patch and could be applied separately.

-- 
Kalle Valo

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Johannes Berg]

On Wed, 2018-05-09 at 11:47 +0300, Kalle Valo wrote:
> 
> I guess these warnings come because Toke's patch increased size of
> struct cfg80211_tid_stats (which is included in struct station_info) and
> both wil6210 and qtnfmac allocate a struct station_info from stack? 

Yes.

> Can
> someone send a fix for the drivers?

I guess Toke/I should do that through my tree.

johannes

Re: [mac80211-next:master 12/14] drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size of 1600 bytes is larger than 1024 bytes

[Kalle Valo]

(adding Maya)

kbuild test robot  writes:

> tree:   
> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> head:   57c6cb81717f957fb741f2e4c79bd0e2f4f55910
> commit: 52539ca89f365d3db530535fbffa88a3cca4d2ec [12/14] cfg80211: Expose TXQ 
> stats and parameters to userspace
> config: i386-randconfig-i1-201818 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> git checkout 52539ca89f365d3db530535fbffa88a3cca4d2ec
> # save the attached .config to linux build tree
> make ARCH=i386 
>
> All warnings (new ones prefixed by >>):
>
>drivers/net//wireless/ath/wil6210/debugfs.c: In function 
> 'wil_link_debugfs_show':
>>> drivers/net//wireless/ath/wil6210/debugfs.c:1245:1: warning: the frame size 
>>> of 1600 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
> --
>drivers/net//wireless/ath/wil6210/wmi.c: In function 'wmi_evt_connect':
>>> drivers/net//wireless/ath/wil6210/wmi.c:979:1: warning: the frame size of 
>>> 1684 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
> --
>drivers/net//wireless/quantenna/qtnfmac/event.c: In function 
> 'qtnf_event_handle_sta_assoc':
>>> drivers/net//wireless/quantenna/qtnfmac/event.c:107:1: warning: the frame 
>>> size of 1616 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^

I guess these warnings come because Toke's patch increased size of
struct cfg80211_tid_stats (which is included in struct station_info) and
both wil6210 and qtnfmac allocate a struct station_info from stack? Can
someone send a fix for the drivers?

-- 
Kalle Valo

[PATCH] ath10k: hw: make consistent usage of ATH10K_FW_DIR in paths

[Marcus Folkesson]

Signed-off-by: Marcus Folkesson 
---
 drivers/net/wireless/ath/ath10k/hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/hw.h 
b/drivers/net/wireless/ath/ath10k/hw.h
index b8bdabe73073..23467e9fefeb 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -85,11 +85,11 @@ enum qca9377_chip_id_rev {
QCA9377_HW_1_1_CHIP_ID_REV = 0x1,
 };
 
-#define QCA6174_HW_2_1_FW_DIR  "ath10k/QCA6174/hw2.1"
+#define QCA6174_HW_2_1_FW_DIR  ATH10K_FW_DIR "/QCA6174/hw2.1"
 #define QCA6174_HW_2_1_BOARD_DATA_FILE "board.bin"
 #define QCA6174_HW_2_1_PATCH_LOAD_ADDR 0x1234
 
-#define QCA6174_HW_3_0_FW_DIR  "ath10k/QCA6174/hw3.0"
+#define QCA6174_HW_3_0_FW_DIR  ATH10K_FW_DIR "/QCA6174/hw3.0"
 #define QCA6174_HW_3_0_BOARD_DATA_FILE "board.bin"
 #define QCA6174_HW_3_0_PATCH_LOAD_ADDR 0x1234
 
-- 
2.16.2