+ Rob On Thu, 25 Jun 2020 at 11:26, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Shimoda-san, > > CC broonie > > On Thu, Jun 25, 2020 at 8:31 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > > From: Geert Uytterhoeven, Sent: Wednesday, June 24, 2020 8:13 PM > > > On Wed, Jun 24, 2020 at 12:06 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > On Mon, 22 Jun 2020 at 04:25, Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > > > > If pm_suspend_via_firmware() returns true, the system will be able > > > > > to cut both vcc and vccq in the suspend. So, call > > > > > mmc_poweroff_nofity() if pm_suspend_via_firmware() returns true. > > > > > > > > > > Note that we should not update the MMC_CAP2_FULL_PWR_CYCLE caps > > > > > because the mmc_select_voltage() checks the caps when attaches > > > > > a mmc/sd. > > > > > > > > --- a/drivers/mmc/core/mmc.c > > > > > +++ b/drivers/mmc/core/mmc.c > > > > > @@ -2038,7 +2039,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > > > > > goto out; > > > > > > > > > > if (mmc_can_poweroff_notify(host->card) && > > > > > - ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend)) > > > > > + ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend || > > > > > + pm_suspend_via_firmware())) > > > > > > > > Sorry, but this doesn't work. > > > > > > > > Even if PSCI is a generic FW interface, it doesn't mean that all PSCI > > > > implementations will cut the vcc and vccq for the MMC card at system > > > > suspend. > > > > > > Indeed, there's nothing guaranteed here. Nor documented how it should > > > behave. Basically the firmware is free to power off the SoC. Or not do that. > > > "If firmware is involved, all odds are off". > > > > I thought we could be guaranteed. But, I understood we could not be guaranteed... > > > > > > Instead, you need to decide this based on some specific DT property. > > > > Perhaps in conjunction with using pm_suspend_via_firmware(). > > > > > > Last time I was involved in a discussion about this, the PSCI people > > > didn't want to add any properties describing particular PSCI behavior... > > > "If firmware is involved, all odds are off". > > > > > > So the only safe thing to do is to expect the worst, and prepare for it... > > > > A headache point is an eMMC device consumes much power if that the system > > doesn't cut the vcc and vccq and doesn’t enter the sleep mode. > > In other words, in power consumption point of view, this patch will > > cause a regression in such a case... > > Indeed. > > > By the way, about adding specific DT property, the regulator can have > > regulator-off-in-suspend property in regulator-state-mem subnode. > > For now, we doesn't seem to get the property from a regulator consumer though. > > So, I'll try to add an API of regulator for it. > > Oh right, the eMMC is described in DT as being connected to two > regulators. > Note that the semantics of regulator-off-in-suspend are that the > regulator should be disabled (by the regulator core) during suspend, not > that the regulator is disabled during suspend by a third party. > No idea if that will work with a fixed-regulator without GPIO control, > but of course you can try. For mmc we already have a DT property, "full-pwr-cycle". Which tells whether the host is able to completely power-cycle the card (some host's manage power internally). However, maybe the proper thing here would be to add a property of regulator instead. If that doesn't make sense, I am also open to add a new MMC property, "full-pwr-cycle-in-suspend". Kind regards Uffe
