Re: [PATCH net-next 13/13] net: dsa: sja1105: add support for the SJA1110 SGMII/2500base-x PCS

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Mon, May 24, 2021 at 07:39:42PM -0700, Florian Fainelli wrote:
> On 5/24/2021 4:22 PM, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > Configure the Synopsys PCS for 10/100/1000 SGMII in autoneg on/off
> > modes, or for fixed 2500base-x.
> > 
> > The portion of PCS configuration that forces the speed is common, but
> > the portion that initializes the PCS and enables/disables autoneg is
> > different, so a new .pcs_config() method was introduced in struct
> > sja1105_info to hide away the differences.
> > 
> > For the xMII Mode Parameters Table to be properly configured for SGMII
> > mode on SJA1110, we need to set the "special" bit, since SGMII is
> > officially bitwise coded as 0b0011 in SJA1105 (decimal 3, equal to
> > XMII_MODE_SGMII), and as 0b1011 in SJA1110 (decimal 11).
> 
> That special bit appears to be write only in the patch you submitted, do
> we need it?

Unfortunately yes.
What happens is this:

In SJA1105, the xMII Mode Parameters Table looks like this:

BIT(2): PHY_MAC: decides the role of the port (MAC or PHY) - relevant
for MII and RMII, irrelevant for RGMII. Defined in this driver as:

typedef enum {
	XMII_MAC = 0,
	XMII_PHY = 1,
} sja1105_mii_role_t;

BITS(1:0): XMII_MODE: defined in this driver as:

typedef enum {
	XMII_MODE_MII		= 0,
	XMII_MODE_RMII		= 1,
	XMII_MODE_RGMII		= 2,
	XMII_MODE_SGMII		= 3,
} sja1105_phy_interface_t;

Here's the interesting part:
In SJA1110, the xMII Mode Parameters Table contains a single XMII_MODE
field which is 4 bit wide. The documentation lists these possible values:

0: MII interface
4: revMII interface
1: RMII interface (RMII TX interface acts as transmit protocol as
   specified in RMII 1.2 spec)
11: SGMII interface
2: RGMII interface
7: INACTIVE (the port is cut off from the outside)
8: 100BASE-TX

If we dissect these values bitwise, we see that revMII in SJA1110 is
numerically equal to XMII_MODE_MII + XMII_PHY from SJA1105, so it makes
sense to keep interpreting the MII mode as 2 separate fields (MAC/PHY
role and MII mode).

Except the SGMII port is decimal 11, and it was decimal 3 in SJA1105.
Bitwise, 11 is 0b1011 and 3 is 0b0011. So while the low-order 3 bits did
not change, the 4th bit needs to be set in order to encode SGMII in
SJA1110.

Similarly, the internal ports connected to the 100base-T1 PHYs are just
XMII_MODE_MII + XMII_MAC (numerically this is equal to decimal 0). But
the internal port connected to the 100base-TX PHY wants to be programmed
to 8. Bitwise that is 0b1000. So the low order 3 bits still encode
XMII_MODE_MII + XMII_MAC.

So the "special" bit 4 is what allows me to keep the XMII_MODE_SGMII
definition at 3 (otherwise SJA1110 wants it to be 11, for a reason I
simply cannot comprehend), and to treat the ports with internal PHYs as
XMII_MODE_MII + XMII_MAC (even if one type of internal PHY wants the
XMII_MODE to be programmed as 0 and the other as 8).

I don't know, I know this is confusing, but I think having an extra
indirection (this is the value of SGMII for this switch) would be
confusing too.

> How much of the programming you are doing is really specific to the
> SJA1110 switch family and how much would be universally (or
> configurable) applicable to a Synopsys PCS driver that would live
> outside of this switch driver?

The pcs-xpcs.c from drivers/net/pcs already handles the same PCS
registers when operating in SGMII mode (their DW_VR_MII_DIG_CTRL1 is my
SJA1105_DC1, their DW_VR_MII_AN_CTRL is my SJA1105_AC, their
DW_VR_MII_AN_INTR_STS is my SJA1105_AIS etc) so there is some
similarity, but it is not as simple as that.

The Designware XPCS block is very customizable, and NXP integrates it
with a non-Designware PMA (this handles serialization and
deserialization of data) and PMD (this implements the CTLE, CDR, PLLs,
line driver, termination resistors). The PMA and PMD are configured
through some registers which are stuffed in the same MDIO PHY/MMD
address that is used to access the PCS. For example, the MDIO_VEND2 MMD
contains the following regions:
- 0000:000f: Designware PCS registers
- 0708:0710: Designware PCS registers
- 8000:8020: Designware PCS registers
- 8030:806e: PMA registers
- 80e1:80e2: Designware PCS registers

Having a non-Synopsys PMA even creates complications for programming the
common Designware PCS registers. For example, in the SJA1105 PMA, the
polarity for the differential TX lane is already inverted (i.e. PLUS is
MINUS and MINUS is PLUS), so in order to obtain the "no TX lane polarity
inversion" effect, one actually needs to enable TX lane polarity
inversion in the _PCS_:

	/* DIGITAL_CONTROL_2: No polarity inversion for TX and RX lanes */
	sja1105_sgmii_write(priv, SJA1105_DC2, SJA1105_DC2_TX_POL_INV_DISABLE);

With the PMA from the SJA1110, this is not necessary and the DIG_CTRL2
register needs to be programmed with 0 in order for both TX and RX lane
polarities to be normal.

So integrating sja1105 with xpcs might be possible, but not all
configuration will be possible to be done there. This is an exploratory
topic that I didn't want to tackle right now, although it is a good time
to discuss it.