Re: [PATCH net] tg3: check NVRAM and control register if SRAM has the placeholder MAC address

[Ivan Delalande <colona@arista.com>]

On Wed, Sep 09, 2026 at 07:17:51PM -0700, Jakub Kicinski wrote:
> On Thu, 3 Sep 2026 16:32:55 -0700 Ivan Delalande wrote:
> > Some of the tg3 NICs we use (BCM57762) reset the SRAM MAC address to the
> > placeholder address on link flaps, tg3_chip_reset, etc. We've typically
> > fixed it from userspace, but since e4c00ba7274b ("tg3: replace
> > placeholder MAC address with device property") was merged, tg3 just
> > fails probe as we don't have a way to get it through the generic
> > device_get_mac_address infrastructure on our systems.
> > 
> > Since that commit effectively made the placeholder address invalid,
> > include this check in the earlier `addr_ok` expression, so we proceed
> > with the NVRAM and control register fallback to find a valid MAC.
> 
> If there are so many cases with known-broken MAC addresses with tg3
> shouldnt we do something like:

I think we could work with this on our systems, though it would be best
if the driver also checked the control register when SRAM is invalid or
the default MAC.

I was testing a slightly more extensive v2 based on Sashiko's comments
as I agree tg3_get_device_address() is pretty awkard in its current
version. But I don't have access to enough different tg3 NICs or
Broadcom internal docs for a confident rewrite, so I was hoping Pavan
and Michael could chime in on all of this (should we just check SRAM,
NVRAM, control register, and device_get_mac_address in sequence and take
the first valid non-placeholder address? should device_get_mac_address
take priority over any of the other sources? should this be controlled
or overridable through module parameters?).

Thanks,

> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -17914,12 +17914,13 @@ static int tg3_init_one(struct pci_dev *pdev,
>         }
>  
>         err = tg3_get_device_address(tp, addr);
> -       if (err) {
> -               dev_err(&pdev->dev,
> -                       "Could not obtain valid ethernet address, aborting\n");
> -               goto err_out_apeunmap;
> +       if (!err) {
> +               eth_hw_addr_set(dev, addr);
> +       } else {
> +               dev_warn(&pdev->dev,
> +                        "Could not obtain valid ethernet address, using a random address\n");
> +               eth_hw_addr_random(dev);
>         }
> -       eth_hw_addr_set(dev, addr);
>  
>         intmbx = MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW;
>         rcvmbx = MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW;
> 
> ?

-- 
Ivan Delalande - Arista Networks