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

[Adam Ford]

On Thu, Oct 1, 2020 at 1:28 PM Tom Rini <trini at konsulko.com> wrote:

> 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!
>

I haven't tried lately, but when booting the Logic PD OMAP3 boards, I was
seeing a message displaying the MAC address while simultaneously showing
the message that it didn't have an address, so DHCP calls would fail.  I
could confirm that ethaddr was not set.  However, if I manually set the
ethaddr to the value dumped by the SMC911x driver, save the environmental
variables then reset the board, the ethernet would work.  It seemed like
the area where the SMC911x displayed the MAC address made sense to update
it since it just finished fetching it. so that's why I set up the patch the
way it was.  I hadn't considered a use case where it wasn't the primary
ethernet controller.

adam



>
> --
> Tom
>