netdev - Re: [PATCH net-next 06/10] net: dsa: mv88e6xxx: extend phylink to Serdes PHYs

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]

Message-ID: <20200305134333.GC25745@shell.armlinux.org.uk>
Date:   Thu, 5 Mar 2020 13:43:33 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Marek Behun <marek.behun@....cz>
Cc:     Andrew Lunn <andrew@...n.ch>,
        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 06/10] net: dsa: mv88e6xxx: extend phylink to
 Serdes PHYs

On Thu, Mar 05, 2020 at 02:38:47PM +0100, Marek Behun wrote:
> On Thu, 05 Mar 2020 12:42:31 +0000
> Russell King <rmk+kernel@...linux.org.uk> wrote:
> 
> > +int mv88e6390_serdes_pcs_link_up(struct mv88e6xxx_chip *chip, int port,
> > +				 u8 lane, int speed, int duplex)
> > +{
> > +	u16 val, bmcr;
> > +	int err;
> > +
> > +	err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
> > +				    MV88E6390_SGMII_BMCR, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	bmcr = val & ~(BMCR_SPEED100 | BMCR_FULLDPLX | BMCR_SPEED1000);
> > +	switch (speed) {
> > +	case SPEED_2500:
> > +	case SPEED_1000:
> > +		bmcr |= BMCR_SPEED1000;
> >  		break;
> > -	case MV88E6XXX_PORT_STS_CMODE_1000BASEX:
> > -		mode = PHY_INTERFACE_MODE_1000BASEX;
> > +	case SPEED_100:
> > +		bmcr |= BMCR_SPEED100;
> >  		break;
> > -	case MV88E6XXX_PORT_STS_CMODE_2500BASEX:
> > -		mode = PHY_INTERFACE_MODE_2500BASEX;
> > +	case SPEED_10:
> >  		break;
> > -	default:
> > -		mode = PHY_INTERFACE_MODE_NA;
> >  	}
> >  
> > -	err = mv88e6xxx_port_setup_mac(chip, port, link, speed, duplex,
> > -				       PAUSE_OFF, mode);
> > -	if (err)
> > -		dev_err(chip->dev, "can't propagate PHY settings to MAC: %d\n",
> > -			err);
> > -	else
> > -		dsa_port_phylink_mac_change(ds, port, link == LINK_FORCED_UP);
> > +	if (duplex == DUPLEX_FULL)
> > +		bmcr |= BMCR_FULLDPLX;
> > +
> > +	if (bmcr == val)
> > +		return 0;
> > +
> > +	return mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> > +				      MV88E6390_SGMII_BMCR, bmcr);
> > +}
> 
> Hi,
> 
> some time ago I wondered if it would make sense to separate the
> SERDES PHY code into a separate phy driver to reside in
> drivers/net/phy/marvell-serdes.c or something like that. Are there
> compatible PHYs which aren't integrated into a switch?

We already have a problem with the copper PHYs in Marvell switches
being handled in this manner - on the 6141, we try to drive them as
our "6390" PHY, but they aren't compatible.  This results in hwmon
being registered, which permanently reports a temperature of -75°C.

I suspect we'll run into the same thing with the serdes.

Then there's the issue that the way we handle serdes PCS is not the
same as a copper PHY.

Lastly, there's the issue with SGMII PCS that there may also be a
copper PHY, and phylib / network devices have no support for stacking
one PHY on top of another.

All this could be solved, but I suspect it will require considerable
effort and restructuring of phylib, phylink and several other bits of
the kernel networking layer.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up