[PATCH net v2] tg3: use random MAC address when tg3_get_device_address fails

[PATCH net v2] tg3: use random MAC address when tg3_get_device_address fails

[Ivan Delalande]

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 as fallback on our systems.

Make the driver assign a random address in this condition instead of
being fatal for probe.

v2: assign random address when tg3_get_device_address fails instead of
    changing its logic.

Fixes: e4c00ba7274b ("tg3: replace placeholder MAC address with device property")
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/netdev/20260903233255.GA4109366@visor/t/#u
Signed-off-by: Ivan Delalande <colona@arista.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 73a4b569b03e..75cb0b0fab2f 100644
--- 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;
-- 
2.43.0

Re: [PATCH net v2] tg3: use random MAC address when tg3_get_device_address fails

[Pavan Chebbi]

On Tue, Sep 15, 2026 at 4:35 AM Ivan Delalande <colona@arista.com> 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 as fallback on our systems.
>
> Make the driver assign a random address in this condition instead of
> being fatal for probe.
>
> v2: assign random address when tg3_get_device_address fails instead of
>     changing its logic.
>
> Fixes: e4c00ba7274b ("tg3: replace placeholder MAC address with device property")
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Link: https://lore.kernel.org/netdev/20260903233255.GA4109366@visor/t/#u
> Signed-off-by: Ivan Delalande <colona@arista.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Thank you.

Re: [PATCH net v2] tg3: use random MAC address when tg3_get_device_address fails

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] tg3_init_one()
  (drivers/net/ethernet/broadcom/tg3.c:17916-17923) now converts *every*…
- [Low] The recorded commit message of eb360ca620ab contains the inter-
  revision changelog line "v2: assign random address when…

--

tg3: use random MAC address when tg3_get_device_address fails

Instead of failing probe when no valid ethernet address can be obtained,
tg3_init_one() now warns and assigns a random MAC address so the device
still probes on systems where the SRAM placeholder address cannot be
replaced through device_get_mac_address().

Link: https://lore.kernel.org/netdev/20260903233255.GA4109366@visor/t/#u

>     v2: assign random address when tg3_get_device_address fails instead of
>         changing its logic.

[Severity: Low]
This isn't a bug, but should the "v2:" changelog line live below the ---
separator instead of in the commit body?  The worked example in
Documentation/process/maintainer-netdev.rst places the per-version
changelog after the --- marker that follows the Signed-off-by tags, so it
does not end up in git history.

> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 73a4b569b03e3..75cb0b0fab2fa 100644
> --- 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);

[Severity: Medium]
Should -EPROBE_DEFER be handled separately here rather than being turned
into a random address?

In the exact case this patch targets, the SRAM holds the placeholder
address, so tg3_get_device_address() returns whatever
device_get_mac_address() gives back:

drivers/net/ethernet/broadcom/tg3.c:tg3_get_device_address() {
	...
	if (tg3_is_default_mac_address(addr))
		return device_get_mac_address(&tp->pdev->dev, addr);

	return 0;
}

That value can be -EPROBE_DEFER:

net/ethernet/eth.c:device_get_mac_address() {
	if (!fwnode_get_mac_address(dev_fwnode(dev), addr))
		return 0;

	return nvmem_get_mac_address(dev, addr);
}

net/ethernet/eth.c:nvmem_get_mac_address() {
	...
	cell = nvmem_cell_get(dev, "mac-address");
	if (IS_ERR(cell))
		return PTR_ERR(cell);
	...
}

drivers/nvmem/core.c:nvmem_cell_get() {
	...
	if (dev->of_node) { /* try dt first */
		cell = of_nvmem_cell_get(dev->of_node, id);
		if (!IS_ERR(cell) || PTR_ERR(cell) == -EPROBE_DEFER)
			return cell;
	}
	...
}

Before this change the error was propagated out of the PCI probe via
'goto err_out_apeunmap', so the driver core retried once the NVMEM
provider registered and the firmware MAC was picked up.  With the new
else branch, does the deferral get swallowed, leaving the interface
registered with a locally-administered random address that changes on
every boot or module reload, with no later retry?

Would the usual idiom help here, i.e. returning early on -EPROBE_DEFER
before falling back to eth_hw_addr_random()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914230521.GA1400582%40visor