[PATCH v2 0/6] iMX6: Implement power off over standby signal

[PATCH v2 0/6] iMX6: Implement power off over standby signal

[Oleksij Rempel]

v2:
 - export pm_power_off_prepare to fix pfuze100-regulator as module compilation.

Oleksij Rempel (6):
  ARM: imx6q: provide documentation for new fsl,pmic_stby_poweroff
    property
  ARM: imx6: register pm_power_off handler if "fsl,pmic_stby_poweroff"
    is set
  kernel/reboot.c: export pm_power_off_prepare
  regulator: pfuze100: add fsl,pmic_stby_poweroff property
  regulator: pfuze100-regulator: provide pm_power_off_prepare handler
  ARM: dts: imx6: RIoTboard provide Standby to PowerOff option

 .../devicetree/bindings/clock/imx6q-clock.txt      |  4 ++
 .../devicetree/bindings/regulator/pfuze100.txt     |  4 ++
 arch/arm/boot/dts/imx6dl-riotboard.dts             |  5 ++
 arch/arm/mach-imx/pm-imx6.c                        | 30 +++++++++
 drivers/regulator/pfuze100-regulator.c             | 76 ++++++++++++++++++++++
 kernel/reboot.c                                    |  1 +
 6 files changed, 120 insertions(+)

Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-clk@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

-- 
2.11.0

[PATCH v2 1/6] ARM: imx6q: provide documentation for new fsl,pmic_stby_poweroff property

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-clk@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/clock/imx6q-clock.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
index 9252912a5b0e..214cb5f461c7 100644
--- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -6,6 +6,10 @@ Required properties:
 - interrupts: Should contain CCM interrupt
 - #clock-cells: Should be <1>
 
+Optional properties:
+- fsl,pmic_stby_poweroff: Configure CCM to assert PMIC_STBY_REQ signal
+  on power off.
+
 The clock consumer should specify the desired clock by having the clock
 ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
 for the full list of i.MX6 Quad and DualLite clock IDs.
-- 
2.11.0

[PATCH v2 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic_stby_poweroff" is set

[Oleksij Rempel]

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:
http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf
page 5083

This patch implements step 4. of this sequence.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-clk@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/mach-imx/pm-imx6.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index e61b1d1027e1..fd7b95f340e5 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -27,6 +27,8 @@
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>
 #include <asm/tlb.h>
+#include <linux/kallsyms.h>
+#include <linux/regulator/machine.h>
 
 #include "common.h"
 #include "hardware.h"
@@ -602,6 +604,31 @@ 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);
+
+	mdelay(1000);
+
+	pr_emerg("Unable to poweroff system\n");
+}
+
+static int imx6_pm_poweroff_probe(void)
+{
+	char symname[KSYM_NAME_LEN];
+
+	if (pm_power_off) {
+		lookup_symbol_name((ulong)pm_power_off, symname);
+		pr_warn("%s: pm_power_off already claimed  %p %s!\n",
+			__func__, pm_power_off, symname);
+		return -EBUSY;
+	}
+
+	pm_power_off = imx6_pm_poweroff;
+	return 0;
+}
+
 void __init imx6_pm_ccm_init(const char *ccm_compat)
 {
 	struct device_node *np;
@@ -618,6 +645,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_poweroff_probe();
 }
 
 void __init imx6q_pm_init(void)
-- 
2.11.0

[PATCH v2 3/6] kernel/reboot.c: export pm_power_off_prepare

[Oleksij Rempel]

we need it to preconfigure PMIC to properly power off the system.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 kernel/reboot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index bd30a973fe94..a6903bf772c7 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -49,6 +49,7 @@ int reboot_force;
  */
 
 void (*pm_power_off_prepare)(void);
+EXPORT_SYMBOL(pm_power_off_prepare);
 
 /**
  *	emergency_restart - reboot the system
-- 
2.11.0

[PATCH v2 4/6] regulator: pfuze100: add fsl,pmic_stby_poweroff property

[Oleksij Rempel]

Document the new optional "fsl,pmic_stby_poweroff" property.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/regulator/pfuze100.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 444c47831a40..216519c4a50d 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,10 @@ Required properties:
 - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000"
 - reg: I2C slave address
 
+Optional properties:
+- fsl,pmic_stby_poweroff: if present, configure the PMIC to shutdown all
+  power rails when PMIC_STBY_REQ line is asserted during the power off sequence.
+
 Required child node:
 - regulators: This is the list of child nodes that specify the regulator
   initialization data for defined regulators. Please refer to below doc
-- 
2.11.0

[PATCH v2 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

[Oleksij Rempel]

On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
about state changes. In this case internal state of PMIC must be
preconfigured for upcomming state change.
It works fine with the current regulator framework, except with the
power-off case.

This patch is providing an optional pm_power_off_prepare handler
which will configure the PMIC_StandBy state to disable all power lines.

In my power consumption test on RIoTBoard, I got the following results:
poweroff without this patch:	320 mA
poweroff with this patch:	2   mA
suspend to ram:			40  mA

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 drivers/regulator/pfuze100-regulator.c | 76 ++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index 63922a2167e5..505852dd38f5 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -28,6 +28,7 @@
 #include <linux/regulator/pfuze100.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/kallsyms.h>
 #include <linux/regmap.h>
 
 #define PFUZE_NUMREGS		128
@@ -42,11 +43,17 @@
 
 #define PFUZE100_COINVOL	0x1a
 #define PFUZE100_SW1ABVOL	0x20
+#define PFUZE100_SW1ABMODE	0x23
 #define PFUZE100_SW1CVOL	0x2e
+#define PFUZE100_SW1CMODE	0x31
 #define PFUZE100_SW2VOL		0x35
+#define PFUZE100_SW2MODE	0x38
 #define PFUZE100_SW3AVOL	0x3c
+#define PFUZE100_SW3AMODE	0x3f
 #define PFUZE100_SW3BVOL	0x43
+#define PFUZE100_SW3BMODE	0x46
 #define PFUZE100_SW4VOL		0x4a
+#define PFUZE100_SW4MODE	0x4d
 #define PFUZE100_SWBSTCON1	0x66
 #define PFUZE100_VREFDDRCON	0x6a
 #define PFUZE100_VSNVSVOL	0x6b
@@ -57,6 +64,13 @@
 #define PFUZE100_VGEN5VOL	0x70
 #define PFUZE100_VGEN6VOL	0x71
 
+#define PFUZE100_SWxMODE_MASK	0xf
+#define PFUZE100_SWxMODE_APS_APS	0x8
+#define PFUZE100_SWxMODE_APS_OFF	0x4
+
+#define PFUZE100_VGENxLPWR	BIT(6)
+#define PFUZE100_VGENxSTBY	BIT(5)
+
 enum chips { PFUZE100, PFUZE200, PFUZE3000 = 3 };
 
 struct pfuze_regulator {
@@ -489,6 +503,64 @@ static inline struct device_node *match_of_node(int index)
 }
 #endif
 
+static struct pfuze_chip *syspm_pfuze_chip;
+
+static void pfuze_poweroff_pre(void)
+{
+	dev_info(syspm_pfuze_chip->dev, "Configure standy mode for poweroff");
+
+	/* Switch from default mode: APS/APS to APS/Off */
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1ABMODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1CMODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW2MODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3AMODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3BMODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW4MODE,
+			   PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN1VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN2VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN3VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN4VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN5VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+	regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN6VOL,
+			   PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+			   PFUZE100_VGENxSTBY);
+}
+
+static int pfuze_poweroff_pre_init(struct pfuze_chip *pfuze_chip)
+{
+	if (pfuze_chip->chip_id != PFUZE100) {
+		dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supoorted chip\n");
+		return -ENODEV;
+	}
+
+	if (pm_power_off_prepare) {
+		dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registred.\n");
+		return -EBUSY;
+	}
+
+	syspm_pfuze_chip = pfuze_chip;
+	pm_power_off_prepare = pfuze_poweroff_pre;
+
+	return 0;
+}
+
 static int pfuze_identify(struct pfuze_chip *pfuze_chip)
 {
 	unsigned int value;
@@ -659,6 +731,10 @@ static int pfuze100_regulator_probe(struct i2c_client *client,
 		}
 	}
 
+	if (of_property_read_bool(client->dev.of_node,
+				  "fsl,pmic_stby_poweroff"))
+		return pfuze_poweroff_pre_init(pfuze_chip);
+
 	return 0;
 }
 
-- 
2.11.0

[PATCH v2 6/6] ARM: dts: imx6: RIoTboard provide Standby to PowerOff option

[Oleksij Rempel]

This board, as well as some other boards with i.MX6 and a PMIC, uses a
"MPIC_STBY_REQ" line to notify the PMIC about a state change.
The PMIC is programmed for a specific state change before triggering the
line.
In this case, PMIC_STBY_REQ can be used for Stand by, Sleep
and Power off modes.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: kernel@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/imx6dl-riotboard.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 2cb72824e800..b5f25fe7b809 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -90,6 +90,10 @@
 	status = "okay";
 };
 
+&clks {
+	fsl,pmic_stby_poweroff;
+};
+
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
@@ -125,6 +129,7 @@
 		reg = <0x08>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <16 8>;
+		fsl,pmic_stby_poweroff;
 
 		regulators {
 			reg_vddcore: sw1ab {				/* VDDARM_IN */
-- 
2.11.0

Re: [PATCH v2 4/6] regulator: pfuze100: add fsl,pmic_stby_poweroff property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

On Wed, May 31, 2017 at 08:14:55AM +0200, Oleksij Rempel wrote:
> Document the new optional "fsl,pmic_stby_poweroff" property.

I'm missing both the cover letter and the rest of the series, what's
going on with dependencies here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/6] ARM: imx6q: provide documentation for new fsl,pmic_stby_poweroff property

[Mark Rutland]

On Wed, May 31, 2017 at 08:14:52AM +0200, Oleksij Rempel wrote:
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Cc: kernel@pengutronix.de
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/clock/imx6q-clock.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> index 9252912a5b0e..214cb5f461c7 100644
> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> @@ -6,6 +6,10 @@ Required properties:
>  - interrupts: Should contain CCM interrupt
>  - #clock-cells: Should be <1>
>  
> +Optional properties:
> +- fsl,pmic_stby_poweroff: Configure CCM to assert PMIC_STBY_REQ signal
> +  on power off.

s/_/-/ in property names, please.

When/why is this needed?

Why should a DT author (not) set this?

Thanks,
Mark.

> +
>  The clock consumer should specify the desired clock by having the clock
>  ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
>  for the full list of i.MX6 Quad and DualLite clock IDs.
> -- 
> 2.11.0
> 

Re: [PATCH v2 4/6] regulator: pfuze100: add fsl,pmic_stby_poweroff property

[Oleksij Rempel]

On Wed, May 31, 2017 at 03:44:38PM +0100, Mark Brown wrote:
> On Wed, May 31, 2017 at 08:14:55AM +0200, Oleksij Rempel wrote:
> > Document the new optional "fsl,pmic_stby_poweroff" property.
> 
> I'm missing both the cover letter and the rest of the series, what's
> going on with dependencies here?

Hmm... I probably needed to add same list of maintainers to all patches.
I will fix it next time. The patches are here:
https://patchwork.kernel.org/bundle/olerem/poweroff/

Should I resend it with complete Cc list?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic_stby_poweroff" is set

[Mark Rutland]

On Wed, May 31, 2017 at 08:14:53AM +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:
> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf
> page 5083

> +static int imx6_pm_poweroff_probe(void)
> +{
> +	char symname[KSYM_NAME_LEN];
> +
> +	if (pm_power_off) {
> +		lookup_symbol_name((ulong)pm_power_off, symname);
> +		pr_warn("%s: pm_power_off already claimed  %p %s!\n",
> +			__func__, pm_power_off, symname);

Please use %pf rather than lookup_symbol_name*(,

> +		return -EBUSY;
> +	}
> +
> +	pm_power_off = imx6_pm_poweroff;
> +	return 0;
> +}
> +
>  void __init imx6_pm_ccm_init(const char *ccm_compat)
>  {
>  	struct device_node *np;
> @@ -618,6 +645,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_poweroff_probe();

This neesd s/_/-/, as commented on the binding.

My what/why query on the binding stadns, regardless.

Thanks,
Mark.

Re: [PATCH v2 4/6] regulator: pfuze100: add fsl,pmic_stby_poweroff property

[Mark Rutland]

On Wed, May 31, 2017 at 08:14:55AM +0200, Oleksij Rempel wrote:
> Document the new optional "fsl,pmic_stby_poweroff" property.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Cc: kernel@pengutronix.de
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/regulator/pfuze100.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt
> index 444c47831a40..216519c4a50d 100644
> --- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
> +++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
> @@ -4,6 +4,10 @@ Required properties:
>  - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000"
>  - reg: I2C slave address
>  
> +Optional properties:
> +- fsl,pmic_stby_poweroff: if present, configure the PMIC to shutdown all
> +  power rails when PMIC_STBY_REQ line is asserted during the power off sequence.

As with patch 1, s/_/-/ please.

Please also explain why/why this is needed.

Thanks,
Mark.

Re: [PATCH v2 4/6] regulator: pfuze100: add fsl,pmic_stby_poweroff property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Wed, May 31, 2017 at 04:55:36PM +0200, Oleksij Rempel wrote:
> On Wed, May 31, 2017 at 03:44:38PM +0100, Mark Brown wrote:

> > I'm missing both the cover letter and the rest of the series, what's
> > going on with dependencies here?

> Hmm... I probably needed to add same list of maintainers to all patches.
> I will fix it next time. The patches are here:
> https://patchwork.kernel.org/bundle/olerem/poweroff/

> Should I resend it with complete Cc list?

Like I say from my point of view I need to know what the dependencies
are somehow.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 3/6] kernel/reboot.c: export pm_power_off_prepare

[Andrew Morton]

On Wed, 31 May 2017 08:14:54 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> we need it to preconfigure PMIC to properly power off the system.

Who is "we", what is a "PMIC", what architecture is affected and where
is this code which references pm_power_off_prepare?

> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -49,6 +49,7 @@ int reboot_force;
>   */
>  
>  void (*pm_power_off_prepare)(void);
> +EXPORT_SYMBOL(pm_power_off_prepare);
>  
>  /**
>   *	emergency_restart - reboot the system
> -- 
> 2.11.0

Re: [PATCH v2 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

[Andrew Morton]

On Wed, 31 May 2017 08:14:56 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
> about state changes. In this case internal state of PMIC must be
> preconfigured for upcomming state change.
> It works fine with the current regulator framework, except with the
> power-off case.
> 
> This patch is providing an optional pm_power_off_prepare handler
> which will configure the PMIC_StandBy state to disable all power lines.
> 
> In my power consumption test on RIoTBoard, I got the following results:
> poweroff without this patch:	320 mA
> poweroff with this patch:	2   mA
> suspend to ram:			40  mA
> 
> ...
>
> +static int pfuze_poweroff_pre_init(struct pfuze_chip *pfuze_chip)
> +{
> +	if (pfuze_chip->chip_id != PFUZE100) {
> +		dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supoorted chip\n");
> +		return -ENODEV;
> +	}
> +
> +	if (pm_power_off_prepare) {
> +		dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registred.\n");
> +		return -EBUSY;
> +	}
> +
> +	syspm_pfuze_chip = pfuze_chip;
> +	pm_power_off_prepare = pfuze_poweroff_pre;
> +
> +	return 0;
> +}
> +

Ah, there it is.

This looks a bit dodgy.  What happens after someone does rmmod on this
driver?

(typo in comment: "supoorted")

(I wish we could get "poweroff" and "power_off" consistent)

Re: [PATCH v2 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 1688 bytes --]

Am 31.05.2017 um 22:59 schrieb Andrew Morton:
> On Wed, 31 May 2017 08:14:56 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
>> On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
>> about state changes. In this case internal state of PMIC must be
>> preconfigured for upcomming state change.
>> It works fine with the current regulator framework, except with the
>> power-off case.
>>
>> This patch is providing an optional pm_power_off_prepare handler
>> which will configure the PMIC_StandBy state to disable all power lines.
>>
>> In my power consumption test on RIoTBoard, I got the following results:
>> poweroff without this patch:	320 mA
>> poweroff with this patch:	2   mA
>> suspend to ram:			40  mA
>>
>> ...
>>
>> +static int pfuze_poweroff_pre_init(struct pfuze_chip *pfuze_chip)
>> +{
>> +	if (pfuze_chip->chip_id != PFUZE100) {
>> +		dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supoorted chip\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	if (pm_power_off_prepare) {
>> +		dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registred.\n");
>> +		return -EBUSY;
>> +	}
>> +
>> +	syspm_pfuze_chip = pfuze_chip;
>> +	pm_power_off_prepare = pfuze_poweroff_pre;
>> +
>> +	return 0;
>> +}
>> +
> 
> Ah, there it is.
> 
> This looks a bit dodgy.  What happens after someone does rmmod on this
> driver?

ok, got it.

> (typo in comment: "supoorted")
> 
> (I wish we could get "poweroff" and "power_off" consistent)

Hm.. only in this case or in DT bindings too? Which variant should be
used? Driver: *_power_off_*; DT: *-power-off-* ?

-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [PATCH v2 1/6] ARM: imx6q: provide documentation for new fsl,pmic_stby_poweroff property

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 2140 bytes --]

Am 31.05.2017 um 16:52 schrieb Mark Rutland:
> On Wed, May 31, 2017 at 08:14:52AM +0200, Oleksij Rempel wrote:
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> Cc: kernel@pengutronix.de
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: Michael Turquette <mturquette@baylibre.com>
>> Cc: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: linux-clk@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  Documentation/devicetree/bindings/clock/imx6q-clock.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> index 9252912a5b0e..214cb5f461c7 100644
>> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> @@ -6,6 +6,10 @@ Required properties:
>>  - interrupts: Should contain CCM interrupt
>>  - #clock-cells: Should be <1>
>>  
>> +Optional properties:
>> +- fsl,pmic_stby_poweroff: Configure CCM to assert PMIC_STBY_REQ signal
>> +  on power off.
> 
> s/_/-/ in property names, please.

ok.

I have now some more questions:
what naming schema should be actually used?
fsl,pmic-stby-poweroff
or
fsl,pmic-stby-power-off

i found some existing case:
sound/rt5660.txt:- realtek,poweroff-in-suspend
so in my case it will be:
fsl,standby-in-poweroff
or
fsl,standby-in-power-off

can we have generic name for this use case?
should I use same binding for PMIC and the SoC or they should be more like:
for SoC "standby-in-power-off"
and for PMIC "power-off-in-standby"



> When/why is this needed?
> 
> Why should a DT author (not) set this?
> 
> Thanks,
> Mark.
> 
>> +
>>  The clock consumer should specify the desired clock by having the clock
>>  ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
>>  for the full list of i.MX6 Quad and DualLite clock IDs.
>> -- 
>> 2.11.0
>>


-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]