linux-kernel - Re: [PATCH v5 08/24] wfx: add bus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Apr 2021 10:22:47 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Kalle Valo <kvalo@...eaurora.org>
Cc:     Jérôme Pouiller <jerome.pouiller@...abs.com>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        driverdevel <devel@...verdev.osuosl.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S . Miller" <davem@...emloft.net>,
        DTML <devicetree@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        Pali Rohár <pali@...nel.org>
Subject: Re: [PATCH v5 08/24] wfx: add bus_sdio.c

On Wed, 7 Apr 2021 at 14:00, Kalle Valo <kvalo@...eaurora.org> wrote:
>
> Ulf Hansson <ulf.hansson@...aro.org> writes:
>
> >> If I follow what has been done in other drivers I would write something
> >> like:
> >>
> >>   static int wfx_sdio_suspend(struct device *dev)
> >>   {
> >>           struct sdio_func *func = dev_to_sdio_func(dev);
> >>           struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
> >>
> >>           config_reg_write_bits(bus->core, CFG_IRQ_ENABLE_DATA, 0);
> >>           // Necessary to keep device firmware in RAM
> >>           return sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
> >
> > This will tell the mmc/sdio core to keep the SDIO card powered on
> > during system suspend. Thus, it doesn't need to re-initialize it at
> > system resume - and the firmware should not need to be re-programmed.
> >
> > On the other hand, if you don't plan to support system wakeups, it
> > would probably be better to power off the card, to avoid wasting
> > energy while the system is suspended. I assume that means you need to
> > re-program the firmware as well. Normally, it's these kinds of things
> > that need to be managed from a ->resume() callback.
>
> Many mac80211 drivers do so that the device is powered off during
> interface down (ifconfig wlan0 down), and as mac80211 does interface
> down automatically during suspend, suspend then works without extra
> handlers.

That sounds simple. :-)

Would you mind elaborating on what is actually being powered off at
interface down - and thus also I am curious what happens at a typical
interface up?

Even if we don't want to use system wakeups (wake-on-lan), the SDIO
core and the SDIO func driver still need to somewhat agree on how to
manage the power for the card during system suspend, I think.

For example, for a non-removable SDIO card, the SDIO/MMC core may
decide to power off the card in system suspend. Then it needs to
restore power to the card and re-initialize it at system resume, of
course. This doesn't mean that the actual corresponding struct device
for it, gets removed/re-added, thus the SDIO func driver isn't being
re-probed after the system has resumed. Although, since the SDIO card
was re-initialized, it's likely that the FW may need to be
re-programmed after the system has been resumed.

Are you saying that re-programming the FW is always happening at
interface up, when there are none system suspend/resume callbacks
assigned for the SDIO func driver?

Kind regards
Uffe