Messages in this thread Patch in this message | | | Date | Fri, 18 Mar 2022 11:47:05 +0100 | Subject | Re: [PATCH] brcmfmac: Avoid keeping power to SDIO card unless WOWL is used | From | Yann Gautier <> |
| |
On 3/17/22 16:28, Ulf Hansson wrote: > Keeping the power to the SDIO card during system wide suspend, consumes > energy. Especially on battery driven embedded systems, this can be a > problem. Therefore, let's change the behaviour into allowing the SDIO card > to be powered off, unless WOWL is supported and enabled. > > Note that, the downside from this change, is that at system resume the SDIO > card needs to be re-initialized and the FW must re-programmed. Even it that > may take some time to complete, it should we worth it, rather than draining > a battery. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > > Please note that, I have only compile-tested this patch, so I am relying on help > from Yann and others to run tests on real HW. > > Kind regards > Ulf Hansson > > --- > .../broadcom/brcm80211/brcmfmac/bcmsdh.c | 33 +++++++++++-------- > 1 file changed, 20 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > index ac02244a6fdf..351886c9d68e 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > @@ -1119,9 +1119,21 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled) > { > struct brcmf_bus *bus_if = dev_get_drvdata(dev); > struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio; > + mmc_pm_flag_t pm_caps = sdio_get_host_pm_caps(sdiodev->func1); > > - brcmf_dbg(SDIO, "Configuring WOWL, enabled=%d\n", enabled); > - sdiodev->wowl_enabled = enabled; > + /* Power must be preserved to be able to support WOWL. */ > + if (!(pm_caps & MMC_PM_KEEP_POWER)) > + goto notsup; > + > + if (sdiodev->settings->bus.sdio.oob_irq_supported || > + pm_caps & MMC_PM_WAKE_SDIO_IRQ) { > + sdiodev->wowl_enabled = enabled; > + brcmf_dbg(SDIO, "Configuring WOWL, enabled=%d\n", enabled); > + return; > + } > + > +notsup: > + brcmf_dbg(SDIO, "WOWL not supported\n"); > } > > #ifdef CONFIG_PM_SLEEP > @@ -1130,7 +1142,7 @@ static int brcmf_ops_sdio_suspend(struct device *dev) > struct sdio_func *func; > struct brcmf_bus *bus_if; > struct brcmf_sdio_dev *sdiodev; > - mmc_pm_flag_t pm_caps, sdio_flags; > + mmc_pm_flag_t sdio_flags; > int ret = 0; > > func = container_of(dev, struct sdio_func, dev); > @@ -1142,20 +1154,15 @@ static int brcmf_ops_sdio_suspend(struct device *dev) > bus_if = dev_get_drvdata(dev); > sdiodev = bus_if->bus_priv.sdio; > > - pm_caps = sdio_get_host_pm_caps(func); > - > - if (pm_caps & MMC_PM_KEEP_POWER) { > - /* preserve card power during suspend */ > + if (sdiodev->wowl_enabled) { > brcmf_sdiod_freezer_on(sdiodev); > brcmf_sdio_wd_timer(sdiodev->bus, 0); > > sdio_flags = MMC_PM_KEEP_POWER; > - if (sdiodev->wowl_enabled) { > - if (sdiodev->settings->bus.sdio.oob_irq_supported) > - enable_irq_wake(sdiodev->settings->bus.sdio.oob_irq_nr); > - else > - sdio_flags |= MMC_PM_WAKE_SDIO_IRQ; > - } > + if (sdiodev->settings->bus.sdio.oob_irq_supported) > + enable_irq_wake(sdiodev->settings->bus.sdio.oob_irq_nr); > + else > + sdio_flags |= MMC_PM_WAKE_SDIO_IRQ; > > if (sdio_set_host_pm_flags(sdiodev->func1, sdio_flags)) > brcmf_err("Failed to set pm_flags %x\n", sdio_flags);
Hi Ulf,
You are missing the counter part in brcmf_ops_sdio_resume:
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -1190,14 +1190,13 @@ static int brcmf_ops_sdio_resume(struct device *dev) if (func->num != 2) return 0;
- if (!(pm_caps & MMC_PM_KEEP_POWER)) { + if (!sdiodev->wowl_enabled) { /* bus was powered off and device removed, probe again */ ret = brcmf_sdiod_probe(sdiodev); if (ret) brcmf_err("Failed to probe device on resume\n"); } else { - if (sdiodev->wowl_enabled && - sdiodev->settings->bus.sdio.oob_irq_supported) + if (sdiodev->settings->bus.sdio.oob_irq_supported) disable_irq_wake(sdiodev->settings->bus.sdio.oob_irq_nr);
Else, we get an oops when calling brcmf_sdio_sleep.
Best regards, Yann
| |