Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
> The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency
> for A72/MSMC clks and the OPP_NOM voltage.
>
> J7200 SOCs may support OPP_LOW Operating Performance Point:
> 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
>
> Hence, add a config check in board_init_f() to select OPP_LOW specs:
> - Check if OPP_LOW AVS voltage read from efuse is valid.
> - Use the device IDs and clock IDs (TISCI docs [0]) to find the A72 and
> MSMC clock frequencies in the devicetree.
> - Fixup the clock frequencies in devicetree as per OPP_LOW spec.
>
> k3_avs driver programs the OPP_LOW AVS voltage for VDD_CPU through
> k3_avs_notify_freq() callback from clk_k3.
>
> [0]: https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/j7200/clocks.html
>
> Signed-off-by: Aniket Limaye <[email protected]>
> ---
[..]
> ---
> arch/arm/mach-k3/j721e/j721e_init.c | 92 ++++++++++++++++++++++++++++-
> 1 file changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-k3/j721e/j721e_init.c
> b/arch/arm/mach-k3/j721e/j721e_init.c
> index e9ed8cb267c..805b28af8e4 100644
> --- a/arch/arm/mach-k3/j721e/j721e_init.c
> +++ b/arch/arm/mach-k3/j721e/j721e_init.c
> @@ -19,6 +19,7 @@
[..]
> +#if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0)
> +static int get_clock_index_by_dev_id(ofnode node, u32 dev_id, u32 clk_id)
I think the following function can be re-usable by other K3 socs too,
can we put it in some common place instead of j721e_init? Maybe clk-k3.c
and exported from there?
> +
> +static int fdt_fixup_a72ss_clock_frequency(void)
> +{
> + int index, size;
> + u32 *rates;
> + ofnode node;
> +
> + node = ofnode_by_compatible(ofnode_null(), "ti,am654-rproc");
> + if (!ofnode_valid(node)) {
> + printf("%s: A72 not found\n", __func__);
> + return -ENODEV;
> + }
> +
> + rates = fdt_getprop_w(ofnode_to_fdt(node), ofnode_to_offset(node),
> + "assigned-clock-rates", &size);
> + if (!rates) {
> + printf("%s: Wrong A72 assigned-clocks-rates configuration\n",
> __func__);
> + return -1;
> + }
> +
> + /* Update A72 Clock Frequency to OPP_LOW spec */
> + index = get_clock_index_by_dev_id(node,
> + DEV_A72SS0_CORE0_0_ID,
> + DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID);
> + if (index < 0 || index >= (size / sizeof(u32))) {
When would we encounter the second if..?
> + printf("%s: Wrong A72 assigned-clocks configuration\n",
> __func__);
> + return -1;
> + }
> + rates[index] = cpu_to_fdt32(1000000000);
> + printf("Changed A72 CPU frequency to %dHz in DT\n", 1000000000);
> +
> + /* Update MSMC Clock Frequency to OPP_LOW spec */
> + index = get_clock_index_by_dev_id(node,
> + DEV_A72SS0_CORE0_ID,
> + DEV_A72SS0_CORE0_MSMC_CLK_ID);
> + if (index < 0 || index >= (size / sizeof(u32))) {
> + printf("%s: Wrong A72 assigned-clocks configuration\n",
> __func__);
> + return -1;
> + }
> + rates[index] = cpu_to_fdt32(500000000);
> + printf("Changed MSMC frequency to %dHz in DT\n", 500000000);
> +
> + return 0;
> +}
> +#endif
[..]
Regards,
Manorit
