On 7/10/26 05:39, Michael Walle wrote:

Hi,

On Fri Jul 10, 2026 at 1:45 AM CEST, Sean Anderson wrote:

quoted

On 7/6/26 08:08, Michael Walle wrote:

quoted

Before converting to the phylink interface, the init function would have
set the correct mode in the maccfg2.

So for reference, the old logic is

         if (iface_speed < SPEED_1000)
                 tmp |= MACCFG2_NIBBLE_MODE;
         else if (iface_speed == SPEED_1000)
                 tmp |= MACCFG2_BYTE_MODE;

which changes between nibble/byte mode depending on the max link speed
(e.g. phylink_interface_max_speed). Notably, neither is set for 2.5G.

Ahh you're right, I totally missed, that the speed parameter was
actually max_speed, which was set by

   priv->speed = phy2speed[macdev->phy_if];
   priv->max_speed = priv->speed;

FWIW, in the DPAA RM I have, there is no 2.5G speed. Are you sure,
there are DTSEC controllers with 2.5Gbps support? The only board I
found in the device trees is the T1023RDB which is using the memac.

It's listed as an option in the P5020/P5040 RMs (under "SerDes Lane
Assignments and Multiplexing"). Of course despite other parts of the
DPAARM referring to the dTSEC's 2.5G capabilities, the the dTSEC chapter
itself makes no mention of any speed higher than 1G.

As unlike U-Boot, Linux promises backwards compatibility even for
out-of-tree devicetrees, I kept the existing behavior when doing my
conversion on the off chance that someone had a board configured for
2.5G.

Given the similarities of 2.5G "SGMII" and SGMII, I suspect that the
settings should probably be the same for 2.5G as for 1G.

quoted

Can you try moving this write to dtsec_mac_config?

That worked! At least for SGMII. I don't have a board with another
interface to test tough. I'll prepare a new version, making
phylink_interface_max_speed() public and setting the correct value
in .mac_config().

Well, we already have switch/case there, so you could just use that
since only RMII would default to nibble mode.

Alternatively, we could move
phylink_interface_max_speed() to the header as static inline, but
that function isn't that small.

quoted

And check in the RM
whether this is configured based on the interface (in which case we should
remove it from dtsec_link_up) or the link speed. And please also check what
the correct behavior for 2.5G should be.

This is an excerpt from the RM:

I/F Mode (bits 22-23):
  This field determines the type of interface to which the MAC is
  connected. Its default is 00.

Of course, this implies that it's OK to leave this field as 0, which is probably
why I left the logic as-is. But since you actually tested this (and I just tried
to piece things together from the RMs) we should set it.

   00 Reserved
   01 Used for all 10Mbps and 100Mbps speeds
   10 Used for all 1Gbps speeds
   11 Reserved

So you could read it both ways? But since the older driver was using
the maximum interface speed..

quoted

At one point I had the P-series RMs downloaded, but it appears I've misplaced
them...

FWIW, I think it's freely available as long as you're logged in with
your NXP account.

I was at home without access to my NXP account :)

--Sean

-michael

quoted

--Sean

quoted

After converting, init will just
set 0 as the mode. According to the "QorIQ Data Path Acceleration
Architecture (DPAA) Reference Manual", this is a reserved value. In
fact, this will prevent the PCS to establish a link to a connected SGMII
PHY. In turn, mac_link_up() is never called. Fix it by setting a
non-reserved mode; mac_link_up() will then set the correct mode later.

Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
FWIW, I've tested this with a Marvell 88E1112 PHY.

   drivers/net/ethernet/freescale/fman/fman_dtsec.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index fe35703c509e..566921d3a884 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c

@@ -402,7 +402,10 @@ static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
   	tmp |= MACCFG1_TX_FLOW;
   	iowrite32be(tmp, &regs->maccfg1);
   
-	tmp = 0;
+	/* write a non-reserved mode, otherwise the PCS won't establish a link
+	 * and .mac_link_up() is never called.
+	 */
+	tmp = MACCFG2_NIBBLE_MODE;
   
   	tmp |= (cfg->preamble_len << MACCFG2_PREAMBLE_LENGTH_SHIFT) &
   		MACCFG2_PREAMBLE_LENGTH_MASK;