mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Trent Piepho <trent.piepho@igorinstitute.com>,
	Michael Grzeschik <m.grzeschik@pengutronix.de>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH] net/eth: read default mac-address default from dts
Date: Mon, 21 Jun 2021 08:42:36 +0200	[thread overview]
Message-ID: <ace6de12-bbf5-78bc-b976-ef8a53a672af@pengutronix.de> (raw)
In-Reply-To: <CAMHeXxNpR01tPQ-M2-Oj8S4s9QxS-i7Vw+H3ED1NH8uO9rZekQ@mail.gmail.com>

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


  reply	other threads:[~2021-06-21  6:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 15:32 Michael Grzeschik
2021-06-15 20:32 ` Trent Piepho
2021-06-21  6:42   ` Ahmad Fatoum [this message]
2021-06-15 17:17 Michael Grzeschik
2021-06-16  7:14 ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ace6de12-bbf5-78bc-b976-ef8a53a672af@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=m.grzeschik@pengutronix.de \
    --cc=trent.piepho@igorinstitute.com \
    --subject='Re: [PATCH] net/eth: read default mac-address default from dts' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox