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@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(-)

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