All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume
@ 2022-07-20 22:25 sean.wang
  2022-07-20 22:25 ` [PATCH v2 2/4] mt76: mt7921s: " sean.wang
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: sean.wang @ 2022-07-20 22:25 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, jenhao.yang, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
	frankgor, kuabhs, druth, abhishekpandit, shawnku, linux-wireless,
	linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

It is unexpected that the reset work is running simultaneously with
the suspend or resume context and it is possible that reset work is still
running even after mt7921 is suspended if we don't fix the race issue.

Thus, the suspend procedure should be waiting until the reset is completed
at the beginning and ignore the subsequent the reset requests.

In case there is an error that happens during either suspend or resume
handler, we will schedule a reset task to recover the error before
returning the error code to ensure we can immediately fix the error there.

Fixes: 0c1ce9884607 ("mt76: mt7921: add wifi reset support")
Co-developed-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: use flush_work instead of cancel_work_sync
---
 drivers/net/wireless/mediatek/mt76/mt7921/mac.c |  5 +++++
 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 13 +++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
index 47f0aa81ab02..6bd9fc9228a2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
@@ -780,6 +780,7 @@ void mt7921_mac_reset_work(struct work_struct *work)
 void mt7921_reset(struct mt76_dev *mdev)
 {
 	struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76);
+	struct mt76_connac_pm *pm = &dev->pm;
 
 	if (!dev->hw_init_done)
 		return;
@@ -787,8 +788,12 @@ void mt7921_reset(struct mt76_dev *mdev)
 	if (dev->hw_full_reset)
 		return;
 
+	if (pm->suspended)
+		return;
+
 	queue_work(dev->mt76.wq, &dev->reset_work);
 }
+EXPORT_SYMBOL_GPL(mt7921_reset);
 
 void mt7921_mac_update_mib_stats(struct mt7921_phy *phy)
 {
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index 9d1ba838e54f..a88561a4e903 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -363,6 +363,7 @@ static int mt7921_pci_suspend(struct device *device)
 	int i, err;
 
 	pm->suspended = true;
+	flush_work(&dev->reset_work);
 	cancel_delayed_work_sync(&pm->ps_work);
 	cancel_work_sync(&pm->wake_work);
 
@@ -424,6 +425,9 @@ static int mt7921_pci_suspend(struct device *device)
 restore_suspend:
 	pm->suspended = false;
 
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
+
 	return err;
 }
 
@@ -437,7 +441,7 @@ static int mt7921_pci_resume(struct device *device)
 
 	err = mt7921_mcu_drv_pmctrl(dev);
 	if (err < 0)
-		return err;
+		goto failed;
 
 	mt7921_wpdma_reinit_cond(dev);
 
@@ -467,11 +471,12 @@ static int mt7921_pci_resume(struct device *device)
 		mt76_connac_mcu_set_deep_sleep(&dev->mt76, false);
 
 	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
-	if (err)
-		return err;
-
+failed:
 	pm->suspended = false;
 
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
+
 	return err;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/4] mt76: mt7921s: fix race issue between reset and suspend/resume
  2022-07-20 22:25 [PATCH v2 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume sean.wang
@ 2022-07-20 22:25 ` sean.wang
  2022-07-20 22:25 ` [PATCH v2 3/4] mt76: mt7921u: " sean.wang
  2022-07-20 22:25 ` [PATCH v2 4/4] mt76: mt7921u: remove unnecessary MT76_STATE_SUSPEND sean.wang
  2 siblings, 0 replies; 4+ messages in thread
From: sean.wang @ 2022-07-20 22:25 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, jenhao.yang, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
	frankgor, kuabhs, druth, abhishekpandit, shawnku, linux-wireless,
	linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

It is unexpected that the reset work is running simultaneously with
the suspend or resume context and it is possible that reset work is still
running even after mt7921 is suspended if we don't fix the race issue.

Thus, the suspend procedure should be waiting until the reset is completed
at the beginning and ignore the subsequent the reset requests.

In case there is an error that happens during either suspend or resume
handler, we will schedule a reset task to recover the error before
returning the error code to ensure we can immediately fix the error there.

Fixes: ca74b9b907f9 ("mt76: mt7921s: add reset support")
Co-developed-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: use flush_work instead of cancel_work_sync
---
 drivers/net/wireless/mediatek/mt76/mt7921/sdio.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
index 487acd6e2be8..2face849fb4f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
@@ -206,6 +206,7 @@ static int mt7921s_suspend(struct device *__dev)
 	pm->suspended = true;
 	set_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
 
+	flush_work(&dev->reset_work);
 	cancel_delayed_work_sync(&pm->ps_work);
 	cancel_work_sync(&pm->wake_work);
 
@@ -261,6 +262,9 @@ static int mt7921s_suspend(struct device *__dev)
 	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
 	pm->suspended = false;
 
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
+
 	return err;
 }
 
@@ -276,7 +280,7 @@ static int mt7921s_resume(struct device *__dev)
 
 	err = mt7921_mcu_drv_pmctrl(dev);
 	if (err < 0)
-		return err;
+		goto failed;
 
 	mt76_worker_enable(&mdev->tx_worker);
 	mt76_worker_enable(&mdev->sdio.txrx_worker);
@@ -288,11 +292,12 @@ static int mt7921s_resume(struct device *__dev)
 		mt76_connac_mcu_set_deep_sleep(mdev, false);
 
 	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
-	if (err)
-		return err;
-
+failed:
 	pm->suspended = false;
 
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
+
 	return err;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 3/4] mt76: mt7921u: fix race issue between reset and suspend/resume
  2022-07-20 22:25 [PATCH v2 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume sean.wang
  2022-07-20 22:25 ` [PATCH v2 2/4] mt76: mt7921s: " sean.wang
@ 2022-07-20 22:25 ` sean.wang
  2022-07-20 22:25 ` [PATCH v2 4/4] mt76: mt7921u: remove unnecessary MT76_STATE_SUSPEND sean.wang
  2 siblings, 0 replies; 4+ messages in thread
From: sean.wang @ 2022-07-20 22:25 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, jenhao.yang, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
	frankgor, kuabhs, druth, abhishekpandit, shawnku, linux-wireless,
	linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

It is unexpected that the reset work is running simultaneously with
the suspend or resume context and it is possible that reset work is still
running even after mt7921 is suspended if we don't fix the race issue.

Thus, the suspend procedure should be waiting until the reset is completed
at the beginning and ignore the subsequent the reset requests.

In case there is an error that happens during either suspend or resume
handler, we will schedule a reset task to recover the error before
returning the error code to ensure we can immediately fix the error there.

Fixes: df3e4143ba8a ("mt76: mt7921u: add suspend/resume support")
Co-developed-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: YN Chen <YN.Chen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: use flush_work instead of cancel_work_sync
---
 .../net/wireless/mediatek/mt76/mt7921/usb.c   | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
index dd3b8884e162..613d5cefffc7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
@@ -300,11 +300,15 @@ static void mt7921u_disconnect(struct usb_interface *usb_intf)
 static int mt7921u_suspend(struct usb_interface *intf, pm_message_t state)
 {
 	struct mt7921_dev *dev = usb_get_intfdata(intf);
+	struct mt76_connac_pm *pm = &dev->pm;
 	int err;
 
+	pm->suspended = true;
+	flush_work(&dev->reset_work);
+
 	err = mt76_connac_mcu_set_hif_suspend(&dev->mt76, true);
 	if (err)
-		return err;
+		goto failed;
 
 	mt76u_stop_rx(&dev->mt76);
 	mt76u_stop_tx(&dev->mt76);
@@ -312,11 +316,20 @@ static int mt7921u_suspend(struct usb_interface *intf, pm_message_t state)
 	set_bit(MT76_STATE_SUSPEND, &dev->mphy.state);
 
 	return 0;
+
+failed:
+	pm->suspended = false;
+
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
+
+	return err;
 }
 
 static int mt7921u_resume(struct usb_interface *intf)
 {
 	struct mt7921_dev *dev = usb_get_intfdata(intf);
+	struct mt76_connac_pm *pm = &dev->pm;
 	bool reinit = true;
 	int err, i;
 
@@ -338,16 +351,23 @@ static int mt7921u_resume(struct usb_interface *intf)
 	if (reinit || mt7921_dma_need_reinit(dev)) {
 		err = mt7921u_dma_init(dev, true);
 		if (err)
-			return err;
+			goto failed;
 	}
 
 	clear_bit(MT76_STATE_SUSPEND, &dev->mphy.state);
 
 	err = mt76u_resume_rx(&dev->mt76);
 	if (err < 0)
-		return err;
+		goto failed;
+
+	err = mt76_connac_mcu_set_hif_suspend(&dev->mt76, false);
+failed:
+	pm->suspended = false;
+
+	if (err < 0)
+		mt7921_reset(&dev->mt76);
 
-	return mt76_connac_mcu_set_hif_suspend(&dev->mt76, false);
+	return err;
 }
 #endif /* CONFIG_PM */
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 4/4] mt76: mt7921u: remove unnecessary MT76_STATE_SUSPEND
  2022-07-20 22:25 [PATCH v2 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume sean.wang
  2022-07-20 22:25 ` [PATCH v2 2/4] mt76: mt7921s: " sean.wang
  2022-07-20 22:25 ` [PATCH v2 3/4] mt76: mt7921u: " sean.wang
@ 2022-07-20 22:25 ` sean.wang
  2 siblings, 0 replies; 4+ messages in thread
From: sean.wang @ 2022-07-20 22:25 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi
  Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
	Deren.Wu, km.lin, jenhao.yang, robin.chiu, Eddie.Chen, ch.yeh,
	posh.sun, ted.huang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
	frankgor, kuabhs, druth, abhishekpandit, shawnku, linux-wireless,
	linux-mediatek

From: Sean Wang <sean.wang@mediatek.com>

remove unnecessary MT76_STATE_SUSPEND manipulation to be consistent
with the mt7921[e, s] driver.

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: add the Acked tag from Lorenzo
---
 drivers/net/wireless/mediatek/mt76/mt7921/usb.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
index 613d5cefffc7..54d11bfb45e2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
@@ -313,8 +313,6 @@ static int mt7921u_suspend(struct usb_interface *intf, pm_message_t state)
 	mt76u_stop_rx(&dev->mt76);
 	mt76u_stop_tx(&dev->mt76);
 
-	set_bit(MT76_STATE_SUSPEND, &dev->mphy.state);
-
 	return 0;
 
 failed:
@@ -354,8 +352,6 @@ static int mt7921u_resume(struct usb_interface *intf)
 			goto failed;
 	}
 
-	clear_bit(MT76_STATE_SUSPEND, &dev->mphy.state);
-
 	err = mt76u_resume_rx(&dev->mt76);
 	if (err < 0)
 		goto failed;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-20 22:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20 22:25 [PATCH v2 1/4] mt76: mt7921e: fix race issue between reset and suspend/resume sean.wang
2022-07-20 22:25 ` [PATCH v2 2/4] mt76: mt7921s: " sean.wang
2022-07-20 22:25 ` [PATCH v2 3/4] mt76: mt7921u: " sean.wang
2022-07-20 22:25 ` [PATCH v2 4/4] mt76: mt7921u: remove unnecessary MT76_STATE_SUSPEND sean.wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.