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

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