linux-kernel - Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

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: <YDE5Ja/O4sk4hewj@lunn.ch>
Date:   Sat, 20 Feb 2021 17:30:29 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Ivan Bornyakov <i.bornyakov@...rotek.ru>, netdev@...r.kernel.org,
        system@...rotek.ru, hkallweit1@...il.com, davem@...emloft.net,
        kuba@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

> > +/* switch line-side interface between 10GBase-R and 1GBase-X
> > + * according to speed */
> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > +	struct mv2222_data *priv = phydev->priv;
> > +
> > +	if (phydev->speed == SPEED_10000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		mv2222_soft_reset(phydev);
> > +	}
> > +
> > +	if (phydev->speed == SPEED_1000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		mv2222_soft_reset(phydev);
> > +	}
> 
> Wouldn't it be better to have a single function to set the line
> interface, used by both this function and your sfp_module_insert
> function? I'm thinking something like:
> 
> static int mv2222_set_line_interface(struct phy_device *phydev,
> 				     phy_interface_t line_interface)
> {
> ...
> }
> 
> and calling that from both these locations to configure the PHY for
> 10GBASE-R, 1000BASE-X and SGMII modes.

Agreed. This got me confused, wondering where the SGMII handling was.

	Andrew