net: net: add negotiation of in-band capabilities (remainder)
Subsystems (9)
Russell King (Oracle)
maintainercover net: net: add negotiation of in-band capabilities (remainder)
Hi,
Here are the last three patches which were not included in the non-RFC posting, but were in the RFC posting. These add the .pcs_inband() method to the Lynx, MTK Lynx and XPCS drivers.
drivers/net/pcs/pcs-lynx.c | 22 ++++++++++++++++++++++ drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ drivers/net/pcs/pcs-xpcs.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)
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!
PATCH 1/3 net: pcs: pcs-lynx: implement pcs_inband_caps() method
Report the PCS in-band capabilities to phylink for the Lynx PCS.
--- drivers/net/pcs/pcs-lynx.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
drivers/net/pcs/pcs-lynx.c +22 -0
@@ -35,6 +35,27 @@ enum sgmii_speed {#define phylink_pcs_to_lynx (pl_pcs) container_of((pl_pcs), struct lynx_pcs, pcs)#define lynx_to_phylink_pcs (lynx) (&(lynx)->pcs) +static unsigned int lynx_pcs_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +case PHY_INTERFACE_MODE_10GBASER : +case PHY_INTERFACE_MODE_2500BASEX : +return LINK_INBAND_DISABLE ; + +case PHY_INTERFACE_MODE_USXGMII : +return LINK_INBAND_ENABLE ; + +default : +return 0 ; + } +} +static void lynx_pcs_get_state_usxgmii (struct mdio_device *pcs,struct phylink_link_state *state) {
@@ -306,6 +327,7 @@ static void lynx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode, }static const struct phylink_pcs_ops lynx_pcs_phylink_ops = { + .pcs_inband_caps = lynx_pcs_inband_caps, .pcs_get_state = lynx_pcs_get_state, .pcs_config = lynx_pcs_config, .pcs_an_restart = lynx_pcs_an_restart,
Signature · 1 line
2.30.2
[REGRESSION] net: pcs-lynx: 10G SFP no longer links up
#regzbot introduced: 6561f0e547be221f411fda5eddfcc5bd8bb058a5
Hi Russell,
Quoted 2 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the Lynx PCS.
The implementation of in-band capabilities has broken SFP+ (10GBase-R) mode on my LS1088 board. The other ports in the system (QSGMII) work fine.
$ dmesg | grep -E (eth8|dpmac2-sfp) sfp dpmac2-sfp: module FS SFPP-AO02 rev B1 sn F1940200586-2 dc 200615 fsl_dpaa2_eth dpni.1 eth8: autoneg setting not compatible with PCS $ sudo ip link set eth8 up fsl_dpaa2_eth dpni.1 eth8: configuring for inband/10gbase-r link mode < no link up occurs>
Reverting this exact commit resolves the issue, and the SFP comes up as normal:
fsl_dpaa2_eth dpni.1 eth8: configuring for inband/10gbase-r link mode fsl_dpaa2_eth dpni.1 eth8: Link is Up - 10Gbps/Full - flow control off
The changes that were merged into net-next recently ("net: phylink: fix PCS without autoneg") did not resolve this problem.
Obviously, I don't think this particular patch is responsible (but it's what the bisect arrives at). Do you have any suggestions on where to insert some tracing/debug?
Transceiver information:
Identifier : 0x03 (SFP)
Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
Connector : 0x21 (Copper pigtail)
Transceiver codes : 0x00 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00
Transceiver type : Active Cable
Encoding : 0x00 (unspecified)
BR, Nominal : 10300MBd
Rate identifier : 0x00 (unspecified)
Length (SMF,km) : 0km
Length (SMF) : 0m
Length (50um) : 0m
Length (62.5um) : 0m
Length (Copper) : 3m
Length (OM3) : 0m
Active Cu cmplnce. : 0x0c (unknown) [SFF-8472 rev10.4 only]
Vendor name : FS
Vendor OUI : 00:17:6a
Vendor PN : SFPP-AO02
Vendor rev : B1
Option values : 0x00 0x12
Option : RX_LOS implemented
Option : TX_DISABLE implemented
BR margin, max : 0%
BR margin, min : 0%
Vendor SN : F1940200586-2
Date code : 200615Passive cables are also affected:
Transceiver codes : 0x00 0x00 0x00 0x00 0x00 0x04 0x00 0x00 0x00
Transceiver type : Passive Cable
Best Regards,
Matt
Quoted 50 lines
Reviewed-by Maxime Chevallier <[email protected]>Signed-off-by Russell King (Oracle) <[email protected] <mailto:rmk%[email protected]>>---
drivers/net/pcs/pcs-lynx.c| 22 ++++++++++++++++++++++1 file changed, 22 insertions(+)drivers/net/pcs/pcs-lynx.c +22
-0@@ -35,6 +35,27 @@ enum sgmii_speed {#define phylink_pcs_to_lynx (pl_pcs) container_of((pl_pcs), struct lynx_pcs, pcs)#define lynx_to_phylink_pcs (lynx) (&(lynx)->pcs) +static unsigned int lynx_pcs_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +case PHY_INTERFACE_MODE_10GBASER : +case PHY_INTERFACE_MODE_2500BASEX : +return LINK_INBAND_DISABLE ; + +case PHY_INTERFACE_MODE_USXGMII : +return LINK_INBAND_ENABLE ; + +default : +return 0 ; + } +} +static void lynx_pcs_get_state_usxgmii (struct mdio_device *pcs,struct phylink_link_state *state) {@@ -306,6 +327,7 @@ static void lynx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode, }static const struct phylink_pcs_ops lynx_pcs_phylink_ops = { + .pcs_inband_caps = lynx_pcs_inband_caps, .pcs_get_state = lynx_pcs_get_state, .pcs_config = lynx_pcs_config, .pcs_an_restart = lynx_pcs_an_restart,Signature · 1 line
2.30.2
Russell King (Oracle)
maintainerRe: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Quoted 10 lines · Mathew McBride
#regzbot introduced:
6561f0e547be221f411fda5eddfcc5bd8bb058a5Hi Russell,
Quoted 2 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the Lynx PCS.
The implementation of in-band capabilities has broken SFP+ (10GBase-R) mode on my LS1088 board. The other ports in the system (QSGMII) work fine.
Thanks for the report.
Please try the diff below:
drivers/net/phy/phylink.c +9 -0
@@ -3624,6 +3624,15 @@ static int phylink_sfp_config_optical(struct phylink *pl)phylink_dbg (pl,"optical SFP: chosen %s interface\n" ,phy_modes (interface)); +/* GBASE-R interfaces with the exception of KR do not have autoneg at +* the PCS. As the PCS is media facing, disable the Autoneg bit in the +* advertisement. +*/ +if (interface ==PHY_INTERFACE_MODE_5GBASER || + interface ==PHY_INTERFACE_MODE_10GBASER || + interface ==PHY_INTERFACE_MODE_25GBASER ) +__clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); +if (!phylink_validate_pcs_inband_autoneg (pl, interface, config.advertising)) {phylink_err (pl,"autoneg setting not compatible with PCS" );
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!
Re: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Quoted 5 lines · Russell King (Oracle)
Quoted 4 lines · Mathew McBride
#regzbot introduced:
6561f0e547be221f411fda5eddfcc5bd8bb058a5Hi Russell,
Quoted 8 lines
Quoted 6 lines · Russell King (Oracle)
Quoted 2 lines
Report the PCS in-band capabilities to phylink for the Lynx PCS.
The implementation of in-band capabilities has broken SFP+ (10GBase-R) mode on my LS1088 board. The other ports in the system (QSGMII) work fine.
Thanks for the report.
Thanks Russell!
The diff below does fix the problem, 10G SFP's now link up again.
I should note that Alex Guzman was the one who originally reported the issue to me, he has also confirmed this diff resolves the issue.
Quoted 27 lines
Please try the diff below:
drivers/net/phy/phylink.c +9
-0@@ -3624,6 +3624,15 @@ static int phylink_sfp_config_optical(struct phylink *pl)phylink_dbg (pl,"optical SFP: chosen %s interface\n" ,phy_modes (interface)); +/* GBASE-R interfaces with the exception of KR do not have autoneg at +* the PCS. As the PCS is media facing, disable the Autoneg bit in the +* advertisement. +*/ +if (interface ==PHY_INTERFACE_MODE_5GBASER || + interface ==PHY_INTERFACE_MODE_10GBASER || + interface ==PHY_INTERFACE_MODE_25GBASER ) +__clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); +if (!phylink_validate_pcs_inband_autoneg (pl, interface, config.advertising)) {phylink_err (pl,"autoneg setting not compatible with PCS" );Signature · 3 lines
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Hi Russell,
Quoted 1 line · Mathew McBride
[snip]
Just following up on this issue where directly connected SFP+ modules stopped linking up after the introduction of in-band capabilities.
The diff you provided below[1] resolved the issue.
Were you planning on submitting it as a patch? If not, I'd be happy to send it in.
Best Regards,
Matt
[1] https://lore.kernel.org/all/[email protected]/
Quoted 35 lines
Thanks Russell!
The diff below does fix the problem, 10G SFP's now link up again.
I should note that Alex Guzman was the one who originally reported the issue to me, he has also confirmed this diff resolves the issue.
Quoted 27 lines
Please try the diff below:
drivers/net/phy/phylink.c +9
-0@@ -3624,6 +3624,15 @@ static int phylink_sfp_config_optical(struct phylink *pl)phylink_dbg (pl,"optical SFP: chosen %s interface\n" ,phy_modes (interface)); +/* GBASE-R interfaces with the exception of KR do not have autoneg at +* the PCS. As the PCS is media facing, disable the Autoneg bit in the +* advertisement. +*/ +if (interface ==PHY_INTERFACE_MODE_5GBASER || + interface ==PHY_INTERFACE_MODE_10GBASER || + interface ==PHY_INTERFACE_MODE_25GBASER ) +__clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); +if (!phylink_validate_pcs_inband_autoneg (pl, interface, config.advertising)) {phylink_err (pl,"autoneg setting not compatible with PCS" );Signature · 3 lines
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Lo!
Quoted 10 lines · Mathew McBride
Hi Russell,
Quoted 1 line · Mathew McBride
[snip]
Just following up on this issue where directly connected SFP+ modules stopped linking up after the introduction of in-band capabilities.
The diff you provided below[1] resolved the issue.
Were you planning on submitting it as a patch? If not, I'd be happy to send it in.
I might be missing something, but from here it looks like it fall through the cracks on Russell's side. This is nothing bad, this can happen, especially during summer and thus vacation time. I'd thus say:
wait two or three days if this reminds him of the patch, otherwise go ahead and submit it yourself to get the regression fixed.
Ciao, Thorsten
Quoted 38 lines
[1] https://lore.kernel.org/all/[email protected]/
Quoted 35 lines
Thanks Russell!
The diff below does fix the problem, 10G SFP's now link up again.
I should note that Alex Guzman was the one who originally reported the issue to me, he has also confirmed this diff resolves the issue.
Quoted 27 lines
Please try the diff below:
drivers/net/phy/phylink.c +9
-0@@ -3624,6 +3624,15 @@ static int phylink_sfp_config_optical(struct phylink *pl)phylink_dbg (pl,"optical SFP: chosen %s interface\n" ,phy_modes (interface)); +/* GBASE-R interfaces with the exception of KR do not have autoneg at +* the PCS. As the PCS is media facing, disable the Autoneg bit in the +* advertisement. +*/ +if (interface ==PHY_INTERFACE_MODE_5GBASER || + interface ==PHY_INTERFACE_MODE_10GBASER || + interface ==PHY_INTERFACE_MODE_25GBASER ) +__clear_bit (ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); +if (!phylink_validate_pcs_inband_autoneg (pl, interface, config.advertising)) {phylink_err (pl,"autoneg setting not compatible with PCS" );Signature · 3 lines
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Russell King (Oracle)
maintainerRe: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Quoted 11 lines · Thorsten Leemhuis
Lo!
Quoted 8 lines · Mathew McBride
Hi Russell,
Quoted 1 line · Mathew McBride
[snip]
Just following up on this issue where directly connected SFP+ modules stopped linking up after the introduction of in-band capabilities.
Quoted 8 lines
Quoted 2 lines
The diff you provided below[1] resolved the issue.
Were you planning on submitting it as a patch? If not, I'd be happy to send it in.I might be missing something, but from here it looks like it fall through the cracks on Russell's side. This is nothing bad, this can happen, especially during summer and thus vacation time. I'd thus say:
wait two or three days if this reminds him of the patch, otherwise go ahead and submit it yourself to get the regression fixed.
Yes, the reminder was sent during July when I wasn't looking at email, and as you can imagine, if I spend three weeks on vacation, I am _not_ going to catch up with that pile of email - if I were, there'd be no point taking vacation because the mental effort would be just the same as having no vacation.
I have been debating whether we should actually do something like this, especially given the issues with 2500base-X:
- if (!phylink_validate_pcs_inband_autoneg(pl, interface,
- config.advertising)) {
- phylink_err(pl, "autoneg setting not compatible with PCS");
- return -EINVAL;
+ while (!phylink_validate_pcs_inband_autoneg(pl, interface,
+ config.advertising)) {
+ if (!test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ config.advertising)) {
+ phylink_err(pl, "autoneg setting is not compatible with PCS");
+ return -EINVAL;
+ }
+
+ __clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising);
}which turns it into something generic - but my problem with that is.. what if the module (e.g. a GPON module immitating a fibre module) requires Autoneg but the PCS doesn't support Autoneg for the selected interface mode.
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!
Russell King (Oracle)
maintainerRe: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
Quoted 45 lines · Russell King (Oracle)
Quoted 11 lines · Thorsten Leemhuis
Lo!
Quoted 8 lines · Mathew McBride
Hi Russell,
Quoted 1 line · Mathew McBride
[snip]
Just following up on this issue where directly connected SFP+ modules stopped linking up after the introduction of in-band capabilities.
Quoted 8 lines
Quoted 2 lines
The diff you provided below[1] resolved the issue.
Were you planning on submitting it as a patch? If not, I'd be happy to send it in.I might be missing something, but from here it looks like it fall through the cracks on Russell's side. This is nothing bad, this can happen, especially during summer and thus vacation time. I'd thus say:
wait two or three days if this reminds him of the patch, otherwise go ahead and submit it yourself to get the regression fixed.Yes, the reminder was sent during July when I wasn't looking at email, and as you can imagine, if I spend three weeks on vacation, I am _not_ going to catch up with that pile of email - if I were, there'd be no point taking vacation because the mental effort would be just the same as having no vacation.
I have been debating whether we should actually do something like this, especially given the issues with 2500base-X:
(quoted diff) +9
-4- if (!phylink_validate_pcs_inband_autoneg(pl, interface, - config.advertising)) { - phylink_err(pl, "autoneg setting not compatible with PCS"); - return -EINVAL; + while (!phylink_validate_pcs_inband_autoneg(pl, interface, + config.advertising)) { + if (!test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + config.advertising)) { + phylink_err(pl, "autoneg setting is not compatible with PCS"); + return -EINVAL; + } + + __clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); }
Quoted 4 lines
which turns it into something generic - but my problem with that is.. what if the module (e.g. a GPON module immitating a fibre module) requires Autoneg but the PCS doesn't support Autoneg for the selected interface mode.
Please note that I'm waiting for a response from those who have the problem... and this thread is again getting buried, so likely I'll forget about it soon.
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!
Re: [REGRESSION] net: pcs-lynx: 10G SFP no longer links up
In reply to Russell King (Oracle)Hi Russell,
Quoted 7 lines · Russell King (Oracle)
Yes, the reminder was sent during July when I wasn't looking at email, and as you can imagine, if I spend three weeks on vacation, I am _not_ going to catch up with that pile of email - if I were, there'd be no point taking vacation because the mental effort would be just the same as having no vacation.
No problem. I was not sure if you were on vacation or not.
Quoted 8 lines
I have been debating whether we should actually do something like this, especially given the issues with 2500base-X:
(quoted diff) +0
-4- if (!phylink_validate_pcs_inband_autoneg(pl, interface, - config.advertising)) { - phylink_err(pl, "autoneg setting not compatible with PCS"); - return -EINVAL;
Review
Quoted 10 lines
(quoted diff) +9
-0+ while (!phylink_validate_pcs_inband_autoneg(pl, interface, + config.advertising)) { + if (!test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + config.advertising)) { + phylink_err(pl, "autoneg setting is not compatible with PCS"); + return -EINVAL; + } + + __clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising); }
I have just given this version a quick test with (at 10G only, I have not tested 1 or 2.5G configs), and it seems to work.
My board doesn't natively support 2.5G (due to a component in the signal path) but I can modify a board to work with 2.5G if testing these GPON or other imitator modules is helpful to you.
Quoted 5 lines
which turns it into something generic - but my problem with that is.. what if the module (e.g. a GPON module immitating a fibre module) requires Autoneg but the PCS doesn't support Autoneg for the selected interface mode.
Many Thanks,
Matt
Quoted 4 lines
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!
Re: [PATCH net-next 1/3] net: pcs: pcs-lynx: implement pcs_inband_caps() method
In reply to Russell King (Oracle)Quoted 41 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the Lynx PCS.
Reviewed-by Maxime Chevallier <[email protected]>Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/pcs/pcs-lynx.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)drivers/net/pcs/pcs-lynx.c +21
-0@@ -35,6 +35,27 @@ enum sgmii_speed {#define phylink_pcs_to_lynx (pl_pcs) container_of((pl_pcs), struct lynx_pcs, pcs)#define lynx_to_phylink_pcs (lynx) (&(lynx)->pcs) +static unsigned int lynx_pcs_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +case PHY_INTERFACE_MODE_10GBASER : +case PHY_INTERFACE_MODE_2500BASEX : +return LINK_INBAND_DISABLE ; + +case PHY_INTERFACE_MODE_USXGMII : +return LINK_INBAND_ENABLE ; + +default : +return 0 ; + } +} +static void lynx_pcs_get_state_usxgmii (struct mdio_device *pcs,struct phylink_link_state *state) {
drivers/net/pcs/pcs-lynx.c static void lynx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode, · line 333
Quoted 7 lines
drivers/net/pcs/pcs-lynx.c +1
-0@@ -306,6 +327,7 @@ static void lynx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode, }static const struct phylink_pcs_ops lynx_pcs_phylink_ops = { + .pcs_inband_caps = lynx_pcs_inband_caps, .pcs_get_state = lynx_pcs_get_state, .pcs_config = lynx_pcs_config, .pcs_an_restart = lynx_pcs_an_restart,
I have noticed a regression on LX2160 Honeycomb and LX2162 Clearfog, between v6.13 and v6.14: When connecting 10G fiber SFP, there are kernel errors and no link-up:
ifconfig eth1 up
[ 27.649571] fsl_dpaa2_eth dpni.10 eth1: configuring for inband/10gbase-r link mode
[ 45.642936] sfp sfp-ab: module FS SFPP-AO02 rev A sn C2405395582-2 dc 240517
[ 45.652543] fsl_dpaa2_eth dpni.10 eth1: autoneg setting not compatible with PCS
Apparently the system starts with AN at 10G, while the pcs-lynx driver reports AN disabled.
Then we receive the error message above.
I don't quite understand what is going wrong here - why the system tries to enable AN
in the first place after inserting SFP ... any ideas?
PATCH 2/3 net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
In reply to Russell King (Oracle)Report the PCS in-band capabilities to phylink for the LynxI PCS.
--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/net/pcs/pcs-mtk-lynxi.c +16 -0
@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); } +static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +default : +return 0 ; + } +} +static void mtk_pcs_lynxi_get_state (struct phylink_pcs *pcs,struct phylink_link_state *state) {
@@ -241,6 +256,7 @@ static void mtk_pcs_lynxi_disable(struct phylink_pcs *pcs) }static const struct phylink_pcs_ops mtk_pcs_lynxi_ops = { + .pcs_inband_caps = mtk_pcs_lynxi_inband_caps, .pcs_get_state = mtk_pcs_lynxi_get_state, .pcs_config = mtk_pcs_lynxi_config, .pcs_an_restart = mtk_pcs_lynxi_restart_an,
Signature · 1 line
2.30.2
Re: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 13 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs);
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 98
Quoted 10 lines
drivers/net/pcs/pcs-mtk-lynxi.c +8
-0} +static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII :
QSGMII is not supported by this PCS.
Apart from that looks good to me.
Russell King (Oracle)
maintainerRe: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 16 lines · Daniel Golle
Quoted 15 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); }
Quoted 10 lines
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 98
Quoted 8 lines
drivers/net/pcs/pcs-mtk-lynxi.c +8
-0+static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII :QSGMII is not supported by this PCS.
Well...
- lynx_pcs_get_state(), lynx_pcs_config(), and lynx_pcs_link_up() include QSGMII in their case statements.
- lynx_pcs_config_giga() refers to QSGMII in a comment, grouping it with SGMII configuration.
I think if the hardware doesn't support QSGMII, these references to it should be removed?
I also think that adding a .pcs_validate() method would be a good idea to reject interface modes that the PCS does not support.
Quoted 3 lines
Apart from that looks good to me.
Reviewed-by Daniel Golle <[email protected]>
Thanks.
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!
Daniel Golle
maintainerRe: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 28 lines · Russell King (Oracle)
Quoted 16 lines · Daniel Golle
Quoted 15 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); }Quoted 10 lines
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 98
Quoted 8 lines
drivers/net/pcs/pcs-mtk-lynxi.c +8
-0+static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII :QSGMII is not supported by this PCS.
Quoted 5 lines
Well...
-lynx_pcs_get_state(),lynx_pcs_config(), andlynx_pcs_link_up()include QSGMII in their case statements.
-lynx_pcs_config_giga()refers to QSGMII in a comment, grouping it with SGMII configuration.
These functions are in pcs-lynx.c and not in pcs-mtk-lynxi.c. There is no reference to QSGMII anywhere in pcs-mtk-lynxi.c, what made you assume it would be supported?
Quoted 6 lines
I think if the hardware doesn't support QSGMII, these references to it should be removed?
I also think that adding a .pcs_validate() method would be a good idea to reject interface modes that the PCS does not support.
I can add the pcs_validate function and send the patch doing that to be included in net-next.
Russell King (Oracle)
maintainerRe: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 40 lines · Daniel Golle
Quoted 28 lines · Russell King (Oracle)
Quoted 16 lines · Daniel Golle
Quoted 15 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); }Quoted 10 lines
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 98
Quoted 8 lines
drivers/net/pcs/pcs-mtk-lynxi.c +8
-0+static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX : +case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII :QSGMII is not supported by this PCS.
Quoted 5 lines
Well...
-lynx_pcs_get_state(),lynx_pcs_config(), andlynx_pcs_link_up()include QSGMII in their case statements.
-lynx_pcs_config_giga()refers to QSGMII in a comment, grouping it with SGMII configuration.These functions are in
pcs-lynx.cand not inpcs-mtk-lynxi.c. There is no reference to QSGMII anywhere inpcs-mtk-lynxi.c, what made you assume it would be supported?Quoted 1 line
Quoted 8 lines
Quoted 5 lines
I think if the hardware doesn't support QSGMII, these references to it should be removed?
I also think that adding a .pcs_validate() method would be a good idea to reject interface modes that the PCS does not support.
I can add the
pcs_validatefunction and send the patch doing that to be included in net-next.
With the addition of the supported_interfaces bitmap, there's no point if all the validation function is doing is checking the interfaces.
The supported_interfaces bitmap takes over that in core phylink code (and if a MAC supplies a PCS for an interface that is in the MAC's supported_interfaces bitmap but does not have the interface bit set in the PCS's supported_interfaces, phylink will produce a kernel message at error severity before .pcs_validate is called.
At a point in the near future, I will be making the PCS supported_interfaces mandatory - but in the mean time this patch set makes it optional.
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!
Re: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
In reply to Russell King (Oracle)Quoted 13 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>Reviewed-by Daniel Golle <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 96
Quoted 9 lines
drivers/net/pcs/pcs-mtk-lynxi.c +6
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); } +static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX :
Isn't this the place now where to report to phylink, that this PCS does not support in-band at 2500base-x?
Best regards,
Eric
Quoted 20 lines
drivers/net/pcs/pcs-mtk-lynxi.c +10
-0+case PHY_INTERFACE_MODE_SGMII : +case PHY_INTERFACE_MODE_QSGMII : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +default : +return 0 ; + } +} +static void mtk_pcs_lynxi_get_state (struct phylink_pcs *pcs,struct phylink_link_state *state) {@@ -241,6 +256,7 @@ static void mtk_pcs_lynxi_disable(struct phylink_pcs *pcs) }static const struct phylink_pcs_ops mtk_pcs_lynxi_ops = { + .pcs_inband_caps = mtk_pcs_lynxi_inband_caps, .pcs_get_state = mtk_pcs_lynxi_get_state, .pcs_config = mtk_pcs_lynxi_config, .pcs_an_restart = mtk_pcs_lynxi_restart_an,
Russell King (Oracle)
maintainerRe: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 17 lines · Eric Woudstra
Quoted 16 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>Reviewed-by Daniel Golle <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); }
Quoted 9 lines
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 96
Quoted 6 lines
drivers/net/pcs/pcs-mtk-lynxi.c +6
-0+static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX :Isn't this the place now where to report to phylink, that this PCS does not support in-band at 2500base-x?
No - look at the arguments to this function. What arguments would this function make a decision whether in-band is supported in any interface mode?
The correct place is the .pcs_inband_caps(), which from reading the code, I understood that in-band can be used at 2500base-X with this PCS. See
https://patch.msgid.link/[email protected]
which was merged at the beginning of December, and if you are correct, the patch was wrong.
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!
Daniel Golle
maintainerRe: [PATCH net-next 2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method
Quoted 32 lines · Russell King (Oracle)
Quoted 17 lines · Eric Woudstra
Quoted 16 lines · Russell King (Oracle)
Report the PCS in-band capabilities to phylink for the LynxI PCS.
Signed-off-by Russell King (Oracle) <[email protected]>Reviewed-by Daniel Golle <[email protected]>--- drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/net/pcs/pcs-mtk-lynxi.c +0
-0@@ -88,6 +88,21 @@ static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) return container_of(pcs, struct mtk_pcs_lynxi, pcs); }Quoted 9 lines
drivers/net/pcs/pcs-mtk-lynxi.c static struct mtk_pcs_lynxi *pcs_to_mtk_pcs_lynxi(struct phylink_pcs *pcs) · line 96
Quoted 6 lines
drivers/net/pcs/pcs-mtk-lynxi.c +6
-0+static unsigned int mtk_pcs_lynxi_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +switch (interface) { +case PHY_INTERFACE_MODE_1000BASEX : +case PHY_INTERFACE_MODE_2500BASEX :Isn't this the place now where to report to phylink, that this PCS does not support in-band at 2500base-x?
No - look at the arguments to this function. What arguments would this function make a decision whether in-band is supported in any interface mode?
Quoted 6 lines
The correct place is the .pcs_inband_caps(), which from reading the code, I understood that in-band can be used at 2500base-X with this PCS. See
https://patch.msgid.link/[email protected]
which was merged at the beginning of December, and if you are correct, the patch was wrong.
Yes, that patch was wrong. Neither is QSGMII supported at all by the LynxI hardware, nor can in-band-status be used in 2500Base-X mode. I will send a patch to fix that.
PATCH 3/3 net: pcs: xpcs: implement pcs_inband_caps() method
In reply to Russell King (Oracle)Report the PCS inband capabilities to phylink for XPCS.
--- drivers/net/pcs/pcs-xpcs.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
drivers/net/pcs/pcs-xpcs.c +28 -0
@@ -567,6 +567,33 @@ static int xpcs_validate(struct phylink_pcs *pcs, unsigned long *supported,return 0 ; } +static unsigned int xpcs_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +struct dw_xpcs *xpcs =phylink_pcs_to_xpcs (pcs); +const struct dw_xpcs_compat *compat; + + compat =xpcs_find_compat (xpcs, interface); +if (!compat) +return 0 ; + +switch (compat->an_mode) { +case DW_AN_C73 : +return LINK_INBAND_ENABLE ; + +case DW_AN_C37_SGMII : +case DW_AN_C37_1000BASEX : +return LINK_INBAND_DISABLE |LINK_INBAND_ENABLE ; + +case DW_10GBASER : +case DW_2500BASEX : +return LINK_INBAND_DISABLE ; + +default : +return 0 ; + } +} +void xpcs_get_interfaces (struct dw_xpcs *xpcs,unsigned long *interfaces) {const struct dw_xpcs_compat *compat;
@@ -1306,6 +1333,7 @@ static const struct dw_xpcs_desc xpcs_desc_list[] = {static const struct phylink_pcs_ops xpcs_phylink_ops = { .pcs_validate = xpcs_validate, + .pcs_inband_caps = xpcs_inband_caps, .pcs_pre_config = xpcs_pre_config, .pcs_config = xpcs_config, .pcs_get_state = xpcs_get_state,
Signature · 1 line
2.30.2
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 10 lines · On Thu, 5 Dec 2024 09:42:07 +0000 you wrote:
Hi,
Here are the last three patches which were not included in the non-RFC posting, but were in the RFC posting. These add the .pcs_inband() method to the Lynx, MTK Lynx and XPCS drivers.
drivers/net/pcs/pcs-lynx.c | 22 ++++++++++++++++++++++ drivers/net/pcs/pcs-mtk-lynxi.c | 16 ++++++++++++++++ drivers/net/pcs/pcs-xpcs.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)
Here is the summary with links:
- [net-next,1/3] net: pcs: pcs-lynx: implement pcs_inband_caps() method
https://git.kernel.org/netdev/net-next/c/6561f0e547be - [net-next,2/3] net: pcs: pcs-mtk-lynxi: implement pcs_inband_caps() method https://git.kernel.org/netdev/net-next/c/520d29bdda86 - [net-next,3/3] net: pcs: xpcs: implement pcs_inband_caps() method https://git.kernel.org/netdev/net-next/c/484d0170d6c6
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html