Re: [PATCH v5 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set

From: Oleksij Rempel
Date: Tue Jun 20 2017 - 00:55:49 EST

Hi Leonard,

On 19.06.2017 13:35, Leonard Crestez wrote:
On Mon, 2017-06-19 at 07:02 +0200, Oleksij Rempel wrote:
One of the Freescale recommended sequences for power off with
PMIC is the following:
3. SoC is programming PMIC for power off when standby is asserted.
4. In CCM STOP mode, Standby is asserted, PMIC gates SoC supplies.

page 5083

This patch implements step 4. of this sequence.

Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
arch/arm/mach-imx/pm-imx6.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-
index e61b1d1027e1..cb76832935f0 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -602,6 +602,28 @@ static void __init imx6_pm_common_init(const
struct imx6_pm_socdata

+static void imx6_pm_poweroff(void)
+ imx6_set_lpm(STOP_POWER_OFF);
+ cpu_suspend(0, imx6q_suspend_finish);

It's a bit strange that you're using parts of the suspend code for
this. Perhaps you could just write your bits to CLPCR and then execute
a WFI?

Well, every thing in this patch set, yell about using standby/suspend path in HW for power off. Even iMX code use STOP_POWER_OFF for configuring this regs. How can it be more strange? :) Should i really duplicate code to hide this?

It would also be nice to rename this to something like
imx6_pm_stby_poweroff to make it clear that it's only for a certain
power off path.