Messages in this thread | | | Date | Mon, 30 Nov 2015 17:53:23 -0800 | From | Stephen Boyd <> | Subject | Re: [PATCH 5/6] clk: qcom: gdsc: Do not check for disabled status on votable gdscs |
| |
On 11/26, Rajendra Nayak wrote: > Some gdscs might be controlled via voting registers and might not > really disable when the kernel intends to disable them (due to other > votes keeping them enabled) > Mark these gdscs with a flag for we do not check/wait on a disable > status for these gdscs within the kernel disable callback. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/clk/qcom/gcc-msm8996.c | 4 ++++ > drivers/clk/qcom/gdsc.c | 4 ++++ > drivers/clk/qcom/gdsc.h | 15 ++++++++------- > drivers/clk/qcom/mmcc-msm8996.c | 3 +++ > 4 files changed, 19 insertions(+), 7 deletions(-)
We should have this patch before adding the gdscs that use it. Prevents any bisect hole.
> static struct gdsc usb30_gdsc = { > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index fb2e43c..984537f 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -65,6 +65,10 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en) > if (ret) > return ret; > > + /* If disabling votable gdscs, don't poll on status */ > + if ((sc->flags & VOTABLE) && !en) > + return 0; > +
There's an explicit delay of 100uS on the disable path for votable gdscs in the downstream code. I don't see that here.
> timeout = jiffies + usecs_to_jiffies(TIMEOUT_US); > > if (sc->gds_hw_ctrl) { > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h > index 66a43be..91cbb09 100644 > --- a/drivers/clk/qcom/gdsc.h > +++ b/drivers/clk/qcom/gdsc.h > @@ -20,13 +20,6 @@ > struct regmap; > struct reset_controller_dev; > > -/* Powerdomain allowable state bitfields */ > -#define PWRSTS_OFF BIT(0) > -#define PWRSTS_RET BIT(1) > -#define PWRSTS_ON BIT(2) > -#define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) > -#define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) > - > /** > * struct gdsc - Globally Distributed Switch Controller > * @pd: generic power domain > @@ -49,6 +42,14 @@ struct gdsc { > unsigned int *cxcs; > unsigned int cxc_count; > const u8 pwrsts; > +/* Powerdomain allowable state bitfields */ > +#define PWRSTS_OFF BIT(0) > +#define PWRSTS_RET BIT(1) > +#define PWRSTS_ON BIT(2) > +#define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) > +#define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON)
Yes! We should have done this already. I guess I'm ok combining it with this patch.
> + const u8 flags; > +#define VOTABLE BIT(0) > struct reset_controller_dev *rcdev; > unsigned int *resets; > unsigned int reset_count;
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
| |