Re: [PATCH net-next v2 1/3] net: macb: fix SGMII with inband aneg disabled
From: Charles Perry
Date: Wed Mar 04 2026 - 10:29:15 EST
On Wed, Mar 04, 2026 at 11:15:43AM +0000, Conor Dooley wrote:
> On Tue, Feb 24, 2026 at 12:28:52PM -0800, Charles Perry wrote:
> > Make it possible to connect a PHY which does not use inband
> > autoneg to a gem MAC using phylink's information.
> >
> > The previous implementation relied on whether or not the link
> > was a fixed-link to disable SGMII autoneg. This commit extend
> > this to all link which are not configured for inband
> > autonegotiation.
> >
> > Signed-off-by: Charles Perry <charles.perry@xxxxxxxxxxxxx>
>
> This breaks the macb on mpfs-icicle-kit, I get stuck with:
>
> [ 7.189102] mpfs-sys-controller syscontroller: Registered MPFS system controller
> [ 7.260946] macb 20110000.ethernet eth0: PHY [20112000.ethernet-ffffffff:08] driver [Vitesse VSC8662] (irq=POLL)
> [ 7.273881] macb 20110000.ethernet eth0: configuring for phy/sgmii link mode
> [ 7.296580] macb 20110000.ethernet: gem-ptp-timer ptp clock registered.
> [ 7.345782] macb 20112000.ethernet eth1: PHY [20112000.ethernet-ffffffff:09] driver [Vitesse VSC8662] (irq=POLL)
> [ 7.358082] macb 20112000.ethernet eth1: configuring for phy/sgmii link mode
> [ 7.380479] macb 20112000.ethernet: gem-ptp-timer ptp clock registered.
> [ 11.376763] macb 20110000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> [ 11.398403] Sending DHCP requests .
> [ 11.472699] macb 20112000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
> [ 13.938425] ..... timed out!
> [ 93.598491] macb 20110000.ethernet eth0: Link is Down
> [ 93.641823] macb 20110000.ethernet: gem-ptp-timer ptp clock unregistered.
> [ 93.659433] macb 20112000.ethernet eth1: Link is Down
> [ 93.691724] macb 20112000.ethernet: gem-ptp-timer ptp clock unregistered.
> [ 93.703977] IP-Config: Retrying forever (NFS root)...
> [ 93.758382] macb 20110000.ethernet eth0: PHY [20112000.ethernet-ffffffff:08] driver [Vitesse VSC8662] (irq=POLL)
> [ 93.770655] macb 20110000.ethernet eth0: configuring for phy/sgmii link mode
> [ 93.786497] macb 20110000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> [ 93.795840] macb 20110000.ethernet: gem-ptp-timer ptp clock registered.
> [ 93.844481] macb 20112000.ethernet eth1: PHY [20112000.ethernet-ffffffff:09] driver [Vitesse VSC8662] (irq=POLL)
> [ 93.856769] macb 20112000.ethernet eth1: configuring for phy/sgmii link mode
> [ 93.870926] macb 20112000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
> [ 93.880302] macb 20112000.ethernet: gem-ptp-timer ptp clock registered.
>
Hello Conor,
I checked the driver for the VSC8662 and it doesn't have the
->inband_caps() and ->config_inband() callbacks so Linux leaves whatever
the bootloader puts or uses the defaults. Looking at the datasheet, this
should be register 23 (Extended PHY Control Set 1) bit 13 (MAC interface
auto-negotiation)
My guess is that this bit is set and since this patch disable inband
autonegotiation (because phylink decides it), there is a mismatch.
Can you add 'managed = "in-band-status"' in your device tree under the macb
node? That's not necessarily the fix, I just want to confirm my theory.
Also '#define DEBUG' in 'drivers/net/phy/phylink.c' can help if you can
recompile your kernel.
> and because I am NFS root, it breaks boot :)
>
> btw, I don't see an explanation in hte commit message for why the
> macb_is_gem(bp) got dropped? Is this now done unconditionally when it
> was conditional before?
>
Well if you get to the ->macb_pcs_config() callback, you most certainly
have an SGMII PCS block. Does your IP have the PCS registers? (0x200 to
0x23c) Also I thought that macb doesn't do SGMII (only GEM does) so it
can't reach ->macb_pcs_config().
Thanks alot for the testing!
Regards,
Charles
> Cheers,
> Conor.
>
> > ---
> >
> > Notes:
> > Changes in v2:
> > * Move my notes to the cover letter
> > * Remove the spinlock in macb_pcs_config
> > * Keep the comment about the PCSSEL register
> >
> > drivers/net/ethernet/cadence/macb_main.c | 36 +++++++++++++-----------
> > 1 file changed, 20 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index f72270a39d25..f4250e76cef0 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -574,6 +574,26 @@ static int macb_pcs_config(struct phylink_pcs *pcs,
> > const unsigned long *advertising,
> > bool permit_pause_to_mac)
> > {
> > + struct macb *bp = container_of(pcs, struct macb, phylink_sgmii_pcs);
> > + u32 old, new;
> > +
> > + old = gem_readl(bp, PCSANADV);
> > + new = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
> > + if (new != -EINVAL && old != new)
> > + gem_writel(bp, PCSANADV, new);
> > +
> > + /* Disable AN if it's not to be used, enable otherwise.
> > + * Must be written after PCSSEL is set in NCFGR which is done in
> > + * macb_mac_config(), otherwise writes will not take effect.
> > + */
> > + old = gem_readl(bp, PCSCNTRL);
> > + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
> > + new = old | BMCR_ANENABLE;
> > + else
> > + new = old & ~BMCR_ANENABLE;
> > + if (old != new)
> > + gem_writel(bp, PCSCNTRL, new);
> > +
> > return 0;
> > }
> >
> > @@ -628,22 +648,6 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
> > if (old_ncr ^ ncr)
> > macb_or_gem_writel(bp, NCR, ncr);
> >
> > - /* Disable AN for SGMII fixed link configuration, enable otherwise.
> > - * Must be written after PCSSEL is set in NCFGR,
> > - * otherwise writes will not take effect.
> > - */
> > - if (macb_is_gem(bp) && state->interface == PHY_INTERFACE_MODE_SGMII) {
> > - u32 pcsctrl, old_pcsctrl;
> > -
> > - old_pcsctrl = gem_readl(bp, PCSCNTRL);
> > - if (mode == MLO_AN_FIXED)
> > - pcsctrl = old_pcsctrl & ~GEM_BIT(PCSAUTONEG);
> > - else
> > - pcsctrl = old_pcsctrl | GEM_BIT(PCSAUTONEG);
> > - if (old_pcsctrl != pcsctrl)
> > - gem_writel(bp, PCSCNTRL, pcsctrl);
> > - }
> > -
> > spin_unlock_irqrestore(&bp->lock, flags);
> > }
> >
> > --
> > 2.47.3
> >