| Message ID | 20210829154757.784699-6-dmitry.baryshkov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 1b771839de054710e1e015b10e29a4d04c41f54b |
| Headers | show |
| Series | clk: qcom: use power-domain for sm8250's clock controllers | expand |
On Sun 29 Aug 08:47 PDT 2021, Dmitry Baryshkov wrote: > On sm8250 dispcc and videocc registers are powered up by the MMCX power > domain. Currently we use a regulator to enable this domain on demand, > however this has some consequences, as genpd code is not reentrant. > > Make gdsc code also use pm_runtime calls to ensure that registers are > accessible during the gdsc_enable/gdsc_disable operations. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/clk/qcom/gdsc.c | 51 ++++++++++++++++++++++++++++++++++++++--- > drivers/clk/qcom/gdsc.h | 2 ++ > 2 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index 4ece326ea233..7e1dd8ccfa38 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -11,6 +11,7 @@ > #include <linux/kernel.h> > #include <linux/ktime.h> > #include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/reset-controller.h> > @@ -50,6 +51,22 @@ enum gdsc_status { > GDSC_ON > }; > > +static int gdsc_pm_runtime_get(struct gdsc *sc) > +{ > + if (!sc->dev) > + return 0; > + > + return pm_runtime_resume_and_get(sc->dev); > +} > + > +static int gdsc_pm_runtime_put(struct gdsc *sc) > +{ > + if (!sc->dev) > + return 0; > + > + return pm_runtime_put_sync(sc->dev); > +} > + > /* Returns 1 if GDSC status is status, 0 if not, and < 0 on error */ > static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status) > { > @@ -232,9 +249,8 @@ static void gdsc_retain_ff_on(struct gdsc *sc) > regmap_update_bits(sc->regmap, sc->gdscr, mask, mask); > } > > -static int gdsc_enable(struct generic_pm_domain *domain) > +static int _gdsc_enable(struct gdsc *sc) > { > - struct gdsc *sc = domain_to_gdsc(domain); > int ret; > > if (sc->pwrsts == PWRSTS_ON) > @@ -290,11 +306,22 @@ static int gdsc_enable(struct generic_pm_domain *domain) > return 0; > } > > -static int gdsc_disable(struct generic_pm_domain *domain) > +static int gdsc_enable(struct generic_pm_domain *domain) > { > struct gdsc *sc = domain_to_gdsc(domain); > int ret; > > + ret = gdsc_pm_runtime_get(sc); > + if (ret) > + return ret; > + > + return _gdsc_enable(sc); > +} > + > +static int _gdsc_disable(struct gdsc *sc) > +{ > + int ret; > + > if (sc->pwrsts == PWRSTS_ON) > return gdsc_assert_reset(sc); > > @@ -329,6 +356,18 @@ static int gdsc_disable(struct generic_pm_domain *domain) > return 0; > } > > +static int gdsc_disable(struct generic_pm_domain *domain) > +{ > + struct gdsc *sc = domain_to_gdsc(domain); > + int ret; > + > + ret = _gdsc_disable(sc); > + > + gdsc_pm_runtime_put(sc); > + > + return ret; > +} > + > static int gdsc_init(struct gdsc *sc) > { > u32 mask, val; > @@ -443,6 +482,8 @@ int gdsc_register(struct gdsc_desc *desc, > for (i = 0; i < num; i++) { > if (!scs[i]) > continue; > + if (pm_runtime_enabled(dev)) > + scs[i]->dev = dev; > scs[i]->regmap = regmap; > scs[i]->rcdev = rcdev; > ret = gdsc_init(scs[i]); > @@ -457,6 +498,8 @@ int gdsc_register(struct gdsc_desc *desc, > continue; > if (scs[i]->parent) > pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); > + else if (!IS_ERR_OR_NULL(dev->pm_domain)) > + pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); > } > > return of_genpd_add_provider_onecell(dev->of_node, data); > @@ -475,6 +518,8 @@ void gdsc_unregister(struct gdsc_desc *desc) > continue; > if (scs[i]->parent) > pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd); > + else if (!IS_ERR_OR_NULL(dev->pm_domain)) > + pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); > } > of_genpd_del_provider(dev->of_node); > } > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h > index 5bb396b344d1..702d47a87af6 100644 > --- a/drivers/clk/qcom/gdsc.h > +++ b/drivers/clk/qcom/gdsc.h > @@ -25,6 +25,7 @@ struct reset_controller_dev; > * @resets: ids of resets associated with this gdsc > * @reset_count: number of @resets > * @rcdev: reset controller > + * @dev: the device holding the GDSC, used for pm_runtime calls > */ > struct gdsc { > struct generic_pm_domain pd; > @@ -58,6 +59,7 @@ struct gdsc { > > const char *supply; > struct regulator *rsupply; > + struct device *dev; > }; > > struct gdsc_desc { > -- > 2.33.0 >
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 4ece326ea233..7e1dd8ccfa38 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -11,6 +11,7 @@ #include <linux/kernel.h> #include <linux/ktime.h> #include <linux/pm_domain.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/reset-controller.h> @@ -50,6 +51,22 @@ enum gdsc_status { GDSC_ON }; +static int gdsc_pm_runtime_get(struct gdsc *sc) +{ + if (!sc->dev) + return 0; + + return pm_runtime_resume_and_get(sc->dev); +} + +static int gdsc_pm_runtime_put(struct gdsc *sc) +{ + if (!sc->dev) + return 0; + + return pm_runtime_put_sync(sc->dev); +} + /* Returns 1 if GDSC status is status, 0 if not, and < 0 on error */ static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status) { @@ -232,9 +249,8 @@ static void gdsc_retain_ff_on(struct gdsc *sc) regmap_update_bits(sc->regmap, sc->gdscr, mask, mask); } -static int gdsc_enable(struct generic_pm_domain *domain) +static int _gdsc_enable(struct gdsc *sc) { - struct gdsc *sc = domain_to_gdsc(domain); int ret; if (sc->pwrsts == PWRSTS_ON) @@ -290,11 +306,22 @@ static int gdsc_enable(struct generic_pm_domain *domain) return 0; } -static int gdsc_disable(struct generic_pm_domain *domain) +static int gdsc_enable(struct generic_pm_domain *domain) { struct gdsc *sc = domain_to_gdsc(domain); int ret; + ret = gdsc_pm_runtime_get(sc); + if (ret) + return ret; + + return _gdsc_enable(sc); +} + +static int _gdsc_disable(struct gdsc *sc) +{ + int ret; + if (sc->pwrsts == PWRSTS_ON) return gdsc_assert_reset(sc); @@ -329,6 +356,18 @@ static int gdsc_disable(struct generic_pm_domain *domain) return 0; } +static int gdsc_disable(struct generic_pm_domain *domain) +{ + struct gdsc *sc = domain_to_gdsc(domain); + int ret; + + ret = _gdsc_disable(sc); + + gdsc_pm_runtime_put(sc); + + return ret; +} + static int gdsc_init(struct gdsc *sc) { u32 mask, val; @@ -443,6 +482,8 @@ int gdsc_register(struct gdsc_desc *desc, for (i = 0; i < num; i++) { if (!scs[i]) continue; + if (pm_runtime_enabled(dev)) + scs[i]->dev = dev; scs[i]->regmap = regmap; scs[i]->rcdev = rcdev; ret = gdsc_init(scs[i]); @@ -457,6 +498,8 @@ int gdsc_register(struct gdsc_desc *desc, continue; if (scs[i]->parent) pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); + else if (!IS_ERR_OR_NULL(dev->pm_domain)) + pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); } return of_genpd_add_provider_onecell(dev->of_node, data); @@ -475,6 +518,8 @@ void gdsc_unregister(struct gdsc_desc *desc) continue; if (scs[i]->parent) pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd); + else if (!IS_ERR_OR_NULL(dev->pm_domain)) + pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); } of_genpd_del_provider(dev->of_node); } diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 5bb396b344d1..702d47a87af6 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -25,6 +25,7 @@ struct reset_controller_dev; * @resets: ids of resets associated with this gdsc * @reset_count: number of @resets * @rcdev: reset controller + * @dev: the device holding the GDSC, used for pm_runtime calls */ struct gdsc { struct generic_pm_domain pd; @@ -58,6 +59,7 @@ struct gdsc { const char *supply; struct regulator *rsupply; + struct device *dev; }; struct gdsc_desc {
On sm8250 dispcc and videocc registers are powered up by the MMCX power domain. Currently we use a regulator to enable this domain on demand, however this has some consequences, as genpd code is not reentrant. Make gdsc code also use pm_runtime calls to ensure that registers are accessible during the gdsc_enable/gdsc_disable operations. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/clk/qcom/gdsc.c | 51 ++++++++++++++++++++++++++++++++++++++--- drivers/clk/qcom/gdsc.h | 2 ++ 2 files changed, 50 insertions(+), 3 deletions(-) -- 2.33.0