Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH v3 2/2] mt76: mt7615: fix slow performance when enable encryption
From: Ryder Lee @ 2019-06-03  6:08 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, Chih-Min Chen,
	linux-wireless, linux-mediatek, linux-kernel, Ryder Lee
In-Reply-To: <a1ff446dfc06e2443552e7ec2d754099aacce7df.1559541944.git.ryder.lee@mediatek.com>

Fix wrong WCID assignment and add RKV (RX Key of this entry is valid)
flag to check if peer uses the same configuration with previous
handshaking.

If the configuration is mismatch, WTBL indicates a “cipher mismatch”
to stop SEC decryption to prevent the packet from damage.

Suggested-by: YF Luo <yf.luo@mediatek.com>
Suggested-by: Yiwei Chung <yiwei.chung@mediatek.com>
Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
Changes since v3 - none
Changes since v2 - none
---
 drivers/net/wireless/mediatek/mt76/mt7615/init.c | 16 ++++++++++------
 drivers/net/wireless/mediatek/mt76/mt7615/main.c |  2 +-
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c  |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
index f860af6a42da..b3e08154ccbe 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
@@ -62,16 +62,11 @@ static void mt7615_mac_init(struct mt7615_dev *dev)
 		 MT_AGG_ARCR_RATE_DOWN_RATIO_EN |
 		 FIELD_PREP(MT_AGG_ARCR_RATE_DOWN_RATIO, 1) |
 		 FIELD_PREP(MT_AGG_ARCR_RATE_UP_EXTRA_TH, 4)));
-
-	dev->mt76.global_wcid.idx = MT7615_WTBL_RESERVED;
-	dev->mt76.global_wcid.hw_key_idx = -1;
-	rcu_assign_pointer(dev->mt76.wcid[MT7615_WTBL_RESERVED],
-			   &dev->mt76.global_wcid);
 }
 
 static int mt7615_init_hardware(struct mt7615_dev *dev)
 {
-	int ret;
+	int ret, idx;
 
 	mt76_wr(dev, MT_INT_SOURCE_CSR, ~0);
 
@@ -98,6 +93,15 @@ static int mt7615_init_hardware(struct mt7615_dev *dev)
 	mt7615_mcu_ctrl_pm_state(dev, 0);
 	mt7615_mcu_del_wtbl_all(dev);
 
+	/* Beacon and mgmt frames should occupy wcid 0 */
+	idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7615_WTBL_STA - 1);
+	if (idx)
+		return -ENOSPC;
+
+	dev->mt76.global_wcid.idx = idx;
+	dev->mt76.global_wcid.hw_key_idx = -1;
+	rcu_assign_pointer(dev->mt76.wcid[idx], &dev->mt76.global_wcid);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index 585e67fa2728..2cdd339453c8 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -95,7 +95,7 @@ static int mt7615_add_interface(struct ieee80211_hw *hw,
 
 	dev->vif_mask |= BIT(mvif->idx);
 	dev->omac_mask |= BIT(mvif->omac_idx);
-	idx = MT7615_WTBL_RESERVED - 1 - mvif->idx;
+	idx = MT7615_WTBL_RESERVED - mvif->idx;
 	mvif->sta.wcid.idx = idx;
 	mvif->sta.wcid.hw_key_idx = -1;
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index e82297048449..b3802f18b37b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -882,6 +882,7 @@ int mt7615_mcu_set_wtbl_key(struct mt7615_dev *dev, int wcid,
 		if (cipher == MT_CIPHER_NONE && key)
 			return -EOPNOTSUPP;
 
+		req.key.rkv = 1;
 		req.key.cipher_id = cipher;
 		req.key.key_id = key->keyidx;
 		req.key.key_len = key->keylen;
-- 
2.18.0


^ permalink raw reply related

* [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Ryder Lee @ 2019-06-03  6:08 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, Chih-Min Chen,
	linux-wireless, linux-mediatek, linux-kernel, Ryder Lee

Enable NL80211_IFTYPE_MESH_POINT and update its path.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
Changes since v3 - fix a wrong expression
Changes since v2 - remove unused definitions
---
 drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++++++
 drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c  | 4 +++-
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.h  | 6 ------
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
index 59f604f3161f..f860af6a42da 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
@@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] = {
 	{
 		.max = MT7615_MAX_INTERFACES,
 		.types = BIT(NL80211_IFTYPE_AP) |
+#ifdef CONFIG_MAC80211_MESH
+			 BIT(NL80211_IFTYPE_MESH_POINT) |
+#endif
 			 BIT(NL80211_IFTYPE_STATION)
 	}
 };
@@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
 	dev->mt76.antenna_mask = 0xf;
 
 	wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
+#ifdef CONFIG_MAC80211_MESH
+				 BIT(NL80211_IFTYPE_MESH_POINT) |
+#endif
 				 BIT(NL80211_IFTYPE_AP);
 
 	ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index b0bb7cc12385..585e67fa2728 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
 
 	switch (type) {
 	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_MESH_POINT:
 		/* ap use hw bssid 0 and ext bssid */
 		if (~mask & BIT(HW_BSSID_0))
 			return HW_BSSID_0;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 43f70195244c..e82297048449 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
 
 	switch (vif->type) {
 	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_MESH_POINT:
 		tx_wlan_idx = mvif->sta.wcid.idx;
 		conn_type = CONNECTION_INFRA_AP;
 		break;
@@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
 		.rx_wtbl = {
 			.tag = cpu_to_le16(WTBL_RX),
 			.len = cpu_to_le16(sizeof(struct wtbl_rx)),
-			.rca1 = vif->type != NL80211_IFTYPE_AP,
+			.rca1 = vif->type == NL80211_IFTYPE_STATION,
 			.rca2 = 1,
 			.rv = 1,
 		},
@@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum nl80211_iftype type, u32 *conn_type)
 {
 	switch (type) {
 	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_MESH_POINT:
 		if (conn_type)
 			*conn_type = CONNECTION_INFRA_STA;
 		break;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
index e96efb13fa4d..0915cb735699 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
@@ -105,25 +105,19 @@ enum {
 #define STA_TYPE_STA		BIT(0)
 #define STA_TYPE_AP		BIT(1)
 #define STA_TYPE_ADHOC		BIT(2)
-#define STA_TYPE_TDLS		BIT(3)
 #define STA_TYPE_WDS		BIT(4)
 #define STA_TYPE_BC		BIT(5)
 
 #define NETWORK_INFRA		BIT(16)
 #define NETWORK_P2P		BIT(17)
 #define NETWORK_IBSS		BIT(18)
-#define NETWORK_MESH		BIT(19)
-#define NETWORK_BOW		BIT(20)
 #define NETWORK_WDS		BIT(21)
 
 #define CONNECTION_INFRA_STA	(STA_TYPE_STA | NETWORK_INFRA)
 #define CONNECTION_INFRA_AP	(STA_TYPE_AP | NETWORK_INFRA)
 #define CONNECTION_P2P_GC	(STA_TYPE_STA | NETWORK_P2P)
 #define CONNECTION_P2P_GO	(STA_TYPE_AP | NETWORK_P2P)
-#define CONNECTION_MESH_STA	(STA_TYPE_STA | NETWORK_MESH)
-#define CONNECTION_MESH_AP	(STA_TYPE_AP | NETWORK_MESH)
 #define CONNECTION_IBSS_ADHOC	(STA_TYPE_ADHOC | NETWORK_IBSS)
-#define CONNECTION_TDLS		(STA_TYPE_STA | NETWORK_INFRA | STA_TYPE_TDLS)
 #define CONNECTION_WDS		(STA_TYPE_WDS | NETWORK_WDS)
 #define CONNECTION_INFRA_BC	(STA_TYPE_BC | NETWORK_INFRA)
 
-- 
2.18.0


^ permalink raw reply related

* RE: [EXT] INFO: trying to register non-static key in del_timer_sync (2)
From: Ganapathi Bhat @ 2019-06-03  8:41 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: syzbot, amitkarwar@gmail.com, andreyknvl@google.com,
	davem@davemloft.net, huxinming820@gmail.com, kvalo@codeaurora.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	nishants@marvell.com, syzkaller-bugs@googlegroups.com
In-Reply-To: <CACT4Y+aQzBkAq86Hx4jNFnAUzjXnq8cS2NZKfeCaFrZa__g-cg@mail.gmail.com>

Hi Dmitry,

> The "fixed" status relates to the similar past bug that was reported and fixed
> more than a year ago:
Oh OK; We understood the issue, working on a change to fix this;

Thanks,
Ganapathi

^ permalink raw reply

* In A Nutshell
From: Emissary @ 2019-05-30 21:23 UTC (permalink / raw)
  To: linux-wireless

Hello, 

We have a private business proposition for you,contact me for more details. 

Thank you,     

Datuk.
5.30.19/135p/28wwe.5

^ permalink raw reply

* Re: [RFC 0/8] nl80211: add 6GHz band support
From: Arend Van Spriel @ 2019-06-03 10:39 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <9ba78df6-18a3-5c1c-6c57-3fa71531b460@broadcom.com>

On 5/27/2019 10:46 PM, Arend Van Spriel wrote:
> On 5/24/2019 8:38 PM, Arend Van Spriel wrote:
>> On May 24, 2019 1:56:43 PM Johannes Berg <johannes@sipsolutions.net> 
>> wrote:
>>
>>> Hi Arend,
>>>
>>> On Mon, 2019-05-20 at 14:00 +0200, Arend van Spriel wrote:
>>>> In 802.11ax D4.0 a new band has been proposed. This series contains
>>>> changes to cfg80211 for supporting this band. With 2GHz and 5GHz there
>>>> was no overlap in channel number. However, this new band has channel
>>>> numbers with a range from 1 up to 253.
>>>
>>> At the wireless workshop in Prague, we looked at this and sort of
>>> decided that it'd be better to put all the 6 GHz channels into the 5 GHz
>>> "band" in nl80211, to avoid all the "5 || 6" since they're really the
>>> same except for very specific places like scanning.
>>
>> Would have liked to be there, but attending is no longer an option for 
>> me. We now have two autistic, non-verbal children and I am the primary 
>> caregiver for the oldest because my wife can't handle him. Guess I 
>> should have checked the workshop notes before working on this :-) Do 
>> you have URL?
> 
> Found the netdev wifi workshop page and looked over the slides quickly, 
> but the notes page is pretty empty ;-)
> 
>> Agree that most functional requirements for 6 GHz are same as 5 GHz. 
>> There are some 6 GHz specifics about beaconing as well.
> 
> This came up in discussion with my colleagues today and I would say from 
> mac80211 perspective there is more to it than just scanning. In short 
> the 6GHz band is for HE-only operation so for example only HE rates may 
> be used. As the bitrates are in ieee80211_supported_band having a 
> separate 6GHz band seems to have a (slight?) advantage.

Hi, Johannes

Any thoughts on this?

Regards,
Arend

^ permalink raw reply

* [PATCH] Revert "cfg80211: fix processing world regdomain when non modular"
From: Hodaszi, Robert @ 2019-06-03 11:41 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org

This reverts commit 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2.

Re-triggering a reg_process_hint with the last request on all events,
can make the regulatory domain fail in case of multiple WiFi modules. On
slower boards (espacially with mdev), enumeration of the WiFi modules
can end up in an intersected regulatory domain, and user cannot set it
with 'iw reg set' anymore.

This is happening, because:
- 1st module enumerates, queues up a regulatory request
- request gets processed by __reg_process_hint_driver():
  - checks if previous was set by CORE -> yes
    - checks if regulator domain changed -> yes, from '00' to e.g. 'US'
      -> sends request to the 'crda'
- 2nd module enumerates, queues up a regulator request (which triggers
the reg_todo() work)
- reg_todo() -> reg_process_pending_hints() sees, that the last request
is not processed yet, so it tries to process it again.
__reg_process_hint driver() will run again, and:
  - checks if the last request's initiator was the core -> no, it was
the driver (1st WiFi module)
  - checks, if the previous initiator was the driver -> yes
    - checks if the regulator domain changed -> yes, it was '00' (set by
core, and crda call did not return yet), and should be changed to 'US'

------> __reg_process_hint_driver calls an intersect

Besides, the reg_process_hint call with the last request is meaningless
since the crda call has a timeout work. If that timeout expires, the
first module's request will lost.

Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
---
 net/wireless/reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4831ad745f91..b00ebf6fc696 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2788,7 +2788,7 @@ static void reg_process_pending_hints(void)
 
 	/* When last_request->processed becomes true this will be rescheduled */
 	if (lr && !lr->processed) {
-		reg_process_hint(lr);
+		REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
 		return;
 	}
 

^ permalink raw reply related

* INFO: trying to register non-static key in mwifiex_unregister_dev
From: syzbot @ 2019-06-03 13:31 UTC (permalink / raw)
  To: amitkarwar, andreyknvl, davem, gbhat, huxinming820, kvalo,
	linux-kernel, linux-usb, linux-wireless, netdev, nishants,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    69bbe8c7 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=1448d0f2a00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=193d8457178b3229
dashboard link: https://syzkaller.appspot.com/bug?extid=373e6719b49912399d21
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16e57ca6a00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1106eda2a00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+373e6719b49912399d21@syzkaller.appspotmail.com

usb 1-1: Using ep0 maxpacket: 8
usb 1-1: config 0 has an invalid interface number: 182 but max is 0
usb 1-1: config 0 has no interface number 0
usb 1-1: New USB device found, idVendor=1286, idProduct=2052,  
bcdDevice=61.43
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
usb 1-1: Direct firmware load for mrvl/usbusb8997_combo_v4.bin failed with  
error -2
usb 1-1: Failed to get firmware mrvl/usbusb8997_combo_v4.bin
usb 1-1: info: _mwifiex_fw_dpc: unregister device
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #10
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: events request_firmware_work_func
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  assign_lock_key kernel/locking/lockdep.c:774 [inline]
  register_lock_class+0x11ae/0x1240 kernel/locking/lockdep.c:1083
  __lock_acquire+0x11d/0x5340 kernel/locking/lockdep.c:3673
  lock_acquire+0x100/0x2b0 kernel/locking/lockdep.c:4302
  del_timer_sync+0x3a/0x130 kernel/time/timer.c:1277
  mwifiex_usb_cleanup_tx_aggr  
drivers/net/wireless/marvell/mwifiex/usb.c:1358 [inline]
  mwifiex_unregister_dev+0x416/0x690  
drivers/net/wireless/marvell/mwifiex/usb.c:1370
  _mwifiex_fw_dpc+0x577/0xda0 drivers/net/wireless/marvell/mwifiex/main.c:651
  request_firmware_work_func+0x126/0x242  
drivers/base/firmware_loader/main.c:785
  process_one_work+0x905/0x1570 kernel/workqueue.c:2268
  worker_thread+0x96/0xe20 kernel/workqueue.c:2414
  kthread+0x30b/0x410 kernel/kthread.c:254
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
------------[ cut here ]------------
ODEBUG: assert_init not available (active state 0) object type: timer_list  
hint: 0x0
WARNING: CPU: 1 PID: 21 at lib/debugobjects.c:325  
debug_print_object+0x160/0x250 lib/debugobjects.c:325


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH] carl9170: Fix misuse of device driver API
From: Alan Stern @ 2019-06-03 14:10 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, Kalle Valo, USB list
In-Reply-To: <20190602090622.13656-1-chunkeey@gmail.com>

On Sun, 2 Jun 2019, Christian Lamparter wrote:

> This patch follows Alan Stern's recent patch:
> "p54: Fix race between disconnect and firmware loading"
> 
> that overhauled carl9170 buggy firmware loading and driver
> unbinding procedures.
> 
> Since the carl9170 code was adapted from p54 it uses the
> same functions and is likely to have the same problem, but
> it's just that the syzbot hasn't reproduce them (yet).
> 
> a summary from the changes (copied from the p54 patch):
>  * Call usb_driver_release_interface() rather than
>    device_release_driver().
> 
>  * Lock udev (the interface's parent) before unbinding the
>    driver instead of locking udev->parent.
> 
>  * During the firmware loading process, take a reference
>    to the USB interface instead of the USB device.
> 
>  * Don't take an unnecessary reference to the device during
>    probe (and then don't drop it during disconnect).
> 
> and
> 
>  * Make sure to prevent use-after-free bugs by explicitly
>    setting the driver context to NULL after signaling the
>    completion.
> 
> Cc: <stable@vger.kernel.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>

This basically looks right.  However...

> ---
>  drivers/net/wireless/ath/carl9170/usb.c | 26 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
> index e7c3f3b8457d..297a7b877d31 100644
> --- a/drivers/net/wireless/ath/carl9170/usb.c
> +++ b/drivers/net/wireless/ath/carl9170/usb.c
> @@ -128,6 +128,8 @@ static const struct usb_device_id carl9170_usb_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(usb, carl9170_usb_ids);
>  
> +static struct usb_driver carl9170_driver;
> +
>  static void carl9170_usb_submit_data_urb(struct ar9170 *ar)
>  {
>  	struct urb *urb;
> @@ -966,7 +968,7 @@ static int carl9170_usb_init_device(struct ar9170 *ar)
>  
>  static void carl9170_usb_firmware_failed(struct ar9170 *ar)
>  {
> -	struct device *parent = ar->udev->dev.parent;
> +	struct usb_interface *intf = ar->intf;
>  	struct usb_device *udev;

It looks a little strange to initialize intf in the definition but to 
initialize udev afterward.  Nothing wrong with it, just odd.

>  
>  	/*
> @@ -978,16 +980,15 @@ static void carl9170_usb_firmware_failed(struct ar9170 *ar)
>  	udev = ar->udev;
>  
>  	complete(&ar->fw_load_wait);
> +	/* at this point 'ar' could be already freed. Don't use it anymore */
> +	ar = NULL;
>  
>  	/* unbind anything failed */
> -	if (parent)
> -		device_lock(parent);
> -
> -	device_release_driver(&udev->dev);
> -	if (parent)
> -		device_unlock(parent);
> +	usb_lock_device(udev);
> +	usb_driver_release_interface(&carl9170_driver, intf);
> +	usb_unlock_device(udev);
>  
> -	usb_put_dev(udev);
> +	usb_put_intf(intf);
>  }
>  
>  static void carl9170_usb_firmware_finish(struct ar9170 *ar)
> @@ -1009,7 +1010,7 @@ static void carl9170_usb_firmware_finish(struct ar9170 *ar)
>  		goto err_unrx;
>  
>  	complete(&ar->fw_load_wait);
> -	usb_put_dev(ar->udev);
> +	usb_put_intf(ar->intf);

But this could be a problem.  As soon as the complete() call runs, ar 
might be deallocated.  The code should copy ar->intf before calling 
complete().

Alan Stern

>  	return;
>  
>  err_unrx:
> @@ -1052,7 +1053,6 @@ static int carl9170_usb_probe(struct usb_interface *intf,
>  		return PTR_ERR(ar);
>  
>  	udev = interface_to_usbdev(intf);
> -	usb_get_dev(udev);
>  	ar->udev = udev;
>  	ar->intf = intf;
>  	ar->features = id->driver_info;
> @@ -1094,15 +1094,14 @@ static int carl9170_usb_probe(struct usb_interface *intf,
>  	atomic_set(&ar->rx_anch_urbs, 0);
>  	atomic_set(&ar->rx_pool_urbs, 0);
>  
> -	usb_get_dev(ar->udev);
> +	usb_get_intf(intf);
>  
>  	carl9170_set_state(ar, CARL9170_STOPPED);
>  
>  	err = request_firmware_nowait(THIS_MODULE, 1, CARL9170FW_NAME,
>  		&ar->udev->dev, GFP_KERNEL, ar, carl9170_usb_firmware_step2);
>  	if (err) {
> -		usb_put_dev(udev);
> -		usb_put_dev(udev);
> +		usb_put_intf(intf);
>  		carl9170_free(ar);
>  	}
>  	return err;
> @@ -1131,7 +1130,6 @@ static void carl9170_usb_disconnect(struct usb_interface *intf)
>  
>  	carl9170_release_firmware(ar);
>  	carl9170_free(ar);
> -	usb_put_dev(udev);
>  }
>  
>  #ifdef CONFIG_PM
> 


^ permalink raw reply

* Re: INFO: trying to register non-static key in mwifiex_unregister_dev
From: Andrey Konovalov @ 2019-06-03 14:31 UTC (permalink / raw)
  To: syzbot
  Cc: amitkarwar, David S. Miller, gbhat, huxinming820, Kalle Valo,
	LKML, USB list, linux-wireless, netdev, nishants, syzkaller-bugs
In-Reply-To: <00000000000044cec9058a6b6003@google.com>

On Mon, Jun 3, 2019 at 3:31 PM syzbot
<syzbot+373e6719b49912399d21@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    69bbe8c7 usb-fuzzer: main usb gadget fuzzer driver
> git tree:       https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=1448d0f2a00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=193d8457178b3229
> dashboard link: https://syzkaller.appspot.com/bug?extid=373e6719b49912399d21
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16e57ca6a00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1106eda2a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+373e6719b49912399d21@syzkaller.appspotmail.com
>
> usb 1-1: Using ep0 maxpacket: 8
> usb 1-1: config 0 has an invalid interface number: 182 but max is 0
> usb 1-1: config 0 has no interface number 0
> usb 1-1: New USB device found, idVendor=1286, idProduct=2052,
> bcdDevice=61.43
> usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> usb 1-1: config 0 descriptor??
> usb 1-1: Direct firmware load for mrvl/usbusb8997_combo_v4.bin failed with
> error -2
> usb 1-1: Failed to get firmware mrvl/usbusb8997_combo_v4.bin
> usb 1-1: info: _mwifiex_fw_dpc: unregister device
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #10
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Workqueue: events request_firmware_work_func
> Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0xca/0x13e lib/dump_stack.c:113
>   assign_lock_key kernel/locking/lockdep.c:774 [inline]
>   register_lock_class+0x11ae/0x1240 kernel/locking/lockdep.c:1083
>   __lock_acquire+0x11d/0x5340 kernel/locking/lockdep.c:3673
>   lock_acquire+0x100/0x2b0 kernel/locking/lockdep.c:4302
>   del_timer_sync+0x3a/0x130 kernel/time/timer.c:1277
>   mwifiex_usb_cleanup_tx_aggr
> drivers/net/wireless/marvell/mwifiex/usb.c:1358 [inline]
>   mwifiex_unregister_dev+0x416/0x690
> drivers/net/wireless/marvell/mwifiex/usb.c:1370
>   _mwifiex_fw_dpc+0x577/0xda0 drivers/net/wireless/marvell/mwifiex/main.c:651
>   request_firmware_work_func+0x126/0x242
> drivers/base/firmware_loader/main.c:785
>   process_one_work+0x905/0x1570 kernel/workqueue.c:2268
>   worker_thread+0x96/0xe20 kernel/workqueue.c:2414
>   kthread+0x30b/0x410 kernel/kthread.c:254
>   ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> ------------[ cut here ]------------
> ODEBUG: assert_init not available (active state 0) object type: timer_list
> hint: 0x0
> WARNING: CPU: 1 PID: 21 at lib/debugobjects.c:325
> debug_print_object+0x160/0x250 lib/debugobjects.c:325
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

#syz dup: INFO: trying to register non-static key in del_timer_sync (2)

^ permalink raw reply

* Re: [PATCH] ath10k: avoid leaving .bss_info_changed prematurely
From: Kalle Valo @ 2019-06-03 14:48 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: ath10k, linux-wireless, Sven Eckelmann, Sriram R
In-Reply-To: <20190311083107.311-1-sven@narfation.org>

Sven Eckelmann <sven@narfation.org> writes:

> From: Sven Eckelmann <seckelmann@datto.com>
>
> ath10k_bss_info_changed() handles various events from the upper layers. It
> parses the changed bitfield and then configures the driver/firmware
> accordingly. Each detected event is handled in a separate scope which is
> independent of each other - but in the same function.
>
> The commit f279294e9ee2 ("ath10k: add support for configuring management
> packet rate") changed this behavior by returning from this function
> prematurely when some precondition was not fulfilled. All new event
> handlers added after the BSS_CHANGED_BASIC_RATES event handler would then
> also be skipped.
>
> Signed-off-by: Sven Eckelmann <seckelmann@datto.com>
> ---
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Sriram R <srirrama@codeaurora.org>
>
> Only compile tested

Unfortunately doesn't apply anymore, please rebase.

Applying: ath10k: avoid leaving .bss_info_changed prematurely
Using index info to reconstruct a base tree...
M       drivers/net/wireless/ath/ath10k/mac.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/ath/ath10k/mac.c
CONFLICT (content): Merge conflict in drivers/net/wireless/ath/ath10k/mac.c
Recorded preimage for 'drivers/net/wireless/ath/ath10k/mac.c'
error: Failed to merge in the changes.
Patch failed at 0001 ath10k: avoid leaving .bss_info_changed prematurely

-- 
Kalle Valo

^ permalink raw reply

* [PATCH v2] ath10k: avoid leaving .bss_info_changed prematurely
From: Sven Eckelmann @ 2019-06-03 15:25 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Sven Eckelmann, Kalle Valo, Sriram R

From: Sven Eckelmann <seckelmann@datto.com>

ath10k_bss_info_changed() handles various events from the upper layers. It
parses the changed bitfield and then configures the driver/firmware
accordingly. Each detected event is handled in a separate scope which is
independent of each other - but in the same function.

The commit f279294e9ee2 ("ath10k: add support for configuring management
packet rate") changed this behavior by returning from this function
prematurely when some precondition was not fulfilled. All new event
handlers added after the BSS_CHANGED_BASIC_RATES event handler would then
also be skipped.

Signed-off-by: Sven Eckelmann <seckelmann@datto.com>
---
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Sriram R <srirrama@codeaurora.org>

Only compile tested

v2:

* rebased on top of commit 9e7251fa3897 ("ath10k: Check tx_stats before
  use it")

 drivers/net/wireless/ath/ath10k/mac.c | 64 ++++++++++++++++-----------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 98a7842e09b1..90f59117a04d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5582,6 +5582,40 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
 	mutex_unlock(&ar->conf_mutex);
 }
 
+static void ath10k_recalculate_mgmt_rate(struct ath10k *ar,
+					 struct ieee80211_vif *vif)
+{
+	struct ath10k_vif *arvif = (void *)vif->drv_priv;
+	const struct ieee80211_supported_band *sband;
+	struct cfg80211_chan_def def;
+	u8 basic_rate_idx;
+	int hw_rate_code;
+	u32 vdev_param;
+	u16 bitrate;
+	int ret;
+
+	lockdep_assert_held(&ar->conf_mutex);
+
+	if (WARN_ON(ath10k_mac_vif_chan(vif, &def)))
+		return;
+
+	sband = ar->hw->wiphy->bands[def.chan->band];
+	basic_rate_idx = ffs(vif->bss_conf.basic_rates) - 1;
+	bitrate = sband->bitrates[basic_rate_idx].bitrate;
+
+	hw_rate_code = ath10k_mac_get_rate_hw_value(bitrate);
+	if (hw_rate_code < 0) {
+		ath10k_warn(ar, "bitrate not supported %d\n", bitrate);
+		return;
+	}
+
+	vdev_param = ar->wmi.vdev_param->mgmt_rate;
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
+					hw_rate_code);
+	if (ret)
+		ath10k_warn(ar, "failed to set mgmt tx rate %d\n", ret);
+}
+
 static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 				    struct ieee80211_vif *vif,
 				    struct ieee80211_bss_conf *info,
@@ -5592,10 +5626,9 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 	struct cfg80211_chan_def def;
 	u32 vdev_param, pdev_param, slottime, preamble;
 	u16 bitrate, hw_value;
-	u8 rate, basic_rate_idx, rateidx;
-	int ret = 0, hw_rate_code, mcast_rate;
+	u8 rate, rateidx;
+	int ret = 0, mcast_rate;
 	enum nl80211_band band;
-	const struct ieee80211_supported_band *sband;
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -5819,29 +5852,8 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 				    arvif->vdev_id,  ret);
 	}
 
-	if (changed & BSS_CHANGED_BASIC_RATES) {
-		if (WARN_ON(ath10k_mac_vif_chan(vif, &def))) {
-			mutex_unlock(&ar->conf_mutex);
-			return;
-		}
-
-		sband = ar->hw->wiphy->bands[def.chan->band];
-		basic_rate_idx = ffs(vif->bss_conf.basic_rates) - 1;
-		bitrate = sband->bitrates[basic_rate_idx].bitrate;
-
-		hw_rate_code = ath10k_mac_get_rate_hw_value(bitrate);
-		if (hw_rate_code < 0) {
-			ath10k_warn(ar, "bitrate not supported %d\n", bitrate);
-			mutex_unlock(&ar->conf_mutex);
-			return;
-		}
-
-		vdev_param = ar->wmi.vdev_param->mgmt_rate;
-		ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
-						hw_rate_code);
-		if (ret)
-			ath10k_warn(ar, "failed to set mgmt tx rate %d\n", ret);
-	}
+	if (changed & BSS_CHANGED_BASIC_RATES)
+		ath10k_recalculate_mgmt_rate(ar, vif);
 
 	mutex_unlock(&ar->conf_mutex);
 }
-- 
2.20.1


^ permalink raw reply related

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

There are certain cases, notably when transitioning between sleep and
active state, when Broadcom SDIO WiFi cards will produce errors on the
SDIO bus.  This is evident from the source code where you can see that
we try commands in a loop until we either get success or we've tried
too many times.  The comment in the code reinforces this by saying
"just one write attempt may fail"

Unfortunately these failures sometimes end up causing an "-EILSEQ"
back to the core which triggers a retuning of the SDIO card and that
blocks all traffic to the card until it's done.

Let's disable retuning around the commands we expect might fail.

Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2: None

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4a750838d8cd..e0cfcc078a54 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -16,6 +16,7 @@
 #include <linux/mmc/sdio_ids.h>
 #include <linux/mmc/sdio_func.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/core.h>
 #include <linux/semaphore.h>
 #include <linux/firmware.h>
 #include <linux/module.h>
@@ -697,6 +698,7 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 		bmask = SBSDIO_FUNC1_SLEEPCSR_KSO_MASK;
 	}
 
+	mmc_expect_errors_begin(bus->sdiodev->func1->card->host);
 	do {
 		/* reliable KSO bit set/clr:
 		 * the sdiod sleep write access is synced to PMU 32khz clk
@@ -719,6 +721,7 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 				   &err);
 
 	} while (try_cnt++ < MAX_KSO_ATTEMPTS);
+	mmc_expect_errors_end(bus->sdiodev->func1->card->host);
 
 	if (try_cnt > 2)
 		brcmf_dbg(SDIO, "try_cnt=%d rd_val=0x%x err=%d\n", try_cnt,
-- 
2.22.0.rc1.311.g5d7573a151-goog


^ permalink raw reply related

* [PATCH v2 2/3] mmc: core: API for temporarily disabling auto-retuning due to errors
From: Douglas Anderson @ 2019-06-03 18:37 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	linux-mmc, linux-kernel, Shawn Lin, Wolfram Sang, Avri Altman
In-Reply-To: <20190603183740.239031-1-dianders@chromium.org>

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

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

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

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

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

Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Note that are are a whole boatload of different ways that we could
provide an API for the Broadcom WiFi SDIO driver.  This patch
illustrates one way but if maintainers feel strongly that this is too
ugly and have a better idea then I can give it a shot too.  From a
purist point of view I kinda felt that the "expect errors" really
belonged as part of the mmc_request structure, but getting it into
there meant changing a whole pile of core SD/MMC APIs.  Simply adding
it to the host seemed to match the current style better and was a less
intrusive change.

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

 drivers/mmc/core/core.c  | 27 +++++++++++++++++++++++++--
 include/linux/mmc/core.h |  2 ++
 include/linux/mmc/host.h |  1 +
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db36dc870b5..ba4f71aa8cd9 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -144,8 +144,9 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
 	int err = cmd->error;
 
 	/* Flag re-tuning needed on CRC errors */
-	if ((cmd->opcode != MMC_SEND_TUNING_BLOCK &&
-	    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200) &&
+	if (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
+	    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
+	    !host->expect_errors &&
 	    (err == -EILSEQ || (mrq->sbc && mrq->sbc->error == -EILSEQ) ||
 	    (mrq->data && mrq->data->error == -EILSEQ) ||
 	    (mrq->stop && mrq->stop->error == -EILSEQ)))
@@ -2163,6 +2164,28 @@ int mmc_sw_reset(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_sw_reset);
 
+void mmc_expect_errors_begin(struct mmc_host *host)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->lock, flags);
+	WARN_ON(host->expect_errors);
+	host->expect_errors = true;
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+EXPORT_SYMBOL_GPL(mmc_expect_errors_begin);
+
+void mmc_expect_errors_end(struct mmc_host *host)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->lock, flags);
+	WARN_ON(!host->expect_errors);
+	host->expect_errors = false;
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+EXPORT_SYMBOL_GPL(mmc_expect_errors_end);
+
 static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 {
 	host->f_init = freq;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 134a6483347a..02a13abf0cda 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -178,6 +178,8 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd,
 
 int mmc_hw_reset(struct mmc_host *host);
 int mmc_sw_reset(struct mmc_host *host);
+void mmc_expect_errors_begin(struct mmc_host *host);
+void mmc_expect_errors_end(struct mmc_host *host);
 void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card);
 
 #endif /* LINUX_MMC_CORE_H */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 43d0f0c496f6..8d553fb8c834 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -398,6 +398,7 @@ struct mmc_host {
 	unsigned int		retune_now:1;	/* do re-tuning at next req */
 	unsigned int		retune_paused:1; /* re-tuning is temporarily disabled */
 	unsigned int		use_blk_mq:1;	/* use blk-mq */
+	unsigned int		expect_errors:1; /* don't trigger retune upon errors */
 
 	int			rescan_disable;	/* disable card detection */
 	int			rescan_entered;	/* used with nonremovable devices */
-- 
2.22.0.rc1.311.g5d7573a151-goog


^ permalink raw reply related

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

This reverts commit 29f6589140a10ece8c1d73f58043ea5b3473ab3e.

After that patch landed I find that my kernel log on
rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep state -110

This seems to happen every time the Broadcom WiFi transitions out of
sleep mode.  Reverting the commit fixes the problem for me, so that's
what this patch does.

Note that, in general, the justification in the original commit seemed
a little weak.  It looked like someone was testing on a SD card
controller that would sometimes die if there were CRC errors on the
bus.  This used to happen back in early days of dw_mmc (the controller
on my boards), but we fixed it.  Disabling a feature on all boards
just because one SD card controller is broken seems bad.

Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos")
Cc: Wright Feng <wright.feng@cypress.com>
Cc: Double Lo <double.lo@cypress.com>
Cc: Madhan Mohan R <madhanmohan.r@cypress.com>
Cc: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

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

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4e15ea57d4f5..4a750838d8cd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3364,11 +3364,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
 
 static bool brcmf_sdio_aos_no_decode(struct brcmf_sdio *bus)
 {
-	if (bus->ci->chip == CY_CC_43012_CHIP_ID ||
-	    bus->ci->chip == CY_CC_4373_CHIP_ID ||
-	    bus->ci->chip == BRCM_CC_4339_CHIP_ID ||
-	    bus->ci->chip == BRCM_CC_4345_CHIP_ID ||
-	    bus->ci->chip == BRCM_CC_4354_CHIP_ID)
+	if (bus->ci->chip == CY_CC_43012_CHIP_ID)
 		return true;
 	else
 		return false;
-- 
2.22.0.rc1.311.g5d7573a151-goog


^ permalink raw reply related

* [PATCH v2 0/3] brcmfmac: sdio: Deal better w/ transmission errors waking from idle
From: Douglas Anderson @ 2019-06-03 18:37 UTC (permalink / raw)
  To: Ulf Hansson, Kalle Valo, Adrian Hunter, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, Douglas Anderson,
	linux-mmc, Shawn Lin, YueHaibing, Hans de Goede, Hante Meuleman,
	Michael Trimarchi, Wolfram Sang, Franky Lin, David S. Miller,
	linux-kernel, Avri Altman

This series attempts to deal better with the expected transmission
errors that we get when waking up (from idle) the SDIO-based WiFi on
rk3288-veyron-minnie, rk3288-veyron-speedy, and rk3288-veyron-mickey.

Some details about those errors can be found in
<https://crbug.com/960222>, but to summarize it here: if we try to
send the wakeup command to the WiFi card at the same time it has
decided to wake up itself then it will behave badly on the SDIO bus.
This can cause timeouts or CRC errors.

When I tested on 4.19 and 4.20 these CRC errors can be seen to cause
re-tuning.  Since I am currently developing on 4.19 this was the
original problem I attempted to solve.

On mainline it turns out that you don't see the retuning errors but
you see tons of spam about timeouts trying to wakeup from sleep.  I
tracked down the commit that was causing that and have partially
reverted it here.  I have no real knowledge about Broadcom WiFi, but
the commit that was causing problems sounds (from the descriptioin) to
be a hack commit penalizing all Broadcom WiFi users because of a bug
in a Cypress SD controller.  I will let others comment if this is
truly the case and, if so, what the right solution should be.

There wasn't a good resolution on v1 and it's been a while, so I'm
sending out a v2.  Other than changing patch #1 to a full revert, the
only other changes here are just to the patch descriptions.

Changes in v2:
- A full revert, not just a partial one (Arend).  ...with explicit Cc.
- Updated commit message to clarify based on discussion of v1.

Douglas Anderson (3):
  Revert "brcmfmac: disable command decode in sdio_aos"
  mmc: core: API for temporarily disabling auto-retuning due to errors
  brcmfmac: sdio: Disable auto-tuning around commands expected to fail

 drivers/mmc/core/core.c                       | 27 +++++++++++++++++--
 .../broadcom/brcm80211/brcmfmac/sdio.c        |  9 +++----
 include/linux/mmc/core.h                      |  2 ++
 include/linux/mmc/host.h                      |  1 +
 4 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.22.0.rc1.311.g5d7573a151-goog


^ permalink raw reply

* Re: [PATCH v2] mmc: dw_mmc: Disable SDIO interrupts while suspended to fix suspend/resume
From: Doug Anderson @ 2019-06-03 18:41 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Jaehoon Chung, Shawn Lin, Kalle Valo, Heiko Stuebner,
	open list:ARM/Rockchip SoC..., Guenter Roeck, Brian Norris,
	linux-wireless, Sonny Rao, Emil Renner Berthing,
	Matthias Kaehlcke, Ryan Case, # 4.0+, linux-mmc@vger.kernel.org,
	Linux Kernel Mailing List
In-Reply-To: <CAD=FV=XMph_CE3pFZGP+5d0K2FgbPbheF1oX72TfZn_dpf8SQA@mail.gmail.com>

Ulf,

On Tue, May 28, 2019 at 3:49 PM Doug Anderson <dianders@chromium.org> wrote:
>
> > 1) As kind of stated above, did you consider a solution where the core
> > simply disables the SDIO IRQ in case it isn't enabled for system
> > wakeup? In this way all host drivers would benefit.
>
> I can give it a shot if you can give me a bunch of specific advice,
> but I only have access to a few devices doing anything with SDIO and
> they are all using Marvell or Broadcom on dw_mmc.
>
> In general I have no idea how SDIO wakeup (plumbed through the SD
> controller) would work.  As per below the only way I've seen it done
> is totally out-of-band.  ...and actually, I'm not sure I've actually
> ever seen even the out of band stuff truly work on a system myself.
> It's always been one of those "we should support wake on WiFi" but
> never made it all the way to implementation.  In any case, if there
> are examples of people plumbing wakeup through the SD controller I'd
> need to figure out how to not break them.  Just doing a solution for
> dw_mmc means I don't have to worry about this since dw_mmc definitely
> doesn't support SDIO wakeup.
>
> Maybe one way to get a more generic solution is if you had an idea for
> a patch that would work for many host controllers then you could post
> it and I could test to confirm that it's happy on dw_mmc?  ...similar
> to when you switched dw_mmc away from the old kthread-based SDIO
> stuff?

Unless you have time to help dig into all the possibilities here to
help understand how this should behave across all the different host
controllers / wakeup setups, maybe we could just land ${SUBJECT} patch
for now and when there is more clarity we can revisit?

Thanks!

-Doug

^ permalink raw reply

* WARNING: suspicious RCU usage in in_dev_dump_addr
From: syzbot @ 2019-06-03 18:51 UTC (permalink / raw)
  To: amitkarwar, anshuman.khandual, axboe, benedictwong, benve,
	coreteam, davej, davem, dbanerje, devel, dledford, doshir,
	edumazet, faisal.latif, fw, gbhat, gregkh, gustavo, huxinming820,
	idosch, jakub.kicinski, jgg, johannes, kadlec, keescook, kuznet,
	kvalo, linux-kernel, linux-rdma, linux-wireless, liuhangbin,
	lucien.xin, matwey, mpe, neescoba, netdev, netfilter-devel,
	nishants, pablo, paulmck, petrm, pkaustub, pv-drivers, romieu,
	shannon.nelson, shiraz.saleem, steffen.klassert, syzkaller-bugs,
	yoshfuji

Hello,

syzbot found the following crash on:

HEAD commit:    b33bc2b8 nexthop: Add entry to MAINTAINERS
git tree:       net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=13f46f52a00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=1004db091673bbaf
dashboard link: https://syzkaller.appspot.com/bug?extid=bad6e32808a3a97b1515
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11dc685aa00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16229e36a00000

The bug was bisected to:

commit 2638eb8b50cfc16240e0bb080b9afbf541a9b39d
Author: Florian Westphal <fw@strlen.de>
Date:   Fri May 31 16:27:09 2019 +0000

     net: ipv4: provide __rcu annotation for ifa_list

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=170e1a0ea00000
final crash:    https://syzkaller.appspot.com/x/report.txt?x=148e1a0ea00000
console output: https://syzkaller.appspot.com/x/log.txt?x=108e1a0ea00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bad6e32808a3a97b1515@syzkaller.appspotmail.com
Fixes: 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")

=============================
WARNING: suspicious RCU usage
5.2.0-rc2+ #13 Not tainted
-----------------------------
net/ipv4/devinet.c:1766 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
1 lock held by syz-executor924/9000:
  #0: 0000000087fe3874 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0  
net/netlink/af_netlink.c:2208

stack backtrace:
CPU: 0 PID: 9000 Comm: syz-executor924 Not tainted 5.2.0-rc2+ #13
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
  in_dev_dump_addr+0x36f/0x3d0 net/ipv4/devinet.c:1766
  inet_dump_ifaddr+0xa8f/0xca0 net/ipv4/devinet.c:1826
  rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3444
  netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2253
  __netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2361
  netlink_dump_start include/linux/netlink.h:226 [inline]
  rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5181
  netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2486
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5236
  netlink_unicast_kernel net/netlink/af_netlink.c:1311 [inline]
  netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1337
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1926
  sock_sendmsg_nosec net/socket.c:652 [inline]
  sock_sendmsg+0xd7/0x130 net/socket.c:671
  ___sys_sendmsg+0x803/0x920 net/socket.c:2292
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2330
  __do_sys_sendmsg net/socket.c:2339 [inline]
  __se_sys_sendmsg net/socket.c:2337 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2337
  do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4402a9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fffe5f26f18 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004402a9
RDX: 0000000000000000 RSI: 0000000020000240 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10:


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [RFC PATCH v4] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Jes Sorensen @ 2019-06-03 19:21 UTC (permalink / raw)
  To: Chris Chiu, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux
In-Reply-To: <20190531091229.93033-1-chiu@endlessm.com>

On 5/31/19 5:12 AM, Chris Chiu wrote:
> We have 3 laptops which connect the wifi by the same RTL8723BU.
> The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
> They have the same problem with the in-kernel rtl8xxxu driver, the
> iperf (as a client to an ethernet-connected server) gets ~1Mbps.
> Nevertheless, the signal strength is reported as around -40dBm,
> which is quite good. From the wireshark capture, the tx rate for each
> data and qos data packet is only 1Mbps. Compare to the Realtek driver
> at https://github.com/lwfinger/rtl8723bu, the same iperf test gets
> ~12Mbps or better. The signal strength is reported similarly around
> -40dBm. That's why we want to improve.
> 
> After reading the source code of the rtl8xxxu driver and Realtek's, the
> major difference is that Realtek's driver has a watchdog which will keep
> monitoring the signal quality and updating the rate mask just like the
> rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
> And this kind of watchdog also exists in rtlwifi driver of some specific
> chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
> the same member function named dm_watchdog and will invoke the
> corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
> mask.
> 
> With this commit, the tx rate of each data and qos data packet will
> be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> the lowest rate from the rate mask and explains why the tx rate of
> data and qos data is always lowest 1Mbps because the default rate mask
> passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> and MCS rate. However, with Realtek's driver, the tx rate observed from
> wireshark under the same condition is almost 65Mbps or 72Mbps.
> 
> I believe the firmware of RTL8723BU may need fix. And I think we
> can still bring in the dm_watchdog as rtlwifi to improve from the
> driver side. Please leave precious comments for my commits and
> suggest what I can do better. Or suggest if there's any better idea
> to fix this. Thanks.
> 
> Signed-off-by: Chris Chiu <chiu@endlessm.com>

I am really pleased to see you're investigating some of these issues,
since I've been pretty swamped and not had time to work on this driver
for a long time.

The firmware should allow for two rate modes, either firmware handled or
controlled by the driver. Ideally we would want the driver to handle it,
but I never was able to make that work reliable.

This fix should at least improve the situation, and it may explain some
of the performance issues with the 8192eu as well?

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 8828baf26e7b..216f603827a8 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1195,6 +1195,44 @@ struct rtl8723bu_c2h {
>  
>  struct rtl8xxxu_fileops;
>  
> +/*mlme related.*/
> +enum wireless_mode {
> +	WIRELESS_MODE_UNKNOWN = 0,
> +	/* Sub-Element */
> +	WIRELESS_MODE_B = BIT(0),
> +	WIRELESS_MODE_G = BIT(1),
> +	WIRELESS_MODE_A = BIT(2),
> +	WIRELESS_MODE_N_24G = BIT(3),
> +	WIRELESS_MODE_N_5G = BIT(4),
> +	WIRELESS_AUTO = BIT(5),
> +	WIRELESS_MODE_AC = BIT(6),
> +	WIRELESS_MODE_MAX = 0x7F,
> +};
> +
> +/* from rtlwifi/wifi.h */
> +enum ratr_table_mode_new {
> +	RATEID_IDX_BGN_40M_2SS = 0,
> +	RATEID_IDX_BGN_40M_1SS = 1,
> +	RATEID_IDX_BGN_20M_2SS_BN = 2,
> +	RATEID_IDX_BGN_20M_1SS_BN = 3,
> +	RATEID_IDX_GN_N2SS = 4,
> +	RATEID_IDX_GN_N1SS = 5,
> +	RATEID_IDX_BG = 6,
> +	RATEID_IDX_G = 7,
> +	RATEID_IDX_B = 8,
> +	RATEID_IDX_VHT_2SS = 9,
> +	RATEID_IDX_VHT_1SS = 10,
> +	RATEID_IDX_MIX1 = 11,
> +	RATEID_IDX_MIX2 = 12,
> +	RATEID_IDX_VHT_3SS = 13,
> +	RATEID_IDX_BGN_3SS = 14,
> +};
> +
> +#define RTL8XXXU_RATR_STA_INIT 0
> +#define RTL8XXXU_RATR_STA_HIGH 1
> +#define RTL8XXXU_RATR_STA_MID  2
> +#define RTL8XXXU_RATR_STA_LOW  3
> +

>  extern struct rtl8xxxu_fileops rtl8192cu_fops;
>  extern struct rtl8xxxu_fileops rtl8192eu_fops;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 26b674aca125..2071ab9fd001 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1645,6 +1645,148 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
>  	rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
>  }
>  
> +static u8 rtl8723b_signal_to_rssi(int signal)
> +{
> +	if (signal < -95)
> +		signal = -95;
> +	return (u8)(signal + 95);
> +}

Could you make this more generic so it can be used by the other sub-drivers?

> +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> +				       int signal, struct ieee80211_sta *sta)
> +{
> +	struct ieee80211_hw *hw = priv->hw;
> +	u16 wireless_mode;
> +	u8 rssi_level, ratr_idx;
> +	u8 txbw_40mhz;
> +	u8 rssi, rssi_thresh_high, rssi_thresh_low;
> +
> +	rssi_level = priv->rssi_level;
> +	rssi = rtl8723b_signal_to_rssi(signal);
> +	txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40) ? 1 : 0;
> +
> +	switch (rssi_level) {
> +	case RTL8XXXU_RATR_STA_HIGH:
> +		rssi_thresh_high = 50;
> +		rssi_thresh_low = 20;
> +		break;
> +	case RTL8XXXU_RATR_STA_MID:
> +		rssi_thresh_high = 55;
> +		rssi_thresh_low = 20;
> +		break;
> +	case RTL8XXXU_RATR_STA_LOW:
> +		rssi_thresh_high = 60;
> +		rssi_thresh_low = 25;
> +		break;
> +	default:
> +		rssi_thresh_high = 50;
> +		rssi_thresh_low = 20;
> +		break;
> +	}

Can we make this use defined values with some explanation rather than
hard coded values?

> +	if (rssi > rssi_thresh_high)
> +		rssi_level = RTL8XXXU_RATR_STA_HIGH;
> +	else if (rssi > rssi_thresh_low)
> +		rssi_level = RTL8XXXU_RATR_STA_MID;
> +	else
> +		rssi_level = RTL8XXXU_RATR_STA_LOW;
> +
> +	if (rssi_level != priv->rssi_level) {
> +		int sgi = 0;
> +		u32 rate_bitmap = 0;
> +
> +		rcu_read_lock();
> +		rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> +				(sta->ht_cap.mcs.rx_mask[0] << 12) |
> +				(sta->ht_cap.mcs.rx_mask[1] << 20);
> +		if (sta->ht_cap.cap &
> +		    (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> +			sgi = 1;
> +		rcu_read_unlock();
> +
> +		wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> +		switch (wireless_mode) {
> +		case WIRELESS_MODE_B:
> +			ratr_idx = RATEID_IDX_B;
> +			if (rate_bitmap & 0x0000000c)
> +				rate_bitmap &= 0x0000000d;
> +			else
> +				rate_bitmap &= 0x0000000f;
> +			break;
> +		case WIRELESS_MODE_A:
> +		case WIRELESS_MODE_G:
> +			ratr_idx = RATEID_IDX_G;
> +			if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> +				rate_bitmap &= 0x00000f00;
> +			else
> +				rate_bitmap &= 0x00000ff0;
> +			break;
> +		case (WIRELESS_MODE_B | WIRELESS_MODE_G):
> +			ratr_idx = RATEID_IDX_BG;
> +			if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> +				rate_bitmap &= 0x00000f00;
> +			else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> +				rate_bitmap &= 0x00000ff0;
> +			else
> +				rate_bitmap &= 0x00000ff5;
> +			break;

It would be nice as well to get all these masks into generic names.

> +		case WIRELESS_MODE_N_24G:
> +		case WIRELESS_MODE_N_5G:
> +		case (WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> +		case (WIRELESS_MODE_A | WIRELESS_MODE_N_5G):
> +			if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +				ratr_idx = RATEID_IDX_GN_N2SS;
> +			else
> +				ratr_idx = RATEID_IDX_GN_N1SS;
> +		case (WIRELESS_MODE_B | WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> +		case (WIRELESS_MODE_B | WIRELESS_MODE_N_24G):
> +			if (txbw_40mhz) {
> +				if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +					ratr_idx = RATEID_IDX_BGN_40M_2SS;
> +				else
> +					ratr_idx = RATEID_IDX_BGN_40M_1SS;
> +			} else {
> +				if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +					ratr_idx = RATEID_IDX_BGN_20M_2SS_BN;
> +				else
> +					ratr_idx = RATEID_IDX_BGN_20M_1SS_BN;
> +			}
> +
> +			if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> +				if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> +					rate_bitmap &= 0x0f8f0000;
> +				} else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> +					rate_bitmap &= 0x0f8ff000;
> +				} else {
> +					if (txbw_40mhz)
> +						rate_bitmap &= 0x0f8ff015;
> +					else
> +						rate_bitmap &= 0x0f8ff005;
> +				}
> +			} else {
> +				if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> +					rate_bitmap &= 0x000f0000;
> +				} else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> +					rate_bitmap &= 0x000ff000;
> +				} else {
> +					if (txbw_40mhz)
> +						rate_bitmap &= 0x000ff015;
> +					else
> +						rate_bitmap &= 0x000ff005;
> +				}
> +			}
> +			break;
> +		default:
> +			ratr_idx = RATEID_IDX_BGN_40M_2SS;
> +			rate_bitmap &= 0x0fffffff;
> +			break;
> +		}
> +
> +		priv->rssi_level = rssi_level;
> +		priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi);
> +	}
> +}
> +

In general I think all of this should be fairly generic and the other
subdrivers should be able to benefit from it?


>  struct rtl8xxxu_fileops rtl8723bu_fops = {
>  	.parse_efuse = rtl8723bu_parse_efuse,
>  	.load_firmware = rtl8723bu_load_firmware,
> @@ -1665,6 +1807,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
>  	.usb_quirks = rtl8xxxu_gen2_usb_quirks,
>  	.set_tx_power = rtl8723b_set_tx_power,
>  	.update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> +	.refresh_rate_mask = rtl8723b_refresh_rate_mask,
>  	.report_connect = rtl8xxxu_gen2_report_connect,
>  	.fill_txdesc = rtl8xxxu_fill_txdesc_v2,
>  	.writeN_block_size = 1024,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 039e5ca9d2e4..be322402ca01 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4311,7 +4311,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
>  	rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>  }
>  
> -void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> +void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> +			       u32 ramask, u8 rateid, int sgi)
>  {
>  	struct h2c_cmd h2c;
>  
> @@ -4331,7 +4332,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
>  }
>  
>  void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> -				    u32 ramask, int sgi)
> +				    u32 ramask, u8 rateid, int sgi)
>  {
>  	struct h2c_cmd h2c;
>  	u8 bw = 0;
> @@ -4345,7 +4346,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
>  	h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
>  
>  	h2c.ramask.arg = 0x80;
> -	h2c.b_macid_cfg.data1 = 0;
> +	h2c.b_macid_cfg.data1 = rateid;
>  	if (sgi)
>  		h2c.b_macid_cfg.data1 |= BIT(7);
>  
> @@ -4485,6 +4486,40 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
>  	rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
>  }
>  
> +u16
> +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> +{
> +	u16 network_type = WIRELESS_MODE_UNKNOWN;
> +	u32 rate_mask;
> +
> +	rate_mask = (sta->supp_rates[0] & 0xfff) |
> +		    (sta->ht_cap.mcs.rx_mask[0] << 12) |
> +		    (sta->ht_cap.mcs.rx_mask[0] << 20);
> +
> +	if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> +		if (sta->vht_cap.vht_supported)
> +			network_type = WIRELESS_MODE_AC;
> +		else if (sta->ht_cap.ht_supported)
> +			network_type = WIRELESS_MODE_N_5G;
> +
> +		network_type |= WIRELESS_MODE_A;
> +	} else {
> +		if (sta->vht_cap.vht_supported)
> +			network_type = WIRELESS_MODE_AC;
> +		else if (sta->ht_cap.ht_supported)
> +			network_type = WIRELESS_MODE_N_24G;
> +
> +		if (sta->supp_rates[0] <= 0xf)
> +			network_type |= WIRELESS_MODE_B;
> +		else if (sta->supp_rates[0] & 0xf)
> +			network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> +		else
> +			network_type |= WIRELESS_MODE_G;
> +	}
> +
> +	return network_type;
> +}

I always hated the wireless_mode nonsense in the realtek driver, but
maybe we cannot avoid it :(

Cheers,
Jes

^ permalink raw reply

* [ANN] wireless-regdb: master-2019-06-03
From: Seth Forshee @ 2019-06-03 21:50 UTC (permalink / raw)
  To: wireless-regdb; +Cc: linux-wireless

A new release of wireless-regdb (master-2019-06-03) is available at:

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

The short log of changes since the 2019-03-01 release is below.

Thanks,
Seth

---

Peter Oh (1):
      wireless-regdb: Update regulatory rules for South Korea

Seth Forshee (2):
      wireless-regdb: Expand 60 GHz band for Japan to 57-66 GHz
      wireless-regdb: update regulatory database based on preceding changes

Vladimir Koutny (1):
      wireless-regdb: Update regulatory rules for Japan (JP) on 5GHz

Xose Vazquez Perez (2):
      wireless-regdb: update source of information for ES
      wireless-regdb: update source of information for CU


^ permalink raw reply

* Re: [PATCH] wireless-regdb: Expand 60 GHz band for Japan to 57-66 GHz
From: Seth Forshee @ 2019-06-03 21:37 UTC (permalink / raw)
  To: wireless-regdb; +Cc: linux-wireless
In-Reply-To: <20190515132617.12852-1-seth.forshee@canonical.com>

On Wed, May 15, 2019 at 08:26:17AM -0500, Seth Forshee wrote:
> The official documents are not feely available, but based on
> summaries such as [1] and numerous third-party resources the 60
> GHz band in Japan has been 57-66 GHz for some time now. Update
> our rules accordingly.
> 
> [1] https://webstore.arib.or.jp/en/products/detail.php?product_id=288
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Applied.

^ permalink raw reply

* Re: [PATCH] wireless-regdb: update source of information for CU
From: Seth Forshee @ 2019-06-03 21:35 UTC (permalink / raw)
  To: Xose Vazquez Perez; +Cc: WIRELESS ML, REGDB ML
In-Reply-To: <20190530110047.3449-1-xose.vazquez@gmail.com>

On Thu, May 30, 2019 at 01:00:47PM +0200, Xose Vazquez Perez wrote:
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: WIRELESS ML <linux-wireless@vger.kernel.org>
> Cc: REGDB ML <wireless-regdb@lists.infradead.org>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

Applied, thanks!

^ permalink raw reply

* RE: [EXT] Re: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage"
From: Ganapathi Bhat @ 2019-06-04  3:03 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-kernel@vger.kernel.org, Amitkumar Karwar,
	Nishant Sarmukadam, Xinming Hu, linux-wireless@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <20190308023401.GA121759@google.com>

Hi Brian,

> >    netif_rx_ni+0xe8/0x120
> >    mwifiex_recv_packet+0xfc/0x10c [mwifiex]
> >    mwifiex_process_rx_packet+0x1d4/0x238 [mwifiex]
> >    mwifiex_11n_dispatch_pkt+0x190/0x1ac [mwifiex]
> >    mwifiex_11n_rx_reorder_pkt+0x28c/0x354 [mwifiex]
> 
> TL;DR: the problem was right here ^^^
> where you started running mwifiex_11n_dispatch_pkt() (via
> mwifiex_11n_scan_and_dispatch()) while holding a spinlock.
> 
> When you do that, you eventually call netif_rx_ni(), which specifically defers
> to softirq contexts. Then, if you happen to have your flush timer expiring just
> before that, you end up in mwifiex_flush_data(), which also needs that
> spinlock.

Understood; Thanks for this detail;

> 
> There are a few possible ways to handle this:
> (a) prevent processing softirqs in that context; e.g., with
>     local_bh_disable(). This seems somewhat of a hack.
>     (Side note: I think most of the locks in this driver really could be
>     spin_lock_bh(), not spin_lock_irqsave() -- we don't really care
>     about hardirq context for 99% of these locks.)
> (b) restructure so that packet processing (e.g., netif_rx_ni()) is done
>     outside of the spinlock.
> 
> It's actually not that hard to do (b). You can just queue your skb's up in a
> temporary sk_buff_head list and process them all at once after you've
> finished processing the reorder table. I have a local patch to do this, and I
> might send it your way if I can give it a bit more testing.


OK; That will be good; We will run a complete test after the patch; (OR we can work on this, share for review);

Regards,
Ganapathi

^ permalink raw reply

* Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
From: Wright Feng @ 2019-06-04  3:20 UTC (permalink / raw)
  To: Arend Van Spriel, Doug Anderson, Kalle Valo
  Cc: Madhan Mohan R, Ulf Hansson, LKML, Hante Meuleman,
	David S. Miller, netdev, Chi-Hsien Lin, Brian Norris,
	linux-wireless, YueHaibing, Adrian Hunter,
	open list:ARM/Rockchip SoC...,
	brcm80211-dev-list.pdl@broadcom.com, Matthias Kaehlcke,
	Naveen Gupta, brcm80211-dev-list, Double Lo, Franky Lin
In-Reply-To: <16aff358a20.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>



On 2019/5/29 上午 12:11, Arend Van Spriel wrote:
> On May 28, 2019 6:09:21 PM Arend Van Spriel 
> <arend.vanspriel@broadcom.com> wrote:
> 
>> On May 28, 2019 5:52:10 PM Doug Anderson <dianders@chromium.org> wrote:
>>
>>> Hi,
>>>
>>> On Tue, May 28, 2019 at 5:18 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>>>>
>>>> Douglas Anderson <dianders@chromium.org> wrote:
>>>>
>>>> > In commit 29f6589140a1 ("brcmfmac: disable command decode in
>>>> > sdio_aos") we disabled something called "command decode in sdio_aos"
>>>> > for a whole bunch of Broadcom SDIO WiFi parts.
>>>> >
>>>> > After that patch landed I find that my kernel log on
>>>> > rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
>>>> >   brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep 
>>>> state -110
>>>> >
>>>> > This seems to happen every time the Broadcom WiFi transitions out of
>>>> > sleep mode.  Reverting the part of the commit that affects the 
>>>> WiFi on
>>>> > my boards fixes the problem for me, so that's what this patch does.
>>>> >
>>>> > Note that, in general, the justification in the original commit 
>>>> seemed
>>>> > a little weak.  It looked like someone was testing on a SD card
>>>> > controller that would sometimes die if there were CRC errors on the
>>>> > bus.  This used to happen back in early days of dw_mmc (the 
>>>> controller
>>>> > on my boards), but we fixed it.  Disabling a feature on all boards
>>>> > just because one SD card controller is broken seems bad.  ...so
>>>> > instead of just this patch possibly the right thing to do is to fully
>>>> > revert the original commit.
>>>> >
Since the commit 29f6589140a1 ("brcmfmac: disable command decode in 
sdio_aos") causes the regression on other SD card controller, it is 
better to revert it as you mentioned.
Actually, without the commit, we hit MMC timeout(-110) and hanged 
instead of CRC error in our test. Would you please share the analysis of 
dw_mmc issue which you fixed? We'd like to compare whether we got the 
same issue.

Regards,
Wright

>>>> > Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos")
>>>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>>>
>>>> I don't see patch 2 in patchwork and I assume discussion continues.
>>>
>>> Apologies.  I made sure to CC you individually on all the patches but
>>> didn't think about the fact that you use patchwork to manage and so
>>> didn't ensure all patches made it to all lists (by default each patch
>>> gets recipients individually from get_maintainer).  I'll make sure to
>>> fix for patch set #2.  If you want to see all the patches, you can at
>>> least find them on lore.kernel.org linked from the cover:
>>>
>>> https://lore.kernel.org/patchwork/cover/1075373/
>>>
>>>
>>>> Please resend if/when I need to apply something.
>>>>
>>>> 2 patches set to Changes Requested.
>>>>
>>>> 10948785 [1/3] brcmfmac: re-enable command decode in sdio_aos for 
>>>> BRCM 4354
>>>
>>> As per Arend I'll change patch #1 to a full revert instead of a
>>> partial revert.  Arend: please yell if you want otherwise.
>>
>> No yelling here. If any it is expected from Cypress. Maybe good to add 
>> them
>> specifically in Cc:
> 
> Of the revert patch that is.
> 
> Gr. AvS
> 
> 

^ permalink raw reply

* Re: [PATCH v2] mmc: dw_mmc: Disable SDIO interrupts while suspended to fix suspend/resume
From: Ulf Hansson @ 2019-06-04  7:30 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jaehoon Chung, Shawn Lin, Kalle Valo, Heiko Stuebner,
	open list:ARM/Rockchip SoC..., Guenter Roeck, Brian Norris,
	linux-wireless, Sonny Rao, Emil Renner Berthing,
	Matthias Kaehlcke, Ryan Case, # 4.0+, linux-mmc@vger.kernel.org,
	Linux Kernel Mailing List
In-Reply-To: <CAD=FV=U7_ek_z7UfaDn9My8UfZfpNom04OJHowoH-sNsGZQnxA@mail.gmail.com>

On Mon, 3 Jun 2019 at 20:41, Doug Anderson <dianders@chromium.org> wrote:
>
> Ulf,
>
> On Tue, May 28, 2019 at 3:49 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > > 1) As kind of stated above, did you consider a solution where the core
> > > simply disables the SDIO IRQ in case it isn't enabled for system
> > > wakeup? In this way all host drivers would benefit.
> >
> > I can give it a shot if you can give me a bunch of specific advice,
> > but I only have access to a few devices doing anything with SDIO and
> > they are all using Marvell or Broadcom on dw_mmc.
> >
> > In general I have no idea how SDIO wakeup (plumbed through the SD
> > controller) would work.  As per below the only way I've seen it done
> > is totally out-of-band.  ...and actually, I'm not sure I've actually
> > ever seen even the out of band stuff truly work on a system myself.
> > It's always been one of those "we should support wake on WiFi" but
> > never made it all the way to implementation.  In any case, if there
> > are examples of people plumbing wakeup through the SD controller I'd
> > need to figure out how to not break them.  Just doing a solution for
> > dw_mmc means I don't have to worry about this since dw_mmc definitely
> > doesn't support SDIO wakeup.
> >
> > Maybe one way to get a more generic solution is if you had an idea for
> > a patch that would work for many host controllers then you could post
> > it and I could test to confirm that it's happy on dw_mmc?  ...similar
> > to when you switched dw_mmc away from the old kthread-based SDIO
> > stuff?

Let me have a look and see if I can post something for you to test.

>
> Unless you have time to help dig into all the possibilities here to
> help understand how this should behave across all the different host
> controllers / wakeup setups, maybe we could just land ${SUBJECT} patch
> for now and when there is more clarity we can revisit?

That's an option. I only fear that the revisit part never happens
(because of me personally being occupied with other things).

If I not able to come up with something within a week, then I will
queue up this as fix.

Kind regards
Uffe

^ permalink raw reply

* RE: [PATCH 11/11] rtw88: debug: dump tx power indexes in use
From: Tony Chuang @ 2019-06-04  8:18 UTC (permalink / raw)
  To: Joe Perches, kvalo@codeaurora.org; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <39d56df83cc8de95969c6ba3003d8101caedc045.camel@perches.com>

> Subject: Re: [PATCH 11/11] rtw88: debug: dump tx power indexes in use
> 
> On Wed, 2019-05-29 at 15:54 +0800, yhchuang@realtek.com wrote:
> > From: Zong-Zhe Yang <kevin_yang@realtek.com>
> >
> > Add a read entry in debugfs to dump current tx power
> > indexes in use for each path and each rate section.
> > The corresponding power bases, power by rate, and
> > power limit are also included.
> >
> > Signed-off-by: Zong-Zhe Yang <kevin_yang@realtek.com>
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> > ---
> >  drivers/net/wireless/realtek/rtw88/debug.c | 112
> +++++++++++++++++++++++++++++
> >  1 file changed, 112 insertions(+)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/debug.c
> b/drivers/net/wireless/realtek/rtw88/debug.c
> > index f0ae260..ee2937c2 100644
> > --- a/drivers/net/wireless/realtek/rtw88/debug.c
> > +++ b/drivers/net/wireless/realtek/rtw88/debug.c
> > @@ -8,6 +8,7 @@
> >  #include "sec.h"
> >  #include "fw.h"
> >  #include "debug.h"
> > +#include "phy.h"
> >
> >  #ifdef CONFIG_RTW88_DEBUGFS
> >
> > @@ -460,6 +461,112 @@ static int rtw_debug_get_rf_dump(struct seq_file
> *m, void *v)
> >  	return 0;
> >  }
> >
> > +static void rtw_print_cck_rate_txt(struct seq_file *m, u8 rate)
> > +{
> > +	static const char * const
> > +	cck_rate[] = {"1M", "2M", "5.5M", "11M"};
> > +	u8 idx = rate - DESC_RATE1M;
> > +
> > +	seq_printf(m, "%5s%-5s", "CCK_", cck_rate[idx]);
> 
> Why use %5s instead of just embedding the prefix directly?
> Also why use %5s at all when the length is 4?
> 
> I think it'd be more sensible as:
> 
> 	seq_printf(m, " CCK_%-5s", cck_rate[idx]);
> 

Ok, it is better.
Will send a v2 later :)
Thanks

Yan-Hsuan

^ permalink raw reply


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