Re: [PATCH 3/3] net: stmmac: dwmac-socfpga: Add mac-mode DT property support
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: 2026-07-01 06:50:12
Also in:
linux-arm-kernel, linux-devicetree, lkml
Hi, On 6/30/26 15:31, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
From: Nazim Amirul <redacted>
Russell King's commit de696c63c1dc ("net: stmmac: socfpga: convert to
use phy_interface") replaced mac_interface with phy_interface in
socfpga_get_plat_phymode(), noting that no upstream DTS files set the
"mac-mode" property, making the two values identical.
The Agilex5 SoCDK TSN Config2 board is an exception: its gmac1 TSN
port uses GMII internally in the MAC while the PHY-side interface is
RGMII, so mac-mode and phy-mode differ. Without restoring mac_interface
support, the MAC is configured with RGMII instead of GMII, causing
connectivity failures on this board.
Add socfpga_of_get_mac_mode() to read the optional "mac-mode" DT
property and store it in a new mac_interface field. When the property
is absent, mac_interface falls back to phy_interface, preserving
the existing behaviour for all other boards.After our discussions, indeed mac-mode seems to be the way to go, however I have some remarks on how it's handled right now.
quoted hunk ↗ jump to hunk
Fixes: de696c63c1dc ("net: stmmac: socfpga: convert to use phy_interface") Signed-off-by: Nazim Amirul <redacted> --- .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c index 1d7f0a57d288..6a6837c4a414 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c@@ -69,12 +69,30 @@ struct socfpga_dwmac { void __iomem *tse_pcs_base; void __iomem *sgmii_adapter_base; bool f2h_ptp_ref_clk; + phy_interface_t mac_interface; const struct socfpga_dwmac_ops *ops; }; +static int socfpga_of_get_mac_mode(struct device_node *np) +{ + const char *pm; + int err, i; + + err = of_property_read_string(np, "mac-mode", &pm); + if (err < 0) + return err; + + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) { + if (!strcasecmp(pm, phy_modes(i))) + return i; + } + + return -ENODEV; +} + static phy_interface_t socfpga_get_plat_phymode(struct socfpga_dwmac *dwmac) { - return dwmac->plat_dat->phy_interface; + return dwmac->mac_interface; }
Taking a look at the logic in socfpga_gen{5|10}_set_phy_mode(), we have :
phy_interface_t phymode = socfpga_get_plat_phymode(dwmac);
u32 val;
socfpga_set_phy_mode_common(phymode, &val)
if (dwmac->splitter_base)
val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII;
[...]
if (phymode == PHY_INTERFACE_MODE_SGMII)
socfpga_sgmii_config(dwmac, true);
With this new patch, we now have 2 different ways of handling this
converter block (splitter presence, and mac-mode presence in DT)
Can you unify this a bit ?
One thing could be adding a helper to get the macmode such as
socfpga_get_plat_macmode()
I think we should move the splitter handling before calling
socfpga_get_plat_macmode() :
if (dwmac->splitter_base)
dwmav->mac_interface = PHY_INTERFACE_MODE_GMII
We'd get the intf_sel value based on the macmode, and also calls to
helpers such as socfpga_sgmii_config(phymode, xxx) need the actual
phymode as a parameter, not the macmode.
Thanks :)
Maxime