Hi Sean,
On Fri, 30 Dec 2022 at 09:36, Sean Anderson <sean.ander...@seco.com> wrote:
>
> On 12/29/22 17:38, Simon Glass wrote:
> > Hi Sean,
> >
> > On Thu, 29 Dec 2022 at 10:54, Sean Anderson <sean.ander...@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.ander...@seco.com>
> >> Reviewed-by: Ramon Fried <rfried....@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.
OK I see. But which filesystem, which filename, ...?
>
> > 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.
Terse is one word for it. Is there actually any documentation? I see
some stuff in README which is the wrong place.
Regards,
Simon
>
> --Sean
>
> >> +
> >> config SYS_QE_FMAN_FW_IN_NOR
> >> bool "NOR flash"
> >>
> >> --
> >> 2.35.1.1320.gc452695387.dirty
> >>
> >
> > Regards,
> > Simon
