Skip to contentSkip to conversation

mt76: mt7921: fix possible resume failure

PATCH [email protected] 2 participants 2 messages Dec 7, 2021 1 message not in the archive Arm MediaTek

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): Ya8ysehFjoEGGEf+@lore-desk--annotate

#1 lore raw

From: Sean Wang <[email protected]>

Quoted 5 lines
Quoted 5 lines

From: Sean Wang <[email protected]>

Fix the possible resume failure due to mt76_connac_mcu_set_hif_suspend timeout.

Quoted 7 lines
Quoted 5 lines

That is because clearing the flag pm->suspended too early opened up a race window, where mt7921_poll_tx/rx scheduled a ps_work to put the device in doze mode, that is unexpected for the device is being resumed from the suspend state and would make the remaining MCU comamnds in resume handler failed to execute.

do we have a similar issue in mt7921s_resume()?

We don't have the same issue found in mt7921s_resume because the driver guarantees all MCU commands (even in mt7921s_resume) always work in the active state.

But we can have the same patch for mt7921s just to make code consistent with mt7921e.

Quoted 40 lines
Quoted 39 lines
Fixes ffa1bf97425b ("mt76: mt7921: introduce PM support")
---
 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/wireless/mediatek/mt76/mt7921/pci.c +5 -3
index 5635de3c80b1..21a48f02ab60 100644
@@ -315,7 +315,6 @@ static int mt7921_pci_resume(struct pci_dev *pdev)
 	struct mt76_connac_pm *pm = &dev->pm;
 	int i, err;
 
-	pm->suspended = false;
 	err = pci_set_power_state(pdev, PCI_D0);
 	if (err)
 		return err;
@@ -353,7 +352,11 @@ static int mt7921_pci_resume(struct pci_dev *pdev)
 	if (!pm->ds_enable)
 		mt76_connac_mcu_set_deep_sleep(&dev->mt76, false);
 
-	return mt76_connac_mcu_set_hif_suspend(mdev, false);
+	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
+
+	pm->suspended = false;
+
+	return err;
 }
 #endif /* CONFIG_PM */
 
--

2.25.1

#2 lore raw
Quoted 13 lines

From: Sean Wang <[email protected]>

Quoted 11 lines
Quoted 10 lines

From: Sean Wang <[email protected]>

Fix the possible resume failure due to mt76_connac_mcu_set_hif_suspend timeout.

That is because clearing the flag pm->suspended too early opened up a race window, where mt7921_poll_tx/rx scheduled a ps_work to put the device in doze mode, that is unexpected for the device is being resumed from the suspend state and would make the remaining MCU comamnds in resume handler failed to execute.

Quoted 8 lines
Quoted 1 line

do we have a similar issue in mt7921s_resume()?

We don't have the same issue found in mt7921s_resume because the driver guarantees all MCU commands (even in mt7921s_resume) always work in the active state.

But we can have the same patch for mt7921s just to make code consistent with mt7921e.

ack, agree.

Regards,
Lorenzo

Quoted 42 lines
Quoted 41 lines
Quoted 40 lines
Fixes ffa1bf97425b ("mt76: mt7921: introduce PM support")
---
 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/wireless/mediatek/mt76/mt7921/pci.c +5 -3
index 5635de3c80b1..21a48f02ab60 100644
@@ -315,7 +315,6 @@ static int mt7921_pci_resume(struct pci_dev *pdev)
 	struct mt76_connac_pm *pm = &dev->pm;
 	int i, err;
 
-	pm->suspended = false;
 	err = pci_set_power_state(pdev, PCI_D0);
 	if (err)
 		return err;
@@ -353,7 +352,11 @@ static int mt7921_pci_resume(struct pci_dev *pdev)
 	if (!pm->ds_enable)
 		mt76_connac_mcu_set_deep_sleep(&dev->mt76, false);
 
-	return mt76_connac_mcu_set_hif_suspend(mdev, false);
+	err = mt76_connac_mcu_set_hif_suspend(mdev, false);
+
+	pm->suspended = false;
+
+	return err;
 }
 #endif /* CONFIG_PM */
 
--

2.25.1