Hi, On Thu, Jul 21, 2022 at 6:25 AM Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote: > > > 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. I think in the past not many device trees actually hooked up the regulators to the DP/eDP but did hook up the regulators to the PHYs? That means they didn't used to get a regulator_set_load() on them and now they do? It should also be noted that we're now setting the load for a bunch of USB PHYs that we didn't used to set a load on... > > 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? I'm not a total fan. I'd love to see evidence to the contrary, but I'm a believer that the amount of extra cruft involved with all these regulator_set_load() calls is overkill for most cases. All the extra code / per-SoC tables added to drivers isn't ideal. Every single LDO on Qualcomm's PMICs seems to be able to be set in "high power mode" and "low power mode", but I think the majority of clients only really care about two things: on and in high power mode vs. off. I think the amount of stuff peripherals can do in low power mode is super limited, so you have to be _really_ sure that the peripheral won't draw too much current without you having a chance to reconfigure the regulator. Of course, if the load could be easily specified in the device tree and we could get rid of all the cruft in the drivers then maybe that would be OK. -Doug
