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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 14 Mar 2020 19:38:22 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Russell King <rmk+kernel@...linux.org.uk>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next 3/8] net: dsa: mv88e6xxx: configure interface
 settings in mac_config

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 <rmk+kernel@...linux.org.uk>
> @@ -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 <andrew@...n.ch>

    Andrew