[PATCH v3 2/3] net: fm: Add firmware name parameter

[Sean Anderson]

On 12/29/22 17:38, Simon Glass wrote:
> Hi Sean,
> 
> On Thu, 29 Dec 2022 at 10:53, Sean Anderson <sean.anderson at 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.anderson at seco.com>
>> Reviewed-by: Ramon Fried <rfried.dev at 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, &reg->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).

--Sean