On Fri, Nov 19, 2021 at 01:30:06PM +0100, Michal Simek wrote: > Hi, > > On 11/18/21 20:54, Tom Rini wrote: > > On Thu, Nov 18, 2021 at 08:04:22PM +0100, Wolfgang Denk wrote: > > > Dear Tom, > > > > > > In message <20211118162920.GH24579@bill-the-cat> you wrote: > > > > > > > > > It is perfectly OK for U-Boot to start with a random MAC address, > > > > > use this for a while, and change it so something else later. This > > > > > is what may happen at production: say the MAC address is stored in > > > > > some EEPROM or fuses, which are initially empty, so U-Boot will use > > > > > a random MAC address, download it's board specific date (serial#, > > > > > MAC address, ...) over network, programm it into the respective > > > > > storage devices, and switch to using the new "official" MAC address. > > > > > > > > Yes. And up until this patch saying I want to use this random MAC with > > > > Linux required user intervention. > > > > > > Correct - this is a bug in the implementation of thispatch, and > > > apparently the few people that ever used it did not notice it or > > > care enough about it to submit fixes. > > > > > > > And I dare say that half the time or > > > > more that was probably just not noticed (everything comes up with > > > > dynamic host names/dns these days, noticing the IP changed between > > > > U-Boot and Linux is easy to miss in those cases). > > > > > > Agreed. > > > > > > > > CONFIG_NET_RANDOM_ETHADDR is the attempt to do the same > > > > > automatically, except that it falls short of setting the "ethaddr" > > > > > environment variable. I consider this a bug. > > > > > > > > Since the code isn't that old, it shouldn't be hard to pull up the > > > > thread / patch on introducing it. So, lets: > > > > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-1-git-sen= > > > > d-email-joe.hershber...@ni.com/ > > > > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-2-git-sen= > > > > d-email-joe.hershber...@ni.com/ > > > > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-3-git-sen= > > > > d-email-joe.hershber...@ni.com/ > > > > > > > > And from there we can take away I think two important things: > > > > 1: Not setting ethaddr with NET_RANDOM_ETHADDR is intentional > > > > > > Ummm... From which part of the patches or the comments do you take > > > this conclusion? Not a single line of code, or comments in the code > > > > The same part I keep asking to have fixed in a v2, the help text by the > > Kconfig option: > > + A new MAC address will be generated on every boot and it will > > + not be added to the environment. > > > > [snip] > > > > So, yes, OK, this is a bug fix, in that NET_RANDOM_ETHADDR was incorrect > > > > at introduction. That means we do need a v2 of this patch that updates > > > > the Kconfig help text as that currently says it will not update the > > > > environment. > > > > > > This makes no sense to me. Instead of documenting the bug we should > > > fix it and add the missing eth_env_set_enetaddr(). > > > > > > If I prepare such patches, will you accept these? > > > > Now I'm confused. Taking the patch this whole thread is attached to, we > > make NET_RANDOM_ETHADDR perform eth_env_set_enetaddr_by_index which is > > missing (well, only in the DM case, the non-DM case isn't updated and I > > guess should also be?). My problem is that the Kconfig help text for > > NET_RANDOM_ETHADDR still reflects what I pasted from the initial commit > > of it, a sentence saying we don't update the environment. This change > > does, so the help is wrong and needs fixing. I have come around to > > agreeing with the concept of this patch, that NET_RANDOM_ETHADDR should > > cause the environment to be updated and in turn fdt_fixup_ethernet() > > will populate this to Linux. > > > > This thread is getting quite long and I think that would be the best if you > (both) can create patch based on how you see it should work and send it > over. Based on it it will be super clear how you think things should work. > My initial intention was to show mac addresses via net list (Actually I was > investigating how to probe all ethernet controllers without calling > networking command for getting IPs out of reset with power domain driver) > which is solved by Michael's patch when he send v2 version.
Reasonable request, done: https://patchwork.ozlabs.org/project/uboot/patch/20211120155358.376540-1-tr...@konsulko.com/ -- Tom
signature.asc
Description: PGP signature