* [PATCH] mt76: mt7921: fix possible resume failure
@ 2021-12-07 2:35 sean.wang
2021-12-07 10:08 ` Lorenzo Bianconi
0 siblings, 1 reply; 4+ messages in thread
From: sean.wang @ 2021-12-07 2:35 UTC (permalink / raw)
To: nbd, lorenzo.bianconi
Cc: sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
Mark-YW.Chen, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, shawnku,
linux-wireless, linux-mediatek
From: Sean Wang <sean.wang@mediatek.com>
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.
Fixes: ffa1bf97425b ("mt76: mt7921: introduce PM 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>
---
drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index 5635de3c80b1..21a48f02ab60 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mt76: mt7921: fix possible resume failure
2021-12-07 2:35 [PATCH] mt76: mt7921: fix possible resume failure sean.wang
@ 2021-12-07 10:08 ` Lorenzo Bianconi
0 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Bianconi @ 2021-12-07 10:08 UTC (permalink / raw)
To: sean.wang
Cc: nbd, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Mark-YW.Chen,
Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh, posh.sun,
ted.huang, Eric.Liang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
frankgor, jemele, abhishekpandit, shawnku, linux-wireless,
linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
> From: Sean Wang <sean.wang@mediatek.com>
>
> 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.
do we have a similar issue in mt7921s_resume()?
>
> Fixes: ffa1bf97425b ("mt76: mt7921: introduce PM 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>
> ---
> drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> index 5635de3c80b1..21a48f02ab60 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> @@ -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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <Ya8ysehFjoEGGEf+@lore-desk--annotate>]
* Re: [PATCH] mt76: mt7921: fix possible resume failure
[not found] <Ya8ysehFjoEGGEf+@lore-desk--annotate>
@ 2021-12-07 17:21 ` sean.wang
2021-12-07 20:08 ` Lorenzo Bianconi
0 siblings, 1 reply; 4+ messages in thread
From: sean.wang @ 2021-12-07 17:21 UTC (permalink / raw)
To: lorenzo.bianconi
Cc: nbd, sean.wang, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang,
Mark-YW.Chen, Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh,
posh.sun, ted.huang, Eric.Liang, Stella.Chang, Tom.Chou,
steve.lee, jsiuda, frankgor, jemele, abhishekpandit, shawnku,
linux-wireless, linux-mediatek
From: Sean Wang <sean.wang@mediatek.com>
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> 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.
>
>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.
>
>>
>> Fixes: ffa1bf97425b ("mt76: mt7921: introduce PM 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>
>> ---
>> drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> index 5635de3c80b1..21a48f02ab60 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> @@ -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
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mt76: mt7921: fix possible resume failure
2021-12-07 17:21 ` sean.wang
@ 2021-12-07 20:08 ` Lorenzo Bianconi
0 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Bianconi @ 2021-12-07 20:08 UTC (permalink / raw)
To: sean.wang
Cc: nbd, Soul.Huang, YN.Chen, Leon.Yen, Eric-SY.Chang, Mark-YW.Chen,
Deren.Wu, km.lin, robin.chiu, Eddie.Chen, ch.yeh, posh.sun,
ted.huang, Eric.Liang, Stella.Chang, Tom.Chou, steve.lee, jsiuda,
frankgor, jemele, abhishekpandit, shawnku, linux-wireless,
linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 2267 bytes --]
> From: Sean Wang <sean.wang@mediatek.com>
>
> >> From: Sean Wang <sean.wang@mediatek.com>
> >>
> >> 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.
> >
> >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
>
> >
> >>
> >> Fixes: ffa1bf97425b ("mt76: mt7921: introduce PM 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>
> >> ---
> >> drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 7 +++++--
> >> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> index 5635de3c80b1..21a48f02ab60 100644
> >> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> >> @@ -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
> >>
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-07 20:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-07 2:35 [PATCH] mt76: mt7921: fix possible resume failure sean.wang
2021-12-07 10:08 ` Lorenzo Bianconi
[not found] <Ya8ysehFjoEGGEf+@lore-desk--annotate>
2021-12-07 17:21 ` sean.wang
2021-12-07 20:08 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox