On 6/9/2021 2:22 AM, Ulf Hansson wrote: > On Wed, 9 Jun 2021 at 05:07, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote: >> >> >> >> On 6/8/2021 5:40 AM, Ulf Hansson wrote: >>> On Wed, 2 Jun 2021 at 21:28, Al Cooper <alcooperx@xxxxxxxxx> wrote: >>>> >>>> Add support for the legacy Arasan sdhci controller on the BCM7211 and >>>> related SoC's. This includes adding a .shutdown callback to increase >>>> the power savings during S5. >>> >>> Please split this into two separate changes. >>> >>> May I also ask about the ->shutdown() callback and in relation to S5. >>> What makes the ->shutdown callback only being invoked for S5? >> >> It is not only called for S5 (entered via poweroff on a prompt) but also >> during kexec or reboot. The poweroff path is via: >> >> kernel_power_off() -> kernel_shutdown_prepare() -> device_shutdown() -> >> .shutdown() >> >> For kexec or reboot we do not really care about power savings since we >> are about to load a new image anyway, however for S5/poweroff we do care >> about quiescing the eMMC controller in a way that its clocks and the >> eMMC device can be put into low power mode since we will stay in that >> mode for seconds/hours/days until someone presses a button on their >> remote (or other wake-up sources). > > Hmm, I am not sure I understand correctly. At shutdown we don't care > about wake-up sources from the kernel point of view, instead we treat > everything as if it will be powered off. The same .shutdown() path is used whether you kexec, reboot or poweroff, but for poweroff we do care about allowing specific wake-up sources configured as such to wake-up the system at a later time, like GPIOs, RTC, etc. > > We put devices into low power state at system suspend and potentially > also during some of the hibernation phases. > > Graceful shutdown of the eMMC is also managed by the mmc core. AFAICT that calls mmc_blk_shutdown() but that is pretty much it, the SDHCI platform_driver still needs to do something in order to conserve power including disabling host->clk, otherwise we would not have done that for sdhci-brcmstb.c. -- Florian