[PATCH v2 0/3] arm64: dts: renesas: salvator-x(s)/ulcb: Configure PMIC for DDR Backup Power

[PATCH v2 0/3] arm64: dts: renesas: salvator-x(s)/ulcb: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

	Hi Simon, Magnus,

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

This patch series is the DT counterpart of the series "[PATCH v2 0/4]
regulator: bd9571mwv: Add support for DDR backup mode" [1] and
"[PATCH/RFC 0/2] regulator: bd9571mwv: Add support for toggle power
switches" [2].

The first patch adds the missing device node for the BD9571 PMIC on the
ULCB boards, and can be applied immediately.
The last two patches add DDR Backup Mode configuration, and depend on
the DT bindings in the driver patch being accepted.

Changes compared to v2:
  - Add support for ULCB,
  - Add "rohm,rstbmode-level" for Salvator-X(S).

This has been tested on M3ULCB (thanks Jacopo!), and on Salvator-X(S).

For testing, driver and DTS patches are available in the
topic/bd9571-ddr-backup-mode-driver-v2 and
topic/bd9571-ddr-backup-mode-dt-v2 branches of my renesas-drivers git
repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.

Thanks for your comments!

[1] https://lkml.org/lkml/2018/3/14/302
[2] https://lkml.org/lkml/2018/3/14/324

Geert Uytterhoeven (3):
  arm64: dts: renesas: ulcb: Add BD9571 PMIC
  arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup
    Power
  arm64: dts: renesas: ulcb: Configure PMIC for DDR Backup Power

 arch/arm64/boot/dts/renesas/salvator-common.dtsi |  2 ++
 arch/arm64/boot/dts/renesas/ulcb.dtsi            | 31 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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 v2 1/3] arm64: dts: renesas: ulcb: Add BD9571 PMIC

[Geert Uytterhoeven]

Add a device node for the ROHM BD9571MWV PMIC.

This was based on the example in the DT binding documentation, but using
IRQ0 instead of a GPIO interrupt, as that matches the schematics, and
because INTC-EX is a simpler block.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 arch/arm64/boot/dts/renesas/ulcb.dtsi | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index 6f814845f8b665f3..bb21ae335e8b8489 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -243,6 +243,30 @@
 
 &i2c_dvfs {
 	status = "okay";
+
+	pmic: pmic at 30 {
+		pinctrl-0 = <&irq0_pins>;
+		pinctrl-names = "default";
+
+		compatible = "rohm,bd9571mwv";
+		reg = <0x30>;
+		interrupt-parent = <&intc_ex>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		regulators {
+			dvfs: dvfs {
+				regulator-name = "dvfs";
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <1030000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &ohci1 {
@@ -276,6 +300,11 @@
 		function = "i2c2";
 	};
 
+	irq0_pins: irq0 {
+		groups = "intc_ex_irq0";
+		function = "intc_ex";
+	};
+
 	scif2_pins: scif2 {
 		groups = "scif2_data_a";
 		function = "scif2";
-- 
2.7.4

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
need to be kept powered when backup mode is enabled.  Reflect this in
the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.

The accessory power switch (SW23) is a toggle switch, hense specify
"rohm,rstbmode-level".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Add rohm,rstbmode-level.
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 2a7f36abd2dd85c6..80794c38c2669d75 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -376,6 +376,8 @@
 		#interrupt-cells = <2>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		rohm,ddr-backup-power = <15>;
+		rohm,rstbmode-level;
 
 		regulators {
 			dvfs: dvfs {
-- 
2.7.4

[PATCH v2 3/3] arm64: dts: renesas: ulcb: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

On the R-Car Starter Kit Premier/Pro, all of the DDR0, DDR1, DDR0C, and
DDR1C power rails need to be kept powered when backup mode is enabled.
Reflect this in the "rohm,ddr-backup-power" property for the BD9571MWV
PMIC node.

The accessory power switch (SW8) is a momentary switch, hense specify
"rohm,rstbmode-pulse".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 arch/arm64/boot/dts/renesas/ulcb.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index bb21ae335e8b8489..6caf2e4b776faf40 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -256,6 +256,8 @@
 		#interrupt-cells = <2>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		rohm,ddr-backup-power = <15>;
+		rohm,rstbmode-pulse;
 
 		regulators {
 			dvfs: dvfs {
-- 
2.7.4

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Sergei Shtylyov]

Hello!

On 03/14/2018 03:09 PM, Geert Uytterhoeven wrote:

> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
> need to be kept powered when backup mode is enabled.  Reflect this in
> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
> 
> The accessory power switch (SW23) is a toggle switch, hense specify
> "rohm,rstbmode-level".
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Add rohm,rstbmode-level.
> ---
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -376,6 +376,8 @@
>  		#interrupt-cells = <2>;
>  		gpio-controller;
>  		#gpio-cells = <2>;
> +		rohm,ddr-backup-power = <15>;

  Why not 0xf if those are all bit flags?

[...]

MBR, Sergei

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Simon Horman]

On Wed, Mar 14, 2018 at 06:17:17PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 03/14/2018 03:09 PM, Geert Uytterhoeven wrote:
> 
> > On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
> > need to be kept powered when backup mode is enabled.  Reflect this in
> > the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
> > 
> > The accessory power switch (SW23) is a toggle switch, hense specify
> > "rohm,rstbmode-level".
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v2:
> >   - Add rohm,rstbmode-level.
> > ---
> >  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > index 2a7f36abd2dd85c6..80794c38c2669d75 100644
> > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > @@ -376,6 +376,8 @@
> >  		#interrupt-cells = <2>;
> >  		gpio-controller;
> >  		#gpio-cells = <2>;
> > +		rohm,ddr-backup-power = <15>;
> 
>   Why not 0xf if those are all bit flags?

Hi Geert,

could you address this feedback when you have some time?

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

Hi Sergei,

On Wed, Mar 14, 2018 at 4:17 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 03/14/2018 03:09 PM, Geert Uytterhoeven wrote:
>> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
>> need to be kept powered when backup mode is enabled.  Reflect this in
>> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
>>
>> The accessory power switch (SW23) is a toggle switch, hense specify
>> "rohm,rstbmode-level".
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> v2:
>>   - Add rohm,rstbmode-level.
>> ---
>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> @@ -376,6 +376,8 @@
>>               #interrupt-cells = <2>;
>>               gpio-controller;
>>               #gpio-cells = <2>;
>> +             rohm,ddr-backup-power = <15>;
>
>   Why not 0xf if those are all bit flags?

Because it's a small number?
Does it matter?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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 v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Sergei Shtylyov]

On 03/16/2018 03:00 PM, Geert Uytterhoeven wrote:

>>> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
>>> need to be kept powered when backup mode is enabled.  Reflect this in
>>> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
>>>
>>> The accessory power switch (SW23) is a toggle switch, hense specify
>>> "rohm,rstbmode-level".
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> v2:
>>>   - Add rohm,rstbmode-level.
>>> ---
>>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>>> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
>>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>>> @@ -376,6 +376,8 @@
>>>               #interrupt-cells = <2>;
>>>               gpio-controller;
>>>               #gpio-cells = <2>;
>>> +             rohm,ddr-backup-power = <15>;
>>
>>   Why not 0xf if those are all bit flags?
> 
> Because it's a small number?
> Does it matter?

   I think hex is preferable for the bit flags. Decimal makes one think it's something
else -- some magnitude, etc...

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Simon Horman]

On Fri, Mar 16, 2018 at 03:08:13PM +0300, Sergei Shtylyov wrote:
> On 03/16/2018 03:00 PM, Geert Uytterhoeven wrote:
> 
> >>> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
> >>> need to be kept powered when backup mode is enabled.  Reflect this in
> >>> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
> >>>
> >>> The accessory power switch (SW23) is a toggle switch, hense specify
> >>> "rohm,rstbmode-level".
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>> ---
> >>> v2:
> >>>   - Add rohm,rstbmode-level.
> >>> ---
> >>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >>> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
> >>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >>> @@ -376,6 +376,8 @@
> >>>               #interrupt-cells = <2>;
> >>>               gpio-controller;
> >>>               #gpio-cells = <2>;
> >>> +             rohm,ddr-backup-power = <15>;
> >>
> >>   Why not 0xf if those are all bit flags?
> > 
> > Because it's a small number?
> > Does it matter?
> 
>    I think hex is preferable for the bit flags. Decimal makes one think it's something
> else -- some magnitude, etc...

Geert, it seems that Rob Herring is of the same opinion.
Would you care to respin this series?

[PATCH v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Geert Uytterhoeven]

Hi Simon,

On Mon, Mar 26, 2018 at 11:06 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Mar 16, 2018 at 03:08:13PM +0300, Sergei Shtylyov wrote:
>> On 03/16/2018 03:00 PM, Geert Uytterhoeven wrote:
>>
>> >>> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
>> >>> need to be kept powered when backup mode is enabled.  Reflect this in
>> >>> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
>> >>>
>> >>> The accessory power switch (SW23) is a toggle switch, hense specify
>> >>> "rohm,rstbmode-level".
>> >>>
>> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >>> ---
>> >>> v2:
>> >>>   - Add rohm,rstbmode-level.
>> >>> ---
>> >>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>> >>>  1 file changed, 2 insertions(+)
>> >>>
>> >>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> >>> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
>> >>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> >>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> >>> @@ -376,6 +376,8 @@
>> >>>               #interrupt-cells = <2>;
>> >>>               gpio-controller;
>> >>>               #gpio-cells = <2>;
>> >>> +             rohm,ddr-backup-power = <15>;
>> >>
>> >>   Why not 0xf if those are all bit flags?
>> >
>> > Because it's a small number?
>> > Does it matter?
>>
>>    I think hex is preferable for the bit flags. Decimal makes one think it's something
>> else -- some magnitude, etc...
>
> Geert, it seems that Rob Herring is of the same opinion.
> Would you care to respin this series?

Sure, after the respun driver series has been accepted.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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 v2 2/3] arm64: dts: renesas: salvator-common: Configure PMIC for DDR Backup Power

[Simon Horman]

On Mon, Mar 26, 2018 at 11:13:54AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Mar 26, 2018 at 11:06 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Mar 16, 2018 at 03:08:13PM +0300, Sergei Shtylyov wrote:
> >> On 03/16/2018 03:00 PM, Geert Uytterhoeven wrote:
> >>
> >> >>> On Salvator-X(S), all of the DDR0, DDR1, DDR0C, and DDR1C power rails
> >> >>> need to be kept powered when backup mode is enabled.  Reflect this in
> >> >>> the "rohm,ddr-backup-power" property for the BD9571MWV PMIC node.
> >> >>>
> >> >>> The accessory power switch (SW23) is a toggle switch, hense specify
> >> >>> "rohm,rstbmode-level".
> >> >>>
> >> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> >>> ---
> >> >>> v2:
> >> >>>   - Add rohm,rstbmode-level.
> >> >>> ---
> >> >>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
> >> >>>  1 file changed, 2 insertions(+)
> >> >>>
> >> >>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> >>> index 2a7f36abd2dd85c6..80794c38c2669d75 100644
> >> >>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> >>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> >>> @@ -376,6 +376,8 @@
> >> >>>               #interrupt-cells = <2>;
> >> >>>               gpio-controller;
> >> >>>               #gpio-cells = <2>;
> >> >>> +             rohm,ddr-backup-power = <15>;
> >> >>
> >> >>   Why not 0xf if those are all bit flags?
> >> >
> >> > Because it's a small number?
> >> > Does it matter?
> >>
> >>    I think hex is preferable for the bit flags. Decimal makes one think it's something
> >> else -- some magnitude, etc...
> >
> > Geert, it seems that Rob Herring is of the same opinion.
> > Would you care to respin this series?
> 
> Sure, after the respun driver series has been accepted.

Perfect, thanks!