| Message ID | 1590fb0e69f6243ac6a961b16bf7ae7534f46949.1678201958.git.daniel@makrotopia.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: mtk_eth_soc: various enhancements | expand |
> Gesendet: Dienstag, 07. März 2023 um 16:54 Uhr > Von: "Daniel Golle" <daniel@makrotopia.org> > Link partner advertised link modes are not reported by the SerDes > hardware if not operating in SGMII mode. Hence we cannot use > phylink_mii_c22_pcs_decode_state() in this case. > Implement reporting link and an_complete only and use speed according to > the interface mode. Hi, have tested Parts 1-12 this on bananapi-r3 (mt7986) with 1G Fiber SFP (no 2g5 available yet) on gmac1 and lan4 (mt7531 p5) Tested-by: Frank Wunderlich <frank-w@public-files.de> Thx Daniel for working on SFP support :) regards Frank
On Tue, Mar 07, 2023 at 03:54:11PM +0000, Daniel Golle wrote: > Link partner advertised link modes are not reported by the SerDes > hardware if not operating in SGMII mode. Hence we cannot use > phylink_mii_c22_pcs_decode_state() in this case. > Implement reporting link and an_complete only and use speed according to > the interface mode. > > Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") > Signed-off-by: Daniel Golle <daniel@makrotopia.org> This has been proven to work by Frank Wunderlich last October, so by making this change, you will be regressing his setup. What are you testing against? Have you proven independently that the link partner is indeed sending a valid advertisement for the LPA register to be filled in?
Am 8. März 2023 12:38:31 MEZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>: >On Tue, Mar 07, 2023 at 03:54:11PM +0000, Daniel Golle wrote: >> Link partner advertised link modes are not reported by the SerDes >> hardware if not operating in SGMII mode. Hence we cannot use >> phylink_mii_c22_pcs_decode_state() in this case. >> Implement reporting link and an_complete only and use speed according to >> the interface mode. >> >> Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") >> Signed-off-by: Daniel Golle <daniel@makrotopia.org> > >This has been proven to work by Frank Wunderlich last October, so by >making this change, you will be regressing his setup. Hi My tests were done with 1 kind of 1g fibre sfp as i only have these atm...have ordered some 2g5 rj54 ones,but don't have them yet. I'm not sure if they are working with/without sgmii (1000base-X) and if they have builtin phy. Daniel have a lot more of different SFPs and some (especially 2g5) were not working after our pcs change. >What are you testing against? Have you proven independently that the >link partner is indeed sending a valid advertisement for the LPA >register to be filled in? > regards Frank
On Wed, Mar 08, 2023 at 12:44:57PM +0100, Frank Wunderlich wrote: > Am 8. März 2023 12:38:31 MEZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>: > >On Tue, Mar 07, 2023 at 03:54:11PM +0000, Daniel Golle wrote: > >> Link partner advertised link modes are not reported by the SerDes > >> hardware if not operating in SGMII mode. Hence we cannot use > >> phylink_mii_c22_pcs_decode_state() in this case. > >> Implement reporting link and an_complete only and use speed according to > >> the interface mode. > >> > >> Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") > >> Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > > >This has been proven to work by Frank Wunderlich last October, so by > >making this change, you will be regressing his setup. > > Hi > > My tests were done with 1 kind of 1g fibre sfp as i only have these atm...have ordered some 2g5 rj54 ones,but don't have them yet. I'm not sure if they are working with/without sgmii (1000base-X) and if they have builtin phy. > > Daniel have a lot more of different SFPs and some (especially 2g5) were not working after our pcs change. Exactly. 1 GBit/s SFPs with built-in PHY and using SGMII are working fine before and after conversion to phylink_pcs. 1000Base-X and 2500Base-X PHYs and SFPs were broken after this. The patch about (and the other one you already NACK'ed) fixes those codepaths which were simply not used in Frank's setup. > > >What are you testing against? Have you proven independently that the > >link partner is indeed sending a valid advertisement for the LPA > >register to be filled in? > > I have a ballpark of different SFPs and MediaTek boards with different PHYs here and tried all of them. I have no way to tell if the SFPs and PHYs which stopped working after the phylink_pcs conversion are sending valid advertisement. The only other boards with SFP slots I got here are RealTek-based switches, and all I can say is that on an RTL8380 based 1G switch both, the SFP modules containing a PHY and operating in SGMII mode as well as the ones without a PHY exposed via i2c-mdio and operating in 1000Base-X mode are working fine with that switch, with both, stock firmware and OpenWrt running on it. However, even should they not send valid advertisement, they are very common parts and they were working before and not after the change to phylink_pcs, for the reasons mentioned in the description of this patch.
On Wed, Mar 08, 2023 at 12:44:57PM +0100, Frank Wunderlich wrote: > Am 8. März 2023 12:38:31 MEZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>: > >On Tue, Mar 07, 2023 at 03:54:11PM +0000, Daniel Golle wrote: > >> Link partner advertised link modes are not reported by the SerDes > >> hardware if not operating in SGMII mode. Hence we cannot use > >> phylink_mii_c22_pcs_decode_state() in this case. > >> Implement reporting link and an_complete only and use speed according to > >> the interface mode. > >> > >> Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") > >> Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > > >This has been proven to work by Frank Wunderlich last October, so by > >making this change, you will be regressing his setup. > > Hi > > My tests were done with 1 kind of 1g fibre sfp as i only have these atm...have ordered some 2g5 rj54 ones,but don't have them yet. I'm not sure if they are working with/without sgmii (1000base-X) and if they have builtin phy. Fiber SFPs do not have a built in PHY. They merely convert the serdes electrical waveform into light and back again, possibly with some retiming of the received waveform, and a bit of monitoring. They're pretty simple devices. They certainly do not change the protocol in any way.
On Wed, Mar 08, 2023 at 12:05:19PM +0000, Daniel Golle wrote: > On Wed, Mar 08, 2023 at 12:44:57PM +0100, Frank Wunderlich wrote: > > Am 8. März 2023 12:38:31 MEZ schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>: > > >On Tue, Mar 07, 2023 at 03:54:11PM +0000, Daniel Golle wrote: > > >> Link partner advertised link modes are not reported by the SerDes > > >> hardware if not operating in SGMII mode. Hence we cannot use > > >> phylink_mii_c22_pcs_decode_state() in this case. > > >> Implement reporting link and an_complete only and use speed according to > > >> the interface mode. > > >> > > >> Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") > > >> Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > > > > >This has been proven to work by Frank Wunderlich last October, so by > > >making this change, you will be regressing his setup. > > > > Hi > > > > My tests were done with 1 kind of 1g fibre sfp as i only have these atm...have ordered some 2g5 rj54 ones,but don't have them yet. I'm not sure if they are working with/without sgmii (1000base-X) and if they have builtin phy. > > > > Daniel have a lot more of different SFPs and some (especially 2g5) were not working after our pcs change. > > Exactly. 1 GBit/s SFPs with built-in PHY and using SGMII are working > fine before and after conversion to phylink_pcs. 1000Base-X and > 2500Base-X PHYs and SFPs were broken after this. > > The patch about (and the other one you already NACK'ed) fixes those > codepaths which were simply not used in Frank's setup. You're replying to "Frank" but I think the "you" you are using there is addressed to me. I beg to differ with your assessment over whether the code paths were used or not. Here's the proof from Frank's testing: https://lore.kernel.org/all/trinity-4470b00b-771b-466e-9f3a-a3df72758208-1666435920485@3c-app-gmx-bs49/ The values at offset 8 (0x40e041a0) _are_ 1000base-X AN values, not SGMII. There are many messages in that thread showing valid 1000base-X AN values in register 8 (that being the local advertisement in the lower 16 bits, and the partner's advertisement in the upper 16 bits. The link partner in this case sent 0x40e0, which is: #define LPA_1000XFULL 0x0020 /* Can do 1000BASE-X full-duplex */ #define LPA_1000XHALF 0x0040 /* Can do 1000BASE-X half-duplex */ #define LPA_1000XPAUSE 0x0080 /* Can do 1000BASE-X pause */ #define LPA_LPACK 0x4000 /* Link partner acked us */ and our advertisement was 0x41a0: #define ADVERTISE_1000XFULL 0x0020 /* Try for 1000BASE-X full-duplex */ #define ADVERTISE_1000XPAUSE 0x0080 /* Try for 1000BASE-X pause */ #define ADVERTISE_1000XPSE_ASYM 0x0100 /* Try for 1000BASE-X asym pause */ #define ADVERTISE_LPACK 0x4000 /* Ack link partners response */ (bit 14 is managed by the PCS.) > I have a ballpark of different SFPs and MediaTek boards with different > PHYs here and tried all of them. > > I have no way to tell if the SFPs and PHYs which stopped working after > the phylink_pcs conversion are sending valid advertisement. The only > other boards with SFP slots I got here are RealTek-based switches, and > all I can say is that on an RTL8380 based 1G switch both, the SFP > modules containing a PHY and operating in SGMII mode as well as the > ones without a PHY exposed via i2c-mdio and operating in 1000Base-X > mode are working fine with that switch, with both, stock firmware and > OpenWrt running on it. > > However, even should they not send valid advertisement, they are very > common parts and they were working before and not after the change to > phylink_pcs, for the reasons mentioned in the description of this > patch. Let me re-iterate in a different way to make this crystal clear: the fibre SFP is completely transparent with respect to the PCS-to-PCS link. The clause 37 autonegotiation is between the PCS at one end and the PCS at the other end. The fibre SFP is not involved in it. All that fibre SFPs do is convert the serdes waveform into a varying optical intensity and back again to a serdes waveform. They do not attempt to interpret the waveform, but they may shape it via retimer circuitry and adjust the gain to provide a compliant electrical signal. So, if the PCS at the far end of the link is sending a zero advertisement or not sending an advertisement at all, then the LPA register will contain a zero irrespective of the fibre SFP module being used. Another case where it may give a zero value but with a copper SFP is if the copper SFP is using 1000base-X or SGMII but without sending an advertisement. As I've already stated, BCM84881 is known to do this, which is used on some copper SFP modules. It may be that some copper SFP modules have the PHY setup to use 1000base-X but without any clause 37 advertisement - that would not surprise me in the least. I would suggest that if you replaced your mediatek board in your setup with something else, e.g. an Armada 388 based Clearfog board which is known to work correctly, used the same fibre SFPs, you'll find that negotiation would not complete and the link wouldn't come up. Then if you use the same fibre SFPs that I do at both ends, you'll find the same problem (because the fibre SFPs make no difference as far as the in-band AN is concerned.) I hope this helps.
diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c index 58d8cb3aa7f4..98d80007d3bd 100644 --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c @@ -23,14 +23,30 @@ static void mtk_pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state) { struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs); - unsigned int bm, adv; + unsigned int bm, bmsr, adv; - /* Read the BMSR and LPA */ + /* Read the BMSR */ regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm); - regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv); + bmsr = FIELD_GET(SGMII_BMSR, bm); - phylink_mii_c22_pcs_decode_state(state, FIELD_GET(SGMII_BMSR, bm), - FIELD_GET(SGMII_LPA, adv)); + /* link partner advertised link modes are not reported by the + * hardware when not operating in SGMII mode. Hence we cannot + * use phylink_mii_c22_pcs_decode_state() in this case. + */ + if (state->interface != PHY_INTERFACE_MODE_SGMII) { + state->link = !!(bmsr & BMSR_LSTATUS); + state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE); + state->speed = (state->interface == + PHY_INTERFACE_MODE_2500BASEX) ? + SPEED_2500 : SPEED_1000; + state->duplex = DUPLEX_FULL; + + return; + } + + /* Read LPA and use standard decode function for SGMII mode */ + regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv); + phylink_mii_c22_pcs_decode_state(state, bmsr, FIELD_GET(SGMII_LPA, adv)); } static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
Link partner advertised link modes are not reported by the SerDes hardware if not operating in SGMII mode. Hence we cannot use phylink_mii_c22_pcs_decode_state() in this case. Implement reporting link and an_complete only and use speed according to the interface mode. Fixes: 14a44ab0330d ("net: mtk_eth_soc: partially convert to phylink_pcs") Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- drivers/net/ethernet/mediatek/mtk_sgmii.c | 26 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)