Hi Ulf, > From: Ulf Hansson, Sent: Friday, June 26, 2020 8:03 PM > > + 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. As other email thread of this trial as v4, I could not get approval [1]. [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdX93Q9WhKLqv_wNPzArbc68NcbVN8jJ9MDKxAcicpBQ5Q@xxxxxxxxxxxxxx/T/#m70883cc5de4fa7fca50118dad743c836d5e3b451 > 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. My v4 patch was using a regulator property. But since I could not get approval, we could not use this way, I think. > If that doesn't make sense, I am also open to add a > new MMC property, "full-pwr-cycle-in-suspend". I thought this way was better. However, I'm wondering if adding such a new MMC property to issue Power Off Notification in mmc_suspend() is really better than the current implementation. This is because we don't have any completely solutions now: - Depend on board configuration (The board doesn't have "full-pwr-cycle"). - Depend on firmware on board [2]. So, even if adding a new MMC property, it cannot sync the firmware condition. In fact, this is possible to cause regression in power consumption point of view [3]. - My environment (PSCI which is one of firmware) doesn't support any ability to sync between the firmware and OS for now [4]. But, what do you think? [2] https://lore.kernel.org/linux-renesas-soc/1592792699-24638-1-git-send-email-yoshihiro.shimoda.uh@xxxxxxxxxxx/T/#mde38d5866b7f3edc9a2ed105ac06b4fda4c3e99e [3] https://lore.kernel.org/linux-renesas-soc/1592792699-24638-1-git-send-email-yoshihiro.shimoda.uh@xxxxxxxxxxx/T/#mf8e7abdf135586ba0b70a1c235410a6f94c6007d [4] https://lore.kernel.org/linux-renesas-soc/CAMuHMdX93Q9WhKLqv_wNPzArbc68NcbVN8jJ9MDKxAcicpBQ5Q@xxxxxxxxxxxxxx/T/#m442a2ce972cfdb3ff33637c120c8d096e4d07af8 Best regards, Yoshihiro Shimoda > Kind regards > Uffe
