On Thu, Oct 01, 2020 at 01:22:37PM -0500, Adam Ford wrote: > On Thu, Oct 1, 2020 at 1:17 PM Tom Rini <tr...@konsulko.com> wrote: > > > On Thu, Oct 01, 2020 at 07:48:32PM +0200, Marek Vasut wrote: > > > On 10/1/20 4:09 PM, Tom Rini wrote: > > > > On Tue, Aug 18, 2020 at 08:19:02AM -0500, Adam Ford wrote: > > > > > > > >> The ethernet controller can read the MAC from EEPROM and display it, > > > >> but if ethaddr is not set, the ethernet is still unavailable. > > > >> > > > >> This patch checks will automatically set the MAC address if it has > > > >> not already been set. > > > >> > > > >> Signed-off-by: Adam Ford <aford...@gmail.com> > > > >> Acked-by: Joe Hershberger <joe.hershber...@ni.com> > > > > > > > > Applied to u-boot/next, thanks! > > > > > > Note that this breaks every single setup where smc911x is not primary > > > ethernet. On systems where smc911x is secondary ethernet, you need to > > > set eth1addr and so on, so please do fix that. > > > > > > Also, this kind of ethXaddr update should happen in the ethernet core > > > instead, drivers shouldn't really modify environment, no ? > > > > Interesting points. So, if smc911x is not the primary ether device, > > something else will have already set "ethaddr", most likely. We do have > > both the common case where "ethaddr" (and "eth1addr" and so forth) are > > set. > > > > Adam, when exactly did you run in to the case where ethaddr wasn't set > > correctly? Was it on a non-DM_ETH case? To Marek's last point, we do > > have drivers that set ethaddr/ethXaddr, but that's in the non-DM_ETH > > case. > > > > The only situation I tested was with DM_ETH since I thought it was going to > be a requirement by 2020.07. If ethaddr is already set, it shouldn't > override it, but I can see an issue where using the SMC911x as a secondary > controller may cause an issue because the driver at this level doesn't know. > > It seems like there should be a way to determine if the MAC address is > readable so the user doesn't need to enter it manually, but it's probably > not at the driver level based on the feedback. > > If you want to revert this patch, I won't object. I don't really have time > to develop a better one right now.
Well, wait. Did you encounter a case where "ethaddr" was not automatically correctly set? A quick skim of the driver and it looks like it's doing everything needed for the common code to set "ethaddr" correctly from enetaddr the driver probed. Thanks! -- Tom
signature.asc
Description: PGP signature
