On Thu, 21 Jul 2022 at 13:31, Johan Hovold <johan@xxxxxxxxxx> wrote: > > On Tue, Jul 05, 2022 at 09:29:13AM -0700, Kuogee Hsieh wrote: > > 0) rebase on https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tree > > 1) add regulator_set_load() to eDP phy > > 2) add regulator_set_load() to DP phy > > 3) remove vdda related function out of eDP/DP controller > > > > Kuogee Hsieh (3): > > phy: qcom-edp: add regulator_set_load to edp phy > > phy: qcom-qmp: add regulator_set_load to dp phy > > drm/msm/dp: delete vdda regulator related functions from eDP/DP > > controller > > > > drivers/gpu/drm/msm/dp/dp_parser.c | 14 ----- > > drivers/gpu/drm/msm/dp/dp_parser.h | 8 --- > > drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------ > > drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++ > > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 ++++++++++--- > > 5 files changed, 46 insertions(+), 124 deletions(-) > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > where the DP PHY regulators are shared with other PHYs whose drivers do > not request a load. I'm trying to understand, why does this series cause the regression. Previously it would be the DP controller voting on the regulators load, now it has been moved to the DP/eDP PHYs. > Specifically, the hard-coded vdda-phy load of 21.8 mA added by this > series, causes several RPMh regulators to now be put in low-power mode. > > I found Doug's suggestion to handle situations like this in regulator > core: > > https://lore.kernel.org/all/20180814170617.100087-1-dianders@xxxxxxxxxxxx/ > > but since that was rejected, how do we deal with this generally? > > In the above thread Doug mentioned adding support for load requests to > further drivers and Bjorn mentioned working around it by adding > regulator-system-load properties to DT. > > It seems quite likely that changes like this one affects other systems > too, and the effects may be hard to debug. So a more general solution > than playing whack-a-mole using DT would be good to have. I think enabling a regulator which supports set_load() and without load being set should cause a warning, at least with CONFIG_REGULATOR_DEBUG. WDYT? -- With best wishes Dmitry
