From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Commit 90f522a20e3d1 ("NET: dwmac: Make dwmac reset unconditional")
asserts a reset in probe when a reset controller is present. This reset
clears the MAC address registers, so a valid address programmed by the
bootloader gets lost and the driver falls back to a random address.

Read the MAC address from the hardware registers before resetting the
hardware. Keep the existing address selection logic when no valid
address is found, and program the selected address back into the MAC
after probe so it remains consistent in hardware.

Export stmmac_bus_clks_config() so the early read path can enable the
bus clocks before accessing the MAC registers.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
Hi all,

Sending this patch as an RFC if there is an alternative/better way to
achieve this.

Cheers,
Prabhakar
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 66 ++++++++++++++++++-
 .../ethernet/stmicro/stmmac/stmmac_platform.c |  3 +-
 .../ethernet/stmicro/stmmac/stmmac_platform.h |  2 +
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 347a0078f622..171bf08d6242 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -44,6 +44,7 @@
 #include <net/page_pool/helpers.h>
 #include <net/pkt_cls.h>
 #include <net/xdp_sock_drv.h>
+#include "stmmac_platform.h"
 #include "stmmac_ptp.h"
 #include "stmmac_fpe.h"
 #include "stmmac.h"
@@ -7659,11 +7660,65 @@ struct plat_stmmacenet_data *stmmac_plat_dat_alloc(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(stmmac_plat_dat_alloc);
 
+static int stmmac_read_mac_before_reset(struct stmmac_priv *priv,
+					u8 *mac_addr)
+{
+	unsigned int hi_addr, lo_addr;
+	int ret;
+
+	eth_zero_addr(mac_addr);
+
+	/* Deassert reset to allow hardware access */
+	if (priv->plat->stmmac_rst) {
+		ret = reset_control_deassert(priv->plat->stmmac_rst);
+		if (ret)
+			return ret;
+	}
+
+	/* Enable bus clocks to read MAC address */
+	ret = stmmac_bus_clks_config(priv, true);
+	if (ret) {
+		dev_warn(priv->device, "failed to enable clocks: %d\n", ret);
+		goto assert_reset;
+	}
+
+	switch (priv->plat->core_type) {
+	case DWMAC_CORE_GMAC4:
+	case DWMAC_CORE_XGMAC:
+		/* GMAC4/XGMAC: MAC Address0 High/Low Register */
+		hi_addr = 0x300;  /* GMAC_ADDR_HIGH(0) */
+		lo_addr = 0x304;  /* GMAC_ADDR_LOW(0) */
+		break;
+	case DWMAC_CORE_GMAC:
+	case DWMAC_CORE_MAC100:
+		/* GMAC/GMAC3/MAC100: MAC Address0 High/Low Register */
+		hi_addr = 0x40;   /* GMAC_ADDR_HIGH(0) */
+		lo_addr = 0x44;   /* GMAC_ADDR_LOW(0) */
+		break;
+	default:
+		hi_addr = 0;
+	}
+
+	if (hi_addr)
+		stmmac_get_mac_addr(priv->ioaddr, mac_addr, hi_addr, lo_addr);
+
+	ret = 0;
+
+	stmmac_bus_clks_config(priv, false);
+
+assert_reset:
+	if (priv->plat->stmmac_rst)
+		reset_control_assert(priv->plat->stmmac_rst);
+
+	return ret;
+}
+
 static int __stmmac_dvr_probe(struct device *device,
 			      struct plat_stmmacenet_data *plat_dat,
 			      struct stmmac_resources *res)
 {
 	struct net_device *ndev = NULL;
+	u8 saved_mac_addr[ETH_ALEN];
 	struct stmmac_priv *priv;
 	u32 rxq;
 	int i, ret = 0;
@@ -7740,6 +7795,9 @@ static int __stmmac_dvr_probe(struct device *device,
 	if ((phyaddr >= 0) && (phyaddr <= 31))
 		priv->plat->phy_addr = phyaddr;
 
+	/* Save MAC address before reset (if bootloader programmed it) */
+	stmmac_read_mac_before_reset(priv, saved_mac_addr);
+
 	if (priv->plat->stmmac_rst) {
 		ret = reset_control_assert(priv->plat->stmmac_rst);
 		reset_control_deassert(priv->plat->stmmac_rst);
@@ -7768,7 +7826,13 @@ static int __stmmac_dvr_probe(struct device *device,
 	if (priv->synopsys_id < DWMAC_CORE_5_20)
 		priv->plat->dma_cfg->dche = false;
 
-	stmmac_check_ether_addr(priv);
+	if (is_valid_ether_addr(saved_mac_addr))
+		eth_hw_addr_set(priv->dev, saved_mac_addr);
+	else
+		stmmac_check_ether_addr(priv);
+
+	/* Store the MAC address in hardware */
+	stmmac_set_umac_addr(priv, priv->hw, priv->dev->dev_addr, 0);
 
 	ndev->netdev_ops = &stmmac_netdev_ops;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 8979a50b5507..6f0393f2992d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -857,7 +857,7 @@ void stmmac_pltfr_remove(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(stmmac_pltfr_remove);
 
-static int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled)
+int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled)
 {
 	struct plat_stmmacenet_data *plat_dat = priv->plat;
 	int ret;
@@ -888,6 +888,7 @@ static int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(stmmac_bus_clks_config);
 
 static int __maybe_unused stmmac_runtime_suspend(struct device *dev)
 {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
index 6e6561e29d6e..2d52197020a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
@@ -20,6 +20,8 @@ struct clk *stmmac_pltfr_find_clk(struct plat_stmmacenet_data *plat_dat,
 int stmmac_get_platform_resources(struct platform_device *pdev,
 				  struct stmmac_resources *stmmac_res);
 
+int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled);
+
 int stmmac_pltfr_probe(struct platform_device *pdev,
 		       struct plat_stmmacenet_data *plat,
 		       struct stmmac_resources *res);
-- 
2.52.0

On Mon, Jan 26, 2026 at 05:25:03PM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Commit 90f522a20e3d1 ("NET: dwmac: Make dwmac reset unconditional")
> asserts a reset in probe when a reset controller is present. This reset
> clears the MAC address registers, so a valid address programmed by the
> bootloader gets lost and the driver falls back to a random address.
> 
> Read the MAC address from the hardware registers before resetting the
> hardware. Keep the existing address selection logic when no valid
> address is found, and program the selected address back into the MAC
> after probe so it remains consistent in hardware.
> 
> Export stmmac_bus_clks_config() so the early read path can enable the
> bus clocks before accessing the MAC registers.

I don't think this is a good idea. stmmac_bus_clks_config() is specific
to using platform devices, but the core stmmac driver also supports
PCI that doesn't use stmmac_bus_clks_config().

stmmac_bus_clks_config() handles:

 - plat_dat->stmmac_clk
 - plat_dat->pclk
 - any clock handled by the plat_dat->clks_config() method

For platform devices, stmmac_probe_config_dt() gets these two clocks
from DT, and prepares and enables them both. So, by the time the
probe function is called, these clocks are already running.

For those handled by the platform glue, the glues that populate
this function:

eic7700: eic7700_clks_config() - this is called from the init/exit
handlers. Will be invoked to enable the clocks by stmmac_dvr_probe().

imx: imx_dwmac_clks_config() - called by imx_dwmac_probe() to enable
clocks prior to stmmac_dvr_probe() being invoked.

mediaktek: mediatek_dwmac_clks_config() - called by
mediatek_dwmac_probe() to enable clocks prior to stmmac_dvr_probe()
being invoked.

qcom-ethqos: ethqos_clks_config() - called by qcom_ethqos_probe() to
enable clocks prior to stmmac_dvr_probe() being invoked.

So, I can confidently say that all clocks should be running by the
time __stmmac_dvr_probe() is called, and thus there should be no
requirement to call stmmac_bus_clks_config() in this code.


The next problem: you place this code to read registers from stmmac
before:

	ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);

Sadly, the binding documentation is too vague to pin down what this
is, as dwmac can have AHB master (which generates bus cycles for
accessing memory) and AHB slave (which would be the target for
register accesses) interfaces.

The problem here is that if some platform glue has wired this reset
such that it resets the AHB slave side, that will prevent register
access, and thus your attempt to read the MAC across all devices
will fail.


The next question that comes up is that we have a perfectly good way
that's been around for years to pass a MAC address from the boot
loader into the kernel for any network interface. I notice that it
isn't mentioned in the DT bindings, presumably to prevent people
from adding it to their in-kernel DT files.

	mac-address =
	local-mac-address =

The old documentation in ethernet.txt was:

- mac-address: array of 6 bytes, specifies the MAC address that was last used by
  the boot program; should be used in cases where the MAC address assigned to
  the device by the boot program is different from the "local-mac-address"
  property;
- local-mac-address: array of 6 bytes, specifies the MAC address that was
  assigned to the network device;

Given that these are interfaces between the boot loader and the kernel,
they can't be deprecated, as platforms will rely upon these properties
to pass the MAC address from the boot loader to the kernel. For example
on one of my systems:

$ vdir /sys/class/net/eth0/of_node/
total 0
-r--r--r-- 1 root root  4 Jan 26 18:08 gop-port-id
-r--r--r-- 1 root root 50 Jan 26 18:08 interrupt-names
-r--r--r-- 1 root root 80 Jan 26 18:08 interrupts
-r--r--r-- 1 root root  6 Jan 26 18:08 local-mac-address
-r--r--r-- 1 root root 14 Jan 26 18:08 name
-r--r--r-- 1 root root  4 Jan 26 18:08 phy
-r--r--r-- 1 root root 10 Jan 26 18:08 phy-mode
-r--r--r-- 1 root root  8 Jan 26 18:08 phys
-r--r--r-- 1 root root  4 Jan 26 18:08 port-id
-r--r--r-- 1 root root  4 Jan 26 18:08 reg
-r--r--r-- 1 root root  5 Jan 26 18:08 status

where "local-mac-address" states the MAC address to be used for eth0,
as specified by the boot loader.

I don't think stmmac needs this extra complication provided platforms
make use of mechanisms that already exist... and I feel it's time to
start saying no to platform specific quirks that can be handled by
those mechanisms.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!