[PATCH v3 02/10] arm: k3: Add support for loading non linux remote cores

Keerthy j-keerthy at ti.com
Thu Jan 23 05:10:41 CET 2020



On 22/01/20 9:55 pm, Andrew F. Davis wrote:
> On 1/21/20 8:10 PM, keerthy wrote:
>>
>>
>> On 1/21/2020 6:26 PM, Andrew F. Davis wrote:
>>> On 1/21/20 6:07 AM, Keerthy wrote:
>>>> Add MAIN domain R5FSS0 remoteproc support from spl. This enables
>>>> loading the elf firmware in SPL and starting the remotecore.
>>>>
>>>> In order to start the core, there should be a file with path
>>>> "/lib/firmware/j7-main-r5f0_0-fw" under filesystem
>>>> of respective boot mode.
>>>>
>>>> Signed-off-by: Keerthy <j-keerthy at ti.com>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
>>>> [Guard start_non_linux_remote_cores under CONFIG_FS_LOADER]
>>>> Signed-off-by: Andreas Dannenberg <dannenberg at ti.com>
>>>> ---
>>>>    arch/arm/mach-k3/common.c     | 84 ++++++++++++++++++++++++++++++++---
>>>>    arch/arm/mach-k3/common.h     |  2 +
>>>>    arch/arm/mach-k3/j721e_init.c | 34 ++++++++++++++
>>>>    3 files changed, 115 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>>>> index 8d1529062d..f0ac0c39f1 100644
>>>> --- a/arch/arm/mach-k3/common.c
>>>> +++ b/arch/arm/mach-k3/common.c
>>>> @@ -16,6 +16,10 @@
>>>>    #include <asm/arch/sys_proto.h>
>>>>    #include <asm/hardware.h>
>>>>    #include <asm/io.h>
>>>> +#include <fs_loader.h>
>>>> +#include <fs.h>
>>>> +#include <env.h>
>>>> +#include <elf.h>
>>>>      struct ti_sci_handle *get_ti_sci_handle(void)
>>>>    {
>>>> @@ -57,6 +61,74 @@ int early_console_init(void)
>>>>    #endif
>>>>      #ifdef CONFIG_SYS_K3_SPL_ATF
>>>> +
>>>> +void init_env(void)
>>>> +{
>>>> +#ifdef CONFIG_SPL_ENV_SUPPORT
>>>> +    char *part;
>>>> +
>>>> +    env_init();
>>>> +    env_load();
>>>> +    switch (spl_boot_device()) {
>>>> +    case BOOT_DEVICE_MMC2:
>>>> +        part = env_get("bootpart");
>>>> +        env_set("storage_interface", "mmc");
>>>> +        env_set("fw_dev_part", part);
>>>> +        break;
>>>> +    case BOOT_DEVICE_SPI:
>>>> +        env_set("storage_interface", "ubi");
>>>> +        env_set("fw_ubi_mtdpart", "UBI");
>>>> +        env_set("fw_ubi_volume", "UBI0");
>>>> +        break;
>>>> +    default:
>>>> +        printf("%s from device %u not supported!\n",
>>>> +               __func__, spl_boot_device());
>>>
>>>
>>> This will print for almost every boot mode..
>>
>> I can keep this under debug.
>>
>>>
>>>
>>>> +        return;
>>>> +    }
>>>> +#endif
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_FS_LOADER
>>>> +int load_firmware(char *name_fw, char *name_loadaddr, u32 *loadaddr)
>>>> +{
>>>> +    struct udevice *fsdev;
>>>> +    char *name = NULL;
>>>> +    int size = 0;
>>>> +
>>>> +    *loadaddr = 0;
>>>> +#ifdef CONFIG_SPL_ENV_SUPPORT
>>>> +    switch (spl_boot_device()) {
>>>> +    case BOOT_DEVICE_MMC2:
>>>> +        name = env_get(name_fw);
>>>> +        *loadaddr = env_get_hex(name_loadaddr, *loadaddr);
>>>> +        break;
>>>> +    default:
>>>> +        printf("Loading rproc fw image from device %u not
>>>> supported!\n",
>>>> +               spl_boot_device());
>>>
>>>
>>> This whole thing seems very MMC specific, if early firmware loading is
>>> important it should work for all boot modes. Find a way to include it in
>>> the next boot stage FIT image (tispl.bin) so it works for all modes.
>>
>> That was not NAKd. We are going with fs_loader approach.
>>
> 
> 
> When, where, link?

I had implemented that way internally. That was rejected for multiple 
right reasons:

1) SPL size would bloat based on the size of the firmware.
2) There are multiple cores that need to be loaded and hence adding all 
the firmwares under a fit can be really painful.
3) Changing firmware means building the tispl.bin again.

The FIT solution can not scale well.

- Keerthy
> 
> 
>>>
>>>
>>>> +        return 0;
>>>> +    }
>>>> +#endif
>>>> +    if (!*loadaddr)
>>>> +        return 0;
>>>> +
>>>> +    if (!uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &fsdev)) {
>>>> +        size = request_firmware_into_buf(fsdev, name, (void
>>>> *)*loadaddr,
>>>> +                         0, 0);
>>>> +    }
>>>> +
>>>> +    return size;
>>>> +}
>>>> +#else
>>>> +int load_firmware(char *name_fw, char *name_loadaddr, u32 *loadaddr)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>> +__weak void start_non_linux_remote_cores(void)
>>>> +{
>>>> +}
>>>> +
>>>>    void __noreturn jump_to_image_no_args(struct spl_image_info
>>>> *spl_image)
>>>>    {
>>>>        struct ti_sci_handle *ti_sci = get_ti_sci_handle();
>>>> @@ -65,15 +137,17 @@ void __noreturn jump_to_image_no_args(struct
>>>> spl_image_info *spl_image)
>>>>        /* Release all the exclusive devices held by SPL before
>>>> starting ATF */
>>>>        ti_sci->ops.dev_ops.release_exclusive_devices(ti_sci);
>>>>    +    ret = rproc_init();
>>>> +    if (ret)
>>>> +        panic("rproc failed to be initialized (%d)\n", ret);
>>>> +
>>>> +    init_env();
>>>> +    start_non_linux_remote_cores();
>>>> +
>>>>        /*
>>>>         * It is assumed that remoteproc device 1 is the corresponding
>>>>         * Cortex-A core which runs ATF. Make sure DT reflects the same.
>>>>         */
>>>> -    ret = rproc_dev_init(1);
>>>> -    if (ret)
>>>> -        panic("%s: ATF failed to initialize on rproc (%d)\n", __func__,
>>>> -              ret);
>>>> -
>>>
>>>
>>> Where did this code go?
>>
>> rproc_init takes care of that.
>>
> 
> 
> Is that new behavior then? It should be it's own patch with a commit
> message about that.
> 
> 
>>>
>>>
>>>>        ret = rproc_load(1, spl_image->entry_point, 0x200);
>>>>        if (ret)
>>>>            panic("%s: ATF failed to load on rproc (%d)\n", __func__,
>>>> ret);
>>>> diff --git a/arch/arm/mach-k3/common.h b/arch/arm/mach-k3/common.h
>>>> index d8b34fe060..42fb8ee6e7 100644
>>>> --- a/arch/arm/mach-k3/common.h
>>>> +++ b/arch/arm/mach-k3/common.h
>>>> @@ -24,3 +24,5 @@ void setup_k3_mpu_regions(void);
>>>>    int early_console_init(void);
>>>>    void disable_linefill_optimization(void);
>>>>    void remove_fwl_configs(struct fwl_data *fwl_data, size_t
>>>> fwl_data_size);
>>>> +void start_non_linux_remote_cores(void);
>>>> +int load_firmware(char *name_fw, char *name_loadaddr, u32 *loadaddr);
>>>> diff --git a/arch/arm/mach-k3/j721e_init.c
>>>> b/arch/arm/mach-k3/j721e_init.c
>>>> index f7f7398081..c5f8ede1a0 100644
>>>> --- a/arch/arm/mach-k3/j721e_init.c
>>>> +++ b/arch/arm/mach-k3/j721e_init.c
>>>> @@ -18,6 +18,7 @@
>>>>    #include <dm.h>
>>>>    #include <dm/uclass-internal.h>
>>>>    #include <dm/pinctrl.h>
>>>> +#include <remoteproc.h>
>>>>      #ifdef CONFIG_SPL_BUILD
>>>>    #ifdef CONFIG_K3_LOAD_SYSFW
>>>> @@ -295,3 +296,36 @@ void release_resources_for_core_shutdown(void)
>>>>        }
>>>>    }
>>>>    #endif
>>>> +
>>>> +#ifdef CONFIG_SYS_K3_SPL_ATF
>>>> +void start_non_linux_remote_cores(void)
>>>> +{
>>>> +    int size = 0, ret;
>>>> +    u32 loadaddr = 0;
>>>> +
>>>> +    size = load_firmware("mainr5f0_0fwname", "mainr5f0_0loadaddr",
>>>> +                 &loadaddr);
>>>> +    if (size <= 0)
>>>> +        goto err_load;
>>>> +
>>>> +    /*  remoteproc 2 is aliased for the needed remotecore */
>>>
>>>
>>> Assuming the big-arm core to boot is remoteproc 1 was reasonable, but
>>> there needs to be a better what than assuming the number for every other
>>> remote core.
>>>
>>>
>>>> +    ret = rproc_load(2, loadaddr, size);
>>>> +    if (ret) {
>>>> +        printf("Firmware failed to start on rproc (%d)\n", ret);
>>>> +        goto err_load;
>>>> +    }
>>>> +
>>>> +    ret = rproc_start(2);
>>>> +    if (ret) {
>>>> +        printf("Firmware init failed on rproc (%d)\n", ret);
>>>> +        goto err_load;
>>>> +    }
>>>> +
>>>> +    printf("Remoteproc 2 started successfully\n");
>>>
>>>
>>> That's useful..
>>
>> That is a print that tells everything went well with rproc 2. Otherwise
>> one has to really find other ways to see if it succeeded or not.
>>
> 
> 
> I'm just a customer booting my board, I have no idea what a "Remoteproc
> 2" is. I'm saying make the message describe what has happened.
> 
> Andrew
> 
> 
>>>
>>> Andrew
>>>
>>>
>>>> +
>>>> +    return;
>>>> +
>>>> +err_load:
>>>> +    rproc_reset(2);
>>>> +}
>>>> +#endif
>>>>


More information about the U-Boot mailing list