Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751104AbdFTEzf (ORCPT ); Tue, 20 Jun 2017 00:55:35 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:43445 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751007AbdFTEzd (ORCPT ); Tue, 20 Jun 2017 00:55:33 -0400 Subject: Re: [PATCH v5 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set To: Leonard Crestez , Oleksij Rempel References: <20170619050245.2632-1-o.rempel@pengutronix.de> <20170619050245.2632-3-o.rempel@pengutronix.de> <1497872133.5380.11.camel@nxp.com> Cc: Andrew Morton , devicetree@vger.kernel.org, Fabio Estevam , kernel@pengutronix.de, Liam Girdwood , linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, Mark Brown , Mark Rutland , Michael Turquette , Rob Herring , Russell King , Shawn Guo , Stephen Boyd From: Oleksij Rempel Message-ID: <7f2db0a8-a316-5ecb-24bf-46bcb625e123@pengutronix.de> Date: Tue, 20 Jun 2017 06:55:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1497872133.5380.11.camel@nxp.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:67c:670:100:3ad5:47ff:feaf:13da X-SA-Exim-Mail-From: ore@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1953 Lines: 55 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 >> external >> 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. >> >> See: >> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww >> .nxp.com%2Fassets%2Fdocuments%2Fdata%2Fen%2Freference- >> manuals%2FIMX6DQRM.pdf&data=01%7C01%7Cleonard.crestez%40nxp.com%7C32c >> df19d0ab44ed8e24d08d4b6d097f4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0& >> sdata=CWDpnEGikPGmSTM3dQH8IMxQCVzcIvHL69nu962d8M0%3D&reserved=0 >> page 5083 >> >> This patch implements step 4. of this sequence. >> >> Signed-off-by: Oleksij Rempel >> --- >> 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- >> imx6.c >> 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 >> IMX6Q_GPR1_GINT); >> } >> >> +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. ok.