Re: [PATCH] net/eth: read default mac-address default from dts

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 15.06.21 22:32, Trent Piepho wrote:
> Once upon a time, it was common for kernel dts files to be booted with
> u-boot to include an all zero mac address property, since u-boot could
> not unpack/pack the fdt.  It could only find an existing property and
> change bytes already present, thus adding a blank mac address to be
> patched.  Barebox has a much better fdt fixup system and does not need
> this.
> 
> But maybe these blank mac address properties are still there in some
> of the dts files, which mostly come from the kernel dts sources?
> Might be worth ignoring an all zero address rather than calling it
> found.
> 
> Unless of_get_mac_address() already includes such logic in a way that works ok?

of_get_mac_address does indeed check for valid MAC addresses, but some device trees
contain dummy mac-addresses that pass the validity check, e.g.
[ 00 10 18 36 23 1a ] in some broadcom DTSIs. I haven't checked whether there is
any board we support that's affected by this.

> On Tue, Jun 15, 2021 at 12:49 PM Michael Grzeschik
> <m.grzeschik@pengutronix.de> wrote:
>>
>> Since we have the functino of_get_mac_address we can
>> use it to set the default mac address vom the dts.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  net/eth.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/net/eth.c b/net/eth.c
>> index 84f99d3aa8..baebf89d89 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -11,6 +11,7 @@
>>  #include <net.h>
>>  #include <dma.h>
>>  #include <of.h>
>> +#include <of_net.h>
>>  #include <linux/phy.h>
>>  #include <errno.h>
>>  #include <malloc.h>
>> @@ -431,6 +432,14 @@ int eth_register(struct eth_device *edev)
>>         if (!ret)
>>                 found = 1;
>>
>> +       if (!found && edev->parent) {
>> +               const u8 *maddr = of_get_mac_address(edev->parent->device_node);
>> +               if (maddr) {
>> +                       memcpy(ethaddr, maddr, ETH_ALEN);
>> +                       found = 1;
>> +               }
>> +       }

For the fetch-ethaddr-from-nvmem patches Sascha just merged, I elected to
parse the device tree in a postenvironment call to allow existing board
code, OTP drivers and network drivers to specify a mac address first, so
existing behavior isn't changed. I'd suggest you go the same route.
That would mean changing of_get_mac_addr_nvmem to of_get_mac_address in
this file in next.

>> +
>>         if (!found) {
>>                 ret = edev->get_ethaddr(edev, ethaddr);
>>                 if (!ret)
>> --
>> 2.29.2
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox