Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to operate in Microchip KSZ9477 switch

From: Russell King (Oracle)
Date: Thu Jan 30 2025 - 07:45:24 EST


On Thu, Jan 30, 2025 at 01:05:49AM +0200, Vladimir Oltean wrote:
> On Wed, Jan 29, 2025 at 10:05:20PM +0000, Russell King (Oracle) wrote:
> > For Vladimir: I've added four hacky patches that build on top of the
> > large RFC series I sent earlier which add probably saner configuration
> > for the SGMII code, hopefully making it more understandable in light
> > of Wangxun's TXGBE using PHY mode there (they were adamant that their
> > hardware requires it.) These do not address Tristram's issue.
>
> Ok, let's sidetrack Tristram's thread, sure.

... and this is no longer a side track, because one of the changes in
Tristram's patches is to manually update the BMCR register on link-up
in SGMII mode, because the older XPCS hardware doesn't support
MAC_AUTO_SW. So, here's a patch that splits DW_XPCS_SGMII_MODE_MAC
introduced in the last patch into an _AUTO and _MANUAL variant.

The intention is - once we work out how to detect the older hardware -
that instead of DW_XPCS_SGMII_MODE_MAC_AUTO being used, Tristram sets
DW_XPCS_SGMII_MODE_MAC_MANUAL, which will attempt to clear MAC_AUTO_SW
(it's not clear to me that this can be done given the information that
Tristram has mentioned thus far - but I don't think that matters) and
xpcs_link_up_sgmii_1000basex() will program the BMCR.

This gives us a single control to enable this behaviour, rather than
introducing a quirk for it. Setting DW_XPCS_SGMII_MODE_MAC_MANUAL with
newer hardware allows these code paths to be tested there as well.

See the attached patch, which builds on top of the four already sent.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
From: "Russell King (Oracle)" <rmk+kernel@xxxxxxxxxxxxxxx>
Subject: [PATCH net-next] net: xpcs: add support for manual update for SGMII

Older revisions of the XPCS IP do not support the MAC_AUTO_SW flag and
need the BMCR register updated with the speed information from the PHY.
Split the DW_XPCS_SGMII_MODE_MAC mode into _AUTO and _MANUAL variants,
where _AUTO mode means the update happens in hardware autonomously,
whereas the _MANUAL mode means that we need to update the BMCR register
when the link comes up.

Signed-off-by: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx>
---
drivers/net/pcs/pcs-xpcs.c | 9 +++++++--
drivers/net/pcs/pcs-xpcs.h | 11 ++++++++---
2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index e7aad3c402a4..f23193d1115a 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -691,11 +691,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
mask = DW_VR_MII_DIG_CTRL1_2G5_EN | DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW;

switch (xpcs->sgmii_mode) {
- case DW_XPCS_SGMII_MODE_MAC:
+ case DW_XPCS_SGMII_MODE_MAC_AUTO:
if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
val = DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW;
break;

+ case DW_XPCS_SGMII_MODE_MAC_MANUAL:
+ break;
+
case DW_XPCS_SGMII_MODE_PHY_HW:
val |= DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL;
fallthrough;
@@ -1123,7 +1126,9 @@ static void xpcs_link_up_sgmii_1000basex(struct dw_xpcs *xpcs,
{
int ret;

- if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
+ if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED &&
+ !(interface == PHY_INTERFACE_MODE_SGMII &&
+ xpcs->sgmii_mode == )
return;

if (interface == PHY_INTERFACE_MODE_1000BASEX) {
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 4bd6a82f1a18..be8fab40b012 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -112,8 +112,12 @@ enum dw_xpcs_sgmii_10_100 {
};

/* The SGMII mode:
- * DW_XPCS_SGMII_MODE_MAC: the XPCS acts as a MAC, reading and acknowledging
- * the config word.
+ * DW_XPCS_SGMII_MODE_MAC_AUTO: the XPCS acts as a MAC, accepting the
+ * parameters from the PHY end of the SGMII link and acknowledging the
+ * config word. The XPCS autonomously switches speed.
+ *
+ * DW_XPCS_SGMII_MODE_MAC_MANUAL: the XPCS acts as a MAC as above, but
+ * does not autonomously switch speed.
*
* DW_XPCS_SGMII_MODE_PHY_HW: the XPCS acts as a PHY, deriving the tx_config
* bits 15 (link), 12 (duplex) and 11:10 (speed) from hardware inputs to the
@@ -125,7 +129,8 @@ enum dw_xpcs_sgmii_10_100 {
* integration documentation states that MII_ADVERTISE must be written last.
*/
enum dw_xpcs_sgmii_mode {
- DW_XPCS_SGMII_MODE_MAC, /* XPCS is MAC on SGMII */
+ DW_XPCS_SGMII_MODE_MAC_AUTO, /* XPCS is MAC, auto update */
+ DW_XPCS_SGMII_MODE_MAC_MANUAL, /* XPCS is MAC, manual update */
DW_XPCS_SGMII_MODE_PHY_HW, /* XPCS is PHY, tx_config from hw */
DW_XPCS_SGMII_MODE_PHY_REG, /* XPCS is PHY, tx_config from regs */
};
--
2.30.2