Hi Sean, On Fri, 30 Dec 2022 at 11:53, Sean Anderson <[email protected]> wrote: > > On 12/30/22 12:51, Simon Glass wrote: > > Hi Sean, > > > > On Fri, 30 Dec 2022 at 09:32, Sean Anderson <[email protected]> wrote: > >> > >> On 12/29/22 17:38, Simon Glass wrote: > >> > Hi Sean, > >> > > >> > On Thu, 29 Dec 2022 at 10:53, Sean Anderson <[email protected]> > >> > wrote: > >> >> > >> >> In order to read the firmware from the filesystem, we need a file name. > >> >> Read the firmware name from the device tree, using the firmware-name > >> >> property. This property is commonly used in Linux to determine the > >> >> correct name to use (and can be seen in several device trees in U-Boot). > >> >> > >> >> Signed-off-by: Sean Anderson <[email protected]> > >> >> Reviewed-by: Ramon Fried <[email protected]> > >> >> --- > >> >> > >> >> (no changes since v1) > >> >> > >> >> drivers/net/fm/fm.c | 15 ++++++++++++--- > >> >> drivers/net/fm/fm.h | 2 +- > >> >> 2 files changed, 13 insertions(+), 4 deletions(-) > >> >> > >> >> diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c > >> >> index 055dd61fbe..457200e766 100644 > >> >> --- a/drivers/net/fm/fm.c > >> >> +++ b/drivers/net/fm/fm.c > >> >> @@ -8,6 +8,7 @@ > >> >> #include <image.h> > >> >> #include <malloc.h> > >> >> #include <asm/io.h> > >> >> +#include <dm/device_compat.h> > >> >> #include <linux/errno.h> > >> >> #include <u-boot/crc.h> > >> >> #include <dm.h> > >> >> @@ -353,7 +354,7 @@ static void fm_init_qmi(struct fm_qmi_common *qmi) > >> >> > >> >> /* Init common part of FM, index is fm num# like fm as above */ > >> >> #ifdef CONFIG_TFABOOT > >> >> -int fm_init_common(int index, struct ccsr_fman *reg) > >> >> +int fm_init_common(int index, struct ccsr_fman *reg, const char > >> >> *firmware_name) > >> >> { > >> >> int rc; > >> >> void *addr = NULL; > >> >> @@ -448,7 +449,7 @@ int fm_init_common(int index, struct ccsr_fman *reg) > >> >> return fm_init_bmi(index, ®->fm_bmi_common); > >> >> } > >> >> #else > >> >> -int fm_init_common(int index, struct ccsr_fman *reg) > >> >> +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) > >> >> @@ -561,6 +562,8 @@ static const struct udevice_id fman_ids[] = { > >> >> > >> >> static int fman_probe(struct udevice *dev) > >> >> { > >> >> + const char *firmware_name = NULL; > >> >> + int ret; > >> >> struct fman_priv *priv = dev_get_priv(dev); > >> >> > >> >> priv->reg = (struct ccsr_fman *)(uintptr_t)dev_read_addr(dev); > >> >> @@ -570,7 +573,13 @@ static int fman_probe(struct udevice *dev) > >> >> return -EINVAL; > >> >> } > >> >> > >> >> - return fm_init_common(priv->fman_id, priv->reg); > >> >> + ret = dev_read_string_index(dev, "firmware-name", 0, > >> >> &firmware_name); > >> >> + if (ret && ret != -EINVAL) { > >> >> + dev_dbg(dev, "Could not read firmware-name\n"); > >> >> + return ret; > >> >> + } > >> >> + > >> >> + return fm_init_common(priv->fman_id, priv->reg, firmware_name); > >> >> } > >> >> > >> >> static int fman_remove(struct udevice *dev) > >> >> diff --git a/drivers/net/fm/fm.h b/drivers/net/fm/fm.h > >> >> index ba858cc24b..a2d5b03429 100644 > >> >> --- a/drivers/net/fm/fm.h > >> >> +++ b/drivers/net/fm/fm.h > >> >> @@ -106,7 +106,7 @@ struct fm_port_global_pram { > >> >> > >> >> void *fm_muram_alloc(int fm_idx, size_t size, ulong align); > >> >> void *fm_muram_base(int fm_idx); > >> >> -int fm_init_common(int index, struct ccsr_fman *reg); > >> >> +int fm_init_common(int index, struct ccsr_fman *reg, const char > >> >> *firmware_name); > >> > > >> > Please add a full function comment. > >> > >> I don't think that's really necessary. This is called from one place, and > >> serves > >> mostly to organize the code (now that non-DM net is gone). > >> > > > > As a matter of good practice, all exported functions should be > > documented in their header files. May as well start now, can perhaps > > include the header docs in an rST file. > > Well, it's not actually used in any other file any more, so I propose just > marking it > as static and removing it from the header.
OK Regards, Simon
