Re: [PATCH 06/13] clk: qcom: cpu-8996: simplify the cpu_clk_notifier_cb

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 11.01.2023 20:19, Dmitry Baryshkov wrote:
> - Do not use the Alt PLL completely. Switch to smux when necessary to
>   prevent overvolting
Is this empirical evidence, or did Qualcomm recommendations change since
msm-3.18 was released?


> - Restore the parent in case the rate change aborts for some reason
> - Do not duplicate resetting the parent in set_parent operation.
These sound good.

Konrad
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/clk-cpu-8996.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
> index 7e5246ca7e7f..ee7e18b37832 100644
> --- a/drivers/clk/qcom/clk-cpu-8996.c
> +++ b/drivers/clk/qcom/clk-cpu-8996.c
> @@ -506,27 +506,34 @@ static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
>  {
>  	struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_nb(nb);
>  	struct clk_notifier_data *cnd = data;
> -	int ret;
>  
>  	switch (event) {
>  	case PRE_RATE_CHANGE:
> -		ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
>  		qcom_cpu_clk_msm8996_acd_init(base);
> +
> +		/*
> +		 * Avoid overvolting. clk_core_set_rate_nolock() walks from top
> +		 * to bottom, so it will change the rate of the PLL before
> +		 * chaging the parent of PMUX. This can result in pmux getting
> +		 * clocked twice the expected rate.
> +		 *
> +		 * Manually switch to PLL/2 here.
> +		 */
> +		if (cnd->new_rate < DIV_2_THRESHOLD &&
> +		    cnd->old_rate > DIV_2_THRESHOLD)
> +			clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, SMUX_INDEX);
> +
>  		break;
> -	case POST_RATE_CHANGE:
> -		if (cnd->new_rate < DIV_2_THRESHOLD)
> -			ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
> -							   SMUX_INDEX);
> -		else
> -			ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
> -							   ACD_INDEX);
> -		break;
> +	case ABORT_RATE_CHANGE:
> +		/* Revert manual change */
> +		if (cnd->new_rate < DIV_2_THRESHOLD &&
> +		    cnd->old_rate > DIV_2_THRESHOLD)
> +			clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ACD_INDEX);
>  	default:
> -		ret = 0;
>  		break;
>  	}
>  
> -	return notifier_from_errno(ret);
> +	return NOTIFY_OK;
>  };
>  
>  static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)