[PATCH/RFC 0/5] arm64: dts: renesas: Salvator-X/XS: Enable DDR Backup Mode

[PATCH/RFC 0/5] arm64: dts: renesas: Salvator-X/XS: Enable DDR Backup Mode

[Geert Uytterhoeven]

	Hi all,

The ROHM BD9571MWV PMIC on the Renesas Salvator-X/XS development boards
supports DDR Backup Power, which means that the DDR power rails can be
kept powered while the main SoC is powered down.

Currently performing a system suspend/resume cycle involves several
manual steps:
  1. Configure the PMIC for Backup Mode, using
     "i2cset -f -y 7 0x30 0x20 0x0f",
  2. Switch off SW23 (the ACC switch, which now changes role from power
     switch to wake-up switch) to prepare for suspend,
  3. Suspend to RAM, using "echo mem > /sys/power/state",
  4. Switch on SW23 to resume.

Especially the first step is cumbersome, as it relies on
  1. Intimate knowledge about the PMIC and actual board design,
  2. direct i2c access,
  3. having the i2cset utility available.
In addition, it has to be redone after system resume.

This patch series integrates Backup Mode configuration into the PMIC
driver, using a sysfs virtual file.  E.g. it can be enabled using:

    echo on > /sys/devices/platform/soc/e60b0000.i2c/i2c-7/7-0030/bd9571mwv-regulator.3.auto/backup_mode

Backup Mode state is retained across suspend/resume cycles.

Which DDR rails are to be kept powered is board-specific, and controlled
using the optional "rohm,ddr-backup-power" property in the board DTS
file.

Note that the last patch depends on "[PATCH/RFC] arm64: dts: renesas:
salvator-common: Add BD9571 PMIC".

Thanks for your comments!

Geert Uytterhoeven (5):
  dt-bindings: mfd: bd9571mwv: Document rohm,ddr-backup-power
  mfd: bd9571mwv: Add DDR Backup Power register bit definitions
  mfd: bd9571mwv: Allow DDR Backup Power register access
  regulator: bd9571mwv: Add support for backup mode
  arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup
  Power

 .../devicetree/bindings/mfd/bd9571mwv.txt          |   7 ++
 arch/arm64/boot/dts/renesas/salvator-common.dtsi   |   1 +
 drivers/mfd/bd9571mwv.c                            |   2 +
 drivers/regulator/bd9571mwv-regulator.c            | 121 ++++++++++++++++++++-
 include/linux/mfd/bd9571mwv.h                      |   5 +
 5 files changed, 134 insertions(+), 2 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH/RFC 1/5] dt-bindings: mfd: bd9571mwv: Document rohm,ddr-backup-power

[Geert Uytterhoeven]

Document the new optional "rohm,ddr-backup-power" property.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/mfd/bd9571mwv.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
index 9ab216a851d5619b..7ea3f2db41d4e501 100644
--- a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
@@ -25,6 +25,12 @@ Required properties:
 			    Each child node is defined using the standard
 			    binding for regulators.
 
+Optional properties:
+  - rohm,ddr-backup-power : Value to use for DDR-Backup Power.  This controls
+			    which DDR rails need to be kept powered when backup
+			    mode is enabled, cfr. the KEEPON_DDR* bits in the
+			    documentation for the "BKUP Mode Cnt" register.
+
 Example:
 
 	pmic: pmic@30 {
@@ -36,6 +42,7 @@ Example:
 		#interrupt-cells = <2>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		rohm,ddr-backup-power = <15>;
 
 		regulators {
 			dvfs: dvfs {
-- 
2.7.4

Re: [PATCH/RFC 1/5] dt-bindings: mfd: bd9571mwv: Document rohm,ddr-backup-power

[Lee Jones]

On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:

> Document the new optional "rohm,ddr-backup-power" property.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/mfd/bd9571mwv.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> index 9ab216a851d5619b..7ea3f2db41d4e501 100644
> --- a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> +++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> @@ -25,6 +25,12 @@ Required properties:
>  			    Each child node is defined using the standard
>  			    binding for regulators.
>  
> +Optional properties:
> +  - rohm,ddr-backup-power : Value to use for DDR-Backup Power.  This controls
> +			    which DDR rails need to be kept powered when backup
> +			    mode is enabled, cfr. the KEEPON_DDR* bits in the

Perhaps it's just me, but I'm confused by this line.

Can you word it another way?

> +			    documentation for the "BKUP Mode Cnt" register.
> +
>  Example:
>  
>  	pmic: pmic@30 {
> @@ -36,6 +42,7 @@ Example:
>  		#interrupt-cells = <2>;
>  		gpio-controller;
>  		#gpio-cells = <2>;
> +		rohm,ddr-backup-power = <15>;

Can you explain what this means?  Is it a mask, or does line 15 need
to be kept on?  What is the range?  Is 0 acceptable?  Clarification
required please.

>  		regulators {
>  			dvfs: dvfs {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH/RFC 1/5] dt-bindings: mfd: bd9571mwv: Document rohm,ddr-backup-power

[Geert Uytterhoeven]

Hi Lee,

On Fri, Oct 13, 2017 at 10:55 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:
>> Document the new optional "rohm,ddr-backup-power" property.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  Documentation/devicetree/bindings/mfd/bd9571mwv.txt | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
>> index 9ab216a851d5619b..7ea3f2db41d4e501 100644
>> --- a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
>> +++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
>> @@ -25,6 +25,12 @@ Required properties:
>>                           Each child node is defined using the standard
>>                           binding for regulators.
>>
>> +Optional properties:
>> +  - rohm,ddr-backup-power : Value to use for DDR-Backup Power.  This controls
>> +                         which DDR rails need to be kept powered when backup
>> +                         mode is enabled, cfr. the KEEPON_DDR* bits in the
>
> Perhaps it's just me, but I'm confused by this line.
>
> Can you word it another way?

I'll ttry... Let me think a bit about it...

>> +                         documentation for the "BKUP Mode Cnt" register.
>> +
>>  Example:
>>
>>       pmic: pmic@30 {
>> @@ -36,6 +42,7 @@ Example:
>>               #interrupt-cells = <2>;
>>               gpio-controller;
>>               #gpio-cells = <2>;
>> +             rohm,ddr-backup-power = <15>;
>
> Can you explain what this means?  Is it a mask, or does line 15 need
> to be kept on?  What is the range?  Is 0 acceptable?  Clarification
> required please.

It's a mask (OR of (in this case all four) needed KEEPON_DDR* bits).
Unfortunately the datasheet is not publicly available.

0 is acceptable, but as the property is optional, you better just leave it out.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC 1/5] dt-bindings: mfd: bd9571mwv: Document rohm,ddr-backup-power

[Lee Jones]

On Fri, 13 Oct 2017, Geert Uytterhoeven wrote:

> Hi Lee,
> 
> On Fri, Oct 13, 2017 at 10:55 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:
> >> Document the new optional "rohm,ddr-backup-power" property.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/bd9571mwv.txt | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> >> index 9ab216a851d5619b..7ea3f2db41d4e501 100644
> >> --- a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> >> @@ -25,6 +25,12 @@ Required properties:
> >>                           Each child node is defined using the standard
> >>                           binding for regulators.
> >>
> >> +Optional properties:
> >> +  - rohm,ddr-backup-power : Value to use for DDR-Backup Power.  This controls
> >> +                         which DDR rails need to be kept powered when backup
> >> +                         mode is enabled, cfr. the KEEPON_DDR* bits in the
> >
> > Perhaps it's just me, but I'm confused by this line.
> >
> > Can you word it another way?
> 
> I'll ttry... Let me think a bit about it...

I think it's just the "cfr" which is throwing me.

> >> +                         documentation for the "BKUP Mode Cnt" register.
> >> +
> >>  Example:
> >>
> >>       pmic: pmic@30 {
> >> @@ -36,6 +42,7 @@ Example:
> >>               #interrupt-cells = <2>;
> >>               gpio-controller;
> >>               #gpio-cells = <2>;
> >> +             rohm,ddr-backup-power = <15>;
> >
> > Can you explain what this means?  Is it a mask, or does line 15 need
> > to be kept on?  What is the range?  Is 0 acceptable?  Clarification
> > required please.
> 
> It's a mask (OR of (in this case all four) needed KEEPON_DDR* bits).
> Unfortunately the datasheet is not publicly available.
> 
> 0 is acceptable, but as the property is optional, you better just leave it out.

Would you mind reflecting that in the doc please?

Maybe a few more examples will help?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH/RFC 2/5] mfd: bd9571mwv: Add DDR Backup Power register bit definitions

[Geert Uytterhoeven]

Add definitions for the KEEPON_* bits in the "BKUP Mode Cnt" register,
which control the DDR rails to be kept powered when backup mode is
enabled.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 include/linux/mfd/bd9571mwv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mfd/bd9571mwv.h b/include/linux/mfd/bd9571mwv.h
index f0708ba4cbbae2dc..eb05569f752bb089 100644
--- a/include/linux/mfd/bd9571mwv.h
+++ b/include/linux/mfd/bd9571mwv.h
@@ -33,6 +33,11 @@
 #define BD9571MWV_I2C_MD2_E1_BIT_2		0x12
 
 #define BD9571MWV_BKUP_MODE_CNT			0x20
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK	GENMASK(3, 0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0	BIT(0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1	BIT(1)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0C	BIT(2)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1C	BIT(3)
 #define BD9571MWV_BKUP_MODE_STATUS		0x21
 #define BD9571MWV_BKUP_RECOVERY_CNT		0x22
 #define BD9571MWV_BKUP_CTRL_TIM_CNT		0x23
-- 
2.7.4

Re: [PATCH/RFC 2/5] mfd: bd9571mwv: Add DDR Backup Power register bit definitions

[Lee Jones]

On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:

> Add definitions for the KEEPON_* bits in the "BKUP Mode Cnt" register,
> which control the DDR rails to be kept powered when backup mode is
> enabled.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  include/linux/mfd/bd9571mwv.h | 5 +++++
>  1 file changed, 5 insertions(+)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Applied "mfd: bd9571mwv: Add DDR Backup Power register bit definitions" to the regulator tree

[Mark Brown]

The patch

   mfd: bd9571mwv: Add DDR Backup Power register bit definitions

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 2ff0dab80a8999e99a93fd70f8d701ec3deab207 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 18 Apr 2018 15:18:02 +0200
Subject: [PATCH] mfd: bd9571mwv: Add DDR Backup Power register bit definitions

Add definitions for the KEEPON_* bits in the "BKUP Mode Cnt" register,
which control the DDR rails to be kept powered when backup mode is
enabled.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/linux/mfd/bd9571mwv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mfd/bd9571mwv.h b/include/linux/mfd/bd9571mwv.h
index f0708ba4cbba..eb05569f752b 100644
--- a/include/linux/mfd/bd9571mwv.h
+++ b/include/linux/mfd/bd9571mwv.h
@@ -33,6 +33,11 @@
 #define BD9571MWV_I2C_MD2_E1_BIT_2		0x12
 
 #define BD9571MWV_BKUP_MODE_CNT			0x20
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK	GENMASK(3, 0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0	BIT(0)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1	BIT(1)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR0C	BIT(2)
+#define BD9571MWV_BKUP_MODE_CNT_KEEPON_DDR1C	BIT(3)
 #define BD9571MWV_BKUP_MODE_STATUS		0x21
 #define BD9571MWV_BKUP_RECOVERY_CNT		0x22
 #define BD9571MWV_BKUP_CTRL_TIM_CNT		0x23
-- 
2.17.0

[PATCH/RFC 3/5] mfd: bd9571mwv: Allow DDR Backup Power register access

[Geert Uytterhoeven]

Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
is a.o. used to configure DDR Backup Power.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/mfd/bd9571mwv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
index 64e088dfe7b05b5b..503979c81dae11bb 100644
--- a/drivers/mfd/bd9571mwv.c
+++ b/drivers/mfd/bd9571mwv.c
@@ -29,6 +29,7 @@ static const struct mfd_cell bd9571mwv_cells[] = {
 
 static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
 	regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+	regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
 	regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
 	regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
 	regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
@@ -44,6 +45,7 @@ static const struct regmap_access_table bd9571mwv_readable_table = {
 };
 
 static const struct regmap_range bd9571mwv_writable_yes_ranges[] = {
+	regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
 	regmap_reg_range(BD9571MWV_AVS_VD09_VID(0), BD9571MWV_AVS_VD09_VID(3)),
 	regmap_reg_range(BD9571MWV_DVFS_SETVID, BD9571MWV_DVFS_SETVID),
 	regmap_reg_range(BD9571MWV_GPIO_DIR, BD9571MWV_GPIO_OUT),
-- 
2.7.4

Re: [PATCH/RFC 3/5] mfd: bd9571mwv: Allow DDR Backup Power register access

[Lee Jones]

On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:

> Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
> is a.o. used to configure DDR Backup Power.

a.o.?  That's a new one on me.

Please ensure commit logs are clear for *everyone* to read/understand
by using descriptive and expanded (rather than shorten) terms.

Code looks fine though, so once fixed:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH/RFC 3/5] mfd: bd9571mwv: Allow DDR Backup Power register access

[Geert Uytterhoeven]

Hi Lee,

On Fri, Oct 13, 2017 at 10:58 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:
>
>> Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
>> is a.o. used to configure DDR Backup Power.
>
> a.o.?  That's a new one on me.

among(st) others

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC 3/5] mfd: bd9571mwv: Allow DDR Backup Power register access

[Lee Jones]

On Fri, 13 Oct 2017, Geert Uytterhoeven wrote:

> Hi Lee,
> 
> On Fri, Oct 13, 2017 at 10:58 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 10 Oct 2017, Geert Uytterhoeven wrote:
> >
> >> Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
> >> is a.o. used to configure DDR Backup Power.
> >
> > a.o.?  That's a new one on me.
> 
> among(st) others

Please just write that instead. :)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Applied "mfd: bd9571mwv: Allow DDR Backup Power register access" to the regulator tree

[Mark Brown]

The patch

   mfd: bd9571mwv: Allow DDR Backup Power register access

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7b569bcb2a2f985fe2d1407aed705882af30cb77 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 18 Apr 2018 15:18:03 +0200
Subject: [PATCH] mfd: bd9571mwv: Allow DDR Backup Power register access

Enable read/write access to the BD9571MWV_BKUP_MODE_CNT register, which
is amongst others used to configure DDR Backup Power.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/mfd/bd9571mwv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
index 64e088dfe7b0..503979c81dae 100644
--- a/drivers/mfd/bd9571mwv.c
+++ b/drivers/mfd/bd9571mwv.c
@@ -29,6 +29,7 @@ static const struct mfd_cell bd9571mwv_cells[] = {
 
 static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
 	regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+	regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
 	regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
 	regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
 	regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
@@ -44,6 +45,7 @@ static const struct regmap_access_table bd9571mwv_readable_table = {
 };
 
 static const struct regmap_range bd9571mwv_writable_yes_ranges[] = {
+	regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
 	regmap_reg_range(BD9571MWV_AVS_VD09_VID(0), BD9571MWV_AVS_VD09_VID(3)),
 	regmap_reg_range(BD9571MWV_DVFS_SETVID, BD9571MWV_DVFS_SETVID),
 	regmap_reg_range(BD9571MWV_GPIO_DIR, BD9571MWV_GPIO_OUT),
-- 
2.17.0

[PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Geert Uytterhoeven]

The BD9571MWV PMIC supports backup mode, which keeps one or more DDR
rails powered while the main SoC is powered down.

Which DDR rails are to be kept powered is board-specific, and controlled
using the optional "rohm,ddr-backup-power" DT property.  In the absence
of this property, backup mode is not available.

Backup mode is not enabled automatically, as e.g. on Renesas
Salvator-X(S) boards enabling backup mode changes the role of the ACC
switch from a power switch to a wake-up switch.  Hence enabling it
prevents the board from being powered off using the ACC switch, which
may confuse the user.

Backup mode can be enabled or disabled by the user using the
"backup_mode" sysfs file, e.g.

    echo on > /sys/devices/platform/soc/e60b0000.i2c/i2c-7/7-0030/bd9571mwv-regulator.3.auto/backup_mode

If enabled by the boot loader, initial backup mode state is retained.
As state is lost by PSCI system suspend, it is restored during system
resume.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
What's the right place to document regulator sysfs files?
---
 drivers/regulator/bd9571mwv-regulator.c | 121 +++++++++++++++++++++++++++++++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/bd9571mwv-regulator.c b/drivers/regulator/bd9571mwv-regulator.c
index c67a83d53c4cb76b..c7ff67938c3eb48a 100644
--- a/drivers/regulator/bd9571mwv-regulator.c
+++ b/drivers/regulator/bd9571mwv-regulator.c
@@ -24,6 +24,14 @@
 
 #include <linux/mfd/bd9571mwv.h>
 
+struct bd9571mwv_reg {
+	struct bd9571mwv *bd;
+
+	/* DDR Backup Power */
+	u8 bkup_mode_cnt_keepon;	/* from "rohm,ddr-backup-power" */
+	u8 bkup_mode_cnt_shadow;
+};
+
 enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
 
 #define BD9571MWV_REG(_name, _of, _id, _ops, _vr, _vm, _nv, _min, _step, _lmin)\
@@ -131,14 +139,99 @@ static struct regulator_desc regulators[] = {
 		      0x80, 600000, 10000, 0x3c),
 };
 
+static int bd9571mwv_bkup_mode_set(struct bd9571mwv_reg *bdreg, u8 mode)
+{
+	int ret;
+
+	ret = regmap_write(bdreg->bd->regmap, BD9571MWV_BKUP_MODE_CNT, mode);
+	if (ret) {
+		dev_err(bdreg->bd->dev,
+			"Failed to configure backup mode 0x%x (ret=%i)\n",
+			mode, ret);
+		return ret;
+	}
+
+	bdreg->bkup_mode_cnt_shadow = mode;
+	return 0;
+}
+
+static ssize_t backup_mode_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+	int enabled = bdreg->bkup_mode_cnt_shadow &
+		      BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK;
+
+	return sprintf(buf, "%s\n", enabled ? "on" : "off");
+}
+
+static ssize_t backup_mode_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+	bool enable;
+	ssize_t ret;
+	u8 mode;
+
+	if (!count)
+		return 0;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	mode = bdreg->bkup_mode_cnt_shadow &
+	       ~BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK;
+	if (enable)
+		mode |= bdreg->bkup_mode_cnt_keepon;
+
+	if (mode == bdreg->bkup_mode_cnt_shadow)
+		return count;
+
+	ret = bd9571mwv_bkup_mode_set(bdreg, mode);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+DEVICE_ATTR_RW(backup_mode);
+
+#ifdef CONFIG_PM_SLEEP
+static int bd9571mwv_resume(struct device *dev)
+{
+	struct bd9571mwv_reg *bdreg = dev_get_drvdata(dev);
+
+	return bd9571mwv_bkup_mode_set(bdreg, bdreg->bkup_mode_cnt_shadow);
+}
+
+static const struct dev_pm_ops bd9571mwv_pm  = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, bd9571mwv_resume)
+};
+
+#define DEV_PM_OPS	&bd9571mwv_pm
+#else
+#define DEV_PM_OPS	NULL
+#endif /* CONFIG_PM_SLEEP */
+
 static int bd9571mwv_regulator_probe(struct platform_device *pdev)
 {
 	struct bd9571mwv *bd = dev_get_drvdata(pdev->dev.parent);
 	struct regulator_config config = { };
+	struct bd9571mwv_reg *bdreg;
 	struct regulator_dev *rdev;
-	int i;
+	unsigned int val;
+	int i, ret;
+
+	bdreg = devm_kzalloc(&pdev->dev, sizeof(*bdreg), GFP_KERNEL);
+	if (!bdreg)
+		return -ENOMEM;
+
+	bdreg->bd = bd;
 
-	platform_set_drvdata(pdev, bd);
+	platform_set_drvdata(pdev, bdreg);
 
 	config.dev = &pdev->dev;
 	config.dev->of_node = bd->dev->of_node;
@@ -155,6 +248,28 @@ static int bd9571mwv_regulator_probe(struct platform_device *pdev)
 		}
 	}
 
+	val = 0;
+	of_property_read_u32(bd->dev->of_node, "rohm,ddr-backup-power", &val);
+	if (val & ~BD9571MWV_BKUP_MODE_CNT_KEEPON_MASK) {
+		dev_err(bd->dev, "invalid %s mode %u\n",
+			"rohm,ddr-backup-power", val);
+		return -EINVAL;
+	}
+	bdreg->bkup_mode_cnt_keepon = val;
+
+	ret = regmap_read(bd->regmap, BD9571MWV_BKUP_MODE_CNT, &val);
+	if (ret) {
+		dev_err(bd->dev, "Failed to read backup mode (ret=%i)\n", ret);
+		return ret;
+	}
+	bdreg->bkup_mode_cnt_shadow = val;
+
+	return device_create_file(&pdev->dev, &dev_attr_backup_mode);
+}
+
+static int bd9571mwv_regulator_remove(struct platform_device *pdev)
+{
+	device_remove_file(&pdev->dev, &dev_attr_backup_mode);
 	return 0;
 }
 
@@ -167,8 +282,10 @@ MODULE_DEVICE_TABLE(platform, bd9571mwv_regulator_id_table);
 static struct platform_driver bd9571mwv_regulator_driver = {
 	.driver = {
 		.name = "bd9571mwv-regulator",
+		.pm = DEV_PM_OPS,
 	},
 	.probe = bd9571mwv_regulator_probe,
+	.remove = bd9571mwv_regulator_remove,
 	.id_table = bd9571mwv_regulator_id_table,
 };
 module_platform_driver(bd9571mwv_regulator_driver);
-- 
2.7.4

Re: [PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Mark Brown]

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

On Tue, Oct 10, 2017 at 05:26:17PM +0200, Geert Uytterhoeven wrote:

> Backup mode is not enabled automatically, as e.g. on Renesas
> Salvator-X(S) boards enabling backup mode changes the role of the ACC
> switch from a power switch to a wake-up switch.  Hence enabling it
> prevents the board from being powered off using the ACC switch, which
> may confuse the user.

This sounds an awful lot like the standard power/wakeup, though the
power change is a bit unexpected there.  I'm also wondering if it makes
sense to just only enable the wakeup mode when suspending which
preserves the power off functionality while also keeping the wakeup
support.

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

Re: [PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Geert Uytterhoeven]

Hi Mark,

On Wed, Oct 18, 2017 at 1:02 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Oct 10, 2017 at 05:26:17PM +0200, Geert Uytterhoeven wrote:
>> Backup mode is not enabled automatically, as e.g. on Renesas
>> Salvator-X(S) boards enabling backup mode changes the role of the ACC
>> switch from a power switch to a wake-up switch.  Hence enabling it
>> prevents the board from being powered off using the ACC switch, which
>> may confuse the user.
>
> This sounds an awful lot like the standard power/wakeup, though the
> power change is a bit unexpected there.  I'm also wondering if it makes
> sense to just only enable the wakeup mode when suspending which
> preserves the power off functionality while also keeping the wakeup
> support.

The ACC switch is not a momentary switch (push button), but a toggle
switch with two positions.
Hence you cannot enable wakeup mode while suspending, as the proper
system suspend/resume procedure is:
  1. Enable backup mode in the PMIC,
  2. Switch ACC off (no-op as backup mode has been enabled),
  3. Suspend to RAM (PSCI suspend) => system suspends,
  4. Switch ACC on => system wakes up.
If you would combine steps 1 and 3, you can no longer do step 2 in between.

Yes, it's complicated :-(

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Mark Brown]

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

On Wed, Oct 18, 2017 at 01:19:08PM +0200, Geert Uytterhoeven wrote:
> On Wed, Oct 18, 2017 at 1:02 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Oct 10, 2017 at 05:26:17PM +0200, Geert Uytterhoeven wrote:

> >> Backup mode is not enabled automatically, as e.g. on Renesas
> >> Salvator-X(S) boards enabling backup mode changes the role of the ACC
> >> switch from a power switch to a wake-up switch.  Hence enabling it
> >> prevents the board from being powered off using the ACC switch, which
> >> may confuse the user.

> > This sounds an awful lot like the standard power/wakeup, though the
> > power change is a bit unexpected there.  I'm also wondering if it makes
> > sense to just only enable the wakeup mode when suspending which
> > preserves the power off functionality while also keeping the wakeup
> > support.

> The ACC switch is not a momentary switch (push button), but a toggle
> switch with two positions.
> Hence you cannot enable wakeup mode while suspending, as the proper
> system suspend/resume procedure is:
>   1. Enable backup mode in the PMIC,
>   2. Switch ACC off (no-op as backup mode has been enabled),
>   3. Suspend to RAM (PSCI suspend) => system suspends,
>   4. Switch ACC on => system wakes up.
> If you would combine steps 1 and 3, you can no longer do step 2 in between.

> Yes, it's complicated :-(

I'm confused, I thought this was a physical switch but that's talking
about this as something software controlled (at least in step 2)?

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

Re: [PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Geert Uytterhoeven]

Hi Mark,

On Wed, Oct 18, 2017 at 1:24 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 18, 2017 at 01:19:08PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Oct 18, 2017 at 1:02 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Tue, Oct 10, 2017 at 05:26:17PM +0200, Geert Uytterhoeven wrote:
>> >> Backup mode is not enabled automatically, as e.g. on Renesas
>> >> Salvator-X(S) boards enabling backup mode changes the role of the ACC
>> >> switch from a power switch to a wake-up switch.  Hence enabling it
>> >> prevents the board from being powered off using the ACC switch, which
>> >> may confuse the user.
>
>> > This sounds an awful lot like the standard power/wakeup, though the
>> > power change is a bit unexpected there.  I'm also wondering if it makes
>> > sense to just only enable the wakeup mode when suspending which
>> > preserves the power off functionality while also keeping the wakeup
>> > support.
>
>> The ACC switch is not a momentary switch (push button), but a toggle
>> switch with two positions.
>> Hence you cannot enable wakeup mode while suspending, as the proper
>> system suspend/resume procedure is:
>>   1. Enable backup mode in the PMIC,
>>   2. Switch ACC off (no-op as backup mode has been enabled),
>>   3. Suspend to RAM (PSCI suspend) => system suspends,
>>   4. Switch ACC on => system wakes up.
>> If you would combine steps 1 and 3, you can no longer do step 2 in between.
>
>> Yes, it's complicated :-(
>
> I'm confused, I thought this was a physical switch but that's talking
> about this as something software controlled (at least in step 2)?

The ACC switch is a physical switch.
Steps 1 and 3 are performed by software running on the board's SoC.
Steps 2 and 4 are performed by the external user (human or remote board
farm control hookup).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC 4/5] regulator: bd9571mwv: Add support for backup mode

[Mark Brown]

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

On Wed, Oct 18, 2017 at 01:28:30PM +0200, Geert Uytterhoeven wrote:
> On Wed, Oct 18, 2017 at 1:24 PM, Mark Brown <broonie@kernel.org> wrote:

> >> Hence you cannot enable wakeup mode while suspending, as the proper
> >> system suspend/resume procedure is:
> >>   1. Enable backup mode in the PMIC,
> >>   2. Switch ACC off (no-op as backup mode has been enabled),
> >>   3. Suspend to RAM (PSCI suspend) => system suspends,
> >>   4. Switch ACC on => system wakes up.
> >> If you would combine steps 1 and 3, you can no longer do step 2 in between.

> >> Yes, it's complicated :-(

> > I'm confused, I thought this was a physical switch but that's talking
> > about this as something software controlled (at least in step 2)?

> The ACC switch is a physical switch.
> Steps 1 and 3 are performed by software running on the board's SoC.
> Steps 2 and 4 are performed by the external user (human or remote board
> farm control hookup).

That's horrible.  There's still the question about potentially using the
existing wakeup file to manage if the device is a wakeup source but
otherwise I guess the only other thing that'd make sense would be just
having a property in the DT.

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

[PATCH/RFC 5/5] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

On Salvator-X(S), all of DDR0, DDR1, DDR0C, and DDR1C need to be kept
powered when backup mode is enabled.

Reflect this in the rohm,ddr-backup-power property for the BD9571MWV
PMIC.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index c171718bac7f52a1..68245d87c305855c 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -366,6 +366,7 @@
 		#interrupt-cells = <2>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		rohm,ddr-backup-power = <15>;
 
 		regulators {
 			dvfs: dvfs {
-- 
2.7.4

Re: [PATCH/RFC 5/5] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Simon Horman]

On Tue, Oct 10, 2017 at 05:26:18PM +0200, Geert Uytterhoeven wrote:
> On Salvator-X(S), all of DDR0, DDR1, DDR0C, and DDR1C need to be kept
> powered when backup mode is enabled.
> 
> Reflect this in the rohm,ddr-backup-power property for the BD9571MWV
> PMIC.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>


Hi,

I have marked this as deferred pending acceptance of the bindings.
Please repost or otherwise ping me once that dependency is in place.