Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220803123332.8138-1-johan+linaro@kernel.org>
In-Reply-To: <20220803123332.8138-1-johan+linaro@kernel.org>
From:   Doug Anderson <dianders@chromium.org>
Date:   Wed, 3 Aug 2022 08:30:09 -0700
Message-ID: <CAD=FV=VoFgMj9iOx1HR72AcnTi9OOSCOFm39kKOGXHkeb2SYBg@mail.gmail.com>
Subject: Re: [PATCH 0/2] phy: qcom: drop regulator loads
To:     Johan Hovold <johan+linaro@kernel.org>
Cc:     Vinod Koul <vkoul@kernel.org>, Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Kuogee Hsieh <quic_khsieh@quicinc.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        linux-phy@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Wed, Aug 3, 2022 at 5:34 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Unless a driver implements an idle mode, there's generally no point in
> specifying an active-mode regulator load.
>
> Drop the regulator loads that were recently added to the Qualcomm QMP
> combo and edp PHY drivers.
>
> For a background discussion on this matter, see the following thread:
>
>         https://lore.kernel.org/r/YuPps+cvVAMugWmy@sirena.org.uk
>
> Johan
>
>
> Johan Hovold (2):
>   phy: qcom-qmp-combo: drop regulator loads
>   phy: qcom-edp: drop regulator loads
>
>  drivers/phy/qualcomm/phy-qcom-edp.c       | 12 -------
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 40 +++++------------------
>  2 files changed, 9 insertions(+), 43 deletions(-)

It's really hard to evaluate this based on the information I have
available to me. :( I'm all for getting rid of all this complexity if
it was added for no reason and I could definitely believe that on most
boards there is no reason for it at all as talked about in other
threads.

I guess I worry that there is some use case where LPM mode is actually
usable to power these devices when they're active. It seems like
_maybe_ it could be but only if nothing else is pulling power from the
same LDO? Some LDOs on the board I have seem to be able to do LPM up
to 30 mA and some of the rails are being specified as ~22mA.

The problem with regulator loads is that using them is kinda an "all
or nothing". Either all the consumers need to specify something or
none of them can. :( This means that once the first user comes in and
is able to run the device in LPM (maybe only if they're the only
consumer?) that everything will break. I honestly have no idea if this
will ever happen, though... Mark said the phrase "actively managing
loads it's probably not doing anything useful" and I think "probably"
is an important word there. If that word was "never" then it would
definitely be OK to remove load management like this, but with
"probably" it becomes a lot harder.

If we needed a hack, I'd somewhat prefer a hack that just bumps the
"mA" value here up to something higher. That would force it to HPM...
...although maybe it still won't work? Then the regulator will still
go down to LPM for other consumers if the PHY ever turns off. In that
case I guess there's no getting around other consumers requesting the
load or finding some way to say that on your board this regulator can
only ever be in HPM mode.


-Doug