Subject: Re: [PATCH net-next 3/8] net: dsa: mv88e6xxx: configure interface settings in mac_config

From: Andrew Lunn <[email-masked]>
Date: Sat, 14 Mar 2020 19:38:22 +0100

On Sat, Mar 14, 2020 at 10:15:38AM +0000, Russell King wrote:
> Only configure the interface settings in mac_config(), leaving the
> speed and duplex settings to mac_link_up to deal with.
> 
> Signed-off-by: Russell King <[email-masked]>
> @@ -603,33 +613,26 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
>  				 const struct phylink_link_state *state)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
> -	int speed, duplex, link, pause, err;
> +	int err;
>  
> +	/* FIXME: is this the correct test? If we're in fixed mode on an
> +	 * internal port, why should we process this any different from
> +	 * PHY mode? On the other hand, the port may be automedia between
> +	 * an internal PHY and the serdes...
> +	 */

> -	err = mv88e6xxx_port_setup_mac(chip, port, link, speed, duplex, pause,
> -				       state->interface);
> +	/* FIXME: should we force the link down here - but if we do, how
> +	 * do we restore the link force/unforce state? The driver layering
> +	 * gets in the way.
> +	 */
> +	err = mv88e6xxx_port_config_interface(chip, port, state->interface);

Hi Russell

I'm not too keen on these FIXMEs, but i don't have time at the moment
to take a closer look.

Reviewed-by: Andrew Lunn <[email-masked]>

    Andrew