| Message ID | ZLptIKEb7qLY5LmS@shell.armlinux.org.uk (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove legacy phylink behaviour | expand |
On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > Hi, > > This series removes the - as far as I can tell - unreachable code in > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > the support in phylink for this legacy behaviour. > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > TRGMII, non-serdes based interface modes, and disables those interface > modes prior to phylink configuration. > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > I believe is also not used - mtk_eth_soc appears not to be used with > SFPs (which would use a kind of in-band mode) nor does any DT appear > to specify in-band mode for any non-serdes based interface mode. > > With both of those dealt with, the kernel is now free of any driver > relying on the phylink legacy mode. Therefore, patch 3 removes support > for this. > > Finally, with the advent of a new driver being submitted today that > makes use of state->speed in the mac_config() path, patch 4 ensures that > any phylink_link_state member that should not be used in mac_config is > either cleared or set to an invalid value. Note: probably be a good idea for the mtk_eth_soc people to review and ack before applying this to net-next. Thanks.
Hi Russell, On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > Hi, > > This series removes the - as far as I can tell - unreachable code in > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > the support in phylink for this legacy behaviour. > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > TRGMII, non-serdes based interface modes, and disables those interface > modes prior to phylink configuration. > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > I believe is also not used - mtk_eth_soc appears not to be used with > SFPs (which would use a kind of in-band mode) nor does any DT appear > to specify in-band mode for any non-serdes based interface mode. > > With both of those dealt with, the kernel is now free of any driver > relying on the phylink legacy mode. Therefore, patch 3 removes support > for this. > > Finally, with the advent of a new driver being submitted today that > makes use of state->speed in the mac_config() path, patch 4 ensures that > any phylink_link_state member that should not be used in mac_config is > either cleared or set to an invalid value. Thank you for taking care of this! For the whole series: Reviewed-by: Daniel Golle <daniel@makrotopia.org> Tested-by: Daniel Golle <daniel@makrotopia.org> Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). All works fine as expected. To apply the series I needed to resolve a minor conflict due to net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros being applied in the meantime. > > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 94 +++++------------------------ > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 1 + > drivers/net/phy/phylink.c | 48 ++++++--------- > include/linux/phylink.h | 45 ++------------ > 4 files changed, 42 insertions(+), 146 deletions(-) > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr > Von: "Daniel Golle" <daniel@makrotopia.org> > > Hi Russell, > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > > Hi, > > > > This series removes the - as far as I can tell - unreachable code in > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > > the support in phylink for this legacy behaviour. > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > > TRGMII, non-serdes based interface modes, and disables those interface > > modes prior to phylink configuration. > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > > I believe is also not used - mtk_eth_soc appears not to be used with > > SFPs (which would use a kind of in-band mode) nor does any DT appear > > to specify in-band mode for any non-serdes based interface mode. > > > > With both of those dealt with, the kernel is now free of any driver > > relying on the phylink legacy mode. Therefore, patch 3 removes support > > for this. > > > > Finally, with the advent of a new driver being submitted today that > > makes use of state->speed in the mac_config() path, patch 4 ensures that > > any phylink_link_state member that should not be used in mac_config is > > either cleared or set to an invalid value. > > Thank you for taking care of this! > For the whole series: > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > Tested-by: Daniel Golle <daniel@makrotopia.org> > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). > All works fine as expected. have you changed anything? in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace. [ 10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode [ 10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode [ 11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode [ 11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode [ 11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode [ 11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode [ 15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar. When booting without cable it hangs too (without the up-message of course). > To apply the series I needed to resolve a minor conflict due to > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros > being applied in the meantime. i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and then my defconfig,buildscript etc. maybe i miss anything? afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;) regards Frank
On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote: > Hi > > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr > > Von: "Daniel Golle" <daniel@makrotopia.org> > > > > Hi Russell, > > > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > > > Hi, > > > > > > This series removes the - as far as I can tell - unreachable code in > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > > > the support in phylink for this legacy behaviour. > > > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > > > TRGMII, non-serdes based interface modes, and disables those interface > > > modes prior to phylink configuration. > > > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > > > I believe is also not used - mtk_eth_soc appears not to be used with > > > SFPs (which would use a kind of in-band mode) nor does any DT appear > > > to specify in-band mode for any non-serdes based interface mode. > > > > > > With both of those dealt with, the kernel is now free of any driver > > > relying on the phylink legacy mode. Therefore, patch 3 removes support > > > for this. > > > > > > Finally, with the advent of a new driver being submitted today that > > > makes use of state->speed in the mac_config() path, patch 4 ensures that > > > any phylink_link_state member that should not be used in mac_config is > > > either cleared or set to an invalid value. > > > > Thank you for taking care of this! > > > For the whole series: > > > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > > Tested-by: Daniel Golle <daniel@makrotopia.org> > > > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). > > All works fine as expected. > > have you changed anything? > > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace. > > [ 10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode > [ 10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx > [ 11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode > [ 11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode > [ 11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode > [ 11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode > [ 11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode > > [ 15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx > > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar. > When booting without cable it hangs too (without the up-message of course). > > > To apply the series I needed to resolve a minor conflict due to > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros > > being applied in the meantime. > > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and > then my defconfig,buildscript etc. maybe i miss anything? I tested in a fairly similar way. > > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;) For TRGMII mode mtk_gmac0_rgmii_adjust behavior should not have changed, only speed-dependent adjustments for RGMII have been dropped. I also didn't not see such a problem when I was testing. However, I guess in my case the MAC interface mode has already been setup in hardware because I booted the board via TFTP, hence U-Boot already took care of initializing everything, so that can also be a reason why it worked fine for me.
On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote: > Hi > > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr > > Von: "Daniel Golle" <daniel@makrotopia.org> > > > > Hi Russell, > > > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > > > Hi, > > > > > > This series removes the - as far as I can tell - unreachable code in > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > > > the support in phylink for this legacy behaviour. > > > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > > > TRGMII, non-serdes based interface modes, and disables those interface > > > modes prior to phylink configuration. > > > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > > > I believe is also not used - mtk_eth_soc appears not to be used with > > > SFPs (which would use a kind of in-band mode) nor does any DT appear > > > to specify in-band mode for any non-serdes based interface mode. > > > > > > With both of those dealt with, the kernel is now free of any driver > > > relying on the phylink legacy mode. Therefore, patch 3 removes support > > > for this. > > > > > > Finally, with the advent of a new driver being submitted today that > > > makes use of state->speed in the mac_config() path, patch 4 ensures that > > > any phylink_link_state member that should not be used in mac_config is > > > either cleared or set to an invalid value. > > > > Thank you for taking care of this! > > > For the whole series: > > > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > > Tested-by: Daniel Golle <daniel@makrotopia.org> > > > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). > > All works fine as expected. > > have you changed anything? > > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace. > > [ 10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode > [ 10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx > [ 11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode > [ 11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode > [ 11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode > [ 11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode > [ 11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode > > [ 15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx > > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar. > When booting without cable it hangs too (without the up-message of course). > > > To apply the series I needed to resolve a minor conflict due to > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros > > being applied in the meantime. > > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and > then my defconfig,buildscript etc. maybe i miss anything? > > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;) Have you tried bisecting the four patches? Does that give any hints? Does the net-next base you used boot successfully? Thanks.
> Gesendet: Samstag, 22. Juli 2023 um 18:14 Uhr > Von: "Russell King (Oracle)" <linux@armlinux.org.uk> > > On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote: > > Hi > > > > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr > > > Von: "Daniel Golle" <daniel@makrotopia.org> > > > > > > Hi Russell, > > > > > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > > > > Hi, > > > > > > > > This series removes the - as far as I can tell - unreachable code in > > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > > > > the support in phylink for this legacy behaviour. > > > > > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > > > > TRGMII, non-serdes based interface modes, and disables those interface > > > > modes prior to phylink configuration. > > > > > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > > > > I believe is also not used - mtk_eth_soc appears not to be used with > > > > SFPs (which would use a kind of in-band mode) nor does any DT appear > > > > to specify in-band mode for any non-serdes based interface mode. > > > > > > > > With both of those dealt with, the kernel is now free of any driver > > > > relying on the phylink legacy mode. Therefore, patch 3 removes support > > > > for this. > > > > > > > > Finally, with the advent of a new driver being submitted today that > > > > makes use of state->speed in the mac_config() path, patch 4 ensures that > > > > any phylink_link_state member that should not be used in mac_config is > > > > either cleared or set to an invalid value. > > > > > > Thank you for taking care of this! > > > > > For the whole series: > > > > > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > > > Tested-by: Daniel Golle <daniel@makrotopia.org> > > > > > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). > > > All works fine as expected. > > > > have you changed anything? > > > > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace. > > > > [ 10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode > > [ 10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx > > [ 11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode > > [ 11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode > > [ 11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode > > [ 11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode > > [ 11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode > > > > [ 15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx > > > > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar. > > When booting without cable it hangs too (without the up-message of course). > > > > > To apply the series I needed to resolve a minor conflict due to > > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros > > > being applied in the meantime. > > > > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and > > then my defconfig,buildscript etc. maybe i miss anything? > > > > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;) > > Have you tried bisecting the four patches? Does that give any hints? > Does the net-next base you used boot successfully? sorry, hang was caused by me (missing rootfs and wait in bootargs). only see throughput of only ~850Mbit (tx) and 900Mbit (rx)..wan is on eth0 which is the trgmii one, but without the patches it is similar (quicktested 6.3-rc1 i had already on this card which has similar results) so for me it looks good so far...i'm not sure we had more than 900 anytime on this device... regards Frank
On Sat, Jul 22, 2023 at 08:00:46PM +0200, Frank Wunderlich wrote: > > > Gesendet: Samstag, 22. Juli 2023 um 18:14 Uhr > > Von: "Russell King (Oracle)" <linux@armlinux.org.uk> > > > > On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote: > > > Hi > > > > > > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr > > > > Von: "Daniel Golle" <daniel@makrotopia.org> > > > > > > > > Hi Russell, > > > > > > > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote: > > > > > Hi, > > > > > > > > > > This series removes the - as far as I can tell - unreachable code in > > > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes > > > > > the support in phylink for this legacy behaviour. > > > > > > > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non- > > > > > TRGMII, non-serdes based interface modes, and disables those interface > > > > > modes prior to phylink configuration. > > > > > > > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which > > > > > I believe is also not used - mtk_eth_soc appears not to be used with > > > > > SFPs (which would use a kind of in-band mode) nor does any DT appear > > > > > to specify in-band mode for any non-serdes based interface mode. > > > > > > > > > > With both of those dealt with, the kernel is now free of any driver > > > > > relying on the phylink legacy mode. Therefore, patch 3 removes support > > > > > for this. > > > > > > > > > > Finally, with the advent of a new driver being submitted today that > > > > > makes use of state->speed in the mac_config() path, patch 4 ensures that > > > > > any phylink_link_state member that should not be used in mac_config is > > > > > either cleared or set to an invalid value. > > > > > > > > Thank you for taking care of this! > > > > > > > For the whole series: > > > > > > > > Reviewed-by: Daniel Golle <daniel@makrotopia.org> > > > > Tested-by: Daniel Golle <daniel@makrotopia.org> > > > > > > > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A). > > > > All works fine as expected. > > > > > > have you changed anything? > > > > > > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace. > > > > > > [ 10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode > > > [ 10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx > > > [ 11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode > > > [ 11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode > > > [ 11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode > > > [ 11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode > > > [ 11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode > > > > > > [ 15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx > > > > > > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar. > > > When booting without cable it hangs too (without the up-message of course). > > > > > > > To apply the series I needed to resolve a minor conflict due to > > > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros > > > > being applied in the meantime. > > > > > > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and > > > then my defconfig,buildscript etc. maybe i miss anything? > > > > > > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;) > > > > Have you tried bisecting the four patches? Does that give any hints? > > Does the net-next base you used boot successfully? > > sorry, hang was caused by me (missing rootfs and wait in bootargs). > > only see throughput of only ~850Mbit (tx) and 900Mbit (rx)..wan is on eth0 which is the trgmii one, but without the patches it is similar (quicktested 6.3-rc1 i had already on this card which has similar results) > > so for me it looks good so far...i'm not sure we had more than 900 anytime on this device... So I take it that nothing broke as a result of the patches, and as you've tested them, I can add your tested-by to the patches? Thanks.