| Message ID | 1500526099-9935-4-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rajendra, few small comments to get code closer to perfection... On 07/20/2017 07:48 AM, Rajendra Nayak wrote: > The devices within a gdsc power domain, quite often have additional > clocks to be turned on/off along with the power domain itself. > Add support for this by specifying a list of clk_hw pointers > per gdsc which would be the clocks turned on/off along with the > powerdomain on/off callbacks. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/clk/qcom/gdsc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-- > drivers/clk/qcom/gdsc.h | 8 +++++++ > 2 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index a4f3580..7e7c051 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -12,6 +12,8 @@ > */ > > #include <linux/bitops.h> > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > #include <linux/delay.h> > #include <linux/err.h> > #include <linux/jiffies.h> > @@ -21,6 +23,7 @@ > #include <linux/regmap.h> > #include <linux/reset-controller.h> > #include <linux/slab.h> > +#include "common.h" > #include "gdsc.h" > > #define PWR_ON_MASK BIT(31) > @@ -166,6 +169,29 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc) > GMEM_CLAMP_IO_MASK, 1); > } > > +static inline int gdsc_clk_enable(struct gdsc *sc) I think the compiler is smart enough so 'inline' can dropped here and below. > +{ > + int i, ret; > + > + for (i = 0; i < sc->clk_count; i++) { > + ret = clk_prepare_enable(sc->clks[i]); > + if (ret) { > + for (i--; i >= 0; i--) > + clk_disable_unprepare(sc->clks[i]); > + return ret; > + } > + } blank line please. > + return 0; > +} > + > +static inline void gdsc_clk_disable(struct gdsc *sc) > +{ > + int i; > + > + for (i = 0; i < sc->clk_count; i++) > + clk_disable_unprepare(sc->clks[i]); can we disable clocks in reverse order, not sure that will make any sense but I've seen issues with some of the clocks in the past. > +} > + > static int gdsc_enable(struct generic_pm_domain *domain) > { > struct gdsc *sc = domain_to_gdsc(domain); > @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) > */ > udelay(1); > > + ret = gdsc_clk_enable(sc); > + if (ret) > + return ret; > + > /* Turn on HW trigger mode if supported */ > if (sc->flags & HW_CTRL) { > ret = gdsc_hwctrl(sc, true); > @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) > return ret; > } > > + gdsc_clk_disable(sc); > + > if (sc->pwrsts & PWRSTS_OFF) > gdsc_clear_mem_on(sc); > > @@ -254,7 +286,27 @@ static int gdsc_disable(struct generic_pm_domain *domain) > return 0; > } > > -static int gdsc_init(struct gdsc *sc) > +static inline int gdsc_clk_get(struct device *dev, struct gdsc *sc) > +{ > + if (sc->clk_count) { could you inverse the logic if (!sc->clk_count) return 0; > + int i; > + > + sc->clks = devm_kcalloc(dev, sc->clk_count, sizeof(*sc->clks), > + GFP_KERNEL); > + if (!sc->clks) > + return -ENOMEM; > + > + for (i = 0; i < sc->clk_count; i++) { > + sc->clks[i] = devm_clk_hw_get_clk(dev, sc->clk_hws[i], > + NULL); > + if (IS_ERR(sc->clks[i])) > + return PTR_ERR(sc->clks[i]); > + } > + } add blank line please > + return 0; > +} > + I will make some tests with venus driver on mainline soon.
On 07/20, Rajendra Nayak wrote: > @@ -166,6 +169,29 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc) > GMEM_CLAMP_IO_MASK, 1); > } > > +static inline int gdsc_clk_enable(struct gdsc *sc) > +{ > + int i, ret; > + > + for (i = 0; i < sc->clk_count; i++) { > + ret = clk_prepare_enable(sc->clks[i]); > + if (ret) { > + for (i--; i >= 0; i--) while (--i >= 0) ? > + clk_disable_unprepare(sc->clks[i]); > + return ret; > + } > + } > + return 0; > +} > + > +static inline void gdsc_clk_disable(struct gdsc *sc) > +{ > + int i; > + > + for (i = 0; i < sc->clk_count; i++) Could also be the same while loop. > + clk_disable_unprepare(sc->clks[i]); > +} > + > static int gdsc_enable(struct generic_pm_domain *domain) > { > struct gdsc *sc = domain_to_gdsc(domain); > @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) > */ > udelay(1); > > + ret = gdsc_clk_enable(sc); > + if (ret) > + return ret; > + > /* Turn on HW trigger mode if supported */ > if (sc->flags & HW_CTRL) { > ret = gdsc_hwctrl(sc, true); > @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) > return ret; > } > > + gdsc_clk_disable(sc); Can we call sleeping APIs (i.e. prepare/unprepare) from within the power domains? I thought that didn't work generically because someone could set their runtime PM configuration to be atomic context friendly. Is there some way we can block that from happening if we hook up a power domain that is no longer safe to call from atomic context? > + > if (sc->pwrsts & PWRSTS_OFF) > gdsc_clear_mem_on(sc); > > @@ -254,7 +286,27 @@ static int gdsc_disable(struct generic_pm_domain *domain) > return 0; > } > > -static int gdsc_init(struct gdsc *sc) > +static inline int gdsc_clk_get(struct device *dev, struct gdsc *sc) > +{ > + if (sc->clk_count) { We could drop this check. kmalloc with size zero is OK as long as we don't use the returned pointer value. We shouldn't use it assuming the for loop is maintained. > + int i; > + > + sc->clks = devm_kcalloc(dev, sc->clk_count, sizeof(*sc->clks), > + GFP_KERNEL); > + if (!sc->clks) > + return -ENOMEM; > + > + for (i = 0; i < sc->clk_count; i++) { > + sc->clks[i] = devm_clk_hw_get_clk(dev, sc->clk_hws[i], > + NULL); > + if (IS_ERR(sc->clks[i])) > + return PTR_ERR(sc->clks[i]); > + } > + } > + return 0; > +} > + > +static int gdsc_init(struct device *dev, struct gdsc *sc) > { > u32 mask, val; > int on, ret; > @@ -284,6 +336,10 @@ static int gdsc_init(struct gdsc *sc) > if (on < 0) > return on; > > + ret = gdsc_clk_get(dev, sc); > + if (ret) > + return ret; > + This concerns me if we do probe defer on orphan clks. We may get into some situation where the gdsc is setup by a clk driver that is trying to probe before some parent clk has registered for the clks we're "getting" here. For example, this could easily happen if we insert XO into the tree at the top and everything defers. I suppose this is not a problem because in this case we don't have any clk here that could be orphaned even if RPM clks are inserted into the clk tree for XO? I mean to say that we won't get into a probe defer due to orphan clk loop. I'm always afraid someone will make hardware that send a clk from one controller to another and then back again (we did that with pcie) and then we'll be unable to get out of the defer loop because we'll be deferred on orphan status. > /* > * Votable GDSCs can be ON due to Vote from other masters. > * If a Votable GDSC is ON, make sure we have a Vote. > @@ -327,7 +383,7 @@ int gdsc_register(struct gdsc_desc *desc, > continue; > scs[i]->regmap = regmap; > scs[i]->rcdev = rcdev; > - ret = gdsc_init(scs[i]); > + ret = gdsc_init(dev, scs[i]); > if (ret) > return ret; > data->domains[i] = &scs[i]->pd;
On 07/28/2017 04:32 AM, Stephen Boyd wrote: > On 07/20, Rajendra Nayak wrote: >> @@ -166,6 +169,29 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc) >> GMEM_CLAMP_IO_MASK, 1); >> } >> >> +static inline int gdsc_clk_enable(struct gdsc *sc) >> +{ >> + int i, ret; >> + >> + for (i = 0; i < sc->clk_count; i++) { >> + ret = clk_prepare_enable(sc->clks[i]); >> + if (ret) { >> + for (i--; i >= 0; i--) > > while (--i >= 0) ? > >> + clk_disable_unprepare(sc->clks[i]); >> + return ret; >> + } >> + } >> + return 0; >> +} >> + >> +static inline void gdsc_clk_disable(struct gdsc *sc) >> +{ >> + int i; >> + >> + for (i = 0; i < sc->clk_count; i++) > > Could also be the same while loop. > >> + clk_disable_unprepare(sc->clks[i]); >> +} >> + >> static int gdsc_enable(struct generic_pm_domain *domain) >> { >> struct gdsc *sc = domain_to_gdsc(domain); >> @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) >> */ >> udelay(1); >> >> + ret = gdsc_clk_enable(sc); >> + if (ret) >> + return ret; >> + >> /* Turn on HW trigger mode if supported */ >> if (sc->flags & HW_CTRL) { >> ret = gdsc_hwctrl(sc, true); >> @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) >> return ret; >> } >> >> + gdsc_clk_disable(sc); > > Can we call sleeping APIs (i.e. prepare/unprepare) from within > the power domains? I thought that didn't work generically because > someone could set their runtime PM configuration to be atomic > context friendly. Is there some way we can block that from > happening if we hook up a power domain that is no longer safe to > call from atomic context? hmm, I don't see a way to do that. Perhaps we could prepare/unprepare these as part of the pm_domain attach/detach to a device and then only enable/disable them as part of the gdsc_enable/disable? > >> + >> if (sc->pwrsts & PWRSTS_OFF) >> gdsc_clear_mem_on(sc); >> >> @@ -254,7 +286,27 @@ static int gdsc_disable(struct generic_pm_domain *domain) >> return 0; >> } >> >> -static int gdsc_init(struct gdsc *sc) >> +static inline int gdsc_clk_get(struct device *dev, struct gdsc *sc) >> +{ >> + if (sc->clk_count) { > > We could drop this check. kmalloc with size zero is OK as long as > we don't use the returned pointer value. We shouldn't use it > assuming the for loop is maintained. > >> + int i; >> + >> + sc->clks = devm_kcalloc(dev, sc->clk_count, sizeof(*sc->clks), >> + GFP_KERNEL); >> + if (!sc->clks) >> + return -ENOMEM; >> + >> + for (i = 0; i < sc->clk_count; i++) { >> + sc->clks[i] = devm_clk_hw_get_clk(dev, sc->clk_hws[i], >> + NULL); >> + if (IS_ERR(sc->clks[i])) >> + return PTR_ERR(sc->clks[i]); >> + } >> + } >> + return 0; >> +} >> + >> +static int gdsc_init(struct device *dev, struct gdsc *sc) >> { >> u32 mask, val; >> int on, ret; >> @@ -284,6 +336,10 @@ static int gdsc_init(struct gdsc *sc) >> if (on < 0) >> return on; >> >> + ret = gdsc_clk_get(dev, sc); >> + if (ret) >> + return ret; >> + > > This concerns me if we do probe defer on orphan clks. We may get > into some situation where the gdsc is setup by a clk driver that > is trying to probe before some parent clk has registered for the > clks we're "getting" here. For example, this could easily happen > if we insert XO into the tree at the top and everything defers. > > I suppose this is not a problem because in this case we don't > have any clk here that could be orphaned even if RPM clks are > inserted into the clk tree for XO? I mean to say that we won't > get into a probe defer due to orphan clk loop. I'm always afraid > someone will make hardware that send a clk from one controller to > another and then back again (we did that with pcie) and then > we'll be unable to get out of the defer loop because we'll be > deferred on orphan status. Yes, we would probably run into these issues with probe defer for orphan clks. One way to handle it could be to decouple the probing of the clocks and powerdomain parts of the controller. Like the clock driver can probe and then register a dummy device for powerdomains (like is done for tsens on 8960) so it could probe independently?
On 07/28, Rajendra Nayak wrote: > > > On 07/28/2017 04:32 AM, Stephen Boyd wrote: > > On 07/20, Rajendra Nayak wrote: > > > >> + clk_disable_unprepare(sc->clks[i]); > >> +} > >> + > >> static int gdsc_enable(struct generic_pm_domain *domain) > >> { > >> struct gdsc *sc = domain_to_gdsc(domain); > >> @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) > >> */ > >> udelay(1); > >> > >> + ret = gdsc_clk_enable(sc); > >> + if (ret) > >> + return ret; > >> + > >> /* Turn on HW trigger mode if supported */ > >> if (sc->flags & HW_CTRL) { > >> ret = gdsc_hwctrl(sc, true); > >> @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) > >> return ret; > >> } > >> > >> + gdsc_clk_disable(sc); > > > > Can we call sleeping APIs (i.e. prepare/unprepare) from within > > the power domains? I thought that didn't work generically because > > someone could set their runtime PM configuration to be atomic > > context friendly. Is there some way we can block that from > > happening if we hook up a power domain that is no longer safe to > > call from atomic context? > > hmm, I don't see a way to do that. Perhaps we could prepare/unprepare > these as part of the pm_domain attach/detach to a device and then > only enable/disable them as part of the gdsc_enable/disable? > The problem there is if we keep these clks prepared while the domain is attached to a device I don't see how we can ever turn off the XO clk and achieve XO shutdown in low power modes. A pm domain is basically never detached, right? This is where we need different power levels in PM domains if I understand correctly. > > > > This concerns me if we do probe defer on orphan clks. We may get > > into some situation where the gdsc is setup by a clk driver that > > is trying to probe before some parent clk has registered for the > > clks we're "getting" here. For example, this could easily happen > > if we insert XO into the tree at the top and everything defers. > > > > I suppose this is not a problem because in this case we don't > > have any clk here that could be orphaned even if RPM clks are > > inserted into the clk tree for XO? I mean to say that we won't > > get into a probe defer due to orphan clk loop. I'm always afraid > > someone will make hardware that send a clk from one controller to > > another and then back again (we did that with pcie) and then > > we'll be unable to get out of the defer loop because we'll be > > deferred on orphan status. > > Yes, we would probably run into these issues with probe defer for > orphan clks. One way to handle it could be to decouple the probing > of the clocks and powerdomain parts of the controller. Like the clock > driver can probe and then register a dummy device for powerdomains > (like is done for tsens on 8960) so it could probe independently? > Well is it a problem right now? I think we're OK here because we don't have a "cycle" between clk providers in the clk provider hierarchy. Can we clk_get() during attach and then fail attach if we can't get clks at that point? If this works, we have a backup plan should something go wrong, but we can ignore this concern for now until it becomes a real problem.
On 07/28/2017 10:07 PM, Stephen Boyd wrote: > On 07/28, Rajendra Nayak wrote: >> >> >> On 07/28/2017 04:32 AM, Stephen Boyd wrote: >>> On 07/20, Rajendra Nayak wrote: >>> >>>> + clk_disable_unprepare(sc->clks[i]); >>>> +} >>>> + >>>> static int gdsc_enable(struct generic_pm_domain *domain) >>>> { >>>> struct gdsc *sc = domain_to_gdsc(domain); >>>> @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) >>>> */ >>>> udelay(1); >>>> >>>> + ret = gdsc_clk_enable(sc); >>>> + if (ret) >>>> + return ret; >>>> + >>>> /* Turn on HW trigger mode if supported */ >>>> if (sc->flags & HW_CTRL) { >>>> ret = gdsc_hwctrl(sc, true); >>>> @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) >>>> return ret; >>>> } >>>> >>>> + gdsc_clk_disable(sc); >>> >>> Can we call sleeping APIs (i.e. prepare/unprepare) from within >>> the power domains? I thought that didn't work generically because >>> someone could set their runtime PM configuration to be atomic >>> context friendly. Is there some way we can block that from >>> happening if we hook up a power domain that is no longer safe to >>> call from atomic context? >> >> hmm, I don't see a way to do that. Perhaps we could prepare/unprepare >> these as part of the pm_domain attach/detach to a device and then >> only enable/disable them as part of the gdsc_enable/disable? >> > > The problem there is if we keep these clks prepared while the > domain is attached to a device I don't see how we can ever turn > off the XO clk and achieve XO shutdown in low power modes. A pm > domain is basically never detached, right? This is where we need > different power levels in PM domains if I understand correctly. right, I did not realize the XO shutdown would happen via a unprepare callback and not disable. So looking some more into this, genpd does have a GENPD_FLAG_IRQ_SAFE used to indicate an IRQ safe domain. We don't set this today for any of the gdscs so genpd uses mutex locking anyway. When we do end up supporting/have a need to support IRQ safe domains we might end up putting a check to make sure we don't associate any clocks with those gdscs and leave it to the devices to handle it from within the drivers. > >>> >>> This concerns me if we do probe defer on orphan clks. We may get >>> into some situation where the gdsc is setup by a clk driver that >>> is trying to probe before some parent clk has registered for the >>> clks we're "getting" here. For example, this could easily happen >>> if we insert XO into the tree at the top and everything defers. >>> >>> I suppose this is not a problem because in this case we don't >>> have any clk here that could be orphaned even if RPM clks are >>> inserted into the clk tree for XO? I mean to say that we won't >>> get into a probe defer due to orphan clk loop. I'm always afraid >>> someone will make hardware that send a clk from one controller to >>> another and then back again (we did that with pcie) and then >>> we'll be unable to get out of the defer loop because we'll be >>> deferred on orphan status. >> >> Yes, we would probably run into these issues with probe defer for >> orphan clks. One way to handle it could be to decouple the probing >> of the clocks and powerdomain parts of the controller. Like the clock >> driver can probe and then register a dummy device for powerdomains >> (like is done for tsens on 8960) so it could probe independently? >> > > Well is it a problem right now? I think we're OK here because we > don't have a "cycle" between clk providers in the clk provider > hierarchy. Can we clk_get() during attach and then fail attach if > we can't get clks at that point? If this works, we have a backup > plan should something go wrong, but we can ignore this concern > for now until it becomes a real problem. we don't have a cycle but I was worried about gcc being deferred until RPM clocks are probed and MMCC getting probed in between, and MMCC init before GCC init would have us run into issues. But I should be able to defer the clk_get()'s to during pm domain attach and it should all work in that case.
On 07/31/2017 01:55 PM, Rajendra Nayak wrote: [].. >>>> This concerns me if we do probe defer on orphan clks. We may get >>>> into some situation where the gdsc is setup by a clk driver that >>>> is trying to probe before some parent clk has registered for the >>>> clks we're "getting" here. For example, this could easily happen >>>> if we insert XO into the tree at the top and everything defers. >>>> >>>> I suppose this is not a problem because in this case we don't >>>> have any clk here that could be orphaned even if RPM clks are >>>> inserted into the clk tree for XO? I mean to say that we won't >>>> get into a probe defer due to orphan clk loop. I'm always afraid >>>> someone will make hardware that send a clk from one controller to >>>> another and then back again (we did that with pcie) and then >>>> we'll be unable to get out of the defer loop because we'll be >>>> deferred on orphan status. >>> >>> Yes, we would probably run into these issues with probe defer for >>> orphan clks. One way to handle it could be to decouple the probing >>> of the clocks and powerdomain parts of the controller. Like the clock >>> driver can probe and then register a dummy device for powerdomains >>> (like is done for tsens on 8960) so it could probe independently? >>> >> >> Well is it a problem right now? I think we're OK here because we >> don't have a "cycle" between clk providers in the clk provider >> hierarchy. Can we clk_get() during attach and then fail attach if >> we can't get clks at that point? If this works, we have a backup >> plan should something go wrong, but we can ignore this concern >> for now until it becomes a real problem. > > we don't have a cycle but I was worried about gcc being deferred until > RPM clocks are probed and MMCC getting probed in between, and MMCC init > before GCC init would have us run into issues. > But I should be able to defer the clk_get()'s to during pm domain attach > and it should all work in that case. So one issue I run into trying to move the clk_get() to during attach is, most of these GDSCs to which we are associating the mmagic clocks are votable and some of them happen to be ON when the gdsc driver probes. And we have this to handle the situation.. /* * Votable GDSCs can be ON due to Vote from other masters. * If a Votable GDSC is ON, make sure we have a Vote. */ if ((sc->flags & VOTABLE) && on) gdsc_enable(&sc->pd); And now since we defer clk_get() to a later point, the gdsc can't really be enabled along with the enable/voting of the associated clocks. I was thinking we do something like this instead, /* * Votable GDSCs can be ON due to Vote from other masters. * If *we* haven't Voted for it, tell genpd its actually OFF. */ if ((sc->flags & VOTABLE) && on) on = !on; What do you think?
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index a4f3580..7e7c051 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -12,6 +12,8 @@ */ #include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/jiffies.h> @@ -21,6 +23,7 @@ #include <linux/regmap.h> #include <linux/reset-controller.h> #include <linux/slab.h> +#include "common.h" #include "gdsc.h" #define PWR_ON_MASK BIT(31) @@ -166,6 +169,29 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc) GMEM_CLAMP_IO_MASK, 1); } +static inline int gdsc_clk_enable(struct gdsc *sc) +{ + int i, ret; + + for (i = 0; i < sc->clk_count; i++) { + ret = clk_prepare_enable(sc->clks[i]); + if (ret) { + for (i--; i >= 0; i--) + clk_disable_unprepare(sc->clks[i]); + return ret; + } + } + return 0; +} + +static inline void gdsc_clk_disable(struct gdsc *sc) +{ + int i; + + for (i = 0; i < sc->clk_count; i++) + clk_disable_unprepare(sc->clks[i]); +} + static int gdsc_enable(struct generic_pm_domain *domain) { struct gdsc *sc = domain_to_gdsc(domain); @@ -193,6 +219,10 @@ static int gdsc_enable(struct generic_pm_domain *domain) */ udelay(1); + ret = gdsc_clk_enable(sc); + if (ret) + return ret; + /* Turn on HW trigger mode if supported */ if (sc->flags & HW_CTRL) { ret = gdsc_hwctrl(sc, true); @@ -241,6 +271,8 @@ static int gdsc_disable(struct generic_pm_domain *domain) return ret; } + gdsc_clk_disable(sc); + if (sc->pwrsts & PWRSTS_OFF) gdsc_clear_mem_on(sc); @@ -254,7 +286,27 @@ static int gdsc_disable(struct generic_pm_domain *domain) return 0; } -static int gdsc_init(struct gdsc *sc) +static inline int gdsc_clk_get(struct device *dev, struct gdsc *sc) +{ + if (sc->clk_count) { + int i; + + sc->clks = devm_kcalloc(dev, sc->clk_count, sizeof(*sc->clks), + GFP_KERNEL); + if (!sc->clks) + return -ENOMEM; + + for (i = 0; i < sc->clk_count; i++) { + sc->clks[i] = devm_clk_hw_get_clk(dev, sc->clk_hws[i], + NULL); + if (IS_ERR(sc->clks[i])) + return PTR_ERR(sc->clks[i]); + } + } + return 0; +} + +static int gdsc_init(struct device *dev, struct gdsc *sc) { u32 mask, val; int on, ret; @@ -284,6 +336,10 @@ static int gdsc_init(struct gdsc *sc) if (on < 0) return on; + ret = gdsc_clk_get(dev, sc); + if (ret) + return ret; + /* * Votable GDSCs can be ON due to Vote from other masters. * If a Votable GDSC is ON, make sure we have a Vote. @@ -327,7 +383,7 @@ int gdsc_register(struct gdsc_desc *desc, continue; scs[i]->regmap = regmap; scs[i]->rcdev = rcdev; - ret = gdsc_init(scs[i]); + ret = gdsc_init(dev, scs[i]); if (ret) return ret; data->domains[i] = &scs[i]->pd; diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 3964834..a7fd51b 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -17,6 +17,8 @@ #include <linux/err.h> #include <linux/pm_domain.h> +struct clk; +struct clk_hw; struct regmap; struct reset_controller_dev; @@ -32,6 +34,9 @@ * @resets: ids of resets associated with this gdsc * @reset_count: number of @resets * @rcdev: reset controller + * @clk_count: number of gdsc clocks + * @clks: clk pointers for gdsc clocks + * @clk_hws: clk_hw pointers for gdsc clocks */ struct gdsc { struct generic_pm_domain pd; @@ -56,6 +61,9 @@ struct gdsc { struct reset_controller_dev *rcdev; unsigned int *resets; unsigned int reset_count; + unsigned int clk_count; + struct clk **clks; + struct clk_hw *clk_hws[]; }; struct gdsc_desc {
The devices within a gdsc power domain, quite often have additional clocks to be turned on/off along with the power domain itself. Add support for this by specifying a list of clk_hw pointers per gdsc which would be the clocks turned on/off along with the powerdomain on/off callbacks. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/clk/qcom/gdsc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-- drivers/clk/qcom/gdsc.h | 8 +++++++ 2 files changed, 66 insertions(+), 2 deletions(-)