On 12/30/22 12:51, Simon Glass wrote:
> Hi Sean,
>
> On Fri, 30 Dec 2022 at 09:32, 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:53, Sean Anderson <sean.ander...@seco.com> 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 <sean.ander...@seco.com>
>> >> Reviewed-by: Ramon Fried <rfried....@gmail.com>
>> >> ---
>> >>
>> >> (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.
--Sean
