[PATCH v3 3/3] net: fm: Support loading firmware from a filesystem

[Sean Anderson]

On 12/29/22 17:38, Simon Glass wrote:
> Hi Sean,
> 
> On Thu, 29 Dec 2022 at 10:54, Sean Anderson <sean.anderson at seco.com> wrote:
>>
>> This adds a new method to load Fman firmware from a filesystem. This
>> allows users to use regular files instead of hard-coded offsets for the
>> firmware.
>>
>> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
>> Reviewed-by: Ramon Fried <rfried.dev at gmail.com>
>> ---
>>
>> (no changes since v1)
>>
>>  drivers/net/fm/fm.c | 25 ++++++++++++++++++++++++-
>>  drivers/qe/Kconfig  |  4 ++++
>>  2 files changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
>> index 457200e766..e1fba24471 100644
>> --- a/drivers/net/fm/fm.c
>> +++ b/drivers/net/fm/fm.c
>> @@ -5,6 +5,7 @@
>>   */
>>  #include <common.h>
>>  #include <env.h>
>> +#include <fs_loader.h>
>>  #include <image.h>
>>  #include <malloc.h>
>>  #include <asm/io.h>
>> @@ -452,7 +453,29 @@ int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
>>  int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
>>  {
>>         int rc;
>> -#if defined(CONFIG_SYS_QE_FMAN_FW_IN_NOR)
>> +#if defined(CONFIG_SYS_QE_FMAN_FW_IN_FS)
> 
> Cam this use C code?

I'll look into it...

>> +       struct udevice *fs_loader;
>> +       void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
> 
> For this you can use something like:
> 
> IF_ENABLED_INT(CONFIG_SYS_QE_FMAN_FW_IN_FS, CONFIG_SYS_QE_FMAN_FW_LENGTH)
> 
> so that C works
> 
>> +
>> +       if (!addr)
>> +               return -ENOMEM;
>> +
>> +       rc = get_fs_loader(&fs_loader);
>> +       if (rc) {
>> +               debug("could not get fs loader: %d\n", rc);
>> +               return rc;
>> +       }
>> +
>> +       if (!firmware_name)
>> +               firmware_name = "fman.itb";
>> +
>> +       rc = request_firmware_into_buf(fs_loader, firmware_name, addr,
>> +                                      CONFIG_SYS_QE_FMAN_FW_LENGTH, 0);
>> +       if (rc < 0) {
>> +               debug("could not request %s: %d\n", firmware_name, rc);
>> +               return rc;
>> +       }
>> +#elif defined(CONFIG_SYS_QE_FMAN_FW_IN_NOR)
>>         void *addr = (void *)CONFIG_SYS_FMAN_FW_ADDR;
>>  #elif defined(CONFIG_SYS_QE_FMAN_FW_IN_NAND)
>>         size_t fw_length = CONFIG_SYS_QE_FW_LENGTH;
>> diff --git a/drivers/qe/Kconfig b/drivers/qe/Kconfig
>> index c44a81f69a..89a75c175b 100644
>> --- a/drivers/qe/Kconfig
>> +++ b/drivers/qe/Kconfig
>> @@ -27,6 +27,10 @@ choice
>>         depends on FMAN_ENET || QE
>>         default SYS_QE_FMAN_FW_IN_ROM
>>
>> +config SYS_QE_FMAN_FW_IN_FS
>> +       depends on FS_LOADER && FMAN_ENET
>> +       bool "Filesystem"
> 
> Should this be a choice?

It is.

> In any case it needs some decent help!

I think it's reasonable in context (the choice is "QUICC Engine FMan
ethernet firmware location"). It's in the same (terse) style as the rest
of the file.

--Sean

>> +
>>  config SYS_QE_FMAN_FW_IN_NOR
>>         bool "NOR flash"
>>
>> --
>> 2.35.1.1320.gc452695387.dirty
>>
> 
> Regards,
> Simon