diff mbox series

[2/2] mt76: mt7921s: fix the device cannot sleep deeply in suspend

Message ID a00f3858dd6896f56a5b730c457a8e825c664028.1637363831.git.objelf@gmail.com (mailing list archive)
State Accepted
Delegated to: Felix Fietkau
Headers show
Series [1/2] mt76: mt7921: move mt76_connac_mcu_set_hif_suspend to bus-related files | expand

Commit Message

Sean Wang Nov. 19, 2021, 11:22 p.m. UTC 
From: Sean Wang <sean.wang@mediatek.com>

According to the MT7921S firmware, the cmd MCU_UNI_CMD_HIF_CTRL have to
be last MCU command to execute in suspend handler and all data traffic
have to be stopped before the cmd MCU_UNI_CMD_HIF_CTRL starts as well
in order that mt7921 can successfully fall into the deep sleep mode.

Where we reuse the flag MT76_STATE_SUSPEND and avoid creating
another global flag to stop all of the traffic onto the SDIO bus.

Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support")
Reported-by: Leon Yen <leon.yen@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../wireless/mediatek/mt76/mt76_connac_mcu.c  |  2 +-
 .../net/wireless/mediatek/mt76/mt7921/main.c  |  3 --
 .../net/wireless/mediatek/mt76/mt7921/sdio.c  | 34 ++++++++++++-------
 drivers/net/wireless/mediatek/mt76/sdio.c     |  3 +-
 .../net/wireless/mediatek/mt76/sdio_txrx.c    |  3 +-
 5 files changed, 27 insertions(+), 18 deletions(-)

Comments

Lorenzo Bianconi Nov. 20, 2021, 1 p.m. UTC | #1 
> From: Sean Wang <sean.wang@mediatek.com>
> 
> According to the MT7921S firmware, the cmd MCU_UNI_CMD_HIF_CTRL have to
> be last MCU command to execute in suspend handler and all data traffic
> have to be stopped before the cmd MCU_UNI_CMD_HIF_CTRL starts as well
> in order that mt7921 can successfully fall into the deep sleep mode.
> 
> Where we reuse the flag MT76_STATE_SUSPEND and avoid creating
> another global flag to stop all of the traffic onto the SDIO bus.
> 
> Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support")
> Reported-by: Leon Yen <leon.yen@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../wireless/mediatek/mt76/mt76_connac_mcu.c  |  2 +-
>  .../net/wireless/mediatek/mt76/mt7921/main.c  |  3 --
>  .../net/wireless/mediatek/mt76/mt7921/sdio.c  | 34 ++++++++++++-------
>  drivers/net/wireless/mediatek/mt76/sdio.c     |  3 +-
>  .../net/wireless/mediatek/mt76/sdio_txrx.c    |  3 +-
>  5 files changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> index 26b4b875dcc0..61c4c86e79c8 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> @@ -2461,7 +2461,7 @@ void mt76_connac_mcu_set_suspend_iter(void *priv, u8 *mac,
>  				      struct ieee80211_vif *vif)
>  {
>  	struct mt76_phy *phy = priv;
> -	bool suspend = test_bit(MT76_STATE_SUSPEND, &phy->state);
> +	bool suspend = !test_bit(MT76_STATE_RUNNING, &phy->state);
>  	struct ieee80211_hw *hw = phy->hw;
>  	struct cfg80211_wowlan *wowlan = hw->wiphy->wowlan_config;
>  	int i;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> index e022251b4006..0b2a6b7f22ea 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> @@ -1242,8 +1242,6 @@ static int mt7921_suspend(struct ieee80211_hw *hw,
>  	mt7921_mutex_acquire(dev);
>  
>  	clear_bit(MT76_STATE_RUNNING, &phy->mt76->state);
> -
> -	set_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
>  	ieee80211_iterate_active_interfaces(hw,
>  					    IEEE80211_IFACE_ITER_RESUME_ALL,
>  					    mt76_connac_mcu_set_suspend_iter,
> @@ -1262,7 +1260,6 @@ static int mt7921_resume(struct ieee80211_hw *hw)
>  	mt7921_mutex_acquire(dev);
>  
>  	set_bit(MT76_STATE_RUNNING, &phy->mt76->state);
> -	clear_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
>  	ieee80211_iterate_active_interfaces(hw,
>  					    IEEE80211_IFACE_ITER_RESUME_ALL,
>  					    mt76_connac_mcu_set_suspend_iter,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> index 5fee489c7a99..5c88b6b8d097 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> @@ -206,6 +206,8 @@ static int mt7921s_suspend(struct device *__dev)
>  	int err;
>  
>  	pm->suspended = true;
> +	set_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
> +
>  	cancel_delayed_work_sync(&pm->ps_work);
>  	cancel_work_sync(&pm->wake_work);
>  
> @@ -213,10 +215,6 @@ static int mt7921s_suspend(struct device *__dev)
>  	if (err < 0)
>  		goto restore_suspend;
>  
> -	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
> -	if (err)
> -		goto restore_suspend;
> -
>  	/* always enable deep sleep during suspend to reduce
>  	 * power consumption
>  	 */
> @@ -224,34 +222,45 @@ static int mt7921s_suspend(struct device *__dev)
>  
>  	mt76_txq_schedule_all(&dev->mphy);
>  	mt76_worker_disable(&mdev->tx_worker);
> -	mt76_worker_disable(&mdev->sdio.txrx_worker);
>  	mt76_worker_disable(&mdev->sdio.status_worker);
> -	mt76_worker_disable(&mdev->sdio.net_worker);
>  	cancel_work_sync(&mdev->sdio.stat_work);
>  	clear_bit(MT76_READING_STATS, &dev->mphy.state);
> -
>  	mt76_tx_status_check(mdev, true);
>  
> -	err = mt7921_mcu_fw_pmctrl(dev);
> +	mt76_worker_schedule(&mdev->sdio.txrx_worker);

I guess mt76_worker_disable() is supposed to do it, right?
I guess we can assume txrx_worker is no longer running here, right?

> +	wait_event_timeout(dev->mt76.sdio.wait,
> +			   mt76s_txqs_empty(&dev->mt76), 5 * HZ);
> +
> +	/* It is supposed that SDIO bus is idle at the point */
> +	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
>  	if (err)
>  		goto restore_worker;
>  
> +	mt76_worker_disable(&mdev->sdio.txrx_worker);
> +	mt76_worker_disable(&mdev->sdio.net_worker);
> +
> +	err = mt7921_mcu_fw_pmctrl(dev);
> +	if (err)
> +		goto restore_txrx_worker;
> +
>  	sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
>  
>  	return 0;
>  
> +restore_txrx_worker:
> +	mt76_worker_enable(&mdev->sdio.net_worker);
> +	mt76_worker_enable(&mdev->sdio.txrx_worker);
> +	mt76_connac_mcu_set_hif_suspend(mdev, false);
> +
>  restore_worker:
>  	mt76_worker_enable(&mdev->tx_worker);
> -	mt76_worker_enable(&mdev->sdio.txrx_worker);
>  	mt76_worker_enable(&mdev->sdio.status_worker);
> -	mt76_worker_enable(&mdev->sdio.net_worker);
>  
>  	if (!pm->ds_enable)
>  		mt76_connac_mcu_set_deep_sleep(mdev, false);
>  
> -	mt76_connac_mcu_set_hif_suspend(mdev, false);
> -
>  restore_suspend:
> +	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
>  	pm->suspended = false;
>  
>  	return err;
> @@ -266,6 +275,7 @@ static int mt7921s_resume(struct device *__dev)
>  	int err;
>  
>  	pm->suspended = false;
> +	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
>  
>  	err = mt7921_mcu_drv_pmctrl(dev);
>  	if (err < 0)
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio.c b/drivers/net/wireless/mediatek/mt76/sdio.c
> index c99acc21225e..b0bc7be0fb1f 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio.c
> @@ -479,7 +479,8 @@ static void mt76s_status_worker(struct mt76_worker *w)
>  			resched = true;
>  
>  		if (dev->drv->tx_status_data &&
> -		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state))
> +		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state) &&
> +		    !test_bit(MT76_STATE_SUSPEND, &dev->phy.state))
>  			queue_work(dev->wq, &dev->sdio.stat_work);
>  	} while (nframes > 0);
>  
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 649a56790b89..801590a0a334 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -317,7 +317,8 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
>  		if (ret > 0)
>  			nframes += ret;
>  
> -		if (test_bit(MT76_MCU_RESET, &dev->phy.state)) {
> +		if (test_bit(MT76_MCU_RESET, &dev->phy.state) ||
> +		    test_bit(MT76_STATE_SUSPEND, &dev->phy.state)) {
>  			if (!mt76s_txqs_empty(dev))
>  				continue;
>  			else

since mt76s_tx_run_queue will not run if MT76_MCU_RESET is set, do we really
need to check it for each iteration or is fine something like:

diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
index 649a56790b89..68f30a83fa5d 100644
--- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
+++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
@@ -317,14 +317,12 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
 		if (ret > 0)
 			nframes += ret;
 
-		if (test_bit(MT76_MCU_RESET, &dev->phy.state)) {
-			if (!mt76s_txqs_empty(dev))
-				continue;
-			else
-				wake_up(&sdio->wait);
-		}
 	} while (nframes > 0);
 
+	if (test_bit(MT76_MCU_RESET, &dev->phy.state) &&
+	    mt76s_txqs_empty(dev))
+		wake_up(&sdio->wait);
+
 	/* enable interrupt */
 	sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL);
 	sdio_release_host(sdio->func);

Regards,
Lorenzo

> -- 
> 2.25.1
>
Lorenzo Bianconi Nov. 20, 2021, 2:18 p.m. UTC | #2 
> > From: Sean Wang <sean.wang@mediatek.com>
> > 
> > According to the MT7921S firmware, the cmd MCU_UNI_CMD_HIF_CTRL have to
> > be last MCU command to execute in suspend handler and all data traffic
> > have to be stopped before the cmd MCU_UNI_CMD_HIF_CTRL starts as well
> > in order that mt7921 can successfully fall into the deep sleep mode.
> > 
> > Where we reuse the flag MT76_STATE_SUSPEND and avoid creating
> > another global flag to stop all of the traffic onto the SDIO bus.
> > 
> > Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support")
> > Reported-by: Leon Yen <leon.yen@mediatek.com>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > ---
> >  .../wireless/mediatek/mt76/mt76_connac_mcu.c  |  2 +-
> >  .../net/wireless/mediatek/mt76/mt7921/main.c  |  3 --
> >  .../net/wireless/mediatek/mt76/mt7921/sdio.c  | 34 ++++++++++++-------
> >  drivers/net/wireless/mediatek/mt76/sdio.c     |  3 +-
> >  .../net/wireless/mediatek/mt76/sdio_txrx.c    |  3 +-
> >  5 files changed, 27 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> > index 26b4b875dcc0..61c4c86e79c8 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> > @@ -2461,7 +2461,7 @@ void mt76_connac_mcu_set_suspend_iter(void *priv, u8 *mac,
> >  				      struct ieee80211_vif *vif)
> >  {
> >  	struct mt76_phy *phy = priv;
> > -	bool suspend = test_bit(MT76_STATE_SUSPEND, &phy->state);
> > +	bool suspend = !test_bit(MT76_STATE_RUNNING, &phy->state);
> >  	struct ieee80211_hw *hw = phy->hw;
> >  	struct cfg80211_wowlan *wowlan = hw->wiphy->wowlan_config;
> >  	int i;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > index e022251b4006..0b2a6b7f22ea 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > @@ -1242,8 +1242,6 @@ static int mt7921_suspend(struct ieee80211_hw *hw,
> >  	mt7921_mutex_acquire(dev);
> >  
> >  	clear_bit(MT76_STATE_RUNNING, &phy->mt76->state);
> > -
> > -	set_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
> >  	ieee80211_iterate_active_interfaces(hw,
> >  					    IEEE80211_IFACE_ITER_RESUME_ALL,
> >  					    mt76_connac_mcu_set_suspend_iter,
> > @@ -1262,7 +1260,6 @@ static int mt7921_resume(struct ieee80211_hw *hw)
> >  	mt7921_mutex_acquire(dev);
> >  
> >  	set_bit(MT76_STATE_RUNNING, &phy->mt76->state);
> > -	clear_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
> >  	ieee80211_iterate_active_interfaces(hw,
> >  					    IEEE80211_IFACE_ITER_RESUME_ALL,
> >  					    mt76_connac_mcu_set_suspend_iter,
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > index 5fee489c7a99..5c88b6b8d097 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > @@ -206,6 +206,8 @@ static int mt7921s_suspend(struct device *__dev)
> >  	int err;
> >  
> >  	pm->suspended = true;
> > +	set_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
> > +
> >  	cancel_delayed_work_sync(&pm->ps_work);
> >  	cancel_work_sync(&pm->wake_work);
> >  
> > @@ -213,10 +215,6 @@ static int mt7921s_suspend(struct device *__dev)
> >  	if (err < 0)
> >  		goto restore_suspend;
> >  
> > -	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
> > -	if (err)
> > -		goto restore_suspend;
> > -
> >  	/* always enable deep sleep during suspend to reduce
> >  	 * power consumption
> >  	 */
> > @@ -224,34 +222,45 @@ static int mt7921s_suspend(struct device *__dev)
> >  
> >  	mt76_txq_schedule_all(&dev->mphy);
> >  	mt76_worker_disable(&mdev->tx_worker);
> > -	mt76_worker_disable(&mdev->sdio.txrx_worker);
> >  	mt76_worker_disable(&mdev->sdio.status_worker);
> > -	mt76_worker_disable(&mdev->sdio.net_worker);
> >  	cancel_work_sync(&mdev->sdio.stat_work);
> >  	clear_bit(MT76_READING_STATS, &dev->mphy.state);
> > -
> >  	mt76_tx_status_check(mdev, true);
> >  
> > -	err = mt7921_mcu_fw_pmctrl(dev);
> > +	mt76_worker_schedule(&mdev->sdio.txrx_worker);
> 
> I guess mt76_worker_disable() is supposed to do it, right?
> I guess we can assume txrx_worker is no longer running here, right?

I can see it now, txrx_worker can be running on the different cpu.
I guess we need to add just the wait_event_timeout() and move
mt76_connac_mcu_set_hif_suspend() below. What do you think?
Can you please try the chunk below?

Regards,
Lorenzo

> 
> > +	wait_event_timeout(dev->mt76.sdio.wait,
> > +			   mt76s_txqs_empty(&dev->mt76), 5 * HZ);
> > +
> > +	/* It is supposed that SDIO bus is idle at the point */
> > +	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
> >  	if (err)
> >  		goto restore_worker;
> >  
> > +	mt76_worker_disable(&mdev->sdio.txrx_worker);
> > +	mt76_worker_disable(&mdev->sdio.net_worker);
> > +
> > +	err = mt7921_mcu_fw_pmctrl(dev);
> > +	if (err)
> > +		goto restore_txrx_worker;
> > +
> >  	sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
> >  
> >  	return 0;
> >  
> > +restore_txrx_worker:
> > +	mt76_worker_enable(&mdev->sdio.net_worker);
> > +	mt76_worker_enable(&mdev->sdio.txrx_worker);
> > +	mt76_connac_mcu_set_hif_suspend(mdev, false);
> > +
> >  restore_worker:
> >  	mt76_worker_enable(&mdev->tx_worker);
> > -	mt76_worker_enable(&mdev->sdio.txrx_worker);
> >  	mt76_worker_enable(&mdev->sdio.status_worker);
> > -	mt76_worker_enable(&mdev->sdio.net_worker);
> >  
> >  	if (!pm->ds_enable)
> >  		mt76_connac_mcu_set_deep_sleep(mdev, false);
> >  
> > -	mt76_connac_mcu_set_hif_suspend(mdev, false);
> > -
> >  restore_suspend:
> > +	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
> >  	pm->suspended = false;
> >  
> >  	return err;
> > @@ -266,6 +275,7 @@ static int mt7921s_resume(struct device *__dev)
> >  	int err;
> >  
> >  	pm->suspended = false;
> > +	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
> >  
> >  	err = mt7921_mcu_drv_pmctrl(dev);
> >  	if (err < 0)
> > diff --git a/drivers/net/wireless/mediatek/mt76/sdio.c b/drivers/net/wireless/mediatek/mt76/sdio.c
> > index c99acc21225e..b0bc7be0fb1f 100644
> > --- a/drivers/net/wireless/mediatek/mt76/sdio.c
> > +++ b/drivers/net/wireless/mediatek/mt76/sdio.c
> > @@ -479,7 +479,8 @@ static void mt76s_status_worker(struct mt76_worker *w)
> >  			resched = true;
> >  
> >  		if (dev->drv->tx_status_data &&
> > -		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state))
> > +		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state) &&
> > +		    !test_bit(MT76_STATE_SUSPEND, &dev->phy.state))
> >  			queue_work(dev->wq, &dev->sdio.stat_work);
> >  	} while (nframes > 0);
> >  
> > diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > index 649a56790b89..801590a0a334 100644
> > --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > @@ -317,7 +317,8 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
> >  		if (ret > 0)
> >  			nframes += ret;
> >  
> > -		if (test_bit(MT76_MCU_RESET, &dev->phy.state)) {
> > +		if (test_bit(MT76_MCU_RESET, &dev->phy.state) ||
> > +		    test_bit(MT76_STATE_SUSPEND, &dev->phy.state)) {
> >  			if (!mt76s_txqs_empty(dev))
> >  				continue;
> >  			else
> 
> since mt76s_tx_run_queue will not run if MT76_MCU_RESET is set, do we really
> need to check it for each iteration or is fine something like:
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 649a56790b89..68f30a83fa5d 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -317,14 +317,12 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
>  		if (ret > 0)
>  			nframes += ret;
>  
> -		if (test_bit(MT76_MCU_RESET, &dev->phy.state)) {
> -			if (!mt76s_txqs_empty(dev))
> -				continue;
> -			else
> -				wake_up(&sdio->wait);
> -		}
>  	} while (nframes > 0);
>  
> +	if (test_bit(MT76_MCU_RESET, &dev->phy.state) &&
> +	    mt76s_txqs_empty(dev))
> +		wake_up(&sdio->wait);
> +
>  	/* enable interrupt */
>  	sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL);
>  	sdio_release_host(sdio->func);
> 
> Regards,
> Lorenzo
> 
> > -- 
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
index 26b4b875dcc0..61c4c86e79c8 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
@@ -2461,7 +2461,7 @@  void mt76_connac_mcu_set_suspend_iter(void *priv, u8 *mac,
 				      struct ieee80211_vif *vif)
 {
 	struct mt76_phy *phy = priv;
-	bool suspend = test_bit(MT76_STATE_SUSPEND, &phy->state);
+	bool suspend = !test_bit(MT76_STATE_RUNNING, &phy->state);
 	struct ieee80211_hw *hw = phy->hw;
 	struct cfg80211_wowlan *wowlan = hw->wiphy->wowlan_config;
 	int i;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
index e022251b4006..0b2a6b7f22ea 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
@@ -1242,8 +1242,6 @@  static int mt7921_suspend(struct ieee80211_hw *hw,
 	mt7921_mutex_acquire(dev);
 
 	clear_bit(MT76_STATE_RUNNING, &phy->mt76->state);
-
-	set_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
 	ieee80211_iterate_active_interfaces(hw,
 					    IEEE80211_IFACE_ITER_RESUME_ALL,
 					    mt76_connac_mcu_set_suspend_iter,
@@ -1262,7 +1260,6 @@  static int mt7921_resume(struct ieee80211_hw *hw)
 	mt7921_mutex_acquire(dev);
 
 	set_bit(MT76_STATE_RUNNING, &phy->mt76->state);
-	clear_bit(MT76_STATE_SUSPEND, &phy->mt76->state);
 	ieee80211_iterate_active_interfaces(hw,
 					    IEEE80211_IFACE_ITER_RESUME_ALL,
 					    mt76_connac_mcu_set_suspend_iter,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
index 5fee489c7a99..5c88b6b8d097 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
@@ -206,6 +206,8 @@  static int mt7921s_suspend(struct device *__dev)
 	int err;
 
 	pm->suspended = true;
+	set_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
+
 	cancel_delayed_work_sync(&pm->ps_work);
 	cancel_work_sync(&pm->wake_work);
 
@@ -213,10 +215,6 @@  static int mt7921s_suspend(struct device *__dev)
 	if (err < 0)
 		goto restore_suspend;
 
-	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
-	if (err)
-		goto restore_suspend;
-
 	/* always enable deep sleep during suspend to reduce
 	 * power consumption
 	 */
@@ -224,34 +222,45 @@  static int mt7921s_suspend(struct device *__dev)
 
 	mt76_txq_schedule_all(&dev->mphy);
 	mt76_worker_disable(&mdev->tx_worker);
-	mt76_worker_disable(&mdev->sdio.txrx_worker);
 	mt76_worker_disable(&mdev->sdio.status_worker);
-	mt76_worker_disable(&mdev->sdio.net_worker);
 	cancel_work_sync(&mdev->sdio.stat_work);
 	clear_bit(MT76_READING_STATS, &dev->mphy.state);
-
 	mt76_tx_status_check(mdev, true);
 
-	err = mt7921_mcu_fw_pmctrl(dev);
+	mt76_worker_schedule(&mdev->sdio.txrx_worker);
+	wait_event_timeout(dev->mt76.sdio.wait,
+			   mt76s_txqs_empty(&dev->mt76), 5 * HZ);
+
+	/* It is supposed that SDIO bus is idle at the point */
+	err = mt76_connac_mcu_set_hif_suspend(mdev, true);
 	if (err)
 		goto restore_worker;
 
+	mt76_worker_disable(&mdev->sdio.txrx_worker);
+	mt76_worker_disable(&mdev->sdio.net_worker);
+
+	err = mt7921_mcu_fw_pmctrl(dev);
+	if (err)
+		goto restore_txrx_worker;
+
 	sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
 
 	return 0;
 
+restore_txrx_worker:
+	mt76_worker_enable(&mdev->sdio.net_worker);
+	mt76_worker_enable(&mdev->sdio.txrx_worker);
+	mt76_connac_mcu_set_hif_suspend(mdev, false);
+
 restore_worker:
 	mt76_worker_enable(&mdev->tx_worker);
-	mt76_worker_enable(&mdev->sdio.txrx_worker);
 	mt76_worker_enable(&mdev->sdio.status_worker);
-	mt76_worker_enable(&mdev->sdio.net_worker);
 
 	if (!pm->ds_enable)
 		mt76_connac_mcu_set_deep_sleep(mdev, false);
 
-	mt76_connac_mcu_set_hif_suspend(mdev, false);
-
 restore_suspend:
+	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
 	pm->suspended = false;
 
 	return err;
@@ -266,6 +275,7 @@  static int mt7921s_resume(struct device *__dev)
 	int err;
 
 	pm->suspended = false;
+	clear_bit(MT76_STATE_SUSPEND, &mdev->phy.state);
 
 	err = mt7921_mcu_drv_pmctrl(dev);
 	if (err < 0)
diff --git a/drivers/net/wireless/mediatek/mt76/sdio.c b/drivers/net/wireless/mediatek/mt76/sdio.c
index c99acc21225e..b0bc7be0fb1f 100644
--- a/drivers/net/wireless/mediatek/mt76/sdio.c
+++ b/drivers/net/wireless/mediatek/mt76/sdio.c
@@ -479,7 +479,8 @@  static void mt76s_status_worker(struct mt76_worker *w)
 			resched = true;
 
 		if (dev->drv->tx_status_data &&
-		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state))
+		    !test_and_set_bit(MT76_READING_STATS, &dev->phy.state) &&
+		    !test_bit(MT76_STATE_SUSPEND, &dev->phy.state))
 			queue_work(dev->wq, &dev->sdio.stat_work);
 	} while (nframes > 0);
 
diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
index 649a56790b89..801590a0a334 100644
--- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
+++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
@@ -317,7 +317,8 @@  void mt76s_txrx_worker(struct mt76_sdio *sdio)
 		if (ret > 0)
 			nframes += ret;
 
-		if (test_bit(MT76_MCU_RESET, &dev->phy.state)) {
+		if (test_bit(MT76_MCU_RESET, &dev->phy.state) ||
+		    test_bit(MT76_STATE_SUSPEND, &dev->phy.state)) {
 			if (!mt76s_txqs_empty(dev))
 				continue;
 			else