Re: [PATCH v2] brcmfmac: Correctly fail to suspend when SDIO does not support power on suspend

[Arend van Spriel <arend.vanspriel@broadcom.com>]

On 01-09-17 15:00, Eric Bentley wrote:
> Return error when failing to set power management capabilities flag.  This will
> cause the suspend to fail but the radio will continue to operate.  Allowing this
> to fail without reporting error will cause the radio to be non-functional on
> resume as it will have lost power.

There is more to this story to tell. Initially, the suspend callback did 
return an error upon failing to set the flags. This was dropped by 
commit 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.") as 
it set the flags only for wowl and that wowl would only be enabled if 
the host supports the capability flags:

+#ifdef CONFIG_PM_SLEEP
+       /* wowl can be supported when KEEP_POWER is true and (WAKE_SDIO_IRQ
+        * is true or when platform data OOB irq is true).
+        */
+       if ((sdio_get_host_pm_caps(sdiodev->func[1]) & MMC_PM_KEEP_POWER) &&
+           ((sdio_get_host_pm_caps(sdiodev->func[1]) & 
MMC_PM_WAKE_SDIO_IRQ) ||
+            (sdiodev->pdata->oob_irq_supported)))
+               bus_if->wowl_supported = true;
+#endif

However, MMC_PM_KEEP_POWER is also needed for the non-wowl case. I 
restored that in commit bdf1340cf20f ("brcmfmac: fix sdio suspend and 
resume"), but overlooked the error code return also needed to be restored.

Also note that the wifi chip (the term "radio" does not quite cover it) 
has not really lost power. It is quite common that it is not powered 
through the SDIO bus. With the power-sequence support in the MMC stack 
these days the suspend may result in loss of power. Otherwise, it is 
just the bus that loses power (and clock) and the flags affect what 
tricks the MMC stack tries to pull to get the device accessible again 
upon resume.

Maybe you can incorporate some of this in the commit message, but you 
should at least add:

Fixes: 330b4e4be937 ("brcmfmac: Add wowl support for SDIO devices.")
Fixes: bdf1340cf20f ("brcmfmac: fix sdio suspend and resume")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Eric Bentley eric.bentley@lairdtech.com
> ---
> v2: corrected errant ( with {
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)