[PATCH v2 06/18] arm: mach-k3: sysfw-loader: Add support for rom loading sysfw image

From: Lokesh Vutla <lokeshvutla@ti.com>
To: u-boot@lists.denx.de
Subject: [PATCH v2 06/18] arm: mach-k3: sysfw-loader: Add support for rom loading sysfw image
Date: Wed, 29 Jul 2020 08:05:39 +0530	[thread overview]
Message-ID: <a977e650-e53b-35d3-a074-4ec513d20b3e@ti.com> (raw)
In-Reply-To: <4b488be6-e2fc-6844-933e-186582231e1e@ti.com>

On 27/07/20 10:47 pm, Suman Anna wrote:
> Hi Lokesh,
> 
> On 7/27/20 4:45 AM, Lokesh Vutla wrote:
>> Starting J7200 SoC, ROM supports for loading sysfw directly from boot
>> image. In such cases, SPL need not load sysfw from boot media, but need
>> to receive boot notification message from sysfw. So separate out
>> remoteproc calls for system controller from sysfw loader and just
>> receive the boot notification if sysfw is already loaded.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> ? arch/arm/mach-k3/am6_init.c????????????????? |? 2 +-
>> ? arch/arm/mach-k3/include/mach/sysfw-loader.h |? 4 +-
>> ? arch/arm/mach-k3/j721e_init.c??????????????? |? 2 +-
>> ? arch/arm/mach-k3/sysfw-loader.c????????????? | 56 +++++++++++++-------
>> ? 4 files changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c
>> index abdec76d73..4250ac355b 100644
>> --- a/arch/arm/mach-k3/am6_init.c
>> +++ b/arch/arm/mach-k3/am6_init.c
>> @@ -155,7 +155,7 @@ void board_init_f(ulong dummy)
>> ?????? * Load, start up, and configure system controller firmware while
>> ?????? * also populating the SYSFW post-PM configuration callback hook.
>> ?????? */
>> -??? k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
>> +??? k3_sysfw_loader(false, k3_mmc_stop_clock, k3_mmc_restart_clock);
>> ? ????? /* Prepare console output */
>> ????? preloader_console_init();
>> diff --git a/arch/arm/mach-k3/include/mach/sysfw-loader.h
>> b/arch/arm/mach-k3/include/mach/sysfw-loader.h
>> index 6f5612b4fd..b23a9e821e 100644
>> --- a/arch/arm/mach-k3/include/mach/sysfw-loader.h
>> +++ b/arch/arm/mach-k3/include/mach/sysfw-loader.h
>> @@ -7,6 +7,8 @@
>> ? #ifndef _SYSFW_LOADER_H_
>> ? #define _SYSFW_LOADER_H_
>> ? -void k3_sysfw_loader(void (*config_pm_pre_callback)(void), void
>> (*config_pm_done_callback)(void));
>> +void k3_sysfw_loader(bool rom_loaded_sysfw,
>> +???????????? void (*config_pm_pre_callback)(void),
>> +???????????? void (*config_pm_done_callback)(void));
>> ? ? #endif
>> diff --git a/arch/arm/mach-k3/j721e_init.c b/arch/arm/mach-k3/j721e_init.c
>> index 2010cab1d1..461a9d7f8f 100644
>> --- a/arch/arm/mach-k3/j721e_init.c
>> +++ b/arch/arm/mach-k3/j721e_init.c
>> @@ -174,7 +174,7 @@ void board_init_f(ulong dummy)
>> ?????? * callback hook, effectively switching on (or over) the console
>> ?????? * output.
>> ?????? */
>> -??? k3_sysfw_loader(k3_mmc_stop_clock, k3_mmc_restart_clock);
>> +??? k3_sysfw_loader(false, k3_mmc_stop_clock, k3_mmc_restart_clock);
> 
> Any reason why you want to add the new argument at the front, rather than the
> end which is typical?

Yes this was intentional. It made more sense to get the sysfw state as a first
parameter and then look for other parameters,

> 
>> ? ????? /* Prepare console output */
>> ????? preloader_console_init();
>> diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c
>> index 513be09c68..f62bfa995c 100644
>> --- a/arch/arm/mach-k3/sysfw-loader.c
>> +++ b/arch/arm/mach-k3/sysfw-loader.c
>> @@ -32,6 +32,12 @@ DECLARE_GLOBAL_DATA_PTR;
>> ? #define SYSFW_CFG_RM??????????? "rm-cfg.bin"
>> ? #define SYSFW_CFG_SEC??????????? "sec-cfg.bin"
>> ? +/*
>> + * It is assumed that remoteproc device 0 is the corresponding
>> + * system-controller that runs SYSFW. Make sure DT reflects the same.
>> + */
>> +#define K3_SYSTEM_CONTROLLER_RPROC_ID??? 0
>> +
>> ? static bool sysfw_loaded;
>> ? static void *sysfw_load_address;
>> ? @@ -71,6 +77,26 @@ static int fit_get_data_by_name(const void *fit, int
>> images, const char *name,
>> ????? return fit_image_get_data(fit, node_offset, addr, size);
>> ? }
>> ? +static void k3_start_system_controller(int rproc_id, bool rproc_loaded,
>> +?????????????????????? ulong addr, ulong size)
> 
> Are you expecting the SYSTEM_CONTROLLER_RPROC_ID to be a different value from 0?
> Do you really need the rproc_id argument, rather than directly using it in code?

I didn't want to hardcode in multiple places and I cannot guarantee if it can
change in future. So I added it as a future proof.

Thanks and regards,
Lokesh