[PATCH v3] clk: ti: Notify AVS driver upon setting clock rate

[Nishanth Menon]

On 17:32-20230920, Udit Kumar wrote:
> AVS is enabled at R5 SPL stage, on few platforms like J721E
> and J7200 clk-k3 is used instead if clk-sci driver.
> 
> Add support in clk-k3 driver as well to notify AVS driver
> on setting clock rate so that voltage is changed accordingly.
> 
> Also add support in clk-sci driver to do AVS notification
> before or after clock setting based upon current rate and new clock
> rate.
> 
> Cc: Keerthy <j-keerthy at ti.com>
> Signed-off-by: Udit Kumar <u-kumar1 at ti.com>
> ---
> Boot logs with v3:
> https://gist.github.com/uditkumarti/12a753a822c0aec9ca3028648c29dfa8
> 
> Change log:
> Change in v3:
> - Updated AVS notification before/after clock setting based upon
> new and current rate
> - Added modifed notification in clk-sci driver as well
> - v2 link
> https://lore.kernel.org/all/20230919140408.2608521-1-u-kumar1@ti.com/
> 
> Changes in v2:
> - Kept clk-sci.c as is because this is used by
> AM64 and AM65 platforms
> - v1 link
> https://lore.kernel.org/all/20230919060649.2518147-1-u-kumar1@ti.com
> 
>  drivers/clk/ti/clk-k3.c  | 11 +++++++++++
>  drivers/clk/ti/clk-sci.c | 11 ++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/ti/clk-k3.c b/drivers/clk/ti/clk-k3.c
> index ba925fa3c4..e8d8f8adf3 100644
> --- a/drivers/clk/ti/clk-k3.c
> +++ b/drivers/clk/ti/clk-k3.c
> @@ -11,6 +11,7 @@
>  #include <errno.h>
>  #include <soc.h>
>  #include <clk-uclass.h>
> +#include <k3-avs.h>
>  #include "k3-clk.h"
>  
>  #define PLL_MIN_FREQ	800000000
> @@ -242,7 +243,13 @@ static ulong ti_clk_set_rate(struct clk *clk, ulong rate)
>  	const struct clk_ops *ops;
>  	ulong new_rate, rem;
>  	ulong diff, new_diff;
> +	int set_avs_after_clock = 1;

int freq_scale_up = rate >= ti_clk_get_rate(clk) ? 1 : 0;

>  
> +	if (rate >= ti_clk_get_rate(clk) && IS_ENABLED(CONFIG_K3_AVS0)) {

I think it is better to put IS_ENABLED() as the first check.
See other usage:
$ git grep IS_ENABLED . |grep if|grep '&&'

> +		k3_avs_notify_freq(data->map[clk->id].dev_id,
> +				   data->map[clk->id].clk_id, rate);
> +		set_avs_after_clock = 0;
> +	}
>  	/*
>  	 * We must propagate rate change to parent if current clock type
>  	 * does not allow setting it.
> @@ -339,6 +346,10 @@ static ulong ti_clk_set_rate(struct clk *clk, ulong rate)
>  		}
>  	}
>  
> +	if (set_avs_after_clock && IS_ENABLED(CONFIG_K3_AVS0))
> +		k3_avs_notify_freq(data->map[clk->id].dev_id,
> +				   data->map[clk->id].clk_id, rate);
> +
>  	return new_rate;
>  }
>  
> diff --git a/drivers/clk/ti/clk-sci.c b/drivers/clk/ti/clk-sci.c
> index 74df5a397b..1926843dcc 100644
> --- a/drivers/clk/ti/clk-sci.c
> +++ b/drivers/clk/ti/clk-sci.c
> @@ -91,12 +91,14 @@ static ulong ti_sci_clk_set_rate(struct clk *clk, ulong rate)
>  	const struct ti_sci_handle *sci = data->sci;
>  	const struct ti_sci_clk_ops *cops = &sci->ops.clk_ops;
>  	int ret;
> +	u8 set_avs_after_clock = 1;
>  
>  	debug("%s(clk=%p, rate=%lu)\n", __func__, clk, rate);
>  
> -#ifdef CONFIG_K3_AVS0
> -	k3_avs_notify_freq(clk->id, clk->data, rate);
> -#endif
> +	if (rate >= ti_sci_clk_get_rate(clk) && IS_ENABLED(CONFIG_K3_AVS0)) {
> +		k3_avs_notify_freq(clk->id, clk->data, rate);
> +		set_avs_after_clock = 0;
> +	}
>  
>  	ret = cops->set_freq(sci, clk->id, clk->data, 0, rate, ULONG_MAX);
>  	if (ret) {
> @@ -104,6 +106,9 @@ static ulong ti_sci_clk_set_rate(struct clk *clk, ulong rate)
>  		return ret;
>  	}
>  
> +	if (set_avs_after_clock && IS_ENABLED(CONFIG_K3_AVS0))
> +		k3_avs_notify_freq(clk->id, clk->data, rate);
> +
>  	return rate;
>  }
>  
> -- 
> 2.34.1
> 

First look - it looks fine, but note: these are two different patches.
the clk-sci.c is a fix for an existing implementation and clk-k3.c is a
new feature addition. please don't mix the two.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D