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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1497969136.3177.10.camel@nxp.com>
Date:   Tue, 20 Jun 2017 17:32:16 +0300
From:   Leonard Crestez <leonard.crestez@....com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        <devicetree@...r.kernel.org>,
        Fabio Estevam <fabio.estevam@....com>, <kernel@...gutronix.de>,
        Liam Girdwood <lgirdwood@...il.com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "Michael Turquette" <mturquette@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Shawn Guo <shawnguo@...nel.org>,
        Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: [PATCH v5 2/6] ARM: imx6: register pm_power_off handler if
 "fsl,pmic-stby-poweroff" is set

On Tue, 2017-06-20 at 06:55 +0200, Oleksij Rempel wrote:
> 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.
> > > 
> > > This patch implements step 4. of this sequence.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> > > ---
> > >  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?
> 
This code is used to set the power state the soc enters on WFI. These
function try to ensure that it is possible to come back up when woken
by an interrupt later. That part you don't want, right?

However I can't think of any obvious way this could misbehave so I
guess it's fine.