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-keer...@ti.com>
> Signed-off-by: Udit Kumar <u-kum...@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-kum...@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-kum...@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
