net: phylink: add missing supported link modes for the fixed-link
Subsystems (5)
PATCH v3 1/1 net: phylink: add missing supported link modes for the fixed-link
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/[email protected]/
---
v3 changes:
Add Ressell's explanation in the commit message
v2: https://lore.kernel.org/imx/[email protected]/
v2 changes:
1. Improve the commit message
2. Collect the Reviewed-by tag
v1: https://lore.kernel.org/imx/[email protected]/
--- drivers/net/phy/phylink.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/net/phy/phylink.c +3 -0
@@ -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);
Signature · 1 line
2.34.1
Russell King (Oracle)
maintainerQuoted 23 lines · Wei Fang
Pause,
Asym_Pauseand Autoneg bits are not set whenpl->supportedis 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_Pauseand 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_advertisingbitmasks 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."
Quoted 5 lines
[1] https://lore.kernel.org/all/[email protected]/
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-link configuration")Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>
NAK. I give up.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Quoted 40 lines · Wei Fang
Quoted 18 lines
Pause,
Asym_Pauseand Autoneg bits are not set whenpl->supportedis 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_Pauseand 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.Quoted 8 lines
"As the advertising and
lp_advertisingbitmasks 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
Quoted 1 line
5%7C02%7Cwei.fang%40nxp.com%7Cf602663c2989492e292c08de267bcf5e%7 C686ea
Quoted 1 line
1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638990510348901384%7CUnkn own%7CT
Quoted 1 line
WFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4 zMiI
Quoted 1 line
sIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Ni8mNVFADhA 1SLOZk
Quoted 9 lines
Quoted 6 lines
z7yq%2FcogtuidA2u5GiK%2Bqm56L8%3D&reserved=0
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-linkconfiguration")
Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>NAK. I give up.
Sorry, could you please tell me what the reason is?
Andrew Lunn
maintainerQuoted 9 lines
Quoted 7 lines
Quoted 4 lines
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-linkconfiguration")
Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>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
Russell King (Oracle)
maintainerQuoted 8 lines · Andrew Lunn
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-linkconfiguration")
Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>NAK. I give up.
Quoted 7 lines
Quoted 1 line
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.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Quoted 11 lines · Andrew Lunn
Quoted 10 lines
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-linkconfiguration")
Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>NAK. I give up.
Sorry, could you please tell me what the reason is?
Quoted 9 lines
Quoted 5 lines
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.
Russell King (Oracle)
maintainerQuoted 21 lines · Wei Fang
Quoted 11 lines · Andrew Lunn
Quoted 10 lines
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Fixes de7d3f87be3c ("net: phylink: Use phy_caps_lookup for fixed-linkconfiguration")
Signed-off-by Wei Fang <[email protected]>Reviewed-by Maxime Chevallier <[email protected]>NAK. I give up.
Sorry, could you please tell me what the reason is?
Quoted 9 lines
Quoted 5 lines
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.
Quoted 8 lines
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.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Jakub Kicinski
maintainerQuoted 35 lines · On Wed, 19 Nov 2025 16:06:03 +0000 Russell King (Oracle) wrote:
Quoted 8 lines · Wei Fang
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.
Quoted 7 lines
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.
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 14 lines · On Mon, 17 Nov 2025 18:29:43 +0800 you wrote:
Pause,
Asym_Pauseand Autoneg bits are not set whenpl->supportedis 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.
Quoted 1 line
[...]
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!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html