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;
Date:   Thu, 25 Aug 2022 10:14:27 -0500
From:   Andrew Halaney <ahalaney@redhat.com>
To:     Douglas Anderson <dianders@chromium.org>
Cc:     Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [RFT PATCH v2 2/2] regulator: core: Don't err if allow-set-load
 but no allowed-modes
Message-ID: <20220825151427.dux3luxs6sxx5tno@halaneylaptop>
References: <20220824142229.RFT.v2.1.I137e6bef4f6d517be7b081be926059321102fd3d@changeid>
 <20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid>
Precedence: bulk

On Wed, Aug 24, 2022 at 02:22:57PM -0700, Douglas Anderson wrote:
> Apparently the device trees of some boards have the property
> "regulator-allow-set-load" for some of their regulators but then they
> don't specify anything for "regulator-allowed-modes". That's not
> really legit, but...
>
> ...before commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") they used to get away with it, at
> least on boards using RPMH regulators. That's because when a regulator
> driver implements set_load() then the core doesn't look at
> "regulator-allowed-modes" when trying to automatically adjust things
> in response to the regulator's load. The core doesn't know what mode
> we'll end up in, so how could it validate it?
>
> Said another way: before commit efb0cb50c427 ("regulator: qcom-rpmh:
> Implement get_optimum_mode(), not set_load()") some boards _were_
> having the regulator mode adjusted despite listing no allowed
> modes. After commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") these same boards were now
> getting an error returned when trying to use their regulators, since
> simply enabling a regulator tries to update its load and that was
> failing.
>
> We don't really want to go back to the behavior from before commit
> efb0cb50c427 ("regulator: qcom-rpmh: Implement get_optimum_mode(), not
> set_load()"). Boards shouldn't have been changing modes if no allowed
> modes were listed. However, the behavior after commit efb0cb50c427
> ("regulator: qcom-rpmh: Implement get_optimum_mode(), not set_load()")
> isn't the best because now boards can't even turn their regulators on.
>
> Let's choose to detect this case and return "no error" from
> drms_uA_update(). The net-result will be _different_ behavior than we
> had before commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()"), but this new behavior seems more
> correct. If a board truly needed the mode switched then its device
> tree should be updated to list the allowed modes.
>
> Reported-by: Andrew Halaney <ahalaney@redhat.com>
> Fixes: efb0cb50c427 ("regulator: qcom-rpmh: Implement get_optimum_mode(), not set_load()")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Tested-by: Andrew Halaney <ahalaney@redhat.com>

As you made clear in the commit message, a good number of boards will
have a change in behavior since efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") and associated fixes. I agree that
these devices are not properly described. Is there any sort of heads up we
should give? Just looking at the Qualcomm devicetrees for aarch64, I see all
of these are affected:

    apq8016-sbc.dts
    apq8096-db820c.dts
    apq8096-ifc6640.dts
    msm8916-alcatel-idol347.dts
    msm8916-asus-z00l.dts
    msm8916-huawei-g7.dts
    msm8916-longcheer-l8150.dts
    msm8916-longcheer-l8910.dts
    msm8916-samsung-a2015-common.dtsi
    msm8916-samsung-j5.dts
    msm8916-samsung-serranove.dts
    msm8916-wingtech-wt88047.dts
    msm8992-lg-bullhead.dtsi
    msm8992-xiaomi-libra.dts
    msm8994-msft-lumia-octagon.dtsi
    msm8994-sony-xperia-kitakami.dtsi
    msm8996-sony-xperia-tone.dtsi
    msm8996-xiaomi-common.dtsi
    msm8998-clamshell.dtsi
    msm8998-fxtec-pro1.dts
    msm8998-mtp.dts
    msm8998-oneplus-common.dtsi
    msm8998-sony-xperia-yoshino.dtsi
    sa8155p-adp.dts
    sa8xxxp-auto-adp.dtsi
    sc8280xp-crd.dts
    sc8280xp-lenovo-thinkpad-x13s.dts
    sda660-inforce-ifc6560.dts
    sdm630-sony-xperia-nile.dtsi
    sdm660-xiaomi-lavender.dts
    sm8150-sony-xperia-kumano.dtsi
    sm8250-sony-xperia-edo.dtsi
    sm8350-hdk.dts

Thanks,
Andrew