[PATCH/RFC v2 0/2] treewide: fix _mmc_suspend() on PSCI

[PATCH/RFC v2 0/2] treewide: fix _mmc_suspend() on PSCI

[Yoshihiro Shimoda]

We can detect whether the system is going to suspend by using
pm_suspend_via_firmware() API. So, this patch series update
both PSCI and MMC driver for my environment (r8a77951-salvator-xs).

We can apply these patches to each subsystem. I believe this is
a correct way, but I still mark RFC now...

Changes from v1:
 - Use pm_suspend_via_firmware() API instead of pm_suspend_target_state.
 - Modify the psci driver to call pm_set_suspend_via_firmware.
 https://patchwork.kernel.org/patch/11557505/

Yoshihiro Shimoda (2):
  firmware: psci: call pm_set_suspend_via_firmware()
  mmc: core: Call mmc_power_off_nofity() if pm_suspend_via_firmware()

 drivers/firmware/psci/psci.c | 8 ++++++++
 drivers/mmc/core/mmc.c       | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH/RFC v2 1/2] firmware: psci: call pm_set_suspend_via_firmware()

[Yoshihiro Shimoda]

Call pm_set_suspend_via_firmware() in .begin() of psci_suspend_ops
to use pm_suspend_via_firmware() on PSCI environment.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/firmware/psci/psci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 92013ec..1c22b01 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -309,6 +309,13 @@ static int psci_system_suspend(unsigned long unused)
 			      __pa_symbol(cpu_resume), 0, 0);
 }
 
+static int psci_system_suspend_begin(suspend_state_t state)
+{
+	pm_set_suspend_via_firmware();
+
+	return 0;
+}
+
 static int psci_system_suspend_enter(suspend_state_t state)
 {
 	return cpu_suspend(0, psci_system_suspend);
@@ -316,6 +323,7 @@ static int psci_system_suspend_enter(suspend_state_t state)
 
 static const struct platform_suspend_ops psci_suspend_ops = {
 	.valid          = suspend_valid_only_mem,
+	.begin          = psci_system_suspend_begin,
 	.enter          = psci_system_suspend_enter,
 };
 
-- 
2.7.4

[PATCH/RFC v2 2/2] mmc: core: Call mmc_power_off_nofity() if pm_suspend_via_firmware()

[Yoshihiro Shimoda]

If pm_suspend_via_firmware() returns true, the system will be able
to cut both vcc and vccq in the suspend. So, call
mmc_power_off_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.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/core/mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 4203303..81941fd 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/stat.h>
+#include <linux/suspend.h>
 #include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
@@ -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()))
 		err = mmc_poweroff_notify(host->card, notify_type);
 	else if (mmc_can_sleep(host->card))
 		err = mmc_sleep(host);
-- 
2.7.4

Re: [PATCH/RFC v2 2/2] mmc: core: Call mmc_power_off_nofity() if pm_suspend_via_firmware()

[Sergei Shtylyov]

Hello!

On 19.06.2020 14:29, Yoshihiro Shimoda 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_power_off_nofity() if pm_suspend_via_firmware() returns true.

    mmc_poweroff_notify()? :-)

> 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.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>   drivers/mmc/core/mmc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 4203303..81941fd 100644
> --- 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()))
>   		err = mmc_poweroff_notify(host->card, notify_type);
>   	else if (mmc_can_sleep(host->card))
>   		err = mmc_sleep(host);
> 

MBR, Sergei

Re: [PATCH/RFC v2 2/2] mmc: core: Call mmc_power_off_nofity() if pm_suspend_via_firmware()

[Sergei Shtylyov]

Hello!

On 19.06.2020 14:29, Yoshihiro Shimoda 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_power_off_nofity() if pm_suspend_via_firmware() returns true.

    mmc_poweroff_notify()? :-)

> 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.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>   drivers/mmc/core/mmc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 4203303..81941fd 100644
> --- 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()))
>   		err = mmc_poweroff_notify(host->card, notify_type);
>   	else if (mmc_can_sleep(host->card))
>   		err = mmc_sleep(host);
> 

MBR, Sergei

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH/RFC v2 0/2] treewide: fix _mmc_suspend() on PSCI

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jun 19, 2020 at 2:42 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> We can detect whether the system is going to suspend by using
> pm_suspend_via_firmware() API. So, this patch series update
> both PSCI and MMC driver for my environment (r8a77951-salvator-xs).

Cool, didn't know about pm_set_.*_via_firmware().

Looks like this can be used in the clock and pin control drivers, too.
Currently they're checking for !psci_ops.cpu_suspend.

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/RFC v2 0/2] treewide: fix _mmc_suspend() on PSCI

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Jun 19, 2020 at 2:42 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> We can detect whether the system is going to suspend by using
> pm_suspend_via_firmware() API. So, this patch series update
> both PSCI and MMC driver for my environment (r8a77951-salvator-xs).

Cool, didn't know about pm_set_.*_via_firmware().

Looks like this can be used in the clock and pin control drivers, too.
Currently they're checking for !psci_ops.cpu_suspend.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH/RFC v2 2/2] mmc: core: Call mmc_power_off_nofity() if pm_suspend_via_firmware()

[Yoshihiro Shimoda]

Hello!

Thank you for your review!

> From: Sergei Shtylyov, Sent: Friday, June 19, 2020 10:41 PM
> 
> Hello!
> 
> On 19.06.2020 14:29, Yoshihiro Shimoda 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_power_off_nofity() if pm_suspend_via_firmware() returns true.
> 
>     mmc_poweroff_notify()? :-)

Oops! I'll fix it.

Best regards,
Yoshihiro Shimoda

> > 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.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >   drivers/mmc/core/mmc.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 4203303..81941fd 100644
> > --- 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()))
> >   		err = mmc_poweroff_notify(host->card, notify_type);
> >   	else if (mmc_can_sleep(host->card))
> >   		err = mmc_sleep(host);
> >
> 
> MBR, Sergei

Re: [PATCH/RFC v2 1/2] firmware: psci: call pm_set_suspend_via_firmware()

[Sudeep Holla]

(This patch never made it to my inbox, may be due to ALKML issues on that day)

On Fri, Jun 19, 2020 at 08:29:02PM +0900, Yoshihiro Shimoda wrote:
> Call pm_set_suspend_via_firmware() in .begin() of psci_suspend_ops
> to use pm_suspend_via_firmware() on PSCI environment.
>

I don't have issues with this change as such, but I need to understand
how and why this is used. I will comment on that separately.

--
Regards,
Sudeep

Re: [PATCH/RFC v2 1/2] firmware: psci: call pm_set_suspend_via_firmware()

[Sudeep Holla]

(This patch never made it to my inbox, may be due to ALKML issues on that day)

On Fri, Jun 19, 2020 at 08:29:02PM +0900, Yoshihiro Shimoda wrote:
> Call pm_set_suspend_via_firmware() in .begin() of psci_suspend_ops
> to use pm_suspend_via_firmware() on PSCI environment.
>

I don't have issues with this change as such, but I need to understand
how and why this is used. I will comment on that separately.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH/RFC v2 1/2] firmware: psci: call pm_set_suspend_via_firmware()

[Yoshihiro Shimoda]

Hi Sudeep,

> From: Sudeep Holla, Sent: Monday, June 29, 2020 11:33 PM
> 
> (This patch never made it to my inbox, may be due to ALKML issues on that day)
> 
> On Fri, Jun 19, 2020 at 08:29:02PM +0900, Yoshihiro Shimoda wrote:
> > Call pm_set_suspend_via_firmware() in .begin() of psci_suspend_ops
> > to use pm_suspend_via_firmware() on PSCI environment.
> >
> 
> I don't have issues with this change as such, but I need to understand
> how and why this is used. I will comment on that separately.

I thought MMC driver could use this API to detect whether the firmware was suspending or not.
However, Ulf said this was not a good way because all implementation of PSCI
will not turn the power off while system suspend [1]. Also, as you and Mark said on other
email thread [2], the current PSCI specification cannot inform system suspend state
to OS (Linux).

So, I think I should recall this patch because this cannot get expected information.


[1]
https://lore.kernel.org/linux-renesas-soc/CAPDyKFq-dEPaU094hrk2xg18VpJAsbnf8enieFmcMhKiB1bW1A@mail.gmail.com/


https://lore.kernel.org/linux-renesas-soc/CAMuHMdXh1Hh6H35Mp8hBJBykUucRNwxTmW+U9SHM+xhrcwasDA@mail.gmail.com/T/#m442a2ce972cfdb3ff33637c120c8d096e4d07af8

Best regards,
Yoshihiro Shimoda

> --
> Regards,
> Sudeep

RE: [PATCH/RFC v2 1/2] firmware: psci: call pm_set_suspend_via_firmware()

[Yoshihiro Shimoda]

Hi Sudeep,

> From: Sudeep Holla, Sent: Monday, June 29, 2020 11:33 PM
> 
> (This patch never made it to my inbox, may be due to ALKML issues on that day)
> 
> On Fri, Jun 19, 2020 at 08:29:02PM +0900, Yoshihiro Shimoda wrote:
> > Call pm_set_suspend_via_firmware() in .begin() of psci_suspend_ops
> > to use pm_suspend_via_firmware() on PSCI environment.
> >
> 
> I don't have issues with this change as such, but I need to understand
> how and why this is used. I will comment on that separately.

I thought MMC driver could use this API to detect whether the firmware was suspending or not.
However, Ulf said this was not a good way because all implementation of PSCI
will not turn the power off while system suspend [1]. Also, as you and Mark said on other
email thread [2], the current PSCI specification cannot inform system suspend state
to OS (Linux).

So, I think I should recall this patch because this cannot get expected information.


[1]
https://lore.kernel.org/linux-renesas-soc/CAPDyKFq-dEPaU094hrk2xg18VpJAsbnf8enieFmcMhKiB1bW1A@mail.gmail.com/


https://lore.kernel.org/linux-renesas-soc/CAMuHMdXh1Hh6H35Mp8hBJBykUucRNwxTmW+U9SHM+xhrcwasDA@mail.gmail.com/T/#m442a2ce972cfdb3ff33637c120c8d096e4d07af8

Best regards,
Yoshihiro Shimoda

> --
> Regards,
> Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel