Received-SPF: pass (google.com: domain of linux-wireless+bounces-9013-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: "=?UTF-8?q?Marcin=20=C5=9Alusarz?=" <marcin.slusarz@gmail.com>
To: linux-wireless@vger.kernel.org
Cc: =?UTF-8?q?Marcin=20=C5=9Alusarz?= <mslusarz@renau.com>
Subject: [PATCH v2 2/2] wifi: rtw88/usb: stop rx work before potential power off
Date: Fri, 14 Jun 2024 14:13:39 +0200
Message-Id: <20240614121339.525935-2-mslusarz@renau.com>
In-Reply-To: <20240614121339.525935-1-mslusarz@renau.com>
References: <CA+GA0_tK_+Rsj0FAxO-_VXBLLFMoxW9FHsWhL6gEoWaOMKwWrA@mail.gmail.com>
 <20240614121339.525935-1-mslusarz@renau.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If power off is disabled (like on 8821CU after previous patch),
the hardware can still fire and deliver data. This may have
undersired impact on the networking stack (e.g.
WARN_ON(!local->started) in ieee80211_rx_list), because hw is
not supposed to do that after power off.

So to prevent that, cancel any pending RX URBs to stop the
completion handlers from being called.

Signed-off-by: Marcin Ślusarz <mslusarz@renau.com>
---
v2: start_rx/stop_rx cbs can be NULL; rx_enabled check added to stop_rx
---
 drivers/net/wireless/realtek/rtw88/hci.h  | 14 ++++++++
 drivers/net/wireless/realtek/rtw88/main.c |  7 +++-
 drivers/net/wireless/realtek/rtw88/usb.c  | 44 ++++++++++++++++-------
 drivers/net/wireless/realtek/rtw88/usb.h  |  1 +
 4 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/hci.h b/drivers/net/wireless/realtek/rtw88/hci.h
index 830d7532f2a3..839b9161014f 100644
--- a/drivers/net/wireless/realtek/rtw88/hci.h
+++ b/drivers/net/wireless/realtek/rtw88/hci.h
@@ -18,6 +18,8 @@ struct rtw_hci_ops {
 	void (*deep_ps)(struct rtw_dev *rtwdev, bool enter);
 	void (*link_ps)(struct rtw_dev *rtwdev, bool enter);
 	void (*interface_cfg)(struct rtw_dev *rtwdev);
+	void (*stop_rx)(struct rtw_dev *rtwdev);
+	void (*start_rx)(struct rtw_dev *rtwdev);
 
 	int (*write_data_rsvd_page)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
 	int (*write_data_h2c)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
@@ -57,6 +59,18 @@ static inline void rtw_hci_stop(struct rtw_dev *rtwdev)
 	rtwdev->hci.ops->stop(rtwdev);
 }
 
+static inline void rtw_hci_start_rx(struct rtw_dev *rtwdev)
+{
+	if (rtwdev->hci.ops->start_rx)
+		rtwdev->hci.ops->start_rx(rtwdev);
+}
+
+static inline void rtw_hci_stop_rx(struct rtw_dev *rtwdev)
+{
+	if (rtwdev->hci.ops->stop_rx)
+		rtwdev->hci.ops->stop_rx(rtwdev);
+}
+
 static inline void rtw_hci_deep_ps(struct rtw_dev *rtwdev, bool enter)
 {
 	rtwdev->hci.ops->deep_ps(rtwdev, enter);
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index a48e919adddb..bb0122d19416 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -1357,7 +1357,7 @@ static int rtw_power_on(struct rtw_dev *rtwdev)
 	int ret;
 
 	if (rtwdev->always_power_on && test_bit(RTW_FLAG_POWERON, rtwdev->flags))
-		return 0;
+		goto success;
 
 	ret = rtw_hci_setup(rtwdev);
 	if (ret) {
@@ -1407,6 +1407,9 @@ static int rtw_power_on(struct rtw_dev *rtwdev)
 	rtw_coex_power_on_setting(rtwdev);
 	rtw_coex_init_hw_config(rtwdev, wifi_only);
 
+success:
+	rtw_hci_start_rx(rtwdev);
+
 	return 0;
 
 err_off:
@@ -1509,6 +1512,8 @@ int rtw_core_start(struct rtw_dev *rtwdev)
 
 static void rtw_power_off(struct rtw_dev *rtwdev)
 {
+	rtw_hci_stop_rx(rtwdev);
+
 	if (rtwdev->always_power_on)
 		return;
 
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 28ff46e96604..8e784c357ee2 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -713,6 +713,34 @@ static void rtw_usb_interface_cfg(struct rtw_dev *rtwdev)
 	/* empty function for rtw_hci_ops */
 }
 
+static void rtw_usb_stop_rx(struct rtw_dev *rtwdev)
+{
+	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
+
+	if (!rtwusb->rx_enabled)
+		return;
+
+	rtw_usb_cancel_rx_bufs(rtwusb);
+	rtwusb->rx_enabled = false;
+}
+
+static void rtw_usb_start_rx(struct rtw_dev *rtwdev)
+{
+	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
+	int i;
+
+	if (rtwusb->rx_enabled)
+		return;
+
+	for (i = 0; i < RTW_USB_RXCB_NUM; i++) {
+		struct rx_usb_ctrl_block *rxcb = &rtwusb->rx_cb[i];
+
+		rtw_usb_rx_resubmit(rtwusb, rxcb);
+	}
+
+	rtwusb->rx_enabled = true;
+}
+
 static struct rtw_hci_ops rtw_usb_ops = {
 	.tx_write = rtw_usb_tx_write,
 	.tx_kick_off = rtw_usb_tx_kick_off,
@@ -722,6 +750,8 @@ static struct rtw_hci_ops rtw_usb_ops = {
 	.deep_ps = rtw_usb_deep_ps,
 	.link_ps = rtw_usb_link_ps,
 	.interface_cfg = rtw_usb_interface_cfg,
+	.stop_rx = rtw_usb_stop_rx,
+	.start_rx = rtw_usb_start_rx,
 
 	.write8  = rtw_usb_write8,
 	.write16 = rtw_usb_write16,
@@ -751,18 +781,6 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
 	return 0;
 }
 
-static void rtw_usb_setup_rx(struct rtw_dev *rtwdev)
-{
-	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
-	int i;
-
-	for (i = 0; i < RTW_USB_RXCB_NUM; i++) {
-		struct rx_usb_ctrl_block *rxcb = &rtwusb->rx_cb[i];
-
-		rtw_usb_rx_resubmit(rtwusb, rxcb);
-	}
-}
-
 static void rtw_usb_deinit_rx(struct rtw_dev *rtwdev)
 {
 	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
@@ -900,7 +918,7 @@ int rtw_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		goto err_destroy_rxwq;
 	}
 
-	rtw_usb_setup_rx(rtwdev);
+	rtw_usb_start_rx(rtwdev);
 
 	return 0;
 
diff --git a/drivers/net/wireless/realtek/rtw88/usb.h b/drivers/net/wireless/realtek/rtw88/usb.h
index 86697a5c0103..a6b004d4f74e 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.h
+++ b/drivers/net/wireless/realtek/rtw88/usb.h
@@ -82,6 +82,7 @@ struct rtw_usb {
 	struct rx_usb_ctrl_block rx_cb[RTW_USB_RXCB_NUM];
 	struct sk_buff_head rx_queue;
 	struct work_struct rx_work;
+	bool rx_enabled;
 };
 
 static inline struct rtw_usb_tx_data *rtw_usb_get_tx_data(struct sk_buff *skb)
-- 
2.25.1