Re: [PATCH net-next] net: macb: add support for 1000BASE-X autonegotiation to PCS

On Wed, Jul 15, 2026 at 12:35:48PM -0400, Nathan Whitehorn wrote:

On 7/15/26 12:29, Charles Perry wrote:

quoted

On Tue, Jul 14, 2026 at 04:09:04PM -0400, Nathan Whitehorn wrote:

quoted

The current PCS code unconditionally uses SGMII autonegotiation, though
the hardware supports both SGMII and 1000BASE-X modes. Decouple the
choice of PCS enablement from use of the SGMII mode when running at
gigabit rates and announce to phylink that 1000BASE-X is a supported
operating mode. This enables direct attachment of the PCS to e.g. an
SFP.

Tested and developed on Microchip Polarfire SoC hardware.

Signed-off-by: Nathan Whitehorn <redacted>
---
  drivers/net/ethernet/cadence/macb_main.c | 31 ++++++++++++++++++------
  1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d394f1f43b68..284a3b03f8c7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -583,7 +583,13 @@ static void macb_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
  static void macb_pcs_an_restart(struct phylink_pcs *pcs)
  {
-	/* Not supported */
+	struct macb *bp = container_of(pcs, struct macb, phylink_sgmii_pcs);
+	u32 old, new;
+
+	old = gem_readl(bp, PCSCNTRL);
+	new = old | BMCR_ANRESTART;
+	if (old != new)
+		gem_writel(bp, PCSCNTRL, new);

This bit is self-clearing so I don't think the old != new check is
required.

Ah, OK, good to know.

quoted

Also, can you comment on why AN restart is needed for this patch? Is it a
requirement for 1000base-x of is it because we now have sgmii and
1000base-x?

Our equipment works well enough without this, and it is just here for
completeness rather than an actual need. I am happy to drop it from the
patch if you prefer.

Well it does look like the ->pcs_an_restart() callback was made
specifically for 1000base-x because phylink_pcs_an_restart() in phylink.c
checks for phy_interface_mode_is_8023z():

https://elixir.bootlin.com/linux/v7.1.3/source/drivers/net/phy/phylink.c#L1028

Maybe you actually need this.

Thanks,
Charles