Re: [PATCH] arm64: dts: qcom: Clean up sc7180-trogdor voltage rails
From: Alexandru M Stan
Date: Tue Dec 08 2020 - 20:09:33 EST
On Mon, Dec 7, 2020 at 2:33 PM Douglas Anderson <dianders@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxx>