Messages in this thread | | | Date | Wed, 23 Mar 2022 14:20:36 +0100 | Subject | Re: [PATCH v2] brcmfmac: Avoid keeping power to SDIO card unless WOWL is used | From | Yann Gautier <> |
| |
On 3/23/22 09:39, 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 during system resume the > SDIO card needs to be re-initialized and the FW must be re-programmed. Even > if this may take some time to complete, it should we worth it, rather than > draining the battery. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > > Changes in v2: > - As pointed out by Yann, the changes for the resume path was missing, > so I have added that too. > > Again, 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 | 39 +++++++++++-------- > 1 file changed, 22 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > index ac02244a6fdf..9c598ea97499 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); > @@ -1176,21 +1183,19 @@ static int brcmf_ops_sdio_resume(struct device *dev) > struct brcmf_bus *bus_if = dev_get_drvdata(dev); > struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio; > struct sdio_func *func = container_of(dev, struct sdio_func, dev); > - mmc_pm_flag_t pm_caps = sdio_get_host_pm_caps(func); > int ret = 0; > > brcmf_dbg(SDIO, "Enter: F%d\n", func->num); > 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); > > brcmf_sdiod_freezer_off(sdiodev);
Hi Ulf,
Thanks for the patch, it is OK, and tested by Christophe (R.). So you can add: Tested-by: Christophe Roullier <christophe.roullier@foss.st.com> Acked-by: Yann Gautier <yann.gautier@foss.st.com>
Best regards, Yann
| |