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@xxxxxxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
