| Message ID | 1479472107-18472-2-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 11/18, Sricharan R wrote: > From: Rajendra Nayak <rnayak@codeaurora.org> > > Some GDSCs might support a HW control mode, where in the power > domain (gdsc) is brought in and out of low power state (while > unsued) without any SW assistance, saving power. > Such GDSCs can be configured in a HW control mode when powered on > until they are explicitly requested to be powered off by software. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- Applied to clk-next
Hi Sricharan, On 11/18/2016 02:28 PM, Sricharan R wrote: > From: Rajendra Nayak <rnayak@codeaurora.org> > > Some GDSCs might support a HW control mode, where in the power > domain (gdsc) is brought in and out of low power state (while > unsued) without any SW assistance, saving power. > Such GDSCs can be configured in a HW control mode when powered on > until they are explicitly requested to be powered off by software. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- > [V2] Fixed to take care of the return value of gdsc_hwctrl > > drivers/clk/qcom/gdsc.c | 19 +++++++++++++++++++ > drivers/clk/qcom/gdsc.h | 1 + > 2 files changed, 20 insertions(+) > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index f12d7b2..57c7c1b 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -55,6 +55,13 @@ static int gdsc_is_enabled(struct gdsc *sc, unsigned int reg) > return !!(val & PWR_ON_MASK); > } > > +static int gdsc_hwctrl(struct gdsc *sc, bool en) > +{ > + u32 val = en ? HW_CONTROL_MASK : 0; > + > + return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val); > +} > + > static int gdsc_toggle_logic(struct gdsc *sc, bool en) > { > int ret; > @@ -164,16 +171,28 @@ static int gdsc_enable(struct generic_pm_domain *domain) > */ > udelay(1); > > + /* Turn on HW trigger mode if supported */ > + if (sc->flags & HW_CTRL) > + return gdsc_hwctrl(sc, true); > + > return 0; > } > > static int gdsc_disable(struct generic_pm_domain *domain) > { > struct gdsc *sc = domain_to_gdsc(domain); > + int ret; > > if (sc->pwrsts == PWRSTS_ON) > return gdsc_assert_reset(sc); > > + /* Turn off HW trigger mode if supported */ > + if (sc->flags & HW_CTRL) { > + ret = gdsc_hwctrl(sc, false); Looking in the downstream implementation the disabling of the hw control bit shouldn't be enough. After disabling hw control bit we must have a 1us delay and polling for enabled PWR_ON bit with timeout of 100us, only then we should continue with disabling the GDSC in software controlled mode.
[].. >> static int gdsc_toggle_logic(struct gdsc *sc, bool en) >> { >> int ret; >> @@ -164,16 +171,28 @@ static int gdsc_enable(struct generic_pm_domain *domain) >> */ >> udelay(1); >> >> + /* Turn on HW trigger mode if supported */ >> + if (sc->flags & HW_CTRL) >> + return gdsc_hwctrl(sc, true); >> + >> return 0; >> } >> >> static int gdsc_disable(struct generic_pm_domain *domain) >> { >> struct gdsc *sc = domain_to_gdsc(domain); >> + int ret; >> >> if (sc->pwrsts == PWRSTS_ON) >> return gdsc_assert_reset(sc); >> >> + /* Turn off HW trigger mode if supported */ >> + if (sc->flags & HW_CTRL) { >> + ret = gdsc_hwctrl(sc, false); > > Looking in the downstream implementation the disabling of the hw control > bit shouldn't be enough. > > After disabling hw control bit we must have a 1us delay and polling for > enabled PWR_ON bit with timeout of 100us, only then we should continue > with disabling the GDSC in software controlled mode. Stan, thats right. I will send a patch to fix this up right-away.
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index f12d7b2..57c7c1b 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -55,6 +55,13 @@ static int gdsc_is_enabled(struct gdsc *sc, unsigned int reg) return !!(val & PWR_ON_MASK); } +static int gdsc_hwctrl(struct gdsc *sc, bool en) +{ + u32 val = en ? HW_CONTROL_MASK : 0; + + return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val); +} + static int gdsc_toggle_logic(struct gdsc *sc, bool en) { int ret; @@ -164,16 +171,28 @@ static int gdsc_enable(struct generic_pm_domain *domain) */ udelay(1); + /* Turn on HW trigger mode if supported */ + if (sc->flags & HW_CTRL) + return gdsc_hwctrl(sc, true); + return 0; } static int gdsc_disable(struct generic_pm_domain *domain) { struct gdsc *sc = domain_to_gdsc(domain); + int ret; if (sc->pwrsts == PWRSTS_ON) return gdsc_assert_reset(sc); + /* Turn off HW trigger mode if supported */ + if (sc->flags & HW_CTRL) { + ret = gdsc_hwctrl(sc, false); + if (ret < 0) + return ret; + } + if (sc->pwrsts & PWRSTS_OFF) gdsc_clear_mem_on(sc); diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 3bf497c..b1f30f8 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -50,6 +50,7 @@ struct gdsc { #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) const u8 flags; #define VOTABLE BIT(0) +#define HW_CTRL BIT(1) struct reset_controller_dev *rcdev; unsigned int *resets; unsigned int reset_count;