On Mon, Nov 18, 2024 at 02:24:33AM +0000, Bryan O'Donoghue wrote: > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c [..] > @@ -177,10 +182,14 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status, > ret = gdsc_poll_status(sc, status); > WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n"); > > - if (!ret && status == GDSC_OFF && sc->rsupply) { > - ret = regulator_disable(sc->rsupply); > - if (ret < 0) > - return ret; > + if (!ret && status == GDSC_OFF) { > + if (pm_runtime_enabled(sc->dev)) > + pm_runtime_put_sync(sc->dev); I already made this mistake, and 4cc47e8add63 ("clk: qcom: gdsc: Remove direct runtime PM calls") covers the reason why it was a mistake. What I think you want is two things: 1) When you're accessing the registers, you want the clock controller's power-domain to be on. 2) When the client vote for a GDSC, you want to have the PM framework also ensure that parent power-domains are kept on. For the single case, this is handled by the pm_genpd_add_subdomain() call below. This, or something along those lines, seems like the appropriate solution. Regards, Bjorn