Re: [PATCH 4/6] arm: mach-k3: j721e-init.c: J7200: Add support for CONFIG_K3_OPP_LOW

[Neha Malcom Francis <n-francis@ti.com>]

Hi Aniket

On 17/10/24 11:59, Aniket Limaye wrote:
> From: Reid Tonking <reidt@ti.com>
> 
> 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 to select OPP_LOW specs:
> - Check if OPP_LOW AVS voltage read from efuse is valid.
> - Update the clock frequencies in devicetree.
> - Program the OPP_LOW AVS voltage for VDD_CPU.
> 
> Signed-off-by: Reid Tonking <reidt@ti.com>
> Signed-off-by: Aniket Limaye <a-limaye@ti.com>
> ---
>   arch/arm/mach-k3/j721e/j721e_init.c | 45 ++++++++++++++++++++++++++++-
>   drivers/misc/k3_avs.c               |  5 ++++
>   2 files changed, 49 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..0620759c36c 100644
> --- a/arch/arm/mach-k3/j721e/j721e_init.c
> +++ b/arch/arm/mach-k3/j721e/j721e_init.c
> @@ -19,6 +19,7 @@
>   #include <fdtdec.h>
>   #include <mmc.h>
>   #include <remoteproc.h>
> +#include <k3-avs.h>
>   
>   #include "../sysfw-loader.h"
>   #include "../common.h"
> @@ -147,6 +148,32 @@ static void setup_navss_nb(void)
>   	writel(NB_THREADMAP_BIT1, (uintptr_t)NAVSS0_NBSS_NB1_CFG_NB_THREADMAP);
>   }
>   
> +int fix_freq(const void *fdt)
> +{
> +		int node, ret;
> +		u32 opp_low_freq[3];
> +
> +		node = fdt_node_offset_by_compatible(fdt, -1, "ti,am654-rproc");
> +			if (node < 0) {
> +				printf("%s: A72 not found\n", __func__);
> +				return node;
> +			}

Indentation seems to be off.

> +
> +		/* j7200 opp low values according to data sheet */
> +		opp_low_freq[0] = cpu_to_fdt32(1000000000); /* 202-2 -> A72SS0_CORE0_0_ARM_CLK */
> +		opp_low_freq[1] = cpu_to_fdt32(200000000); /* 61-1 -> GTC0_GTC_CLK */
> +		opp_low_freq[2] = cpu_to_fdt32(500000000); /* 4-1 -> A72SS0_CORE0_MSMC_CLK */
> +
> +		ret = fdt_setprop((void *)fdt, node, "assigned-clock-rates",
> +				  opp_low_freq, sizeof(opp_low_freq));
> +			if (ret) {
> +				printf("%s: Can not set value\n", __func__);
> +				return ret;
> +			}
> +
> +		return 0;
> +}
> +
>   /*
>    * This uninitialized global variable would normal end up in the .bss section,
>    * but the .bss is cleared between writing and reading this variable, so move
> @@ -301,8 +328,24 @@ void board_init_f(ulong dummy)
>   #if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0)
>   	ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(k3_avs),
>   					  &dev);
> -	if (ret)
> +	if (!ret) {
> +		if (IS_ENABLED(CONFIG_K3_OPP_LOW)) {
> +			ret = k3_check_opp(dev, J721E_VDD_MPU, AM6_OPP_LOW);
> +			if (!ret) {
> +				ret = fix_freq(gd->fdt_blob);
> +				if (ret)
> +					printf("Failed to set OPP_LOW frequency\n");
> +
> +				ret = k3_avs_set_opp(dev, J721E_VDD_MPU, AM6_OPP_LOW);
> +				if (ret)
> +					printf("Failed to set OPP_LOW voltage\n");
> +			} else {
> +				printf("Failed to enable K3_OPP_LOW\n");
> +			}
> +		}
> +	} else {
>   		printf("AVS init failed: %d\n", ret);
> +	}
>   #endif
>   
>   #if defined(CONFIG_K3_J721E_DDRSS)
> diff --git a/drivers/misc/k3_avs.c b/drivers/misc/k3_avs.c
> index 90cd9dfe7f9..932b355a5c1 100644
> --- a/drivers/misc/k3_avs.c
> +++ b/drivers/misc/k3_avs.c
> @@ -121,6 +121,11 @@ static int k3_avs_program_voltage(struct k3_avs_privdata *priv,
>   	if (!vd->supply)
>   		return -ENODEV;
>   
> +	if (!volt) {
> +		dev_err(priv->dev, "Fuse is not set for selected opp %d\n", opp_id);

s/Fuse/Efuse
> +		return -EINVAL;
> +	}
> +
>   	vd->opp = opp_id;
>   	vd->flags |= VD_FLAG_INIT_DONE;
>   

-- 
Thanking You
Neha Malcom Francis