Re: [PATCH net] net: dpaa: always set a valid mode I/F mode

["Michael Walle" <mwalle@kernel.org>]

Hi,

On Fri Jul 10, 2026 at 1:45 AM CEST, Sean Anderson wrote:
> On 7/6/26 08:08, Michael Walle wrote:
>> 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.

> 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(). Alternatively, we could move
phylink_interface_max_speed() to the header as static inline, but
that function isn't that small.

> 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.
  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..

> 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.

-michael

>
> --Sean
>
>> 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;