| Message ID | 20180726092220.17250-1-o.rempel@pengutronix.de |
|---|---|
| Headers | show |
| Series | provide power off support for iMX6 with external PMIC | expand |
Hi Oleksij, Am 26.07.2018 um 11:22 schrieb Oleksij Rempel: > 2018.07.26: > v8 is a rebase against kernel v4.18-rc6. No other changes are made. > Added: linux-imx@nxp.com and yibin.gong@nxp.com to the CC. your patches won't apply to regulator-next or linux-next because "regulator: pfuze100: add pfuze3001 support" has been merged before. Since the pfuze3001 doesn't support standby, it would be nice to make the driver take care of this. Best regards Stefan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] > Sent: 2018年7月26日 17:22 > To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; > Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton > <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; > Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael > Turquette <mturquette@baylibre.com>; Stephen Boyd > <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell > King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong > <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> > 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&data=02%7C01%7Cyibin.gong%40nxp.com%7C193fd19e3a > 40416ffa4a08d5f2d9583c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1 > %7C636681937661914076&sdata=lICAelYpUh4%2Ft%2Fs7N9mdk2cLQMi > cHcOqQ07vTOUoyNY%3D&reserved=0 > page 5083 > > This patch implements step 4. of this sequence. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.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 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? > + 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
> -----Original Message----- > From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] > Sent: 2018年7月26日 17:22 > To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; > Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton > <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; > Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael > Turquette <mturquette@baylibre.com>; Stephen Boyd > <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell > King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong > <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> > Subject: [PATCH v8 6/6] ARM: dts: imx6: RIoTboard provide standby on power > off option > > This board, as well as some other boards with i.MX6 and a PMIC, uses a > "PMIC_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> > --- > 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 2e98c92adff7..a0e9753ee767 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; > +}; It's better remove the default "syscon-poweroff" power off node. > + > &fec { > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_enet>; > @@ -170,6 +174,7 @@ > reg = <0x08>; > interrupt-parent = <&gpio5>; > interrupts = <16 8>; > + fsl,pmic-stby-poweroff; > > regulators { > reg_vddcore: sw1ab { /* VDDARM_IN */ > -- > 2.18.0
On 27.07.2018 11:32, Robin Gong wrote: > > >> -----Original Message----- >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] >> Sent: 2018年7月26日 17:22 >> To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; >> Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton >> <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; >> Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael >> Turquette <mturquette@baylibre.com>; Stephen Boyd >> <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell >> King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong >> <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> >> Subject: [PATCH v8 5/6] regulator: pfuze100-regulator: provide >> pm_power_off_prepare handler >> >> 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 standby state of the PMIC to disable all power lines. >> >> In my power consumption test on RIoTBoard, I got the following results: >> power off without this patch: 320 mA >> power off with this patch: 2 mA >> suspend to ram: 40 mA >> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> --- >> drivers/regulator/pfuze100-regulator.c | 92 ++++++++++++++++++++++++++ >> 1 file changed, 92 insertions(+) >> >> diff --git a/drivers/regulator/pfuze100-regulator.c >> b/drivers/regulator/pfuze100-regulator.c >> index 8d9dbcc775ea..e386e9acb3f7 100644 >> --- a/drivers/regulator/pfuze100-regulator.c >> +++ b/drivers/regulator/pfuze100-regulator.c >> @@ -15,6 +15,7 @@ >> #include <linux/regulator/pfuze100.h> >> #include <linux/i2c.h> >> #include <linux/slab.h> >> +#include <linux/kallsyms.h> > Is it necessary? yes, for pm_power_off_prepare >> #include <linux/regmap.h> >> >> #define PFUZE_NUMREGS 128 >> @@ -29,11 +30,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 >> @@ -44,6 +51,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 { >> @@ -492,6 +506,69 @@ static inline struct device_node *match_of_node(int >> index) } #endif >> >> +static struct pfuze_chip *syspm_pfuze_chip; >> + >> +static void pfuze_power_off_prepare(void) >> + dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power >> +off"); > Add 'if (syspm_pfuze_chip ->chip_id == PFUZE100))' here is easy for extend > Support on pfuze200/3000.. in the feature. ok. >> + >> + /* 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_power_off_prepare_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 supported chip\n"); >> + return -ENODEV; >> + } >> + >> + if (pm_power_off_prepare) { >> + dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already >> registered.\n"); >> + return -EBUSY; >> + } >> + >> + if (syspm_pfuze_chip) { >> + dev_warn(pfuze_chip->dev, "syspm_pfuze_chip is already set.\n"); >> + return -EBUSY; >> + } >> + >> + syspm_pfuze_chip = pfuze_chip; >> + pm_power_off_prepare = pfuze_power_off_prepare; >> + >> + return 0; >> +} >> + >> static int pfuze_identify(struct pfuze_chip *pfuze_chip) { >> unsigned int value; >> @@ -661,6 +738,20 @@ static int pfuze100_regulator_probe(struct i2c_client >> *client, >> } >> } >> >> + if (of_property_read_bool(client->dev.of_node, >> + "fsl,pmic-stby-poweroff")) >> + return pfuze_power_off_prepare_init(pfuze_chip); >> + >> + return 0; >> +} >> + >> +static int pfuze100_regulator_remove(struct i2c_client *client) { >> + if (syspm_pfuze_chip) { >> + syspm_pfuze_chip = NULL; >> + pm_power_off_prepare = NULL; >> + } >> + >> return 0; >> } >> >> @@ -671,6 +762,7 @@ static struct i2c_driver pfuze_driver = { >> .of_match_table = pfuze_dt_ids, >> }, >> .probe = pfuze100_regulator_probe, >> + .remove = pfuze100_regulator_remove, >> }; >> module_i2c_driver(pfuze_driver); >> >> -- >> 2.18.0 >
On 27.07.2018 11:15, Robin Gong wrote: > > >> -----Original Message----- >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] >> Sent: 2018年7月26日 17:22 >> To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; >> Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton >> <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; >> Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael >> Turquette <mturquette@baylibre.com>; Stephen Boyd >> <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell >> King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong >> <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> >> 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&data=02%7C01%7Cyibin.gong%40nxp.com%7C193fd19e3a >> 40416ffa4a08d5f2d9583c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1 >> %7C636681937661914076&sdata=lICAelYpUh4%2Ft%2Fs7N9mdk2cLQMi >> cHcOqQ07vTOUoyNY%3D&reserved=0 >> page 5083 >> >> This patch implements step 4. of this sequence. >> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.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 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 >
On 27.07.2018 11:33, Robin Gong wrote: > > >> -----Original Message----- >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] >> Sent: 2018年7月26日 17:22 >> To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; >> Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton >> <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; >> Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael >> Turquette <mturquette@baylibre.com>; Stephen Boyd >> <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell >> King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong >> <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> >> Subject: [PATCH v8 6/6] ARM: dts: imx6: RIoTboard provide standby on power >> off option >> >> This board, as well as some other boards with i.MX6 and a PMIC, uses a >> "PMIC_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> >> --- >> 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 2e98c92adff7..a0e9753ee767 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; >> +}; > It's better remove the default "syscon-poweroff" power off node. ok >> &fec { >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_enet>; >> @@ -170,6 +174,7 @@ >> reg = <0x08>; >> interrupt-parent = <&gpio5>; >> interrupts = <16 8>; >> + fsl,pmic-stby-poweroff; >> >> regulators { >> reg_vddcore: sw1ab { /* VDDARM_IN */ >> -- >> 2.18.0 >
On Mon, Jul 30, 2018 at 09:50:55AM +0200, Oleksij Rempel wrote: > On 27.07.2018 11:32, Robin Gong wrote: > >> #include <linux/slab.h> > >> +#include <linux/kallsyms.h> > > Is it necessary? > yes, for pm_power_off_prepare That's a *weird* header to have to use for that symbol, are you sure there isn't something more specific - if there isn't there should be.
On 30.07.2018 12:24, Mark Brown wrote: > On Mon, Jul 30, 2018 at 09:50:55AM +0200, Oleksij Rempel wrote: >> On 27.07.2018 11:32, Robin Gong wrote: > >>>> #include <linux/slab.h> >>>> +#include <linux/kallsyms.h> > >>> Is it necessary? > >> yes, for pm_power_off_prepare > > That's a *weird* header to have to use for that symbol, are you sure > there isn't something more specific - if there isn't there should be. Hm... you right. Removed.
On 27.07.2018 11:32, Robin Gong wrote: > > >> -----Original Message----- >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] >> Sent: 2018年7月26日 17:22 >> To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; >> Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton >> <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; >> Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael >> Turquette <mturquette@baylibre.com>; Stephen Boyd >> <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell >> King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong >> <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> >> Subject: [PATCH v8 5/6] regulator: pfuze100-regulator: provide >> pm_power_off_prepare handler >> >> 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 standby state of the PMIC to disable all power lines. >> >> In my power consumption test on RIoTBoard, I got the following results: >> power off without this patch: 320 mA >> power off with this patch: 2 mA >> suspend to ram: 40 mA >> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> --- >> drivers/regulator/pfuze100-regulator.c | 92 ++++++++++++++++++++++++++ >> 1 file changed, 92 insertions(+) >> >> diff --git a/drivers/regulator/pfuze100-regulator.c >> b/drivers/regulator/pfuze100-regulator.c >> index 8d9dbcc775ea..e386e9acb3f7 100644 >> --- a/drivers/regulator/pfuze100-regulator.c >> +++ b/drivers/regulator/pfuze100-regulator.c >> @@ -15,6 +15,7 @@ >> #include <linux/regulator/pfuze100.h> >> #include <linux/i2c.h> >> #include <linux/slab.h> >> +#include <linux/kallsyms.h> > Is it necessary? >> #include <linux/regmap.h> >> >> #define PFUZE_NUMREGS 128 >> @@ -29,11 +30,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 >> @@ -44,6 +51,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 { >> @@ -492,6 +506,69 @@ static inline struct device_node *match_of_node(int >> index) } #endif >> >> +static struct pfuze_chip *syspm_pfuze_chip; >> + >> +static void pfuze_power_off_prepare(void) >> + dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power >> +off"); > Add 'if (syspm_pfuze_chip ->chip_id == PFUZE100))' here is easy for extend > Support on pfuze200/3000.. in the feature. There is already: static int pfuze_power_off_prepare_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 supported chip\n"); return -ENODEV; } No need to add it in pfuze_power_off_prepare() >> + >> + /* 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_power_off_prepare_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 supported chip\n"); >> + return -ENODEV; >> + } >> + >> + if (pm_power_off_prepare) { >> + dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already >> registered.\n"); >> + return -EBUSY; >> + } >> + >> + if (syspm_pfuze_chip) { >> + dev_warn(pfuze_chip->dev, "syspm_pfuze_chip is already set.\n"); >> + return -EBUSY; >> + } >> + >> + syspm_pfuze_chip = pfuze_chip; >> + pm_power_off_prepare = pfuze_power_off_prepare; >> + >> + return 0; >> +} >> + >> static int pfuze_identify(struct pfuze_chip *pfuze_chip) { >> unsigned int value; >> @@ -661,6 +738,20 @@ static int pfuze100_regulator_probe(struct i2c_client >> *client, >> } >> } >> >> + if (of_property_read_bool(client->dev.of_node, >> + "fsl,pmic-stby-poweroff")) >> + return pfuze_power_off_prepare_init(pfuze_chip); >> + >> + return 0; >> +} >> + >> +static int pfuze100_regulator_remove(struct i2c_client *client) { >> + if (syspm_pfuze_chip) { >> + syspm_pfuze_chip = NULL; >> + pm_power_off_prepare = NULL; >> + } >> + >> return 0; >> } >> >> @@ -671,6 +762,7 @@ static struct i2c_driver pfuze_driver = { >> .of_match_table = pfuze_dt_ids, >> }, >> .probe = pfuze100_regulator_probe, >> + .remove = pfuze100_regulator_remove, >> }; >> module_i2c_driver(pfuze_driver); >> >> -- >> 2.18.0 >
On 27.07.2018 11:33, Robin Gong wrote: > > >> -----Original Message----- >> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] >> Sent: 2018年7月26日 17:22 >> To: Shawn Guo <shawnguo@kernel.org>; Mark Brown <broonie@kernel.org>; >> Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew Morton >> <akpm@linux-foundation.org>; Liam Girdwood <lgirdwood@gmail.com>; >> Leonard Crestez <leonard.crestez@nxp.com>; Rob Herring >> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Michael >> Turquette <mturquette@baylibre.com>; Stephen Boyd >> <sboyd@codeaurora.org>; Fabio Estevam <fabio.estevam@nxp.com>; Russell >> King <linux@armlinux.org.uk>; dl-linux-imx <linux-imx@nxp.com>; Robin Gong >> <yibin.gong@nxp.com>; A.s. Dong <aisheng.dong@nxp.com> >> Subject: [PATCH v8 6/6] ARM: dts: imx6: RIoTboard provide standby on power >> off option >> >> This board, as well as some other boards with i.MX6 and a PMIC, uses a >> "PMIC_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> >> --- >> 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 2e98c92adff7..a0e9753ee767 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; >> +}; > It's better remove the default "syscon-poweroff" power off node. "syscon-poweroff" is by default disabled and not enabled in arch/arm/boot/dts/imx6dl-riotboard.dts
> >> +static struct pfuze_chip *syspm_pfuze_chip; > >> + > >> +static void pfuze_power_off_prepare(void) > >> + dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power > >> +off"); > > Add 'if (syspm_pfuze_chip ->chip_id == PFUZE100))' here is easy for > > extend Support on pfuze200/3000.. in the feature. > There is already: > static int pfuze_power_off_prepare_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 supported chip\n"); > return -ENODEV; > } > > > No need to add it in pfuze_power_off_prepare() I saw you add chip check in pfuze_power_off_prepare_init(), but I'm saying In the future case pfuze200/3000 may should still support this feature, but registers are different between different chips, thus, move checking chip into pfuze_power_off_prepare() could make the later patch for pfuze200/3000 more clear, less and easier. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018.07.26: v8 is a rebase against kernel v4.18-rc6. No other changes are made. Added: linux-imx@nxp.com and yibin.gong@nxp.com to the CC. 2018.05.17: update patches to version v7 This patch series is providing power off support for Freescale/NXP iMX6 based boards with external power management integrated circuit (PMIC). As a first step the PMIC is configured to turn off the system if the standby pin is asserted. On second step we assert the standby pin. For this reason we need to use pm_power_off_prepare. Usage of stnadby pin for power off is described in official iMX6 documentation. 2018.03.05: As this patch set touches multiple subsystems I think it would make sense for Shawn Guo to take the all patch set. The only part which didn't receive an ACK is regulator stuff. So I would hope that Mark Brown can ACK it. Kind regards, Oleksij Rempel 2017.12.06: Adding Linus. Probably there is no maintainer for this patch set. No changes are made, tested on v4.15-rc1. 2017.10.27: Last version of this patch set was send at 20 Jun 2017, this is a rebase against kernel v4.14-rc6. Probably this set got lost. If I forgot to address some comments, please point me. changes: v7: - use EXPORT_SYMBOL_GPL(pm_power_off_prepare) instead of EXPORT_SYMBOL - call imx6q_suspend_finish() directly without cpu_suspend() v6: - rename imx6_pm_poweroff to imx6_pm_stby_poweroff - fix "MPIC_STBY_REQ" typo in the comment. v5: - remove useless includes from pm-imx6.c patch - add Acked-by to "regulator: pfuze100: add fsl,pmic-stby-poweroff property" patch v4: - update comment in "regulator: pfuze100: add fsl,pmic-stby-poweroff ..." patch - add Acked-by to "ARM: imx6q: provide documentation for new ..." patch v3: - set pm_power_off_prepare = NULL on .remove. - documentation and spelling fixes. - use %pf instead of lookup_symbol_name. 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 on power off option .../devicetree/bindings/clock/imx6q-clock.txt | 8 ++ .../bindings/regulator/pfuze100.txt | 7 ++ arch/arm/boot/dts/imx6dl-riotboard.dts | 5 + arch/arm/mach-imx/pm-imx6.c | 25 +++++ drivers/regulator/pfuze100-regulator.c | 92 +++++++++++++++++++ kernel/reboot.c | 1 + 6 files changed, 138 insertions(+)