Re: [PATCH 04/10] fdt_support: Implement fdt_ethernet_set_macaddr()

["Alex G." <mr.nuke.me@gmail.com>]

Hi Sean,

On 8/26/21 6:35 PM, Sean Anderson wrote:
> 
> 
> On 8/26/21 5:42 PM, Alexandru Gagniuc wrote:
>> Oftentimes we have MAC address information stored in a ROM or OTP. The
>> way to add that to the FDT would be through the u-boot environment,
>> and then fdt_fixup_ethernet(). This is not very useful in SPL.
>>
>> It would be more helpful to be able to "set interface x to MAC y".
>> This is where fdt_ethernet_set_macaddr() comes in. It is similar in
>> function to fdt_fixup_ethernet(), but only updates one interface,
>> without using the u-boot env, and without string processing.
> 
> Have you considered adopting the nvmem-cells property for ethernet
> controllers (added in Linux commit 0e839df92cf3 ("net: ethernet: provide
> nvmem_get_mac_address()"))?

Obviously I haven't. It sounds like a great idea. Thank you for pointing 
me to it.

Alex

> --Sean
> 
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> ---
>>   common/fdt_support.c  | 30 ++++++++++++++++++++++++++++++
>>   include/fdt_support.h | 17 +++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index 4341d84bd5..c4cbd4060e 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -592,6 +592,36 @@ void fdt_fixup_ethernet(void *fdt)
>>       }
>>   }
>>
>> +int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t 
>> *mac_addr)
>> +{
>> +    const char *path, *name;
>> +    int prop, aliases_node;
>> +    char eth_name[16] = "ethernet";
>> +
>> +    aliases_node = fdt_path_offset(fdt, "/aliases");
>> +    if (aliases_node < 0)
>> +        return aliases_node;
>> +
>> +    if (ethnum >= 0)
>> +        sprintf(eth_name, "ethernet%d", ethnum);
>> +
>> +    fdt_for_each_property_offset(prop, fdt, aliases_node) {
>> +        path = fdt_getprop_by_offset(fdt, prop, &name, NULL);
>> +        if (!strcmp(name, eth_name))
>> +            break;
>> +
>> +        path = NULL;
>> +    }
>> +
>> +    if (!path)
>> +        return -FDT_ERR_NOTFOUND;
>> +
>> +    do_fixup_by_path(fdt, path, "mac-address", mac_addr, 6, 0);
>> +    do_fixup_by_path(fdt, path, "local-mac-address", mac_addr, 6, 1);
>> +
>> +    return 0;
>> +}
>> +
>>   int fdt_record_loadable(void *blob, u32 index, const char *name,
>>               uintptr_t load_addr, u32 size, uintptr_t entry_point,
>>               const char *type, const char *os, const char *arch)
>> diff --git a/include/fdt_support.h b/include/fdt_support.h
>> index f6f46bb8e9..3f0bcb5a00 100644
>> --- a/include/fdt_support.h
>> +++ b/include/fdt_support.h
>> @@ -119,6 +119,23 @@ static inline int fdt_fixup_memory_banks(void 
>> *blob, u64 start[], u64 size[],
>>   #endif
>>
>>   void fdt_fixup_ethernet(void *fdt);
>> +
>> +/**
>> + * Set the "mac-address" and "local-mac-address" of ethernet node
>> + * The ethernet node is located from the "/aliases" section of the 
>> fdt. When
>> + * 'ethnum' is positive, then the name is matched exactly, e.g 
>> "ethernet0".
>> + * When ethnum is negative, the first ethernet alias is updated.
>> + * Unlike fdt_fixup_ethernet(), this function only updates one 
>> ethernet node,
>> + * and soes not use the "ethaddr" from the u-boot environment. This 
>> is useful,
>> + * for example, in SPL, when the environment is not initialized or 
>> available.
>> + *
>> + * @param fdt        FDT blob to update
>> + * @param ethnum    Ethernet device index, or negative for any ethernet
>> + * @param mac_addr    Pointer to 6-byte array containing the MAC address
>> + *
>> + * @return 0 if ok, or -FDT_ERR_... on error
>> + */
>> +int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t 
>> *mac_addr);
>>   int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
>>                const void *val, int len, int create);
>>   void fdt_fixup_qe_firmware(void *fdt);
>>