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

[Nishanth Menon]

On 22:58-20230919, Kumar, Udit wrote:
> 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.

Thanks.

> > 
> > 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.

We should handle both cases, ideally.

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