On Thu, Jun 18, 2020 at 03:08:36PM +0300, Ioana Ciornei wrote:

Add a Lynx PCS MDIO module which exposes the necessary operations to
drive the PCS using PHYLINK.

The majority of the code is extracted from the Felix DSA driver, which
will be also modified in a later patch, and exposed as a separate module
for code reusability purposes.

At the moment, USXGMII (only with in-band AN and speeds up to 2500),
SGMII, QSGMII and 2500Base-X (only w/o in-band AN) are supported by the
Lynx PCS MDIO module since these were also supported by Felix.

The module can only be enabled by the drivers in need and not user
selectable.

Is this the same PCS found in the LX2160A?  It looks very similar.

+/* 2500Base-X is SerDes protocol 7 on Felix and 6 on ENETC. It is a SerDes lane
+ * clocked at 3.125 GHz which encodes symbols with 8b/10b and does not have
+ * auto-negotiation of any link parameters. Electrically it is compatible with
+ * a single lane of XAUI.
+ * The hardware reference manual wants to call this mode SGMII, but it isn't
+ * really, since the fundamental features of SGMII:
+ * - Downgrading the link speed by duplicating symbols
+ * - Auto-negotiation
+ * are not there.

I welcome that others are waking up to the industry wide obfuscation of
terminology surrounding "SGMII" and "1000base-X", and calling it out
where it is blatently incorrectly described in documentation.

+ * The speed is configured at 1000 in the IF_MODE because the clock frequency
+ * is actually given by a PLL configured in the Reset Configuration Word (RCW).
+ * Since there is no difference between fixed speed SGMII w/o AN and 802.3z w/o
+ * AN, we call this PHY interface type 2500Base-X. In case a PHY negotiates a
+ * lower link speed on line side, the system-side interface remains fixed at
+ * 2500 Mbps and we do rate adaptation through pause frames.

We have systems that do have AN with 2500base-X however - which is what
you want when you couple two potentially remote systems over a fibre
cable.  The AN in 802.3z (1000base-X) is used to negotiate:

- duplex
- pause mode

although in practice, half-duplex is not supported by lots of hardware,
which leaves just pause mode.  It is useful to have pause mode
negotiation remain present, whether it's 1000base-X or 2500base-X, but
obviously within the hardware boundaries.

I suspect the hardware is capable of supporting 802.3z AN when operating
at 2500base-X, but not the SGMII symbol duplication for slower speeds.

+struct mdio_lynx_pcs *mdio_lynx_pcs_create(struct mdio_device *mdio_dev)
+{
+	struct mdio_lynx_pcs *pcs;
+
+	if (WARN_ON(!mdio_dev))
+		return NULL;
+
+	pcs = kzalloc(sizeof(*pcs), GFP_KERNEL);
+	if (!pcs)
+		return NULL;
+
+	pcs->dev = mdio_dev;
+	pcs->an_restart = lynx_pcs_an_restart;
+	pcs->get_state = lynx_pcs_get_state;
+	pcs->link_up = lynx_pcs_link_up;
+	pcs->config = lynx_pcs_config;

We really should not have these private structure interfaces.  Private
structure-based driver specific interfaces really don't constitute a
sane approach to interface design.

Would it work if there was a "struct mdio_device" add to the
phylink_config structure, and then you could have the phylink_pcs_ops
embedded into this driver?

If not, then we need some kind of mdio_pcs_device that offers this
kind of functionality.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!