[PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Wei Fang]

Pause, Asym_Pause and Autoneg bits are not set when pl->supported is
initialized, so these link modes will not work for the fixed-link. This
leads to a TCP performance degradation issue observed on the i.MX943
platform.

The switch CPU port of i.MX943 is connected to an ENETC MAC, this link
is a fixed link and the link speed is 2.5Gbps. And one of the switch
user ports is the RGMII interface, and its link speed is 1Gbps. If the
flow-control of the fixed link is not enabled, we can easily observe
the iperf performance of TCP packets is very low. Because the inbound
rate on the CPU port is greater than the outbound rate on the user port,
the switch is prone to congestion, leading to the loss of some TCP
packets and requiring multiple retransmissions.

Solving this problem should be as simple as setting the Asym_Pause and
Pause bits. The reason why the Autoneg bit needs to be set, Russell
has gave a very good explanation in the thread [1], see below.

"As the advertising and lp_advertising bitmasks have to be non-empty,
and the swphy reports aneg capable, aneg complete, and AN enabled, then
for consistency with that state, Autoneg should be set. This is how it
was prior to the blamed commit."

[1] https://lore.kernel.org/all/aRjqLN8eQDIQfBjS@shell.armlinux.org.uk/

Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link configuration")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
v3 changes:
Add Ressell's explanation in the commit message
v2: https://lore.kernel.org/imx/20251116023823.1445099-1-wei.fang@nxp.com/
v2 changes:
1. Improve the commit message
2. Collect the Reviewed-by tag
v1: https://lore.kernel.org/imx/20251114052808.1129942-1-wei.fang@nxp.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 9d7799ea1c17..918244308215 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -637,6 +637,9 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
 
 static void phylink_fill_fixedlink_supported(unsigned long *supported)
 {
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
-- 
2.34.1

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Russell King (Oracle)]

On Mon, Nov 17, 2025 at 06:29:43PM +0800, Wei Fang wrote:
> Pause, Asym_Pause and Autoneg bits are not set when pl->supported is
> initialized, so these link modes will not work for the fixed-link. This
> leads to a TCP performance degradation issue observed on the i.MX943
> platform.
> 
> The switch CPU port of i.MX943 is connected to an ENETC MAC, this link
> is a fixed link and the link speed is 2.5Gbps. And one of the switch
> user ports is the RGMII interface, and its link speed is 1Gbps. If the
> flow-control of the fixed link is not enabled, we can easily observe
> the iperf performance of TCP packets is very low. Because the inbound
> rate on the CPU port is greater than the outbound rate on the user port,
> the switch is prone to congestion, leading to the loss of some TCP
> packets and requiring multiple retransmissions.
> 
> Solving this problem should be as simple as setting the Asym_Pause and
> Pause bits. The reason why the Autoneg bit needs to be set, Russell
> has gave a very good explanation in the thread [1], see below.
> 
> "As the advertising and lp_advertising bitmasks have to be non-empty,
> and the swphy reports aneg capable, aneg complete, and AN enabled, then
> for consistency with that state, Autoneg should be set. This is how it
> was prior to the blamed commit."
> 
> [1] https://lore.kernel.org/all/aRjqLN8eQDIQfBjS@shell.armlinux.org.uk/
> 
> Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link configuration")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

NAK. I give up.

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

RE: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Wei Fang]

> On Mon, Nov 17, 2025 at 06:29:43PM +0800, Wei Fang wrote:
> > Pause, Asym_Pause and Autoneg bits are not set when pl->supported is
> > initialized, so these link modes will not work for the fixed-link.
> > This leads to a TCP performance degradation issue observed on the
> > i.MX943 platform.
> >
> > The switch CPU port of i.MX943 is connected to an ENETC MAC, this link
> > is a fixed link and the link speed is 2.5Gbps. And one of the switch
> > user ports is the RGMII interface, and its link speed is 1Gbps. If the
> > flow-control of the fixed link is not enabled, we can easily observe
> > the iperf performance of TCP packets is very low. Because the inbound
> > rate on the CPU port is greater than the outbound rate on the user
> > port, the switch is prone to congestion, leading to the loss of some
> > TCP packets and requiring multiple retransmissions.
> >
> > Solving this problem should be as simple as setting the Asym_Pause and
> > Pause bits. The reason why the Autoneg bit needs to be set, Russell
> > has gave a very good explanation in the thread [1], see below.
> >
> > "As the advertising and lp_advertising bitmasks have to be non-empty,
> > and the swphy reports aneg capable, aneg complete, and AN enabled,
> > then for consistency with that state, Autoneg should be set. This is
> > how it was prior to the blamed commit."
> >
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2FaRjqLN8eQDIQfBjS%40shell.armlinux.org.uk%2F&data=
> 0
> >
> 5%7C02%7Cwei.fang%40nxp.com%7Cf602663c2989492e292c08de267bcf5e%7
> C686ea
> >
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638990510348901384%7CUnkn
> own%7CT
> >
> WFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4
> zMiI
> >
> sIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Ni8mNVFADhA
> 1SLOZk
> > z7yq%2FcogtuidA2u5GiK%2Bqm56L8%3D&reserved=0
> >
> > Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link
> > configuration")
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> 
> NAK. I give up.
> 

Sorry, could you please tell me what the reason is?

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Andrew Lunn]

> > > Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link
> > > configuration")
> > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > 
> > NAK. I give up.
> > 
> 
> Sorry, could you please tell me what the reason is?

I think Russell is referring to the commit message, and how you only
quoted a little section of his explanation. There is no limit to
commit messages, they don't need to be short. It is actually better if
they are long. So you could use his whole explanation. And then you
don't need the link.

	Andrew

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Russell King (Oracle)]

On Tue, Nov 18, 2025 at 03:00:00PM +0100, Andrew Lunn wrote:
> > > > Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link
> > > > configuration")
> > > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > > 
> > > NAK. I give up.
> > > 
> > 
> > Sorry, could you please tell me what the reason is?
> 
> I think Russell is referring to the commit message, and how you only
> quoted a little section of his explanation. There is no limit to
> commit messages, they don't need to be short. It is actually better if
> they are long. So you could use his whole explanation. And then you
> don't need the link.

Worse than that. I gave my reviewed-by, which seems to have been a waste
of time.

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

RE: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Wei Fang]

> On Tue, Nov 18, 2025 at 03:00:00PM +0100, Andrew Lunn wrote:
> > > > > Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link
> > > > > configuration")
> > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > > >
> > > > NAK. I give up.
> > > >
> > >
> > > Sorry, could you please tell me what the reason is?
> >
> > I think Russell is referring to the commit message, and how you only
> > quoted a little section of his explanation. There is no limit to
> > commit messages, they don't need to be short. It is actually better if
> > they are long. So you could use his whole explanation. And then you
> > don't need the link.
> 
> Worse than that. I gave my reviewed-by, which seems to have been a waste
> of time.
> 

I'm sorry, I was in a rush to send out the v3 patch, and I hadn't received
your Reviewed-by tag at that time, so the tag was not added. When I saw
that you gave the Review-by in v2, I realized that I could no longer add it
to v3, so I replied that I had sent v3, hoping that you could resend your
Reviewed-by tag.

If you don't mind, I will refine the commit message as Andrew suggested
and add your Revived-by tag from v2 to v4. I apologize again.

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Russell King (Oracle)]

On Wed, Nov 19, 2025 at 01:22:17AM +0000, Wei Fang wrote:
> > On Tue, Nov 18, 2025 at 03:00:00PM +0100, Andrew Lunn wrote:
> > > > > > Fixes: de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link
> > > > > > configuration")
> > > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > > > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > > > >
> > > > > NAK. I give up.
> > > > >
> > > >
> > > > Sorry, could you please tell me what the reason is?
> > >
> > > I think Russell is referring to the commit message, and how you only
> > > quoted a little section of his explanation. There is no limit to
> > > commit messages, they don't need to be short. It is actually better if
> > > they are long. So you could use his whole explanation. And then you
> > > don't need the link.
> > 
> > Worse than that. I gave my reviewed-by, which seems to have been a waste
> > of time.
> > 
> 
> I'm sorry, I was in a rush to send out the v3 patch, and I hadn't received
> your Reviewed-by tag at that time, so the tag was not added. When I saw
> that you gave the Review-by in v2, I realized that I could no longer add it
> to v3, so I replied that I had sent v3, hoping that you could resend your
> Reviewed-by tag.
> 
> If you don't mind, I will refine the commit message as Andrew suggested
> and add your Revived-by tag from v2 to v4. I apologize again.

I also question the need to refine the commit message this much. One
of the points of lore.kernel.org is that it provides a stable source
for mailing list archives. We use URLs to that site extensively in
the kernel development process - e.g. it's recommended to use it in
Closes: tags, and to reference discussion from commit messages. If
I look at the number of times lore.kernel.org has been mentioned in
commit messages since 6.17, it comes out at around 5700 to date.
Looking back to 6.16, it's about 13000.

So, lore.kernel.org is already an insanely valuable resource to the
kernel community, and the loss of it would result in a lot of
context being lost.

We have had problems with other sites - lkml.org used to be the
popular site, but that became unreliable and stuff broke. However,
the difference is that lore.kernel.org is maintained by the same
people who look after the rest of the kernel.org infrastructure.

Moreover, using lore.kernel.org is encouraged when one wishes to
link to discussion. See "Linking to list discussions from commits"
at the bottom of https://www.kernel.org/lore.html

So, I think there was no need to go through v3, inflating the commit
message, and end up in this situation.

Every time a patch gets reposted, the netdev cycle (as far as the
netdev maintainers are concerned) restarts, and it means a multi-day
delay before the change gets committed. As things stand, this is
likely to miss tomorrow's linux-net tree submission, which is
highly likely to be the last one before 6.18 is released. So we're
not going to get this fixed before the final 6.18 now. And for
what value? None as far as I can see. The patch was ready at v2.

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

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[Jakub Kicinski]

On Wed, 19 Nov 2025 16:06:03 +0000 Russell King (Oracle) wrote:
> On Wed, Nov 19, 2025 at 01:22:17AM +0000, Wei Fang wrote:
> > I'm sorry, I was in a rush to send out the v3 patch, and I hadn't received
> > your Reviewed-by tag at that time, so the tag was not added. When I saw
> > that you gave the Review-by in v2, I realized that I could no longer add it
> > to v3, so I replied that I had sent v3, hoping that you could resend your
> > Reviewed-by tag.
> > 
> > If you don't mind, I will refine the commit message as Andrew suggested
> > and add your Revived-by tag from v2 to v4. I apologize again.  
> 
> I also question the need to refine the commit message this much. One
> of the points of lore.kernel.org is that it provides a stable source
> for mailing list archives. We use URLs to that site extensively in
> the kernel development process - e.g. it's recommended to use it in
> Closes: tags, and to reference discussion from commit messages. If
> I look at the number of times lore.kernel.org has been mentioned in
> commit messages since 6.17, it comes out at around 5700 to date.
> Looking back to 6.16, it's about 13000.
> 
> So, lore.kernel.org is already an insanely valuable resource to the
> kernel community, and the loss of it would result in a lot of
> context being lost.
> 
> We have had problems with other sites - lkml.org used to be the
> popular site, but that became unreliable and stuff broke. However,
> the difference is that lore.kernel.org is maintained by the same
> people who look after the rest of the kernel.org infrastructure.
> 
> Moreover, using lore.kernel.org is encouraged when one wishes to
> link to discussion. See "Linking to list discussions from commits"
> at the bottom of https://www.kernel.org/lore.html
> 
> So, I think there was no need to go through v3, inflating the commit
> message, and end up in this situation.
> 
> Every time a patch gets reposted, the netdev cycle (as far as the
> netdev maintainers are concerned) restarts, and it means a multi-day
> delay before the change gets committed. As things stand, this is
> likely to miss tomorrow's linux-net tree submission, which is
> highly likely to be the last one before 6.18 is released. So we're
> not going to get this fixed before the final 6.18 now. And for
> what value? None as far as I can see. The patch was ready at v2.

We would have also avoided a lot of wasted time here if the authors
just mentioned in the discussion on v2 that v3 is out :|

Let me fix up the Link tag here and apply this so we can move on.

Re: [PATCH v3 net] net: phylink: add missing supported link modes for the fixed-link

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 17 Nov 2025 18:29:43 +0800 you wrote:
> Pause, Asym_Pause and Autoneg bits are not set when pl->supported is
> initialized, so these link modes will not work for the fixed-link. This
> leads to a TCP performance degradation issue observed on the i.MX943
> platform.
> 
> The switch CPU port of i.MX943 is connected to an ENETC MAC, this link
> is a fixed link and the link speed is 2.5Gbps. And one of the switch
> user ports is the RGMII interface, and its link speed is 1Gbps. If the
> flow-control of the fixed link is not enabled, we can easily observe
> the iperf performance of TCP packets is very low. Because the inbound
> rate on the CPU port is greater than the outbound rate on the user port,
> the switch is prone to congestion, leading to the loss of some TCP
> packets and requiring multiple retransmissions.
> 
> [...]

Here is the summary with links:
  - [v3,net] net: phylink: add missing supported link modes for the fixed-link
    https://git.kernel.org/netdev/net/c/e31a11be41cd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html