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

[Oleksij Rempel]

On 27.07.2018 11:15, Robin Gong wrote:
> 
> 
>> -----Original Message-----
>> From: Oleksij Rempel [mailto:o.rempel@xxxxxxxxxxxxxx]
>> Sent: 2018年7月26日 17:22
>> To: Shawn Guo <shawnguo@xxxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>;
>> Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> Cc: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>; kernel@xxxxxxxxxxxxxx;
>> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
>> linux-clk@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Andrew Morton
>> <akpm@xxxxxxxxxxxxxxxxxxxx>; Liam Girdwood <lgirdwood@xxxxxxxxx>;
>> Leonard Crestez <leonard.crestez@xxxxxxx>; Rob Herring
>> <robh+dt@xxxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; Michael
>> Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd
>> <sboyd@xxxxxxxxxxxxxx>; Fabio Estevam <fabio.estevam@xxxxxxx>; Russell
>> King <linux@xxxxxxxxxxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>; Robin Gong
>> <yibin.gong@xxxxxxx>; A.s. Dong <aisheng.dong@xxxxxxx>
>> Subject: [PATCH v8 2/6] ARM: imx6: register pm_power_off handler if
>> "fsl,pmic-stby-poweroff" is set
>>
>> 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%2FIM
>> X6DQRM.pdf&amp;data=02%7C01%7Cyibin.gong%40nxp.com%7C193fd19e3a
>> 40416ffa4a08d5f2d9583c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1
>> %7C636681937661914076&amp;sdata=lICAelYpUh4%2Ft%2Fs7N9mdk2cLQMi
>> cHcOqQ07vTOUoyNY%3D&amp;reserved=0
>> 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-imx6.c
>> index 017539dd712b..2f5c643f62fb 100644
>> --- a/arch/arm/mach-imx/pm-imx6.c
>> +++ b/arch/arm/mach-imx/pm-imx6.c
>> @@ -601,6 +601,28 @@ static void __init imx6_pm_common_init(const struct
>> imx6_pm_socdata
>>  				   IMX6Q_GPR1_GINT);
>>  }
>>
>> +static void imx6_pm_stby_poweroff(void) {
>> +	imx6_set_lpm(STOP_POWER_OFF);
>> +	imx6q_suspend_finish(0);
>> +
>> +	mdelay(1000);
>> +
>> +	pr_emerg("Unable to poweroff system\n"); }
>> +
>> +static int imx6_pm_stby_poweroff_probe(void) {
>> +	if (pm_power_off) {
>> +		pr_warn("%s: pm_power_off already claimed  %p %pf!\n",
>> +			__func__, pm_power_off, pm_power_off);

> 'syscon-poweroff' and 'pmic-stby-poweroff ' should be chosen as a single
> Poweroff way for any i.mx6 board. Why not delete directly 'syscon-poweroff' in dts 
> to avoid such two power off ways coexist?

pm_power_off can be registred by any part of the kernel. So, we need it
to avoid conflicts or at least to be able see them.

On other hand, you are right. syscon-poweroff should be disabled for
this board.

>> +		return -EBUSY;
>> +	}
>> +
>> +	pm_power_off = imx6_pm_stby_poweroff;
>> +	return 0;
>> +}
>> +
>>  void __init imx6_pm_ccm_init(const char *ccm_compat)  {
>>  	struct device_node *np;
>> @@ -617,6 +639,9 @@ void __init imx6_pm_ccm_init(const char
>> *ccm_compat)
>>  	val = readl_relaxed(ccm_base + CLPCR);
>>  	val &= ~BM_CLPCR_LPM;
>>  	writel_relaxed(val, ccm_base + CLPCR);
>> +
>> +	if (of_property_read_bool(np, "fsl,pmic-stby-poweroff"))
>> +		imx6_pm_stby_poweroff_probe();
>>  }
>>
>>  void __init imx6q_pm_init(void)
>> --
>> 2.18.0
>

Attachment: signature.asc
Description: OpenPGP digital signature