On Mon, 8 Jun 2020 at 14:36, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Ulf,
>
> On Mon, Jun 8, 2020 at 1:47 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > On Mon, 8 Jun 2020 at 12:39, Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > > > From: Ulf Hansson, Sent: Monday, June 8, 2020 5:14 PM
> > > > On Thu, 4 Jun 2020 at 14:17, Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > > > > > From: Yoshihiro Shimoda, Sent: Tuesday, May 19, 2020 8:33 PM
> > > > > >
> > > > > > The commit 432356793415 ("mmc: core: Enable power_off_notify for
> > > > > > eMMC shutdown sequence") enabled the power off notification
> > > > > > even if MMC_CAP2_POWEROFF_NOTIFY (MMC_CAP2_FULL_PWR_CYCLE now) is
> > > > > > not set. However, the mmc core lacks to issue the power off
> > > > > > notificaiton when Suspend-to-{RAM,Disk} happens on the system.
> > > > > >
> > > > > > So, add Suspend-to-RAM support at first because this is easy to
> > > > > > check by using pm_suspend_target_state condition on _mmc_suspend().
> > > > > >
> > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > > > >
> > > > > I'd like to add more detail why this patch is needed.
> > > > > I think we should think some events (which are Shutdown, Suspend-to-idle,
> > > > > Suspend-to-RAM) for the Power off Notification control.
> > > > > I described these events like below.
> > > > >
> > > > > Assumption of the host : MMC_CAP2_FULL_PWR_CYCLE=false
> > > > > Assumption of the eMMC : in POWERED_ON
> > > > >
> > > > > 1) Event : Shutdown
> > > > > - power : going to VCC=OFF & VCCQ=OFF
> > > > > - ideal : Either POWER_OFF_LONG or POWER_OFF_SHORT
> > > > > - current : POWER_OFF_LONG --> Perfect
> > > > > - Remarks : the commit 432356793415
> > > > >
> > > > > 2) Event : Suspend-to-Idle
> > > > > - power : Keep VCC=ON & VCCQ=ON
> > > > > - ideal : issue MMC_SLEEP_AWAKE and keep the power (because the host could not change VCC=OFF)
> > > > > - current : issue MMC_SLEEP_AWAKE and keep the power --> Perfect
> > > > > - Remarks : IIUC, even if the eMMC is in POWERED_ON, a host can issue CMD5 (sleep).
> > > >
> > > > As a matter of fact, VCCQ *must* remain on in sleep state, while VCC
> > > > can be powered off.
> > >
> > > I got it.
> > >
> > > > >
> > > > > 3) Event : Suspend-to-RAM
> > > > > - power : going to VCC=OFF & VCCQ=OFF
> > > >
> > > > I don't understand why you think S2R should be treated differently
> > > > from S2I? At least from the MMC subsystem point of view, there is no
> > > > difference. No?
> > >
> > > On my environment, VCC & VCCQ condition differs like below.
> > > S2I: VCC=ON & VCCQ=ON
> > > S2R: VCC=OFF & VCCQ=OFF
> >
> > Can you explain why it differs? Who is managing the regulators and who
> > decides to turn them off?
>
> The firmware does, through PSCI system suspend.
> And what it does exactly is not standardized.
This sounds really weird. Especially, to let PSCI handle the VCC
regulator seems wrong, while PSCI is about power for CPUs and CPU
clusters (and corresponding power rails).
Oh well, nevermind.
> Perhaps we do need an "arm,psci-system-suspend-is-power-down"[1]
> DT property?
Hmm.
I wouldn't limit this to PSCI, but rather see this as a generic FW issue.
In principle, it sounds to me, like we need to dynamically allow the
mmc host to update MMC_CAP2_FULL_PWR_CYCLE, depending on what system
suspend mode we are about to enter. Or something along those lines.
>
> > Perhaps this is a regulator-enable usage count problem?
>
> Unfortunately not. Else we could fix it :-)
I see.
[...]
Kind regards
Uffe
