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

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

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