diff mbox

ath10k: fix core PCI suspend when WoWLAN is supported but disabled

Message ID 20170919232416.108247-1-briannorris@chromium.org (mailing list archive)
State Accepted
Commit 96378bd2c6cda5f04d0f6da2cd35d4670a982c38
Delegated to: Kalle Valo
Headers show

Commit Message

Brian Norris Sept. 19, 2017, 11:24 p.m. UTC
For devices where the FW supports WoWLAN but user-space has not
configured it, we don't do any PCI-specific suspend/resume operations,
because mac80211 doesn't call drv_suspend() when !wowlan. This has
particularly bad effects for some platforms, because we don't stop the
power-save timer, and if this timer goes off after the PCI controller
has suspended the link, Bad Things will happen.

Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
got some of this right, in that it understood there was a problem on
non-WoWLAN firmware. But it forgot the $subject case.

Fix this by moving all the PCI driver suspend/resume logic exclusively
into the driver PM hooks. This shouldn't affect WoWLAN support much
(this just gets executed later on).

I would just as well kill the entirety of ath10k_hif_suspend(), as it's
not even implemented on the USB or SDIO drivers. I expect that we don't
need the callback, except to return "supported" (i.e., 0) or "not
supported" (i.e., -EOPNOTSUPP).

Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
Signed-off-by: Brian Norris <briannorris@chromium.org>
Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Cc: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Brian Norris Oct. 12, 2017, 12:38 a.m. UTC | #1
Ping? Any comments? I know there's more than one way to slice this
problem, but it's most definitely a problem...

Brian

On Tue, Sep 19, 2017 at 04:24:16PM -0700, Brian Norris wrote:
> For devices where the FW supports WoWLAN but user-space has not
> configured it, we don't do any PCI-specific suspend/resume operations,
> because mac80211 doesn't call drv_suspend() when !wowlan. This has
> particularly bad effects for some platforms, because we don't stop the
> power-save timer, and if this timer goes off after the PCI controller
> has suspended the link, Bad Things will happen.
> 
> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> got some of this right, in that it understood there was a problem on
> non-WoWLAN firmware. But it forgot the $subject case.
> 
> Fix this by moving all the PCI driver suspend/resume logic exclusively
> into the driver PM hooks. This shouldn't affect WoWLAN support much
> (this just gets executed later on).
> 
> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
> not even implemented on the USB or SDIO drivers. I expect that we don't
> need the callback, except to return "supported" (i.e., 0) or "not
> supported" (i.e., -EOPNOTSUPP).
> 
> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> Cc: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/pci.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index bc1633945a56..4655c944e3fd 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -2580,6 +2580,12 @@ void ath10k_pci_hif_power_down(struct ath10k *ar)
>  #ifdef CONFIG_PM
>  
>  static int ath10k_pci_hif_suspend(struct ath10k *ar)
> +{
> +	/* Nothing to do; the important stuff is in the driver suspend. */
> +	return 0;
> +}
> +
> +static int ath10k_pci_suspend(struct ath10k *ar)
>  {
>  	/* The grace timer can still be counting down and ar->ps_awake be true.
>  	 * It is known that the device may be asleep after resuming regardless
> @@ -2592,6 +2598,12 @@ static int ath10k_pci_hif_suspend(struct ath10k *ar)
>  }
>  
>  static int ath10k_pci_hif_resume(struct ath10k *ar)
> +{
> +	/* Nothing to do; the important stuff is in the driver resume. */
> +	return 0;
> +}
> +
> +static int ath10k_pci_resume(struct ath10k *ar)
>  {
>  	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
>  	struct pci_dev *pdev = ar_pci->pdev;
> @@ -3403,11 +3415,7 @@ static int ath10k_pci_pm_suspend(struct device *dev)
>  	struct ath10k *ar = dev_get_drvdata(dev);
>  	int ret;
>  
> -	if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
> -		     ar->running_fw->fw_file.fw_features))
> -		return 0;
> -
> -	ret = ath10k_hif_suspend(ar);
> +	ret = ath10k_pci_suspend(ar);
>  	if (ret)
>  		ath10k_warn(ar, "failed to suspend hif: %d\n", ret);
>  
> @@ -3419,11 +3427,7 @@ static int ath10k_pci_pm_resume(struct device *dev)
>  	struct ath10k *ar = dev_get_drvdata(dev);
>  	int ret;
>  
> -	if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
> -		     ar->running_fw->fw_file.fw_features))
> -		return 0;
> -
> -	ret = ath10k_hif_resume(ar);
> +	ret = ath10k_pci_resume(ar);
>  	if (ret)
>  		ath10k_warn(ar, "failed to resume hif: %d\n", ret);
>  
> -- 
> 2.14.1.690.gbb1197296e-goog
>
Adrian Chadd Oct. 12, 2017, 12:43 a.m. UTC | #2
fwiw - I did this on my in progress ath10k port to freebsd, and I've
tested it on Rome and Peregrine. It seems to be the right thing to do
during suspend to at least cleanly shut things down.


-adrian


On 11 October 2017 at 17:38, Brian Norris <briannorris@chromium.org> wrote:
> Ping? Any comments? I know there's more than one way to slice this
> problem, but it's most definitely a problem...
>
> Brian
>
> On Tue, Sep 19, 2017 at 04:24:16PM -0700, Brian Norris wrote:
>> For devices where the FW supports WoWLAN but user-space has not
>> configured it, we don't do any PCI-specific suspend/resume operations,
>> because mac80211 doesn't call drv_suspend() when !wowlan. This has
>> particularly bad effects for some platforms, because we don't stop the
>> power-save timer, and if this timer goes off after the PCI controller
>> has suspended the link, Bad Things will happen.
>>
>> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> got some of this right, in that it understood there was a problem on
>> non-WoWLAN firmware. But it forgot the $subject case.
>>
>> Fix this by moving all the PCI driver suspend/resume logic exclusively
>> into the driver PM hooks. This shouldn't affect WoWLAN support much
>> (this just gets executed later on).
>>
>> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
>> not even implemented on the USB or SDIO drivers. I expect that we don't
>> need the callback, except to return "supported" (i.e., 0) or "not
>> supported" (i.e., -EOPNOTSUPP).
>>
>> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
>> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
>> Cc: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/pci.c | 24 ++++++++++++++----------
>>  1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index bc1633945a56..4655c944e3fd 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -2580,6 +2580,12 @@ void ath10k_pci_hif_power_down(struct ath10k *ar)
>>  #ifdef CONFIG_PM
>>
>>  static int ath10k_pci_hif_suspend(struct ath10k *ar)
>> +{
>> +     /* Nothing to do; the important stuff is in the driver suspend. */
>> +     return 0;
>> +}
>> +
>> +static int ath10k_pci_suspend(struct ath10k *ar)
>>  {
>>       /* The grace timer can still be counting down and ar->ps_awake be true.
>>        * It is known that the device may be asleep after resuming regardless
>> @@ -2592,6 +2598,12 @@ static int ath10k_pci_hif_suspend(struct ath10k *ar)
>>  }
>>
>>  static int ath10k_pci_hif_resume(struct ath10k *ar)
>> +{
>> +     /* Nothing to do; the important stuff is in the driver resume. */
>> +     return 0;
>> +}
>> +
>> +static int ath10k_pci_resume(struct ath10k *ar)
>>  {
>>       struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
>>       struct pci_dev *pdev = ar_pci->pdev;
>> @@ -3403,11 +3415,7 @@ static int ath10k_pci_pm_suspend(struct device *dev)
>>       struct ath10k *ar = dev_get_drvdata(dev);
>>       int ret;
>>
>> -     if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
>> -                  ar->running_fw->fw_file.fw_features))
>> -             return 0;
>> -
>> -     ret = ath10k_hif_suspend(ar);
>> +     ret = ath10k_pci_suspend(ar);
>>       if (ret)
>>               ath10k_warn(ar, "failed to suspend hif: %d\n", ret);
>>
>> @@ -3419,11 +3427,7 @@ static int ath10k_pci_pm_resume(struct device *dev)
>>       struct ath10k *ar = dev_get_drvdata(dev);
>>       int ret;
>>
>> -     if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
>> -                  ar->running_fw->fw_file.fw_features))
>> -             return 0;
>> -
>> -     ret = ath10k_hif_resume(ar);
>> +     ret = ath10k_pci_resume(ar);
>>       if (ret)
>>               ath10k_warn(ar, "failed to resume hif: %d\n", ret);
>>
>> --
>> 2.14.1.690.gbb1197296e-goog
>>
Kalle Valo Oct. 12, 2017, 3:58 a.m. UTC | #3
Brian Norris <briannorris@chromium.org> writes:

> Ping? Any comments? I know there's more than one way to slice this
> problem, but it's most definitely a problem...

I'm lagging behind with patches but I'll try to catch up this week.
Sorry.
Kalle Valo Oct. 13, 2017, 11:37 a.m. UTC | #4
Brian Norris <briannorris@chromium.org> wrote:

> For devices where the FW supports WoWLAN but user-space has not
> configured it, we don't do any PCI-specific suspend/resume operations,
> because mac80211 doesn't call drv_suspend() when !wowlan. This has
> particularly bad effects for some platforms, because we don't stop the
> power-save timer, and if this timer goes off after the PCI controller
> has suspended the link, Bad Things will happen.
> 
> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> got some of this right, in that it understood there was a problem on
> non-WoWLAN firmware. But it forgot the $subject case.
> 
> Fix this by moving all the PCI driver suspend/resume logic exclusively
> into the driver PM hooks. This shouldn't affect WoWLAN support much
> (this just gets executed later on).
> 
> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
> not even implemented on the USB or SDIO drivers. I expect that we don't
> need the callback, except to return "supported" (i.e., 0) or "not
> supported" (i.e., -EOPNOTSUPP).
> 
> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> Cc: Michal Kazior <michal.kazior@tieto.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

Patch applied to ath-next branch of ath.git, thanks.

96378bd2c6cd ath10k: fix core PCI suspend when WoWLAN is supported but disabled
Kalle Valo Oct. 19, 2017, 2:32 p.m. UTC | #5
Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Brian Norris <briannorris@chromium.org> wrote:
>
>> For devices where the FW supports WoWLAN but user-space has not
>> configured it, we don't do any PCI-specific suspend/resume operations,
>> because mac80211 doesn't call drv_suspend() when !wowlan. This has
>> particularly bad effects for some platforms, because we don't stop the
>> power-save timer, and if this timer goes off after the PCI controller
>> has suspended the link, Bad Things will happen.
>> 
>> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> got some of this right, in that it understood there was a problem on
>> non-WoWLAN firmware. But it forgot the $subject case.
>> 
>> Fix this by moving all the PCI driver suspend/resume logic exclusively
>> into the driver PM hooks. This shouldn't affect WoWLAN support much
>> (this just gets executed later on).
>> 
>> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
>> not even implemented on the USB or SDIO drivers. I expect that we don't
>> need the callback, except to return "supported" (i.e., 0) or "not
>> supported" (i.e., -EOPNOTSUPP).
>> 
>> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
>> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
>> Cc: Michal Kazior <michal.kazior@tieto.com>
>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
>
> Patch applied to ath-next branch of ath.git, thanks.
>
> 96378bd2c6cd ath10k: fix core PCI suspend when WoWLAN is supported but disabled

Kbuild found a build problem, I suspect it's caused by this patch:

drivers/net/wireless/ath/ath10k/pci.c:3416:8: error: implicit
declaration of function 'ath10k_pci_suspend'
[-Werror=implicit-function-declaration]

drivers/net/wireless/ath/ath10k/pci.c:3428:8: error: implicit
declaration of function 'ath10k_pci_resume'
[-Werror=implicit-function-declaration]

http://lists.infradead.org/pipermail/ath10k/2017-October/010269.html

The .config.gz there doesn't have CONFIG_PM set, maybe that's the
problem?
Brian Norris Oct. 19, 2017, 5:12 p.m. UTC | #6
+ Arnd

On Thu, Oct 19, 2017 at 02:32:45PM +0000, Kalle Valo wrote:
> Kalle Valo <kvalo@qca.qualcomm.com> writes:
> 
> > Brian Norris <briannorris@chromium.org> wrote:
> >
> >> For devices where the FW supports WoWLAN but user-space has not
> >> configured it, we don't do any PCI-specific suspend/resume operations,
> >> because mac80211 doesn't call drv_suspend() when !wowlan. This has
> >> particularly bad effects for some platforms, because we don't stop the
> >> power-save timer, and if this timer goes off after the PCI controller
> >> has suspended the link, Bad Things will happen.
> >> 
> >> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> >> got some of this right, in that it understood there was a problem on
> >> non-WoWLAN firmware. But it forgot the $subject case.
> >> 
> >> Fix this by moving all the PCI driver suspend/resume logic exclusively
> >> into the driver PM hooks. This shouldn't affect WoWLAN support much
> >> (this just gets executed later on).
> >> 
> >> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
> >> not even implemented on the USB or SDIO drivers. I expect that we don't
> >> need the callback, except to return "supported" (i.e., 0) or "not
> >> supported" (i.e., -EOPNOTSUPP).
> >> 
> >> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> >> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
> >> Signed-off-by: Brian Norris <briannorris@chromium.org>
> >> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
> >> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> >> Cc: Michal Kazior <michal.kazior@tieto.com>
> >> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
> >
> > Patch applied to ath-next branch of ath.git, thanks.
> >
> > 96378bd2c6cd ath10k: fix core PCI suspend when WoWLAN is supported but disabled
> 
> Kbuild found a build problem, I suspect it's caused by this patch:

Actually, it's the interaction of this patch and Arnd's patch:

6af1de2e4ec4 ath10k: mark PM functions as __maybe_unused

I see that's now in these branches:

  ath/ath-current
  ath/ath-qca
  ath/master
  ath/master-pending
  wireless-drivers-next/master
  wireless-drivers-next/pending

Whereas mine got applied to:

  ath/ath-next

So technically, the problem is in your merge here :)

096ad2a15fd8 Merge branch 'ath-next'

> drivers/net/wireless/ath/ath10k/pci.c:3416:8: error: implicit
> declaration of function 'ath10k_pci_suspend'
> [-Werror=implicit-function-declaration]
> 
> drivers/net/wireless/ath/ath10k/pci.c:3428:8: error: implicit
> declaration of function 'ath10k_pci_resume'
> [-Werror=implicit-function-declaration]
> 
> http://lists.infradead.org/pipermail/ath10k/2017-October/010269.html
> 
> The .config.gz there doesn't have CONFIG_PM set, maybe that's the
> problem?

Yes, indirectly that's also the problem.

The solution would seem to be either to kill the #ifdefs around
ath10k_pci_{suspend,resume}() and friends (and use __maybe_unused
instead, to further extend Arnd's patch), or else revert Arnd's stuff
and go with CONFIG_PM_SLEEP everywhere, which would resolve the original
warning (promoted to error) that Arnd was resolving.

I can send out one of these if you'd like.

Brian
Kalle Valo Oct. 20, 2017, 6:24 a.m. UTC | #7
Brian Norris <briannorris@chromium.org> writes:

> + Arnd
>
> On Thu, Oct 19, 2017 at 02:32:45PM +0000, Kalle Valo wrote:
>> Kalle Valo <kvalo@qca.qualcomm.com> writes:
>> 
>> > Brian Norris <briannorris@chromium.org> wrote:
>> >
>> >> For devices where the FW supports WoWLAN but user-space has not
>> >> configured it, we don't do any PCI-specific suspend/resume operations,
>> >> because mac80211 doesn't call drv_suspend() when !wowlan. This has
>> >> particularly bad effects for some platforms, because we don't stop the
>> >> power-save timer, and if this timer goes off after the PCI controller
>> >> has suspended the link, Bad Things will happen.
>> >> 
>> >> Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> >> got some of this right, in that it understood there was a problem on
>> >> non-WoWLAN firmware. But it forgot the $subject case.
>> >> 
>> >> Fix this by moving all the PCI driver suspend/resume logic exclusively
>> >> into the driver PM hooks. This shouldn't affect WoWLAN support much
>> >> (this just gets executed later on).
>> >> 
>> >> I would just as well kill the entirety of ath10k_hif_suspend(), as it's
>> >> not even implemented on the USB or SDIO drivers. I expect that we don't
>> >> need the callback, except to return "supported" (i.e., 0) or "not
>> >> supported" (i.e., -EOPNOTSUPP).
>> >> 
>> >> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
>> >> Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
>> >> Signed-off-by: Brian Norris <briannorris@chromium.org>
>> >> Cc: Ryan Hsu <ryanhsu@qti.qualcomm.com>
>> >> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
>> >> Cc: Michal Kazior <michal.kazior@tieto.com>
>> >> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
>> >
>> > Patch applied to ath-next branch of ath.git, thanks.
>> >
>> > 96378bd2c6cd ath10k: fix core PCI suspend when WoWLAN is supported but disabled
>> 
>> Kbuild found a build problem, I suspect it's caused by this patch:
>
> Actually, it's the interaction of this patch and Arnd's patch:
>
> 6af1de2e4ec4 ath10k: mark PM functions as __maybe_unused
>
> I see that's now in these branches:
>
>   ath/ath-current
>   ath/ath-qca
>   ath/master
>   ath/master-pending
>   wireless-drivers-next/master
>   wireless-drivers-next/pending
>
> Whereas mine got applied to:
>
>   ath/ath-next
>
> So technically, the problem is in your merge here :)
>
> 096ad2a15fd8 Merge branch 'ath-next'

Ah, that's why kbuild bot didn't report about this problem while your
patch was in my pending branch. And I was also really puzzled why it
claimed that my merge was at fault :) Thanks for the good explanation.

Just to clarify: ath-current is for patches going to 4.14 and ath-next
to 4.15, that's why they were applied to a different branch.

>> drivers/net/wireless/ath/ath10k/pci.c:3416:8: error: implicit
>> declaration of function 'ath10k_pci_suspend'
>> [-Werror=implicit-function-declaration]
>> 
>> drivers/net/wireless/ath/ath10k/pci.c:3428:8: error: implicit
>> declaration of function 'ath10k_pci_resume'
>> [-Werror=implicit-function-declaration]
>> 
>> http://lists.infradead.org/pipermail/ath10k/2017-October/010269.html
>> 
>> The .config.gz there doesn't have CONFIG_PM set, maybe that's the
>> problem?
>
> Yes, indirectly that's also the problem.
>
> The solution would seem to be either to kill the #ifdefs around
> ath10k_pci_{suspend,resume}() and friends (and use __maybe_unused
> instead, to further extend Arnd's patch), or else revert Arnd's stuff
> and go with CONFIG_PM_SLEEP everywhere, which would resolve the original
> warning (promoted to error) that Arnd was resolving.
>
> I can send out one of these if you'd like.

I see that you already sent the patch, thanks!
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index bc1633945a56..4655c944e3fd 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2580,6 +2580,12 @@  void ath10k_pci_hif_power_down(struct ath10k *ar)
 #ifdef CONFIG_PM
 
 static int ath10k_pci_hif_suspend(struct ath10k *ar)
+{
+	/* Nothing to do; the important stuff is in the driver suspend. */
+	return 0;
+}
+
+static int ath10k_pci_suspend(struct ath10k *ar)
 {
 	/* The grace timer can still be counting down and ar->ps_awake be true.
 	 * It is known that the device may be asleep after resuming regardless
@@ -2592,6 +2598,12 @@  static int ath10k_pci_hif_suspend(struct ath10k *ar)
 }
 
 static int ath10k_pci_hif_resume(struct ath10k *ar)
+{
+	/* Nothing to do; the important stuff is in the driver resume. */
+	return 0;
+}
+
+static int ath10k_pci_resume(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct pci_dev *pdev = ar_pci->pdev;
@@ -3403,11 +3415,7 @@  static int ath10k_pci_pm_suspend(struct device *dev)
 	struct ath10k *ar = dev_get_drvdata(dev);
 	int ret;
 
-	if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
-		     ar->running_fw->fw_file.fw_features))
-		return 0;
-
-	ret = ath10k_hif_suspend(ar);
+	ret = ath10k_pci_suspend(ar);
 	if (ret)
 		ath10k_warn(ar, "failed to suspend hif: %d\n", ret);
 
@@ -3419,11 +3427,7 @@  static int ath10k_pci_pm_resume(struct device *dev)
 	struct ath10k *ar = dev_get_drvdata(dev);
 	int ret;
 
-	if (test_bit(ATH10K_FW_FEATURE_WOWLAN_SUPPORT,
-		     ar->running_fw->fw_file.fw_features))
-		return 0;
-
-	ret = ath10k_hif_resume(ar);
+	ret = ath10k_pci_resume(ar);
 	if (ret)
 		ath10k_warn(ar, "failed to resume hif: %d\n", ret);