Hi Arend,
Le lun., avril 18 2022 at 09:09:46 +0200, Arend van Spriel
<arend.vanspriel@xxxxxxxxxxxx> a écrit :
On 4/15/2022 10:03 PM, Paul Cercueil wrote:
Use the new DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr() macros to
handle the .suspend/.resume callbacks.
These macros allow the suspend and resume functions to be
automatically
dropped by the compiler when CONFIG_SUSPEND is disabled, without
having
to use #ifdef guards. The advantage is then that these functions are
not
conditionally compiled.
The advantage stated here may not be obvious to everyone and that is
because it only scratches the surface. The code is always compiled
independent from the Kconfig options used and because of that the
real advantage is that build regressions are easier to catch.
Exactly. I will improve the commit message to make this a bit more
explicit.
Some other functions not directly called by the .suspend/.resume
callbacks, but still related to PM were also taken outside #ifdef
guards.
a few comments on this inline...
Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
---
.../broadcom/brcm80211/brcmfmac/bcmsdh.c | 44
+++++++------------
.../broadcom/brcm80211/brcmfmac/sdio.c | 5 +--
.../broadcom/brcm80211/brcmfmac/sdio.h | 16 -------
3 files changed, 19 insertions(+), 46 deletions(-)
diff --git
a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index ac02244a6fdf..a8cf5a570101 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
[...]
@@ -873,7 +865,8 @@ int brcmf_sdiod_remove(struct brcmf_sdio_dev
*sdiodev)
sdiodev->bus = NULL;
}
- brcmf_sdiod_freezer_detach(sdiodev);
+ if (IS_ENABLED(CONFIG_PM_SLEEP))
+ brcmf_sdiod_freezer_detach(sdiodev);
Please move the if statement inside the function to keep the code
flow in the calling function the same as before.
/* Disable Function 2 */
sdio_claim_host(sdiodev->func2);
@@ -949,9 +942,11 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev
*sdiodev)
goto out;
}
- ret = brcmf_sdiod_freezer_attach(sdiodev);
- if (ret)
- goto out;
+ if (IS_ENABLED(CONFIG_PM_SLEEP)) {
+ ret = brcmf_sdiod_freezer_attach(sdiodev);
+ if (ret)
+ goto out;
+ }
Dito. Move the if statement inside the function.
Sure.
Cheers,
-Paul
/* try to attach to the target device */
sdiodev->bus = brcmf_sdio_probe(sdiodev);
