[PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Andrzej Hajda]

LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
5433 SoC. Disabling it causes serious current leak - about 200mA.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 5f1e172..b22bec8 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -432,6 +432,10 @@
 				regulator-name = "VDD18_MIPI2L_1.8V_AP";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo8_reg: LDO8 {
-- 
2.7.4

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Krzysztof Kozlowski]

On Tue, Jan 24, 2017 at 01:45:28PM +0100, Andrzej Hajda wrote:
> LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
> 5433 SoC. Disabling it causes serious current leak - about 200mA.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 5f1e172..b22bec8 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -432,6 +432,10 @@
>  				regulator-name = "VDD18_MIPI2L_1.8V_AP";
>  				regulator-min-microvolt = <1800000>;
>  				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};

Thanks, applied.

Just to satisfy my curiosity, how disabling the regulator causes current
leak? What happens exactly?

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Javier Martinez Canillas]

Hello Krzysztof,

On 01/24/2017 02:52 PM, Krzysztof Kozlowski wrote:
> On Tue, Jan 24, 2017 at 01:45:28PM +0100, Andrzej Hajda wrote:
>> LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
>> 5433 SoC. Disabling it causes serious current leak - about 200mA.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index 5f1e172..b22bec8 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -432,6 +432,10 @@
>>  				regulator-name = "VDD18_MIPI2L_1.8V_AP";
>>  				regulator-min-microvolt = <1800000>;
>>  				regulator-max-microvolt = <1800000>;

I see you already applied, but I think it would be good to have some comments
here explaining why the regulator should be always-on. Otherwise, in future a
developer might want to disable the regulator since it may think that is not
needed due dsi and hdmi already enabling it.

>> +				regulator-always-on;
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
> 
> Thanks, applied.
>
> Just to satisfy my curiosity, how disabling the regulator causes current
> leak? What happens exactly?
>

I was wondering the same.

> Best regards,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Krzysztof Kozlowski]

On Tue, Jan 24, 2017 at 02:57:00PM -0300, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 01/24/2017 02:52 PM, Krzysztof Kozlowski wrote:
> > On Tue, Jan 24, 2017 at 01:45:28PM +0100, Andrzej Hajda wrote:
> >> LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
> >> 5433 SoC. Disabling it causes serious current leak - about 200mA.
> >>
> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >> ---
> >>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> index 5f1e172..b22bec8 100644
> >> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> >> @@ -432,6 +432,10 @@
> >>  				regulator-name = "VDD18_MIPI2L_1.8V_AP";
> >>  				regulator-min-microvolt = <1800000>;
> >>  				regulator-max-microvolt = <1800000>;
> 
> I see you already applied, but I think it would be good to have some comments
> here explaining why the regulator should be always-on. Otherwise, in future a
> developer might want to disable the regulator since it may think that is not
> needed due dsi and hdmi already enabling it.

It is a good idea however we never put such information for other
regulators... If agreed, we might make it a requirement for new DTS and
new changes.

Anyway one could get this from the log itself.

> 
> >> +				regulator-always-on;
> >> +				regulator-state-mem {
> >> +					regulator-off-in-suspend;
> >> +				};
> > 
> > Thanks, applied.
> >
> > Just to satisfy my curiosity, how disabling the regulator causes current
> > leak? What happens exactly?
> >
> 
> I was wondering the same.
> 

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Marek Szyprowski]

Hi Javier,


On 2017-01-24 18:57, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>
> On 01/24/2017 02:52 PM, Krzysztof Kozlowski wrote:
>> On Tue, Jan 24, 2017 at 01:45:28PM +0100, Andrzej Hajda wrote:
>>> LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
>>> 5433 SoC. Disabling it causes serious current leak - about 200mA.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> index 5f1e172..b22bec8 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> @@ -432,6 +432,10 @@
>>>   				regulator-name = "VDD18_MIPI2L_1.8V_AP";
>>>   				regulator-min-microvolt = <1800000>;
>>>   				regulator-max-microvolt = <1800000>;
> I see you already applied, but I think it would be good to have some comments
> here explaining why the regulator should be always-on. Otherwise, in future a
> developer might want to disable the regulator since it may think that is not
> needed due dsi and hdmi already enabling it.

If such change is added in a separate commit, one can easily find the reason
using "git blame".

>>> +				regulator-always-on;
>>> +				regulator-state-mem {
>>> +					regulator-off-in-suspend;
>>> +				};
>> Thanks, applied.
>>
>> Just to satisfy my curiosity, how disabling the regulator causes current
>> leak? What happens exactly?

Such leakage usually happens if there is a hw block, which have more 
than one
power supply. Disabling power for the one power supply might turn the hw 
logic
into some meta state, in which it still conduct a current from the other 
source.

The best example of such behavior is Odroid U3 and its problems with 
reset when
HDMI cable is connected. Sometimes the current leaks from the HDMI 
connector to
the board when the main power supply is turned off and the board enters some
kind of a a "meta state".

We still have no idea which hw block causes this leakage in case of TM2/TM2e
though.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Krzysztof Kozlowski]

On Wed, Jan 25, 2017 at 9:13 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Such leakage usually happens if there is a hw block, which have more than
> one
> power supply. Disabling power for the one power supply might turn the hw
> logic
> into some meta state, in which it still conduct a current from the other
> source.
>
> The best example of such behavior is Odroid U3 and its problems with reset
> when
> HDMI cable is connected. Sometimes the current leaks from the HDMI connector
> to
> the board when the main power supply is turned off and the board enters some
> kind of a a "meta state".
>
> We still have no idea which hw block causes this leakage in case of TM2/TM2e
> though.

Interesting topic... Thanks for explanation!

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: exynos: set LDO7 regulator as always on

[Javier Martinez Canillas]

Hello Marek,

On 01/25/2017 04:13 AM, Marek Szyprowski wrote:
> Hi Javier,
> 
> 
> On 2017-01-24 18:57, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>
>> On 01/24/2017 02:52 PM, Krzysztof Kozlowski wrote:
>>> On Tue, Jan 24, 2017 at 01:45:28PM +0100, Andrzej Hajda wrote:
>>>> LDO7 regulator beside DSI and HDMI provides power for core blocks in Exynos
>>>> 5433 SoC. Disabling it causes serious current leak - about 200mA.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>>> index 5f1e172..b22bec8 100644
>>>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>>> @@ -432,6 +432,10 @@
>>>>                   regulator-name = "VDD18_MIPI2L_1.8V_AP";
>>>>                   regulator-min-microvolt = <1800000>;
>>>>                   regulator-max-microvolt = <1800000>;
>> I see you already applied, but I think it would be good to have some comments
>> here explaining why the regulator should be always-on. Otherwise, in future a
>> developer might want to disable the regulator since it may think that is not
>> needed due dsi and hdmi already enabling it.
> 
> If such change is added in a separate commit, one can easily find the reason
> using "git blame".
>

Yes, that's true.
 
>>>> +                regulator-always-on;
>>>> +                regulator-state-mem {
>>>> +                    regulator-off-in-suspend;
>>>> +                };
>>> Thanks, applied.
>>>
>>> Just to satisfy my curiosity, how disabling the regulator causes current
>>> leak? What happens exactly?
> 
> Such leakage usually happens if there is a hw block, which have more than one
> power supply. Disabling power for the one power supply might turn the hw logic
> into some meta state, in which it still conduct a current from the other source.
> 
> The best example of such behavior is Odroid U3 and its problems with reset when
> HDMI cable is connected. Sometimes the current leaks from the HDMI connector to
> the board when the main power supply is turned off and the board enters some
> kind of a a "meta state".
> 

I didn't know about all this, thanks a lot for the explanation.

> We still have no idea which hw block causes this leakage in case of TM2/TM2e
> though.
> 
> Best regards

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America