Subject: Re: [PATCH net 1/3] net: dsa: mv88e6xxx: Fix forcing speed & duplex when changing to 2500base-x mode

From: "Russell King (Oracle)" <[email-masked]>
Date: Wed, 10 Nov 2021 08:01:48 +0000

Please wait before applying; I have comments on these patches, but I
won't have time to provide those comments for a couple of days.
Thanks.

On Wed, Nov 10, 2021 at 05:10:08AM +0100, Marek Behún wrote:
> Commit 64d47d50be7a ("net: dsa: mv88e6xxx: configure interface settings
> in mac_config") removed forcing of speed and duplex from
> mv88e6xxx_mac_config(), where the link is forced down, and left it only
> in mv88e6xxx_mac_link_up(), by which time link is unforced.
> 
> It seems that in 2500base-x (at least on 88E6190), if the link is not
> forced down, the forcing of new settings for speed and duplex doesn't
> take.
> 
> Fix this by forcing link down.
> 
> Fixes: 64d47d50be7a ("net: dsa: mv88e6xxx: configure interface settings in mac_config")
> Signed-off-by: Marek Behún <[email-masked]>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index f00cbf5753b9..ddb13cecb3ac 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -785,12 +785,17 @@ static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
>  	if ((!mv88e6xxx_phy_is_internal(ds, port) &&
>  	     !mv88e6xxx_port_ppu_updates(chip, port)) ||
>  	    mode == MLO_AN_FIXED) {
> -		/* FIXME: for an automedia port, should we force the link
> -		 * down here - what if the link comes up due to "other" media
> -		 * while we're bringing the port up, how is the exclusivity
> -		 * handled in the Marvell hardware? E.g. port 2 on 88E6390
> -		 * shared between internal PHY and Serdes.
> +		/* FIXME: we need to force the link down here, otherwise the
> +		 * forcing of link speed and duplex by .port_set_speed_duplex()
> +		 * doesn't work for some modes.
> +		 * But what if the link comes up due to "other" media while
> +		 * we're bringing the port up, how is the exclusivity handled in
> +		 * the Marvell hardware? E.g. port 2 on 88E6390 shared between
> +		 * internal PHY and Serdes.
>  		 */
> +		if (ops->port_sync_link)
> +			err = ops->port_sync_link(chip, port, mode, false);
> +
>  		err = mv88e6xxx_serdes_pcs_link_up(chip, port, mode, speed,
>  						   duplex);
>  		if (err)
> -- 
> 2.32.0
> 
> 

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