Brian Norris [off-list ref] writes:

+ Arnd

On Thu, Oct 19, 2017 at 02:32:45PM +0000, Kalle Valo wrote:

quoted

Kalle Valo [off-list ref] writes:
=20

quoted

Brian Norris [off-list ref] wrote:

quoted

For devices where the FW supports WoWLAN but user-space has not
configured it, we don't do any PCI-specific suspend/resume operations=

,

quoted

quoted

quoted

because mac80211 doesn't call drv_suspend() when !wowlan. This has
particularly bad effects for some platforms, because we don't stop th=

e

quoted

quoted

quoted

power-save timer, and if this timer goes off after the PCI controller
has suspended the link, Bad Things will happen.
=20
Commit 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops"=

)

quoted

quoted

quoted

got some of this right, in that it understood there was a problem on
non-WoWLAN firmware. But it forgot the $subject case.
=20
Fix this by moving all the PCI driver suspend/resume logic exclusivel=

y

quoted

quoted

quoted

into the driver PM hooks. This shouldn't affect WoWLAN support much
(this just gets executed later on).
=20
I would just as well kill the entirety of ath10k_hif_suspend(), as it=

's

quoted

quoted

quoted

not even implemented on the USB or SDIO drivers. I expect that we don=

't

quoted

quoted

quoted

need the callback, except to return "supported" (i.e., 0) or "not
supported" (i.e., -EOPNOTSUPP).
=20
Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops"=

)

quoted

quoted

quoted

Fixes: 77258d409ce4 ("ath10k: enable pci soc powersaving")
Signed-off-by: Brian Norris <briannorris@chromium.org>
Cc: Ryan Hsu <redacted>
Cc: Kalle Valo <redacted>
Cc: Michal Kazior <redacted>
Signed-off-by: Kalle Valo <redacted>

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

96378bd2c6cd ath10k: fix core PCI suspend when WoWLAN is supported but=

 disabled

quoted

=20
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.

quoted

drivers/net/wireless/ath/ath10k/pci.c:3416:8: error: implicit
declaration of function 'ath10k_pci_suspend'
[-Werror=3Dimplicit-function-declaration]
=20
drivers/net/wireless/ath/ath10k/pci.c:3428:8: error: implicit
declaration of function 'ath10k_pci_resume'
[-Werror=3Dimplicit-function-declaration]
=20
http://lists.infradead.org/pipermail/ath10k/2017-October/010269.html
=20
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!

--=20
Kalle Valo=