2016-04-16 06:39:10

by Ivaylo Dimitrov

[permalink] [raw]
Subject: [PATCH] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Without that, regulators are left in the mode last set by the bootloader or
by the kernel the device was rebooted from. This leads to various problems
like non-working peripherals.

Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index b3c26a9..1bb36e2 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -329,6 +329,7 @@
regulator-name = "V28";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
+ regulator-initial-mode = <0x0e>;
regulator-always-on; /* due to battery cover sensor */
};

@@ -336,30 +337,35 @@
regulator-name = "VCSI";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>;
};

&vaux3 {
regulator-name = "VMMC2_30";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <0x0e>;
};

&vaux4 {
regulator-name = "VCAM_ANA_28";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
+ regulator-initial-mode = <0x0e>;
};

&vmmc1 {
regulator-name = "VMMC1";
regulator-min-microvolt = <1850000>;
regulator-max-microvolt = <3150000>;
+ regulator-initial-mode = <0x0e>;
};

&vmmc2 {
regulator-name = "V28_A";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <0x0e>;
regulator-always-on; /* due VIO leak to AIC34 VDDs */
};

@@ -367,6 +373,7 @@
regulator-name = "VPLL";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>;
regulator-always-on;
};

@@ -374,6 +381,7 @@
regulator-name = "VSDI_CSI";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>;
regulator-always-on;
};

@@ -381,6 +389,7 @@
regulator-name = "VMMC2_IO_18";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>;
};

&vio {
--
1.9.1


2016-04-17 00:05:44

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Hi Ivo,

On Sat, Apr 16, 2016 at 09:37:23AM +0300, Ivaylo Dimitrov wrote:
> Without that, regulators are left in the mode last set by the bootloader or
> by the kernel the device was rebooted from. This leads to various problems
> like non-working peripherals.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> ---
> arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index b3c26a9..1bb36e2 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -329,6 +329,7 @@
> regulator-name = "V28";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> + regulator-initial-mode = <0x0e>;
> regulator-always-on; /* due to battery cover sensor */
> };

I think this should either get an additional
comment like /* MODE_NORMAL */ or implemented
using a define and a TWL4030_REGULATOR_MODE_NORMAL
constant to keep the *.dts easily readable.

-- Sebastian


Attachments:
(No filename) (1.03 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-17 06:14:16

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode



On 17.04.2016 03:05, Sebastian Reichel wrote:
> Hi Ivo,
>
> On Sat, Apr 16, 2016 at 09:37:23AM +0300, Ivaylo Dimitrov wrote:
>> Without that, regulators are left in the mode last set by the bootloader or
>> by the kernel the device was rebooted from. This leads to various problems
>> like non-working peripherals.
>>
>> Signed-off-by: Ivaylo Dimitrov <[email protected]>
>> ---
>> arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
>> index b3c26a9..1bb36e2 100644
>> --- a/arch/arm/boot/dts/omap3-n900.dts
>> +++ b/arch/arm/boot/dts/omap3-n900.dts
>> @@ -329,6 +329,7 @@
>> regulator-name = "V28";
>> regulator-min-microvolt = <2800000>;
>> regulator-max-microvolt = <2800000>;
>> + regulator-initial-mode = <0x0e>;
>> regulator-always-on; /* due to battery cover sensor */
>> };
>
> I think this should either get an additional
> comment like /* MODE_NORMAL */ or implemented

According to the TRM, this is 'ACTIVE state', but that does not fit in
the regulator framework terminology.

> using a define and a TWL4030_REGULATOR_MODE_NORMAL
> constant to keep the *.dts easily readable.

We already have RES_STATE_ACTIVE defined in linux/i2c/twl.h, is there a
way to include that in a dts?

Ivo

2016-04-17 12:29:40

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Hi,

On Sun, Apr 17, 2016 at 09:14:08AM +0300, Ivaylo Dimitrov wrote:
> On 17.04.2016 03:05, Sebastian Reichel wrote:
> >On Sat, Apr 16, 2016 at 09:37:23AM +0300, Ivaylo Dimitrov wrote:
> >>Without that, regulators are left in the mode last set by the bootloader or
> >>by the kernel the device was rebooted from. This leads to various problems
> >>like non-working peripherals.
> >>
> >>Signed-off-by: Ivaylo Dimitrov <[email protected]>
> >>---
> >> arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >>diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> >>index b3c26a9..1bb36e2 100644
> >>--- a/arch/arm/boot/dts/omap3-n900.dts
> >>+++ b/arch/arm/boot/dts/omap3-n900.dts
> >>@@ -329,6 +329,7 @@
> >> regulator-name = "V28";
> >> regulator-min-microvolt = <2800000>;
> >> regulator-max-microvolt = <2800000>;
> >>+ regulator-initial-mode = <0x0e>;
> >> regulator-always-on; /* due to battery cover sensor */
> >> };
> >
> >I think this should either get an additional
> >comment like /* MODE_NORMAL */ or implemented
>
> According to the TRM, this is 'ACTIVE state', but that does not fit in the
> regulator framework terminology.

No problem with using STATE_ACTIVE or any other fitting description. IMHO
that "active" is misleading for regulators without "always-on/boot-on" tag,
but that was TI's decision.

> >using a define and a TWL4030_REGULATOR_MODE_NORMAL
> >constant to keep the *.dts easily readable.
>
> We already have RES_STATE_ACTIVE defined in linux/i2c/twl.h, is there a way
> to include that in a dts?

Not in its current state. During kernel build the C preprocessor is
applied on the *.dts files. So the header may only contain
preprocessor macros (e.g. #define). For that solution something like
~/src/linux/include/dt-bindings/regulator/maxim,max77802.h should
be created for the twl regulator.

-- Sebastian


Attachments:
(No filename) (1.88 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-17 14:29:51

by Ivaylo Dimitrov

[permalink] [raw]
Subject: [PATCH v1] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Without that, regulators are left in the mode last set by the bootloader or
by the kernel the device was rebooted from. This leads to various problems,
like non-working peripherals.

Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index b3c26a9..d9e2d9c 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -329,6 +329,7 @@
regulator-name = "V28";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
regulator-always-on; /* due to battery cover sensor */
};

@@ -336,30 +337,35 @@
regulator-name = "VCSI";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
};

&vaux3 {
regulator-name = "VMMC2_30";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
};

&vaux4 {
regulator-name = "VCAM_ANA_28";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
};

&vmmc1 {
regulator-name = "VMMC1";
regulator-min-microvolt = <1850000>;
regulator-max-microvolt = <3150000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
};

&vmmc2 {
regulator-name = "V28_A";
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
regulator-always-on; /* due VIO leak to AIC34 VDDs */
};

@@ -367,6 +373,7 @@
regulator-name = "VPLL";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
regulator-always-on;
};

@@ -374,6 +381,7 @@
regulator-name = "VSDI_CSI";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
regulator-always-on;
};

@@ -381,6 +389,7 @@
regulator-name = "VMMC2_IO_18";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
};

&vio {
--
1.9.1

2016-04-18 05:11:53

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v1] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Hi,

On Sun, Apr 17, 2016 at 05:29:23PM +0300, Ivaylo Dimitrov wrote:
> Without that, regulators are left in the mode last set by the bootloader or
> by the kernel the device was rebooted from. This leads to various problems,
> like non-working peripherals.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>

Reviewed-By: Sebastian Reichel <[email protected]>

-- Sebastian


Attachments:
(No filename) (387.00 B)
signature.asc (819.00 B)
Download all attachments

2016-04-19 21:18:37

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v1] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

Hello Ivaylo,

On Sun, Apr 17, 2016 at 10:29 AM, Ivaylo Dimitrov
<[email protected]> wrote:
> Without that, regulators are left in the mode last set by the bootloader or
> by the kernel the device was rebooted from. This leads to various problems,
> like non-working peripherals.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> ---
> arch/arm/boot/dts/omap3-n900.dts | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index b3c26a9..d9e2d9c 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -329,6 +329,7 @@
> regulator-name = "V28";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> + regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */

As Sebastian said, it would be nice if instead of magic numbers +
comments, we have defines for the modes by moving the definitions from
include/linux/i2c/twl.h to include/dt-bindings/regulator/ so they can
be used by DTS.

But that can be done as a follow-up though, to avoid adding a
dependency between the regulator and arm-soc subsystems to get this
fix applied.

Reviewed-by: Javier Martinez Canillas <[email protected]>

Best regards,
Javier

2016-04-24 10:08:35

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v1] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

On Sun 2016-04-17 17:29:23, Ivaylo Dimitrov wrote:
> Without that, regulators are left in the mode last set by the bootloader or
> by the kernel the device was rebooted from. This leads to various problems,
> like non-working peripherals.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>

Reviewed-by: Pavel Machek <[email protected]>

> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -329,6 +329,7 @@
> regulator-name = "V28";
> regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> + regulator-initial-mode = <0x0e>; /* RES_STATE_ACTIVE */
> regulator-always-on; /* due to battery cover sensor */
> };
>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2016-04-26 17:14:46

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v1] ARM: dts: omap3-n900: Specify peripherals LDO regulators initial mode

* Pavel Machek <[email protected]> [160424 03:09]:
> On Sun 2016-04-17 17:29:23, Ivaylo Dimitrov wrote:
> > Without that, regulators are left in the mode last set by the bootloader or
> > by the kernel the device was rebooted from. This leads to various problems,
> > like non-working peripherals.
> >
> > Signed-off-by: Ivaylo Dimitrov <[email protected]>
>
> Reviewed-by: Pavel Machek <[email protected]>

Thanks applying into omap-for-v4.6/fixes-rc5.

Tony