Messages in this thread | | | Date | Thu, 25 Aug 2022 12:00:41 -0500 | From | Andrew Halaney <> | Subject | Re: [RFT PATCH v2 2/2] regulator: core: Don't err if allow-set-load but no allowed-modes |
| |
On Thu, Aug 25, 2022 at 09:43:45AM -0700, Doug Anderson wrote: > Hi, > > On Thu, Aug 25, 2022 at 8:14 AM Andrew Halaney <ahalaney@redhat.com> wrote: > > > > 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 > > True, it would be a good idea to send out fixes. OK, so let's see. We > can probably get fairly close to seeing who is affected with these > greps: > > rpmh_users=$(git grep -l -i rpmh -- arch/arm*/boot/dts/qcom) > set_modes=$(grep -l regulator-allow-set-load ${rpmh_users}) > but_no_allowed_modes=$(grep -l -v regulator-allowed-modes ${set_modes}) > > That actually gives a (much)shorter list than yours. Why? > > Ah. Your list includes not just RPMH users but also RPM users. RPM > users _won't_ be affected. In RPM regulators we don't actually track > the modes in the kernel--we actually pass the load directly to the > remote processor and it handles translating that into loads. RPM > regulators don't even have a way to directly set the mode. > > ...so we only need to fix a small number (7) boards. > > Posting up patches now. OK, the cover letter should show up shortly at: > > https://lore.kernel.org/r/20220825164205.4060647-1-dianders@chromium.org > > -Doug >
Thanks for the grep-fu, I didn't even think about RPM vs RPMH, I will try and review those today!
- Andrew
| |