[PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Eric Woudstra]

The situation: mtk lynxi pcs (eth1 on BananaPi R3) with a rollball
rtl8221b sfp.

When setting eth1 link up, the phy is not immediately attached. It takes
a few seconds, pl->phydev is not set yet.

So when setting link eth1 up:

phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,
0000e240 pause=04

When the phy is attached the link an mode does not change and
phylink_mac_initial_config() is not called. No message 'Link is Up' found
in the kernel log.

We need:

phylink_mac_config: mode=phy/2500base-x/none adv=00,00000000,00008000,
000060ef pause=04

Perhaps forcing phylink_mac_initial_config() always for a phy is not the
desired approach, so I send this patch as RFC to see which approach
will be most suitable.

Log before this patch is applied:
[root@bpir3 ~]# dmesg | grep eth1
[    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
[   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
[   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
[   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
[   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
[   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
[   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
[   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
[   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
[   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
[   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
[   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
[   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
[   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
[   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
[   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
[   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
[   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
[   40.397090] brlan: port 5(eth1) entered blocking state
[   40.402223] brlan: port 5(eth1) entered disabled state
[   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
[   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
[   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
[   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)

Log after this patch is applied:
[root@bpir3 ~]# dmesg | grep eth1
[    2.515149] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082400000, irq 123
[   38.989414] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
[   38.997838] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
[   39.006029] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
[   39.014818] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
[   39.024368] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
[   39.960119] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
[   39.969738] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
[   39.979409] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
[   39.989153] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
[   39.999063] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
[   40.334663] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
[   40.343980] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
[   40.390049] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
[   40.400234] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
[   40.411005] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
[   40.424730] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
[   40.436368] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
[   40.444539] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
[   40.453307] mtk_soc_eth 15100000.ethernet eth1: major config, active phy/outband/2500base-x
[   40.461653] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=phy/2500base-x/none adv=00,00000000,00008000,000060ef pause=04
[   41.170029] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
[   41.187047] brlan: port 5(eth1) entered blocking state
[   41.192213] brlan: port 5(eth1) entered disabled state
[   41.197404] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
[   41.204358] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
[   46.600038] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
[   46.600057] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 2.5Gbps/Full - flow control off
[   46.616926] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
[   46.634003] brlan: port 5(eth1) entered blocking state
[   46.639155] brlan: port 5(eth1) entered forwarding state

Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 drivers/net/phy/phylink.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6d50c2fdb190..6fd66ba9002a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3424,6 +3424,9 @@ static void phylink_sfp_set_config(struct phylink *pl,
 			     phy_modes(state->interface));
 	}
 
+	if (pl->phydev)
+		changed = true;
+
 	if (changed && !test_bit(PHYLINK_DISABLE_STOPPED,
 				 &pl->phylink_disable_state))
 		phylink_mac_initial_config(pl, false);
-- 
2.47.1

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

Going through the log...

On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
> Log before this patch is applied:
> [root@bpir3 ~]# dmesg | grep eth1
> [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
> [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
> [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
> [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
> [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x

This is indeed without the PHY. We're using inband, although the PCS
mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
used. As there is no PHY, we can't switch to MLO_AN_PHY.

> [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
> [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
> [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
> [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
> [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23

The PHY comes along...

> [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
> [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
> [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef

We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.

> [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
> [   40.397090] brlan: port 5(eth1) entered blocking state
> [   40.402223] brlan: port 5(eth1) entered disabled state
> [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
> [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
> [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
> [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)

... but we don't see link-up reported by the PCS after the PHY comes
up. Why is that - I think that needs investigation before we proceed
to patch the issue, because that suggests the PCS isn't seeing
valid 2500base-X from the PHY.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Eric Woudstra]

On 1/7/25 1:47 PM, Russell King (Oracle) wrote:
> Going through the log...
> 
> On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
>> Log before this patch is applied:
>> [root@bpir3 ~]# dmesg | grep eth1
>> [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
>> [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
>> [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
>> [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
>> [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
> 
> This is indeed without the PHY. We're using inband, although the PCS
> mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
> used. As there is no PHY, we can't switch to MLO_AN_PHY.
> 
>> [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
>> [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
>> [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
>> [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
>> [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
>> [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
>> [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
> 
> The PHY comes along...
> 
>> [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
>> [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
>> [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
>> [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
>> [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
> 
> We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.
> 
>> [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
>> [   40.397090] brlan: port 5(eth1) entered blocking state
>> [   40.402223] brlan: port 5(eth1) entered disabled state
>> [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
>> [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
>> [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
>> [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
> 
> ... but we don't see link-up reported by the PCS after the PHY comes
> up. Why is that - I think that needs investigation before we proceed
> to patch the issue, because that suggests the PCS isn't seeing
> valid 2500base-X from the PHY.
> 

I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
it needs to be set to MLO_AN_PHY, so that only the phy determines the
link state:

phylink_resolve() {
    ...
	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
		link_state = pl->phy_state;
    ...
}

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Daniel Golle]

On Tue, Jan 07, 2025 at 12:47:14PM +0000, Russell King (Oracle) wrote:
> Going through the log...
> 
> On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
> > Log before this patch is applied:
> > [root@bpir3 ~]# dmesg | grep eth1
> > [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
> > [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
> > [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
> > [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
> > [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
> 
> This is indeed without the PHY. We're using inband, although the PCS
> mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
> used. As there is no PHY, we can't switch to MLO_AN_PHY.
> 
> > [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
> > [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
> > [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
> > [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> > [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
> > [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> > [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
> 
> The PHY comes along...
> 
> > [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> > [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> > [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
> > [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
> > [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
> 
> We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.
> 
> > [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
> > [   40.397090] brlan: port 5(eth1) entered blocking state
> > [   40.402223] brlan: port 5(eth1) entered disabled state
> > [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
> > [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
> > [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
> > [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
> 
> ... but we don't see link-up reported by the PCS after the PHY comes
> up. Why is that - I think that needs investigation before we proceed
> to patch the issue, because that suggests the PCS isn't seeing
> valid 2500base-X from the PHY.

The PCS doesn't support in-band status in 2500Base-X mode, or at least
the implementation isn't compatible with those RealTek PHYs.

In OpenWrt we carry a downstream patch to disable in-band status on the
side of the PHY which fixes the issue:

https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-6.6/720-02-net-phy-realtek-disable-SGMII-in-band-AN-for-2-5G-PHYs.patch;h=7e48c16515db8e401495dc1c505319424773ee11;hb=HEAD

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Andrew Lunn]

> I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
> it needs to be set to MLO_AN_PHY, so that only the phy determines the
> link state:
> 
> phylink_resolve() {
>     ...
> 	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
> 		link_state = pl->phy_state;
>     ...
> }

phylink tries to determine the whole chain is up. As Russell says, it
could be the PCS has not got sync with the PHY for some reason. So
even if you ignore the PCS state, it might not work. This is actually
a useful pieces of information. Does the link actually work end to end
if you only look at the media state? If it does, that would indicate
the PCS is maybe missing an interrupt, or needs polling for change in
state.

	Andrew

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Andrew Lunn]

> The PCS doesn't support in-band status in 2500Base-X mode, or at least
> the implementation isn't compatible with those RealTek PHYs.

Is one or the other not actually 2500Base-X, but SGMII over clocked?

Overclocked SGMII cannot do in-band signalling. It could well be the
PCS is 2500BaseX, but the PHY is over clocked SGMII, and so the PCS is
not seeing the in-band signalling it expects, so never reports link.

	Andrew

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Eric Woudstra]

On 1/7/25 2:20 PM, Andrew Lunn wrote:
>> I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
>> it needs to be set to MLO_AN_PHY, so that only the phy determines the
>> link state:
>>
>> phylink_resolve() {
>>     ...
>> 	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
>> 		link_state = pl->phy_state;
>>     ...
>> }
> 
> phylink tries to determine the whole chain is up. As Russell says, it
> could be the PCS has not got sync with the PHY for some reason. So
> even if you ignore the PCS state, it might not work. This is actually
> a useful pieces of information. Does the link actually work end to end
> if you only look at the media state? If it does, that would indicate
> the PCS is maybe missing an interrupt, or needs polling for change in
> state.
> 
> 	Andrew

After phylink_mac_initial_config() is re-triggered with the phy
attached, either by the patch, or even with:

ethtool -s eth1 advertise 0x28
(switches to sgmii)
ethtool -s eth1 advertise 0x800000000028
(switches mac back to 2500base-x)

mode is set to MLO_AN_PHY in phylink_pcs_neg_mode() and the link works
end to end.

So the an-mode can be 2 different values, one after link up and another
after these ethtool commands.

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

On Tue, Jan 07, 2025 at 01:16:33PM +0000, Daniel Golle wrote:
> On Tue, Jan 07, 2025 at 12:47:14PM +0000, Russell King (Oracle) wrote:
> > ... but we don't see link-up reported by the PCS after the PHY comes
> > up. Why is that - I think that needs investigation before we proceed
> > to patch the issue, because that suggests the PCS isn't seeing
> > valid 2500base-X from the PHY.
> 
> The PCS doesn't support in-band status in 2500Base-X mode, or at least
> the implementation isn't compatible with those RealTek PHYs.

There is in-band for base-X, which involves 16-bit control words to
report the capabilities of either end (basically half/full duplex,
pause modes). Unlike SGMII, it doesn't contain any status bits for
link up, because that is irrelevant.

Link-up state in base-X modes comes from the PCS itself, whether the
PCS is in sync with the media, and whether it has valid format. This
has *nothing* to do with in-band.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

On Tue, Jan 07, 2025 at 02:14:03PM +0100, Eric Woudstra wrote:
> 
> 
> On 1/7/25 1:47 PM, Russell King (Oracle) wrote:
> > Going through the log...
> > 
> > On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
> >> Log before this patch is applied:
> >> [root@bpir3 ~]# dmesg | grep eth1
> >> [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
> >> [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
> >> [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
> >> [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
> >> [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
> > 
> > This is indeed without the PHY. We're using inband, although the PCS
> > mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
> > used. As there is no PHY, we can't switch to MLO_AN_PHY.
> > 
> >> [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
> >> [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
> >> [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
> >> [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> >> [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
> >> [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> >> [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
> > 
> > The PHY comes along...
> > 
> >> [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> >> [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> >> [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
> >> [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
> >> [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
> > 
> > We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.
> > 
> >> [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
> >> [   40.397090] brlan: port 5(eth1) entered blocking state
> >> [   40.402223] brlan: port 5(eth1) entered disabled state
> >> [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
> >> [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
> >> [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
> >> [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
> > 
> > ... but we don't see link-up reported by the PCS after the PHY comes
> > up. Why is that - I think that needs investigation before we proceed
> > to patch the issue, because that suggests the PCS isn't seeing
> > valid 2500base-X from the PHY.
> > 
> 
> I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
> it needs to be set to MLO_AN_PHY, so that only the phy determines the
> link state:
> 
> phylink_resolve() {
>     ...
> 	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
> 		link_state = pl->phy_state;
>     ...
> }

Please see my reply to Daniel. The PCS should still be capable of
reporting whether its link is up or down irrespective of whether
in-band status is being used or not.

While it is correct that PHY mode needs to be used here, your report
has pointed out that the driver is not reporting the PCS link state
correctly when in-band is disabled.

Given that the current state of affairs has revealed this other bug,
I would like that addressed first while there is a trivial test case
here.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Eric Woudstra]

On 1/7/25 4:03 PM, Russell King (Oracle) wrote:
> On Tue, Jan 07, 2025 at 02:14:03PM +0100, Eric Woudstra wrote:
>>
>>
>> On 1/7/25 1:47 PM, Russell King (Oracle) wrote:
>>> Going through the log...
>>>
>>> On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
>>>> Log before this patch is applied:
>>>> [root@bpir3 ~]# dmesg | grep eth1
>>>> [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
>>>> [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
>>>> [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
>>>> [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
>>>> [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
>>>
>>> This is indeed without the PHY. We're using inband, although the PCS
>>> mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
>>> used. As there is no PHY, we can't switch to MLO_AN_PHY.
>>>
>>>> [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
>>>> [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
>>>> [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
>>>> [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
>>>> [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
>>>> [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
>>>> [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
>>>
>>> The PHY comes along...
>>>
>>>> [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
>>>> [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
>>>> [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
>>>> [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
>>>> [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
>>>
>>> We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.
>>>
>>>> [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
>>>> [   40.397090] brlan: port 5(eth1) entered blocking state
>>>> [   40.402223] brlan: port 5(eth1) entered disabled state
>>>> [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
>>>> [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
>>>> [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
>>>> [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
>>>
>>> ... but we don't see link-up reported by the PCS after the PHY comes
>>> up. Why is that - I think that needs investigation before we proceed
>>> to patch the issue, because that suggests the PCS isn't seeing
>>> valid 2500base-X from the PHY.
>>>
>>
>> I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
>> it needs to be set to MLO_AN_PHY, so that only the phy determines the
>> link state:
>>
>> phylink_resolve() {
>>     ...
>> 	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
>> 		link_state = pl->phy_state;
>>     ...
>> }
> 
> Please see my reply to Daniel. The PCS should still be capable of
> reporting whether its link is up or down irrespective of whether
> in-band status is being used or not.
> 
> While it is correct that PHY mode needs to be used here, your report
> has pointed out that the driver is not reporting the PCS link state
> correctly when in-band is disabled.
> 
> Given that the current state of affairs has revealed this other bug,
> I would like that addressed first while there is a trivial test case
> here.
> 

So I've narrowed down the problem a bit:

At first state->link is set to true, while looking at the bmsr.

But because linkmode_test_bit(fd_bit, state->advertising) and
linkmode_test_bit(fd_bit, state->lp_advertising) are both false,
state->link is set to false after looking at the bmsr.

void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
				      u16 bmsr, u16 lpa)
{
	state->link = !!(bmsr & BMSR_LSTATUS);
	...
	case PHY_INTERFACE_MODE_2500BASEX:
		phylink_decode_c37_word(state, lpa, SPEED_2500);
	...
}

static void phylink_decode_c37_word(struct phylink_link_state *state,
				    uint16_t config_reg, int speed)
{
	...
	if (linkmode_test_bit(fd_bit, state->advertising) &&
	    linkmode_test_bit(fd_bit, state->lp_advertising)) {
		state->speed = speed;
		state->duplex = DUPLEX_FULL;
	} else {
		/* negotiation failure */
		state->link = false;
	}
	...
}

And I can confirm, if I change the part above into the part below
(removing the if statement), the PCS also reports the link is up and the
connection is functional end-to-end. Not that I'm saying this change is
the solution, only for narrowing down the problem.

static void phylink_decode_c37_word(struct phylink_link_state *state,
				    uint16_t config_reg, int speed)
{
	...
	state->speed = speed;
	state->duplex = DUPLEX_FULL;
	...
}

Also worth mentioning, up until v12 of the pcs-mtk-lynxy.c
mtk_pcs_lynxi_get_state() did not call
phylink_mii_c22_pcs_decode_state() at all for 2500base-x.

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

On Thu, Jan 09, 2025 at 09:56:17AM +0100, Eric Woudstra wrote:
> So I've narrowed down the problem a bit:
> 
> At first state->link is set to true, while looking at the bmsr.
> 
> But because linkmode_test_bit(fd_bit, state->advertising) and
> linkmode_test_bit(fd_bit, state->lp_advertising) are both false,
> state->link is set to false after looking at the bmsr.

We shouldn't be getting that far if aneg isn't being used. The problem
is this is no longer sufficient:

        if (!state->link || !linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
                                               state->advertising))
                return;

since whether we use aneg or not now depends on state other than just
the Autoneg bit. It isn't going to be a simple fix, because we need
the PCS neg_mode here, but we don't have it as an argument to the
.pcs_get_state() method. I'll look at what we can do for this today.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Ilya K]

On 2025-01-09 12:15, Russell King (Oracle) wrote:
> On Thu, Jan 09, 2025 at 09:56:17AM +0100, Eric Woudstra wrote:
> I'll look at what we can do for this today.
Hey folks, don't really want to necrobump this, but the original hack patch is the only way I can get my weirdo Realtek (noticing a pattern here) GPON modem-on-a-stick to probe at 2500base-x on a BPi-R4. I've actually tracked down the issue myself and wrote basically the same patch first :( Is there anything I can do (with my extremely limited phylink understanding) to help move this forward so other people don't have to suffer like I did?

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

On Mon, Jun 02, 2025 at 10:19:22AM +0300, Ilya K wrote:
> On 2025-01-09 12:15, Russell King (Oracle) wrote:
> > On Thu, Jan 09, 2025 at 09:56:17AM +0100, Eric Woudstra wrote:
> > I'll look at what we can do for this today.
> Hey folks, don't really want to necrobump this, but the original hack patch is the only way I can get my weirdo Realtek (noticing a pattern here) GPON modem-on-a-stick to probe at 2500base-x on a BPi-R4. I've actually tracked down the issue myself and wrote basically the same patch first :( Is there anything I can do (with my extremely limited phylink understanding) to help move this forward so other people don't have to suffer like I did?

There were two issues that were identified. The first was that when in
2500Base-X mode, the PCS wasn't reporting link-up. That was addressed
by the patch series:

Z4TbR93B-X8A8iHe@shell.armlinux.org.uk

then, the replacement for Eric's patch was:

E1tYhxp-001FHf-Ac@rmk-PC.armlinux.org.uk

So the problem should be solved with these applied, as when the PHY is
attached, "changed" will always be true, which is in effect what Eric's
patch was doing, but in a cleaner way.

Please confirm that you have all six patches applied.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Ilya K]

On 2025-06-02 11:48, Russell King (Oracle) wrote:
> On Mon, Jun 02, 2025 at 10:19:22AM +0300, Ilya K wrote:
>> On 2025-01-09 12:15, Russell King (Oracle) wrote:
>>> On Thu, Jan 09, 2025 at 09:56:17AM +0100, Eric Woudstra wrote:
>>> I'll look at what we can do for this today.
>> Hey folks, don't really want to necrobump this, but the original hack patch is the only way I can get my weirdo Realtek (noticing a pattern here) GPON modem-on-a-stick to probe at 2500base-x on a BPi-R4. I've actually tracked down the issue myself and wrote basically the same patch first :( Is there anything I can do (with my extremely limited phylink understanding) to help move this forward so other people don't have to suffer like I did?
> 
> There were two issues that were identified. The first was that when in
> 2500Base-X mode, the PCS wasn't reporting link-up. That was addressed
> by the patch series:
> 
> Z4TbR93B-X8A8iHe@shell.armlinux.org.uk
> 
> then, the replacement for Eric's patch was:
> 
> E1tYhxp-001FHf-Ac@rmk-PC.armlinux.org.uk
> 
> So the problem should be solved with these applied, as when the PHY is
> attached, "changed" will always be true, which is in effect what Eric's
> patch was doing, but in a cleaner way.
> 
> Please confirm that you have all six patches applied.
> 

That's weird, because I do have all the patches applied. I think this may have been broken by the pcs_inband_caps changes, because without the patch I'm just getting "autoneg setting not compatible with PCS", after which it bails, when it should really reconfigure the MAC instead.

(resending because I replied instead of reply-all by accident)

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Russell King (Oracle)]

On Mon, Jun 02, 2025 at 04:00:14PM +0300, Ilya K wrote:
> That's weird, because I do have all the patches applied. I think this may have been broken by the pcs_inband_caps changes, because without the patch I'm just getting "autoneg setting not compatible with PCS", after which it bails, when it should really reconfigure the MAC instead.

Please enable phylink and sfp debug (adding #define DEBUG to the top of
phylink.c and sfp.c, and then send the kernel messages after reproducing
the issue.

Thanks.

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

Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

[Ilya K]

On 2025-06-02 18:25, Russell King (Oracle) wrote:
> On Mon, Jun 02, 2025 at 04:00:14PM +0300, Ilya K wrote:
>> That's weird, because I do have all the patches applied. I think this may have been broken by the pcs_inband_caps changes, because without the patch I'm just getting "autoneg setting not compatible with PCS", after which it bails, when it should really reconfigure the MAC instead.
> 
> Please enable phylink and sfp debug (adding #define DEBUG to the top of
> phylink.c and sfp.c, and then send the kernel messages after reproducing
> the issue.
> 
> Thanks.
> 

OK, somehow it just works normally now, even with the patch reverted. I think I grew a few gray hairs over the last few days. Sorry for the trouble, everyone, I'm off to not touch this thing ever again.