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

[Kumar, Udit]

Hi Nishanth,

On 9/19/2023 9:07 PM, Nishanth Menon wrote:
> On 19:34-20230919, 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
>> upon setting clock rate so that voltage is changed accordingly.
>>
>> Cc: Keerthy <j-keerthy at ti.com>
>> Signed-off-by: Udit Kumar <u-kumar1 at ti.com>
>> ---
>> Change log:
>> 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 | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/clk/ti/clk-k3.c b/drivers/clk/ti/clk-k3.c
>> index ba925fa3c4..8016c3784a 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
>> @@ -339,6 +340,10 @@ static ulong ti_clk_set_rate(struct clk *clk, ulong rate)
>>   		}
>>   	}
>>   
>> +	if (IS_ENABLED(CONFIG_K3_AVS0))
>> +		k3_avs_notify_freq(data->map[clk->id].dev_id,
>> +				   data->map[clk->id].clk_id, rate);
>> +
> Don't you want to do this prior to actual clock change?

Not really, As i see new freq to be less than current one.

> F1, V1 -> F2, V2
>
> Seq 1:
> F1, V1
> F2, V1
> F2, V2
>
> Seq 2:
> F1, V1
> F1, V2
> F2, V2
>
> If V2 < V1 (implying F2 < F1), then Seq 1 is valid. But if V2 > V1 (F2 >
> F1) Seq 2 is valid.
>
> We seem to ignore this constraint. Can you explain this in the commit
> message?

Sure, will do , Below is reasoning, Please see if make sense

On J7 devices,  In default device tree, A72 freq is set to 2Ghz, few 
SOCs can operate at lower freq.

So while setting OPP freq (which is less than default one), First freq 
needs to set

before changing voltage.


On other hand, I am thinking to fix properly for both cases, 
irrespective of seq 1 is

valid for current use case.


>
> clk-sci.c seems to use Seq 2, vs this patch seems to take Seq 1.

I think, something to be fixed for clk-sci.c as well.
As I see three freq point 
https://elixir.bootlin.com/u-boot/latest/source/drivers/misc/k3_avs.c#L339

So we can choose either way new-freq > current-freq or new-freq < 
current-freq,

Based upon new-freq , voltage to be set before/after clock change.


>
>>   	return new_rate;
>>   }
>>   
>> -- 
>> 2.34.1
>>