linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
@ 2014-01-06 15:25 Thierry Reding
  2014-01-06 20:59 ` Stephen Warren
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2014-01-06 15:25 UTC (permalink / raw)
  To: linux-arm-kernel

GPIO 1 and 2 of the PMIC are not used for the described purpose, so
remove them.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Note: Removing these makes the work-in-progress eDP support work again.
---
 arch/arm/boot/dts/tegra124-venice2.dts | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c6dcef513e5d..4b3f3658e7ff 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -980,28 +980,6 @@
 			regulator-always-on;
 		};
 
-		vdd_3v3_reg: regulator at 1 {
-			compatible = "regulator-fixed";
-			reg = <1>;
-			regulator-name = "vdd_3v3";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-			regulator-boot-on;
-			enable-active-high;
-			gpio = <&as3722 1 GPIO_ACTIVE_HIGH>;
-		};
-
-		vdd_3v3_modem_reg: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "vdd-modem-3v3";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			enable-active-high;
-			gpio = <&as3722 2 GPIO_ACTIVE_HIGH>;
-		};
-
 		vdd_hdmi_5v0_reg: regulator at 3 {
 			compatible = "regulator-fixed";
 			reg = <3>;
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
  2014-01-06 15:25 [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators Thierry Reding
@ 2014-01-06 20:59 ` Stephen Warren
  2014-01-07 12:14   ` Laxman Dewangan
  2014-01-08 13:41   ` Thierry Reding
  0 siblings, 2 replies; 6+ messages in thread
From: Stephen Warren @ 2014-01-06 20:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/06/2014 08:25 AM, Thierry Reding wrote:
> GPIO 1 and 2 of the PMIC are not used for the described purpose, so
> remove them.

As far as I can tell, this patch is correct, since those GPIOs are in
fact used to discharge the rails after disabling them, rather than to
enable/disable the rails.

Equally, these GPIOs affect multiple rails at once, so listing the GPIO
as a property of a single regulator seems wrong either way.

However, PMU_REGEN1 does seem to feed the "EN" pin of U13C1, a DC/DC
switcher for power rail 3.3v_modem, so perhaps there's more going on
here than I see?

In summary, I need Laxman to comment on this and ack the change, and
explain why these GPIOs were listed as regulator enables when it doesn't
seem that they are.

> Note: Removing these makes the work-in-progress eDP support work again.

> diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts

> -		vdd_3v3_reg: regulator at 1 {
> -			compatible = "regulator-fixed";
> -			reg = <1>;
> -			regulator-name = "vdd_3v3";
> -			regulator-min-microvolt = <3300000>;
> -			regulator-max-microvolt = <3300000>;
> -			regulator-always-on;
> -			regulator-boot-on;
> -			enable-active-high;
> -			gpio = <&as3722 1 GPIO_ACTIVE_HIGH>;
> -		};
> -
> -		vdd_3v3_modem_reg: regulator at 2 {
> -			compatible = "regulator-fixed";
> -			reg = <2>;
> -			regulator-name = "vdd-modem-3v3";
> -			regulator-min-microvolt = <3300000>;
> -			regulator-max-microvolt = <3300000>;
> -			enable-active-high;
> -			gpio = <&as3722 2 GPIO_ACTIVE_HIGH>;
> -		};

Don't you want to simply remove the "enable-active-high" and "gpio"
properties, but leave the regulator definitions present, in case
something wants to reference these fixed(?) rails as their supply?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
  2014-01-06 20:59 ` Stephen Warren
@ 2014-01-07 12:14   ` Laxman Dewangan
  2014-01-07 16:43     ` Stephen Warren
  2014-01-08 13:41   ` Thierry Reding
  1 sibling, 1 reply; 6+ messages in thread
From: Laxman Dewangan @ 2014-01-07 12:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 07 January 2014 02:29 AM, Stephen Warren wrote:
> On 01/06/2014 08:25 AM, Thierry Reding wrote:
>> GPIO 1 and 2 of the PMIC are not used for the described purpose, so
>> remove them.
> As far as I can tell, this patch is correct, since those GPIOs are in
> fact used to discharge the rails after disabling them, rather than to
> enable/disable the rails.
>
> Equally, these GPIOs affect multiple rails at once, so listing the GPIO
> as a property of a single regulator seems wrong either way.
>
> However, PMU_REGEN1 does seem to feed the "EN" pin of U13C1, a DC/DC
> switcher for power rail 3.3v_modem, so perhaps there's more going on
> here than I see?
>
> In summary, I need Laxman to comment on this and ack the change, and
> explain why these GPIOs were listed as regulator enables when it doesn't
> seem that they are.
>
PMU_REGEN1 is going to U13C1 (DCDC switcher). So if EN is 0, the output 
will be 0 and when it is 1, the output will be 3.3V.
Because this is coming from GPIO, it is added as the fixed regulator.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
  2014-01-07 12:14   ` Laxman Dewangan
@ 2014-01-07 16:43     ` Stephen Warren
  2014-01-08 13:04       ` Laxman Dewangan
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2014-01-07 16:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/07/2014 05:14 AM, Laxman Dewangan wrote:
> On Tuesday 07 January 2014 02:29 AM, Stephen Warren wrote:
>> On 01/06/2014 08:25 AM, Thierry Reding wrote:
>>> GPIO 1 and 2 of the PMIC are not used for the described purpose, so
>>> remove them.
>> As far as I can tell, this patch is correct, since those GPIOs are in
>> fact used to discharge the rails after disabling them, rather than to
>> enable/disable the rails.
>>
>> Equally, these GPIOs affect multiple rails at once, so listing the GPIO
>> as a property of a single regulator seems wrong either way.
>>
>> However, PMU_REGEN1 does seem to feed the "EN" pin of U13C1, a DC/DC
>> switcher for power rail 3.3v_modem, so perhaps there's more going on
>> here than I see?
>>
>> In summary, I need Laxman to comment on this and ack the change, and
>> explain why these GPIOs were listed as regulator enables when it doesn't
>> seem that they are.
>>
> PMU_REGEN1 is going to U13C1 (DCDC switcher). So if EN is 0, the output
> will be 0 and when it is 1, the output will be 3.3V.
> Because this is coming from GPIO, it is added as the fixed regulator.

What about PMU_REGEN3, for which that isn't the case, and what about the
fact that PMU_REGEN1 does a lot more than just enable that DC/DC switcher?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
  2014-01-07 16:43     ` Stephen Warren
@ 2014-01-08 13:04       ` Laxman Dewangan
  0 siblings, 0 replies; 6+ messages in thread
From: Laxman Dewangan @ 2014-01-08 13:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 07 January 2014 10:13 PM, Stephen Warren wrote:
> On 01/07/2014 05:14 AM, Laxman Dewangan wrote:
>> On Tuesday 07 January 2014 02:29 AM, Stephen Warren wrote:
>>> On 01/06/2014 08:25 AM, Thierry Reding wrote:
>>>> GPIO 1 and 2 of the PMIC are not used for the described purpose, so
>>>> remove them.
>>> As far as I can tell, this patch is correct, since those GPIOs are in
>>> fact used to discharge the rails after disabling them, rather than to
>>> enable/disable the rails.
>>>
>>> Equally, these GPIOs affect multiple rails at once, so listing the GPIO
>>> as a property of a single regulator seems wrong either way.
>>>
>>> However, PMU_REGEN1 does seem to feed the "EN" pin of U13C1, a DC/DC
>>> switcher for power rail 3.3v_modem, so perhaps there's more going on
>>> here than I see?
>>>
>>> In summary, I need Laxman to comment on this and ack the change, and
>>> explain why these GPIOs were listed as regulator enables when it doesn't
>>> seem that they are.
>>>
>> PMU_REGEN1 is going to U13C1 (DCDC switcher). So if EN is 0, the output
>> will be 0 and when it is 1, the output will be 3.3V.
>> Because this is coming from GPIO, it is added as the fixed regulator.
> What about PMU_REGEN3, for which that isn't the case, and what about the
> fact that PMU_REGEN1 does a lot more than just enable that DC/DC switcher?
>
Based on my understanding with schematics:

PMU_REGEN1;
if high then PMU_REGEN1_L is 0 and so Q20B6, Q20B7, Q20B8 are open and 
all rails are not connected to ground through their register.
If Low then PMUC_REGEN1_L is 1 and so Q20B6, Q20B7, Q20B8 are connected 
to D to S and all rails are discharged through the register.
This means we need to keep PMU_REGN1 to high for using the rails in 
normal case.

Similar for the PMU_REGEN3.

PMU_REGEN1 is also enabling the U13C1 when it is High then output will 
have voltage otherowise Vout = 0.
PMU_REGEN1 is feeding to U20A6, which works as sequencer.
The PMU_REGEN1 is expected to be always High (even in LP0) so that it 
can keep the some rail ON on LP0 also. (Based on comment)

The PMU_REGEN3 is high in normal case and should be in low for LP0 so 
that it can off the rails in LP0.

So regulator, involved with this should have the always ON property 
enabled. If the gpio need to be disable during LP0 then it need to 
written explicitly in suspend/resume.

BTW, how this is effecting the edp functionality? Does EDP expect the 
level of this GPIOs to be 0?
Anything I can debug here to narrow-down the issue? Probably we need to 
scope the gpio status?
Also if you can pass the register dump in wokring and non-working case 
then it will be very easy to analyse the issue.


Thanks,
Laxman

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators
  2014-01-06 20:59 ` Stephen Warren
  2014-01-07 12:14   ` Laxman Dewangan
@ 2014-01-08 13:41   ` Thierry Reding
  1 sibling, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2014-01-08 13:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 06, 2014 at 01:59:27PM -0700, Stephen Warren wrote:
> On 01/06/2014 08:25 AM, Thierry Reding wrote:
> > GPIO 1 and 2 of the PMIC are not used for the described purpose, so
> > remove them.
> 
> As far as I can tell, this patch is correct, since those GPIOs are in
> fact used to discharge the rails after disabling them, rather than to
> enable/disable the rails.
> 
> Equally, these GPIOs affect multiple rails at once, so listing the GPIO
> as a property of a single regulator seems wrong either way.
> 
> However, PMU_REGEN1 does seem to feed the "EN" pin of U13C1, a DC/DC
> switcher for power rail 3.3v_modem, so perhaps there's more going on
> here than I see?
> 
> In summary, I need Laxman to comment on this and ack the change, and
> explain why these GPIOs were listed as regulator enables when it doesn't
> seem that they are.
> 
> > Note: Removing these makes the work-in-progress eDP support work again.
> 
> > diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
> 
> > -		vdd_3v3_reg: regulator at 1 {
> > -			compatible = "regulator-fixed";
> > -			reg = <1>;
> > -			regulator-name = "vdd_3v3";
> > -			regulator-min-microvolt = <3300000>;
> > -			regulator-max-microvolt = <3300000>;
> > -			regulator-always-on;
> > -			regulator-boot-on;
> > -			enable-active-high;
> > -			gpio = <&as3722 1 GPIO_ACTIVE_HIGH>;
> > -		};
> > -
> > -		vdd_3v3_modem_reg: regulator at 2 {
> > -			compatible = "regulator-fixed";
> > -			reg = <2>;
> > -			regulator-name = "vdd-modem-3v3";
> > -			regulator-min-microvolt = <3300000>;
> > -			regulator-max-microvolt = <3300000>;
> > -			enable-active-high;
> > -			gpio = <&as3722 2 GPIO_ACTIVE_HIGH>;
> > -		};
> 
> Don't you want to simply remove the "enable-active-high" and "gpio"
> properties, but leave the regulator definitions present, in case
> something wants to reference these fixed(?) rails as their supply?

Yes, I guess that could probably work.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140108/4a37ae61/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-08 13:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-06 15:25 [PATCH] ARM: tegra: Remove 3.3V supply and modem regulators Thierry Reding
2014-01-06 20:59 ` Stephen Warren
2014-01-07 12:14   ` Laxman Dewangan
2014-01-07 16:43     ` Stephen Warren
2014-01-08 13:04       ` Laxman Dewangan
2014-01-08 13:41   ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).