[PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[Douglas Anderson]

For a bunch of rails we really don't do anything with them in Linux.
These are things like modem voltage rails that the modem manages these
itself and core rails (like IO rails) that are setup to just
automagically do the right thing by the firmware.

Let's stop even listing those rails in our device tree.

The net result of this is that some of these rails might be able to go
down to a lower voltage or perhaps transition to LPM (low power mode)
sometimes.

Here's a list of what we're doing and why:

* L1A - only goes to SoC and doesn't seem associated with any
  particular peripheral. Kernel isn't doing anything with
  this. Removing from dts. NET IMPACT: rail might drop from 1.2V to
  1.178V and switch to LPM in some cases depending on firmware.
* L2A - only goes to SoC and doesn't seem associated with any
  particular peripheral. Kernel isn't doing anything with
  this. Removing from dts. NET IMPACT: rail might switch to LPM in
  some cases depending on firmware.
* L3A - only goes to SoC and doesn't seem associated with any
  particular peripheral. Kernel isn't doing anything with
  this. Removing from dts. NET IMPACT: rail might switch to LPM in
  some cases depending on firmware.
* L5A - seems to be totally unused as far as I can tell and doesn't
  even come off QSIP. Removing from dts.
* L6A - only goes to SoC and doesn't seem associated with any
  particular peripheral (I think?). Kernel isn't doing anything with
  this. Removing from dts. NET IMPACT: rail might switch to LPM in
  some cases depending on firmware.
* L16A - Looks like this is only used for internal RF stuff. Removing
  from dts. NET IMPACT: rail might switch to LPM in some cases
  depending on firmware.
* L1C - Just goes to WiFi / Bluetooth. Trust how IDP has this set and
  put this back at 1.616V min.
* L4C - This goes out to the eSIM among other places. This looks like
  it's intended to be for SIM card and modem manages. NET IMPACT:
  rail might switch to LPM in some cases depending on firmware.
* L5C - This goes to the physical SIM.  This looks like it's intended
  to be for SIM card and modem manages. NET IMPACT: rail might drop
  from 1.8V to 1.648V and switch to LPM in some cases depending on
  firmware.

NOTE: in general for anything which is supposed to be managed by Linux
I still left it all forced to HPM since I'm not 100% sure that all the
needed calls to regulator_set_load() are in place and HPM is safer.
Switching more things to LPM can happen in a future patch.

ALSO NOTE: Power measurements showed no measurable difference after
applying this patch, so perhaps it should be viewed more as a cleanup
than any power savings.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 82 ++------------------
 1 file changed, 7 insertions(+), 75 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 8ed7dd39f6e3..43dfe7833ad9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -298,40 +298,6 @@ pp1125_s1a: smps1 {
 			regulator-max-microvolt = <1128000>;
 		};
 
-		/*
-		 * pp2040_s5a (smps5) and pp1056_s4a (smps4) are just
-		 * inputs to other rails on AOP-managed PMICs on trogdor.
-		 * The system is already configured to manage these rails
-		 * automatically (enable when needed, adjust voltage for
-		 * headroom) so we won't specify anything here.
-		 *
-		 * NOTE: though the rails have a voltage implied by their
-		 * name, the automatic headroom calculation might not result
-		 * in them being that voltage.  ...and that's OK.
-		 * Specifically the only point of these rails is to provide
-		 * an input source for other rails and if we can satisify the
-		 * needs of those other rails with a lower source voltage then
-		 * we save power.
-		 */
-
-		pp1200_l1a: ldo1 {
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
-		pp1000_l2a: ldo2 {
-			regulator-min-microvolt = <944000>;
-			regulator-max-microvolt = <1056000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
-		pp1000_l3a: ldo3 {
-			regulator-min-microvolt = <968000>;
-			regulator-max-microvolt = <1064000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
 		vdd_qlink_lv:
 		vdd_qlink_lv_ck:
 		vdd_qusb_hs0_core:
@@ -350,24 +316,6 @@ pp900_l4a: ldo4 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
-		pp2700_l5a: ldo5 {
-			regulator-min-microvolt = <2704000>;
-			regulator-max-microvolt = <2704000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
-		ebi0_cal:
-		ebi1_cal:
-		vddio_ck_ebi0:
-		vddio_ck_ebi1:
-		vddio_ebi0:
-		vddq:
-		pp600_l6a: ldo6 {
-			regulator-min-microvolt = <568000>;
-			regulator-max-microvolt = <648000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
 		vdd_cx_wlan:
 		pp800_l9a: ldo9 {
 			regulator-min-microvolt = <488000>;
@@ -404,6 +352,11 @@ pp1800_l12a_r: ldo12 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
+		/*
+		 * On trogdor this needs to match l10a since we use it to
+		 * give power to things like SPI flash which communicate back
+		 * on lines powered by l10a.  Thus we force to 1.8V.
+		 */
 		pp1800_l13a: ldo13 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
@@ -424,12 +377,6 @@ pp1800_l15a: ldo15 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
-		pp2700_l16a: ldo16 {
-			regulator-min-microvolt = <2496000>;
-			regulator-max-microvolt = <3304000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
 		vdda_qusb_hs0_3p1:
 		vdd_pdphy:
 		pp3100_l17a: ldo17 {
@@ -463,8 +410,8 @@ pp1300_s8c: smps8 {
 		};
 
 		pp1800_l1c: ldo1 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
+			regulator-min-microvolt = <1616000>;
+			regulator-max-microvolt = <1984000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
@@ -491,21 +438,6 @@ pp1200_l3c: ldo3 {
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
 		};
 
-		ld_pp1800_esim_l4c:
-		vddpx_5:
-		pp1800_l4c: ldo4 {
-			regulator-min-microvolt = <1648000>;
-			regulator-max-microvolt = <3304000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
-		vddpx_6:
-		pp1800_l5c: ldo5 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-		};
-
 		vddpx_2:
 		ppvar_l6c: ldo6 {
 			regulator-min-microvolt = <1800000>;
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[Alexandru M Stan]

On Mon, Dec 7, 2020 at 2:33 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> For a bunch of rails we really don't do anything with them in Linux.
> These are things like modem voltage rails that the modem manages these
> itself and core rails (like IO rails) that are setup to just
> automagically do the right thing by the firmware.
>
> Let's stop even listing those rails in our device tree.
>
> The net result of this is that some of these rails might be able to go
> down to a lower voltage or perhaps transition to LPM (low power mode)
> sometimes.
>
> Here's a list of what we're doing and why:
>
> * L1A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might drop from 1.2V to
>   1.178V and switch to LPM in some cases depending on firmware.
> * L2A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L3A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L5A - seems to be totally unused as far as I can tell and doesn't
>   even come off QSIP. Removing from dts.
> * L6A - only goes to SoC and doesn't seem associated with any
>   particular peripheral (I think?). Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L16A - Looks like this is only used for internal RF stuff. Removing
>   from dts. NET IMPACT: rail might switch to LPM in some cases
>   depending on firmware.
> * L1C - Just goes to WiFi / Bluetooth. Trust how IDP has this set and
>   put this back at 1.616V min.
> * L4C - This goes out to the eSIM among other places. This looks like
>   it's intended to be for SIM card and modem manages. NET IMPACT:
>   rail might switch to LPM in some cases depending on firmware.
> * L5C - This goes to the physical SIM.  This looks like it's intended
>   to be for SIM card and modem manages. NET IMPACT: rail might drop
>   from 1.8V to 1.648V and switch to LPM in some cases depending on
>   firmware.
>
> NOTE: in general for anything which is supposed to be managed by Linux
> I still left it all forced to HPM since I'm not 100% sure that all the
> needed calls to regulator_set_load() are in place and HPM is safer.
> Switching more things to LPM can happen in a future patch.
>
> ALSO NOTE: Power measurements showed no measurable difference after
> applying this patch, so perhaps it should be viewed more as a cleanup
> than any power savings.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 82 ++------------------
>  1 file changed, 7 insertions(+), 75 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index 8ed7dd39f6e3..43dfe7833ad9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -298,40 +298,6 @@ pp1125_s1a: smps1 {
>                         regulator-max-microvolt = <1128000>;
>                 };
>
> -               /*
> -                * pp2040_s5a (smps5) and pp1056_s4a (smps4) are just
> -                * inputs to other rails on AOP-managed PMICs on trogdor.
> -                * The system is already configured to manage these rails
> -                * automatically (enable when needed, adjust voltage for
> -                * headroom) so we won't specify anything here.
> -                *
> -                * NOTE: though the rails have a voltage implied by their
> -                * name, the automatic headroom calculation might not result
> -                * in them being that voltage.  ...and that's OK.
> -                * Specifically the only point of these rails is to provide
> -                * an input source for other rails and if we can satisify the
> -                * needs of those other rails with a lower source voltage then
> -                * we save power.
> -                */
> -
> -               pp1200_l1a: ldo1 {
> -                       regulator-min-microvolt = <1200000>;
> -                       regulator-max-microvolt = <1200000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
> -               pp1000_l2a: ldo2 {
> -                       regulator-min-microvolt = <944000>;
> -                       regulator-max-microvolt = <1056000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
> -               pp1000_l3a: ldo3 {
> -                       regulator-min-microvolt = <968000>;
> -                       regulator-max-microvolt = <1064000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
>                 vdd_qlink_lv:
>                 vdd_qlink_lv_ck:
>                 vdd_qusb_hs0_core:
> @@ -350,24 +316,6 @@ pp900_l4a: ldo4 {
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> -               pp2700_l5a: ldo5 {
> -                       regulator-min-microvolt = <2704000>;
> -                       regulator-max-microvolt = <2704000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
> -               ebi0_cal:
> -               ebi1_cal:
> -               vddio_ck_ebi0:
> -               vddio_ck_ebi1:
> -               vddio_ebi0:
> -               vddq:
> -               pp600_l6a: ldo6 {
> -                       regulator-min-microvolt = <568000>;
> -                       regulator-max-microvolt = <648000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
>                 vdd_cx_wlan:
>                 pp800_l9a: ldo9 {
>                         regulator-min-microvolt = <488000>;
> @@ -404,6 +352,11 @@ pp1800_l12a_r: ldo12 {
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> +               /*
> +                * On trogdor this needs to match l10a since we use it to
> +                * give power to things like SPI flash which communicate back
> +                * on lines powered by l10a.  Thus we force to 1.8V.
> +                */
>                 pp1800_l13a: ldo13 {
>                         regulator-min-microvolt = <1800000>;
>                         regulator-max-microvolt = <1800000>;
> @@ -424,12 +377,6 @@ pp1800_l15a: ldo15 {
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> -               pp2700_l16a: ldo16 {
> -                       regulator-min-microvolt = <2496000>;
> -                       regulator-max-microvolt = <3304000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
>                 vdda_qusb_hs0_3p1:
>                 vdd_pdphy:
>                 pp3100_l17a: ldo17 {
> @@ -463,8 +410,8 @@ pp1300_s8c: smps8 {
>                 };
>
>                 pp1800_l1c: ldo1 {
> -                       regulator-min-microvolt = <1800000>;
> -                       regulator-max-microvolt = <1800000>;
> +                       regulator-min-microvolt = <1616000>;
> +                       regulator-max-microvolt = <1984000>;
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> @@ -491,21 +438,6 @@ pp1200_l3c: ldo3 {
>                         regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>                 };
>
> -               ld_pp1800_esim_l4c:
> -               vddpx_5:
> -               pp1800_l4c: ldo4 {
> -                       regulator-min-microvolt = <1648000>;
> -                       regulator-max-microvolt = <3304000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
> -               vddpx_6:
> -               pp1800_l5c: ldo5 {
> -                       regulator-min-microvolt = <1800000>;
> -                       regulator-max-microvolt = <1800000>;
> -                       regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -               };
> -
>                 vddpx_2:
>                 ppvar_l6c: ldo6 {
>                         regulator-min-microvolt = <1800000>;
> --
> 2.29.2.576.ga3fc446d84-goog
>

Reviewed-by: Alexandru M Stan <amstan@google.com>

Re: [PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[Doug Anderson]

Hi Bjorn,

On Mon, Dec 7, 2020 at 2:33 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> For a bunch of rails we really don't do anything with them in Linux.
> These are things like modem voltage rails that the modem manages these
> itself and core rails (like IO rails) that are setup to just
> automagically do the right thing by the firmware.
>
> Let's stop even listing those rails in our device tree.
>
> The net result of this is that some of these rails might be able to go
> down to a lower voltage or perhaps transition to LPM (low power mode)
> sometimes.
>
> Here's a list of what we're doing and why:
>
> * L1A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might drop from 1.2V to
>   1.178V and switch to LPM in some cases depending on firmware.
> * L2A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L3A - only goes to SoC and doesn't seem associated with any
>   particular peripheral. Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L5A - seems to be totally unused as far as I can tell and doesn't
>   even come off QSIP. Removing from dts.
> * L6A - only goes to SoC and doesn't seem associated with any
>   particular peripheral (I think?). Kernel isn't doing anything with
>   this. Removing from dts. NET IMPACT: rail might switch to LPM in
>   some cases depending on firmware.
> * L16A - Looks like this is only used for internal RF stuff. Removing
>   from dts. NET IMPACT: rail might switch to LPM in some cases
>   depending on firmware.
> * L1C - Just goes to WiFi / Bluetooth. Trust how IDP has this set and
>   put this back at 1.616V min.
> * L4C - This goes out to the eSIM among other places. This looks like
>   it's intended to be for SIM card and modem manages. NET IMPACT:
>   rail might switch to LPM in some cases depending on firmware.
> * L5C - This goes to the physical SIM.  This looks like it's intended
>   to be for SIM card and modem manages. NET IMPACT: rail might drop
>   from 1.8V to 1.648V and switch to LPM in some cases depending on
>   firmware.
>
> NOTE: in general for anything which is supposed to be managed by Linux
> I still left it all forced to HPM since I'm not 100% sure that all the
> needed calls to regulator_set_load() are in place and HPM is safer.
> Switching more things to LPM can happen in a future patch.
>
> ALSO NOTE: Power measurements showed no measurable difference after
> applying this patch, so perhaps it should be viewed more as a cleanup
> than any power savings.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 82 ++------------------
>  1 file changed, 7 insertions(+), 75 deletions(-)

We've been running with this in the downstream tree since December 8th
and nobody has yelled.  You can see <https://crrev.com/c/2573506>.  Is
it a good time for it to land upstream?

Thanks!

-Doug

Re: [PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[Bjorn Andersson]

On Mon 11 Jan 15:48 CST 2021, Doug Anderson wrote:

> Hi Bjorn,
> 
> On Mon, Dec 7, 2020 at 2:33 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > For a bunch of rails we really don't do anything with them in Linux.
> > These are things like modem voltage rails that the modem manages these
> > itself and core rails (like IO rails) that are setup to just
> > automagically do the right thing by the firmware.
> >
> > Let's stop even listing those rails in our device tree.
> >
> > The net result of this is that some of these rails might be able to go
> > down to a lower voltage or perhaps transition to LPM (low power mode)
> > sometimes.
> >
> > Here's a list of what we're doing and why:
> >
> > * L1A - only goes to SoC and doesn't seem associated with any
> >   particular peripheral. Kernel isn't doing anything with
> >   this. Removing from dts. NET IMPACT: rail might drop from 1.2V to
> >   1.178V and switch to LPM in some cases depending on firmware.
> > * L2A - only goes to SoC and doesn't seem associated with any
> >   particular peripheral. Kernel isn't doing anything with
> >   this. Removing from dts. NET IMPACT: rail might switch to LPM in
> >   some cases depending on firmware.
> > * L3A - only goes to SoC and doesn't seem associated with any
> >   particular peripheral. Kernel isn't doing anything with
> >   this. Removing from dts. NET IMPACT: rail might switch to LPM in
> >   some cases depending on firmware.
> > * L5A - seems to be totally unused as far as I can tell and doesn't
> >   even come off QSIP. Removing from dts.
> > * L6A - only goes to SoC and doesn't seem associated with any
> >   particular peripheral (I think?). Kernel isn't doing anything with
> >   this. Removing from dts. NET IMPACT: rail might switch to LPM in
> >   some cases depending on firmware.
> > * L16A - Looks like this is only used for internal RF stuff. Removing
> >   from dts. NET IMPACT: rail might switch to LPM in some cases
> >   depending on firmware.
> > * L1C - Just goes to WiFi / Bluetooth. Trust how IDP has this set and
> >   put this back at 1.616V min.
> > * L4C - This goes out to the eSIM among other places. This looks like
> >   it's intended to be for SIM card and modem manages. NET IMPACT:
> >   rail might switch to LPM in some cases depending on firmware.
> > * L5C - This goes to the physical SIM.  This looks like it's intended
> >   to be for SIM card and modem manages. NET IMPACT: rail might drop
> >   from 1.8V to 1.648V and switch to LPM in some cases depending on
> >   firmware.
> >
> > NOTE: in general for anything which is supposed to be managed by Linux
> > I still left it all forced to HPM since I'm not 100% sure that all the
> > needed calls to regulator_set_load() are in place and HPM is safer.
> > Switching more things to LPM can happen in a future patch.
> >
> > ALSO NOTE: Power measurements showed no measurable difference after
> > applying this patch, so perhaps it should be viewed more as a cleanup
> > than any power savings.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 82 ++------------------
> >  1 file changed, 7 insertions(+), 75 deletions(-)
> 
> We've been running with this in the downstream tree since December 8th
> and nobody has yelled.  You can see <https://crrev.com/c/2573506>.  Is
> it a good time for it to land upstream?
> 

Sure thing, I will pick it up. Thanks for the ping!

Regards,
Bjorn

Re: [PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails

[patchwork-bot+linux-arm-msm]

Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Mon,  7 Dec 2020 14:33:02 -0800 you wrote:
> For a bunch of rails we really don't do anything with them in Linux.
> These are things like modem voltage rails that the modem manages these
> itself and core rails (like IO rails) that are setup to just
> automagically do the right thing by the firmware.
> 
> Let's stop even listing those rails in our device tree.
> 
> [...]

Here is the summary with links:
  - arm64: dts: qcom: Clean up sc7180-trogdor voltage rails
    https://git.kernel.org/qcom/c/e5376f2ea2e2

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html