[PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Yoshihiro Shimoda]

Add full-pwr-cycle-in-suspend property to do a graceful shutdown of
the eMMC device in system suspend.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 This new property has been merged into mmc.git / next branch:
https://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git/commit/?h=next&id=475923f495c0cdd4f35c469681013dddd09d3728

 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index dc24cec4..7402cfa 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -715,6 +715,7 @@
 	mmc-hs400-1_8v;
 	bus-width = <8>;
 	non-removable;
+	full-pwr-cycle-in-suspend;
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 98bbcaf..1bf7795 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -833,6 +833,7 @@
 	mmc-hs400-1_8v;
 	non-removable;
 	fixed-emmc-driver-type = <1>;
+	full-pwr-cycle-in-suspend;
 	status = "okay";
 };
 
-- 
2.7.4

Re: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Add full-pwr-cycle-in-suspend property to do a graceful shutdown of
> the eMMC device in system suspend.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.

>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +

I assume we need this on other boards, too?
At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
implementation as Salvator-X(S) and Ebisu.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Add full-pwr-cycle-in-suspend property to do a graceful shutdown of
> > the eMMC device in system suspend.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v5.9.

Thanks!

> >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> 
> I assume we need this on other boards, too?
> At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> implementation as Salvator-X(S) and Ebisu.

I think so. And, I also thin ULCB+KF should not have this because
it doesn't support Suspend-to-RAM. But, what do you think?
JFYI, I could such environment if I added the property into ulcb.dtsi
and added "/delete-property/" into ulcb-kf.dtsi. 

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jul 17, 2020 at 7:44 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> > On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > Add full-pwr-cycle-in-suspend property to do a graceful shutdown of
> > > the eMMC device in system suspend.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > Thanks for your patch!
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in renesas-devel for v5.9.
>
> Thanks!
>
> > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> >
> > I assume we need this on other boards, too?
> > At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> > implementation as Salvator-X(S) and Ebisu.
>
> I think so. And, I also thin ULCB+KF should not have this because
> it doesn't support Suspend-to-RAM. But, what do you think?

How come ULCB+KF doesn't support s2ram?
Isn't KingFisher just an extension board for ULCB?
Does it require a firmware upgrade?

> JFYI, I could such environment if I added the property into ulcb.dtsi
> and added "/delete-property/" into ulcb-kf.dtsi.

Iff that's the case, that's a valid solution.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 4:13 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Jul 17, 2020 at 7:44 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> > > On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@renesas.com> wrote:
<snip>
> > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> > > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> > >
> > > I assume we need this on other boards, too?
> > > At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> > > implementation as Salvator-X(S) and Ebisu.
> >
> > I think so. And, I also thin ULCB+KF should not have this because
> > it doesn't support Suspend-to-RAM. But, what do you think?
> 
> How come ULCB+KF doesn't support s2ram?
> Isn't KingFisher just an extension board for ULCB?
> Does it require a firmware upgrade?

It's related to board design. If ULCB+KF, we must not use 5V AC to ULCB side
and must use 12V AC to KF side. And KF has power switch as SW6 for both ULCB+KF.
https://elinux.org/R-Car/Boards/Kingfisher

However, PMIC on ULCB cannot control the KF power unfortunately.
So, we have no chance to use s2ram on ULCB+KF...

> > JFYI, I could such environment if I added the property into ulcb.dtsi
> > and added "/delete-property/" into ulcb-kf.dtsi.
> 
> Iff that's the case, that's a valid solution.

I think that's a valid solution. But, what do you think?

Best regards,
Yoshihiro Shimoda

Re: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jul 17, 2020 at 10:29 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 4:13 PM
> > On Fri, Jul 17, 2020 at 7:44 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> > > > On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> <snip>
> > > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> > > > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> > > >
> > > > I assume we need this on other boards, too?
> > > > At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> > > > implementation as Salvator-X(S) and Ebisu.
> > >
> > > I think so. And, I also thin ULCB+KF should not have this because
> > > it doesn't support Suspend-to-RAM. But, what do you think?
> >
> > How come ULCB+KF doesn't support s2ram?
> > Isn't KingFisher just an extension board for ULCB?
> > Does it require a firmware upgrade?
>
> It's related to board design. If ULCB+KF, we must not use 5V AC to ULCB side
> and must use 12V AC to KF side. And KF has power switch as SW6 for both ULCB+KF.
> https://elinux.org/R-Car/Boards/Kingfisher

OK.

> However, PMIC on ULCB cannot control the KF power unfortunately.
> So, we have no chance to use s2ram on ULCB+KF...

Makes sense.
BTW, what happens if you try s2ram? I guess the system just wakes up again
immediately? Or does it crash?

> > > JFYI, I could such environment if I added the property into ulcb.dtsi
> > > and added "/delete-property/" into ulcb-kf.dtsi.
> >
> > Iff that's the case, that's a valid solution.
>
> I think that's a valid solution. But, what do you think?

Thanks, looks good to me.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 5:35 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Jul 17, 2020 at 10:29 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 4:13 PM
> > > On Fri, Jul 17, 2020 at 7:44 AM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> > > > > On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > <snip>
> > > > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> > > > > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> > > > >
> > > > > I assume we need this on other boards, too?
> > > > > At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> > > > > implementation as Salvator-X(S) and Ebisu.
> > > >
> > > > I think so. And, I also thin ULCB+KF should not have this because
> > > > it doesn't support Suspend-to-RAM. But, what do you think?
> > >
> > > How come ULCB+KF doesn't support s2ram?
> > > Isn't KingFisher just an extension board for ULCB?
> > > Does it require a firmware upgrade?
> >
> > It's related to board design. If ULCB+KF, we must not use 5V AC to ULCB side
> > and must use 12V AC to KF side. And KF has power switch as SW6 for both ULCB+KF.
> > https://elinux.org/R-Car/Boards/Kingfisher
> 
> OK.
> 
> > However, PMIC on ULCB cannot control the KF power unfortunately.
> > So, we have no chance to use s2ram on ULCB+KF...
> 
> Makes sense.
> BTW, what happens if you try s2ram? I guess the system just wakes up again
> immediately? Or does it crash?

It's frozen after s2ram. Some LEDs on ULCB is OFF. But, LEDs on KF is still ON.
So, I'm wondering if it's better to add the property on ULCB+KF too
because it seems ULCB is turned off. But, what do you think?

--- log ---
root@m3ulcb:~# echo N > /sys/module/printk/parameters/con... 
root@m3ulcb:~# echo N > /sys/module/printk/parameters/console_suspend
root@m3ulcb:~# echo mem > /sys/power/state
[  109.870240] PM: suspend entry (deep)
[  109.873993] Filesystems sync: 0.000 seconds
[  109.879371] Freezing user space processes ... (elapsed 0.001 seconds) done.
[  109.888121] OOM killer disabled.
[  109.891358] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[  109.929977] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[  110.019772] ravb e6800000.ethernet eth0: Link is Down
[  110.222978] Disabling non-boot CPUs ...
[  110.227379] CPU1: shutdown
[  110.230108] psci: CPU1 killed (polled 0 ms)
[  110.236452] CPU2: shutdown
[  110.239216] psci: CPU2 killed (polled 0 ms)
[  110.245304] CPU3: shutdown
[  110.248033] psci: CPU3 killed (polled 0 ms)
[  110.253642] CPU4: shutdown
[  110.256376] psci: CPU4 killed (polled 0 ms)
[  110.261872] CPU5: shutdown
[  110.264638] psci: CPU5 killed (polled 0 ms) 
---

Best regards,
Yoshihiro Shimoda

Re: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jul 17, 2020 at 12:02 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 5:35 PM
> > On Fri, Jul 17, 2020 at 10:29 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 4:13 PM
> > > > On Fri, Jul 17, 2020 at 7:44 AM Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > > From: Geert Uytterhoeven, Sent: Wednesday, July 15, 2020 6:05 PM
> > > > > > On Fri, Jul 10, 2020 at 2:03 PM Yoshihiro Shimoda
> > > > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > <snip>
> > > > > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts   | 1 +
> > > > > > >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
> > > > > >
> > > > > > I assume we need this on other boards, too?
> > > > > > At least ULCB uses the BD9571 PMIC, and has a similar PSCI s2ram
> > > > > > implementation as Salvator-X(S) and Ebisu.
> > > > >
> > > > > I think so. And, I also thin ULCB+KF should not have this because
> > > > > it doesn't support Suspend-to-RAM. But, what do you think?
> > > >
> > > > How come ULCB+KF doesn't support s2ram?
> > > > Isn't KingFisher just an extension board for ULCB?
> > > > Does it require a firmware upgrade?
> > >
> > > It's related to board design. If ULCB+KF, we must not use 5V AC to ULCB side
> > > and must use 12V AC to KF side. And KF has power switch as SW6 for both ULCB+KF.
> > > https://elinux.org/R-Car/Boards/Kingfisher
> >
> > OK.
> >
> > > However, PMIC on ULCB cannot control the KF power unfortunately.
> > > So, we have no chance to use s2ram on ULCB+KF...
> >
> > Makes sense.
> > BTW, what happens if you try s2ram? I guess the system just wakes up again
> > immediately? Or does it crash?
>
> It's frozen after s2ram. Some LEDs on ULCB is OFF. But, LEDs on KF is still ON.

Oops.

> So, I'm wondering if it's better to add the property on ULCB+KF too
> because it seems ULCB is turned off. But, what do you think?

IMHO, a graceful shutdown is better.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] arm64: dts: renesas: add full-pwr-cycle-in-suspend into eMMC nodes

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, July 17, 2020 7:07 PM
<snip>
> > > > However, PMIC on ULCB cannot control the KF power unfortunately.
> > > > So, we have no chance to use s2ram on ULCB+KF...
> > >
> > > Makes sense.
> > > BTW, what happens if you try s2ram? I guess the system just wakes up again
> > > immediately? Or does it crash?
> >
> > It's frozen after s2ram. Some LEDs on ULCB is OFF. But, LEDs on KF is still ON.
> 
> Oops.
> 
> > So, I'm wondering if it's better to add the property on ULCB+KF too
> > because it seems ULCB is turned off. But, what do you think?
> 
> IMHO, a graceful shutdown is better.

Thank you for your opinion. I think so. So, I'll submit a patch.

Best regards,
Yoshihiro Shimoda