[PATCH V2] net: smc911x: Automatically Update ethaddr with MAC

[Tom Rini]

On Thu, Oct 01, 2020 at 01:22:37PM -0500, Adam Ford wrote:
> On Thu, Oct 1, 2020 at 1:17 PM Tom Rini <trini at 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 <aford173 at gmail.com>
> > > >> Acked-by: Joe Hershberger <joe.hershberger at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201001/52c6e60d/attachment.sig>