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

[Sean Anderson]

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.

Can you try moving this write to dtsec_mac_config? 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.

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

--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@xxxxxxxxxx>
> ---
> 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;