netdev - Re: [PATCH v2 net-next] net: dsa: sja1105: Add support for the SGMII port

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] [day] [month] [year] [list]

Message-ID: <20200320090036.GK25745@shell.armlinux.org.uk>
Date:   Fri, 20 Mar 2020 09:00:36 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch,
        f.fainelli@...il.com, vivien.didelot@...il.com
Subject: Re: [PATCH v2 net-next] net: dsa: sja1105: Add support for the SGMII
 port

On Fri, Mar 20, 2020 at 02:54:20AM +0200, Vladimir Oltean wrote:
> +static void sja1105_sgmii_config(struct sja1105_private *priv, int port,
> +				 int speed)
> +{
> +	int pcs_speed;
> +
> +	/* DIGITAL_CONTROL_1: Enable vendor-specific MMD1, allow the PHY to
> +	 * stop the clock during LPI mode, make the MAC reconfigure
> +	 * autonomously after PCS autoneg is done, flush the internal FIFOs.
> +	 */
> +	sja1105_sgmii_write(priv, 0x8000, SJA1105_DC1_EN_VSMMD1 |
> +					  SJA1105_DC1_CLOCK_STOP_EN |
> +					  SJA1105_DC1_MAC_AUTO_SW |
> +					  SJA1105_DC1_INIT);
> +	/* DIGITAL_CONTROL_2: No polarity inversion for TX and RX lanes */
> +	sja1105_sgmii_write(priv, 0x80e1, SJA1105_DC2_TX_POL_INV_DISABLE);
> +	/* AUTONEG_CONTROL: Use SGMII autoneg */
> +	sja1105_sgmii_write(priv, 0x8001, SJA1105_AC_AUTONEG_MODE_SGMII);

The above should be configured at mac_config() time.

> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		pcs_speed = BMCR_SPEED1000;
> +		break;
> +	case SPEED_100:
> +		pcs_speed = BMCR_SPEED100;
> +		break;
> +	case SPEED_10:
> +		pcs_speed = BMCR_SPEED10;
> +		break;
> +	default:
> +		dev_err(priv->ds->dev, "Invalid speed %d\n", speed);
> +		return;
> +	}
> +	sja1105_sgmii_write(priv, MII_BMCR, pcs_speed | BMCR_FULLDPLX);

And this should be (as it now is) at mac_link_up().

If you want in-band AN (slave) to work, and that is documented as
working by enabling the AN bit in MII_BMCR, then I would suggest
setting that bit in the part called from mac_config() only if
phylink_autoneg_inband(mode) returns true, and avoiding the above
in mac_link_up() if phylink_autoneg_inband(mode) is also true.
Yes, I realise you can't actually test that, but up to you whether
or not to add that.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up