| Message ID | 20221229165301.2621563-4-sean.anderson@seco.com |
|---|---|
| State | Accepted |
| Commit | f4426fd68d604fc3f823979d7f643938894df167 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | net: fm: Add support for loading firmware from filesystem | expand |
Hi Sean, On Thu, 29 Dec 2022 at 10:54, Sean Anderson <sean.anderson@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@seco.com> > Reviewed-by: Ramon Fried <rfried.dev@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? > + 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? In any case it needs some decent help! e > + > config SYS_QE_FMAN_FW_IN_NOR > bool "NOR flash" > > -- > 2.35.1.1320.gc452695387.dirty > Regards, Simon
On 12/29/22 17:38, Simon Glass wrote: > Hi Sean, > > On Thu, 29 Dec 2022 at 10:54, Sean Anderson <sean.anderson@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@seco.com> >> Reviewed-by: Ramon Fried <rfried.dev@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
Hi Sean, On Fri, 30 Dec 2022 at 09:36, Sean Anderson <sean.anderson@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.anderson@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@seco.com> > >> Reviewed-by: Ramon Fried <rfried.dev@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
On Thu, Dec 29, 2022 at 11:53:01AM -0500, Sean Anderson 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@seco.com> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com> Applied to u-boot/master, thanks!
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) + struct udevice *fs_loader; + void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH); + + 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_FMAN_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" + config SYS_QE_FMAN_FW_IN_NOR bool "NOR flash"