| Message ID | 20230211074113.2782508-7-o.rempel@pengutronix.de |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | net: add EEE support for KSZ9477 switch family | expand |
On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > It should work as before except write operation to the EEE adv registers > will be done only if some EEE abilities was detected. > > If some driver will have a regression, related driver should provide own > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> This patch causes network interface failures with all my xtensa qemu emulations. Reverting it fixes the problem. Bisect log is attached for reference. Guenter --- # bad: [e4bc15889506723d7b93c053ad4a75cd58248d74] Merge tag 'leds-next-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds # good: [c9c3395d5e3dcc6daee66c6908354d47bf98cb0c] Linux 6.2 git bisect start 'HEAD' 'c9c3395d5e3d' # bad: [5b7c4cabbb65f5c469464da6c5f614cbd7f730f2] Merge tag 'net-next-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next git bisect bad 5b7c4cabbb65f5c469464da6c5f614cbd7f730f2 # good: [877934769e5b91798d304d4641647900ee614ce8] Merge tag 'x86_cpu_for_v6.3_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect good 877934769e5b91798d304d4641647900ee614ce8 # good: [c5ebba75c7625e5cb62cb5423883cc3764779420] net: ipa: use bitmasks for GSI IRQ values git bisect good c5ebba75c7625e5cb62cb5423883cc3764779420 # bad: [871489dd01b67483248edc8873c389a66e469f30] Merge tag 'ieee802154-for-net-next-2023-02-20' of git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan-next git bisect bad 871489dd01b67483248edc8873c389a66e469f30 # good: [986e43b19ae9176093da35e0a844e65c8bf9ede7] wifi: mac80211: fix receiving A-MSDU frames on mesh interfaces git bisect good 986e43b19ae9176093da35e0a844e65c8bf9ede7 # bad: [ca0df43d211039dded5a8f8553356414c9a74731] Merge tag 'wireless-next-2023-03-16' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next git bisect bad ca0df43d211039dded5a8f8553356414c9a74731 # bad: [388a9c907a51489bf566165c72e4e8aa4d62ab49] Merge branch 'devlink-cleanups-and-move-devlink-health-functionality-to-separate-file' git bisect bad 388a9c907a51489bf566165c72e4e8aa4d62ab49 # bad: [1a940b00013a468c0c9dd79dbb485c3ad273939e] net: stmmac: dwc-qos: Make struct dwc_eth_dwmac_data::remove return void git bisect bad 1a940b00013a468c0c9dd79dbb485c3ad273939e # good: [8024edf3590c83f467374857d7c3082d4b3bf079] Merge branch 'net-ipa-GSI-regs' git bisect good 8024edf3590c83f467374857d7c3082d4b3bf079 # bad: [9b01c885be364526d8c05794f8358b3e563b7ff8] net: phy: c22: migrate to genphy_c45_write_eee_adv() git bisect bad 9b01c885be364526d8c05794f8358b3e563b7ff8 # good: [79cdf17e5131ccdee0792f6f25d3db0e34861998] Merge branch 'ionic-on-chip-desc' git bisect good 79cdf17e5131ccdee0792f6f25d3db0e34861998 # good: [48fb19940f2ba6b50dfea70f671be9340fb63d60] net: phy: micrel: add ksz9477_get_features() git bisect good 48fb19940f2ba6b50dfea70f671be9340fb63d60 # good: [022c3f87f88e2d68e90be7687d981c9cb893a3b1] net: phy: add genphy_c45_ethtool_get/set_eee() support git bisect good 022c3f87f88e2d68e90be7687d981c9cb893a3b1 # first bad commit: [9b01c885be364526d8c05794f8358b3e563b7ff8] net: phy: c22: migrate to genphy_c45_write_eee_adv()
On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > It should work as before except write operation to the EEE adv registers > > will be done only if some EEE abilities was detected. > > > > If some driver will have a regression, related driver should provide own > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > This patch causes network interface failures with all my xtensa qemu > emulations. Reverting it fixes the problem. Bisect log is attached > for reference. > Also affected are arm:cubieboard emulations, with same symptom. arm:bletchley-bmc emulations crash. In both cases, reverting this patch fixes the problem. Guenter
Hallo Guenter, On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > It should work as before except write operation to the EEE adv registers > > > will be done only if some EEE abilities was detected. > > > > > > If some driver will have a regression, related driver should provide own > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > This patch causes network interface failures with all my xtensa qemu > > emulations. Reverting it fixes the problem. Bisect log is attached > > for reference. > > > > Also affected are arm:cubieboard emulations, with same symptom. > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > fixes the problem. Please test this fixes: https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ Regards, Oleksij
On 2/23/23 20:53, Oleksij Rempel wrote: > Hallo Guenter, > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: >> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: >>> On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: >>>> Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). >>>> >>>> It should work as before except write operation to the EEE adv registers >>>> will be done only if some EEE abilities was detected. >>>> >>>> If some driver will have a regression, related driver should provide own >>>> .get_features callback. See micrel.c:ksz9477_get_features() as example. >>>> >>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch> >>> >>> This patch causes network interface failures with all my xtensa qemu >>> emulations. Reverting it fixes the problem. Bisect log is attached >>> for reference. >>> >> >> Also affected are arm:cubieboard emulations, with same symptom. >> arm:bletchley-bmc emulations crash. In both cases, reverting this patch >> fixes the problem. > > Please test this fixes: > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ > Applied and tested 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes 068a35a8d62c net: phy: do not force EEE support 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation on top of d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm No change for xtensa and arm:cubieboard; network interfaces still fail. On the plus side, the failures with arm:bletchley-bmc (warnings, crash) are longer seen. Guenter
On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote: > On 2/23/23 20:53, Oleksij Rempel wrote: > > Hallo Guenter, > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > > > > > It should work as before except write operation to the EEE adv registers > > > > > will be done only if some EEE abilities was detected. > > > > > > > > > > If some driver will have a regression, related driver should provide own > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > > > > > This patch causes network interface failures with all my xtensa qemu > > > > emulations. Reverting it fixes the problem. Bisect log is attached > > > > for reference. > > > > > > > > > > Also affected are arm:cubieboard emulations, with same symptom. > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > > > fixes the problem. > > > > Please test this fixes: > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ > > > > Applied and tested > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes > 068a35a8d62c net: phy: do not force EEE support > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation > > on top of > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > No change for xtensa and arm:cubieboard; network interfaces still fail. Huh, interesting. can you please send me the kernel logs. > On the plus side, the failures with arm:bletchley-bmc (warnings, crash) > are longer seen. s/longer/no longer/ ? Regards, Oleksij
Copying regzbot. #regzbot ^introduced 9b01c885be36 #regzbot title Network interface initialization failures on xtensa, arm:cubieboard #regzbot ignore-activity On Thu, Feb 23, 2023 at 08:16:06PM -0800, Guenter Roeck wrote: > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > It should work as before except write operation to the EEE adv registers > > > will be done only if some EEE abilities was detected. > > > > > > If some driver will have a regression, related driver should provide own > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > This patch causes network interface failures with all my xtensa qemu > > emulations. Reverting it fixes the problem. Bisect log is attached > > for reference. > > > > Also affected are arm:cubieboard emulations, with same symptom. > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > fixes the problem. > > Guenter
On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote: > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote: > > On 2/23/23 20:53, Oleksij Rempel wrote: > > > Hallo Guenter, > > > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > > > > > > > It should work as before except write operation to the EEE adv registers > > > > > > will be done only if some EEE abilities was detected. > > > > > > > > > > > > If some driver will have a regression, related driver should provide own > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > > > > > > > This patch causes network interface failures with all my xtensa qemu > > > > > emulations. Reverting it fixes the problem. Bisect log is attached > > > > > for reference. > > > > > > > > > > > > > Also affected are arm:cubieboard emulations, with same symptom. > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > > > > fixes the problem. > > > > > > Please test this fixes: > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ > > > > > > > Applied and tested > > > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes > > 068a35a8d62c net: phy: do not force EEE support > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation > > > > on top of > > > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > > > No change for xtensa and arm:cubieboard; network interfaces still fail. > > Huh, interesting. > > can you please send me the kernel logs. > There is nothing useful there, or at least I don't see anything useful. The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa) just don't come up. Sample logs: cubieboard: https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio xtensa: https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio and, for completeness, bletchley-bmc: https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio Those logs are without the above set of patches, but I don't see a difference with the patches applied for cubieboard and xtensa. I started a complete test run (for all emulations) with the patches applied; that should take about an hour to complete. I could also add some debug logging, but you'd have to give me some hints about what to add and where. > > On the plus side, the failures with arm:bletchley-bmc (warnings, crash) > > are longer seen. > > s/longer/no longer/ ? Yes, sorry. Thanks, Guenter
On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote: > On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote: > > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote: > > > On 2/23/23 20:53, Oleksij Rempel wrote: > > > > Hallo Guenter, > > > > > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: > > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > > > > > > > > > It should work as before except write operation to the EEE adv registers > > > > > > > will be done only if some EEE abilities was detected. > > > > > > > > > > > > > > If some driver will have a regression, related driver should provide own > > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > > > > > > > > > This patch causes network interface failures with all my xtensa qemu > > > > > > emulations. Reverting it fixes the problem. Bisect log is attached > > > > > > for reference. > > > > > > > > > > > > > > > > Also affected are arm:cubieboard emulations, with same symptom. > > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > > > > > fixes the problem. > > > > > > > > Please test this fixes: > > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ > > > > > > > > > > Applied and tested > > > > > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes > > > 068a35a8d62c net: phy: do not force EEE support > > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function > > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation > > > > > > on top of > > > > > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > > > > > No change for xtensa and arm:cubieboard; network interfaces still fail. > > > > Huh, interesting. > > > > can you please send me the kernel logs. > > > There is nothing useful there, or at least I don't see anything useful. > The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa) > just don't come up. > > Sample logs: > > cubieboard: > > https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio > > xtensa: > > https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio > > and, for completeness, bletchley-bmc: > > https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio > > Those logs are without the above set of patches, but I don't see a > difference with the patches applied for cubieboard and xtensa. I > started a complete test run (for all emulations) with the patches > applied; that should take about an hour to complete. > I could also add some debug logging, but you'd have to give me > some hints about what to add and where. OK, interesting. These are emulated PHYs. QEMU seems to return 0 or 0xFFFF on unsupported registers. May be I'm wrong. All EEE read/write accesses depend on initial capability read genphy_c45_read_eee_cap1() Can you please add this trace: diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index f595acd0a895..67dac9f0e71d 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev) * (Register 3.20) */ val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); + printk("MDIO_PCS_EEE_ABLE = 0x%04x", val); if (val < 0) return val;
On 2/24/23 10:36, Oleksij Rempel wrote: > On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote: >> On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote: >>> On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote: >>>> On 2/23/23 20:53, Oleksij Rempel wrote: >>>>> Hallo Guenter, >>>>> >>>>> On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: >>>>>> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: >>>>>>> On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: >>>>>>>> Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). >>>>>>>> >>>>>>>> It should work as before except write operation to the EEE adv registers >>>>>>>> will be done only if some EEE abilities was detected. >>>>>>>> >>>>>>>> If some driver will have a regression, related driver should provide own >>>>>>>> .get_features callback. See micrel.c:ksz9477_get_features() as example. >>>>>>>> >>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch> >>>>>>> >>>>>>> This patch causes network interface failures with all my xtensa qemu >>>>>>> emulations. Reverting it fixes the problem. Bisect log is attached >>>>>>> for reference. >>>>>>> >>>>>> >>>>>> Also affected are arm:cubieboard emulations, with same symptom. >>>>>> arm:bletchley-bmc emulations crash. In both cases, reverting this patch >>>>>> fixes the problem. >>>>> >>>>> Please test this fixes: >>>>> https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ >>>>> >>>> >>>> Applied and tested >>>> >>>> 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes >>>> 068a35a8d62c net: phy: do not force EEE support >>>> 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function >>>> ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation >>>> >>>> on top of >>>> >>>> d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm >>>> >>>> No change for xtensa and arm:cubieboard; network interfaces still fail. >>> >>> Huh, interesting. >>> >>> can you please send me the kernel logs. >>> >> There is nothing useful there, or at least I don't see anything useful. >> The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa) >> just don't come up. >> >> Sample logs: >> >> cubieboard: >> >> https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio >> >> xtensa: >> >> https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio >> >> and, for completeness, bletchley-bmc: >> >> https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio >> >> Those logs are without the above set of patches, but I don't see a >> difference with the patches applied for cubieboard and xtensa. I >> started a complete test run (for all emulations) with the patches >> applied; that should take about an hour to complete. >> I could also add some debug logging, but you'd have to give me >> some hints about what to add and where. > > OK, interesting. These are emulated PHYs. QEMU seems to return 0 or > 0xFFFF on unsupported registers. May be I'm wrong. > All EEE read/write accesses depend on initial capability read > genphy_c45_read_eee_cap1() > > Can you please add this trace: > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index f595acd0a895..67dac9f0e71d 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev) > * (Register 3.20) > */ > val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); > + printk("MDIO_PCS_EEE_ABLE = 0x%04x", val); > if (val < 0) > return val; > For cubieboard: MDIO_PCS_EEE_ABLE = 0x0000 qemu reports attempts to access unsupported registers. I had a look at the Allwinner mdio driver. There is no indication suggesting what the real hardware would return when trying to access unsupported registers, and the Ethernet controller datasheet is not public. For xtensa: MDIO_PCS_EEE_ABLE = 0x0014 I didn't try to find out what that means. qemu did not report attempts to access unsupported registers. Guenter
On Fri, Feb 24, 2023 at 11:17:24AM -0800, Guenter Roeck wrote: > On 2/24/23 10:36, Oleksij Rempel wrote: > > On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote: > > > On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote: > > > > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote: > > > > > On 2/23/23 20:53, Oleksij Rempel wrote: > > > > > > Hallo Guenter, > > > > > > > > > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote: > > > > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > > > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > > > > > > > > > > > > > It should work as before except write operation to the EEE adv registers > > > > > > > > > will be done only if some EEE abilities was detected. > > > > > > > > > > > > > > > > > > If some driver will have a regression, related driver should provide own > > > > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > > > > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > > > > > > > > > > > > > This patch causes network interface failures with all my xtensa qemu > > > > > > > > emulations. Reverting it fixes the problem. Bisect log is attached > > > > > > > > for reference. > > > > > > > > > > > > > > > > > > > > > > Also affected are arm:cubieboard emulations, with same symptom. > > > > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > > > > > > > fixes the problem. > > > > > > > > > > > > Please test this fixes: > > > > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/ > > > > > > > > > > > > > > > > Applied and tested > > > > > > > > > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes > > > > > 068a35a8d62c net: phy: do not force EEE support > > > > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function > > > > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation > > > > > > > > > > on top of > > > > > > > > > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > > > > > > > > > No change for xtensa and arm:cubieboard; network interfaces still fail. > > > > > > > > Huh, interesting. > > > > > > > > can you please send me the kernel logs. > > > > > > > There is nothing useful there, or at least I don't see anything useful. > > > The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa) > > > just don't come up. > > > > > > Sample logs: > > > > > > cubieboard: > > > > > > https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio > > > > > > xtensa: > > > > > > https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio > > > > > > and, for completeness, bletchley-bmc: > > > > > > https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio > > > > > > Those logs are without the above set of patches, but I don't see a > > > difference with the patches applied for cubieboard and xtensa. I > > > started a complete test run (for all emulations) with the patches > > > applied; that should take about an hour to complete. > > > I could also add some debug logging, but you'd have to give me > > > some hints about what to add and where. > > > > OK, interesting. These are emulated PHYs. QEMU seems to return 0 or > > 0xFFFF on unsupported registers. May be I'm wrong. > > All EEE read/write accesses depend on initial capability read > > genphy_c45_read_eee_cap1() > > > > Can you please add this trace: > > > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > > index f595acd0a895..67dac9f0e71d 100644 > > --- a/drivers/net/phy/phy-c45.c > > +++ b/drivers/net/phy/phy-c45.c > > @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev) > > * (Register 3.20) > > */ > > val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); > > + printk("MDIO_PCS_EEE_ABLE = 0x%04x", val); > > if (val < 0) > > return val; > > > > For cubieboard: > > MDIO_PCS_EEE_ABLE = 0x0000 > > qemu reports attempts to access unsupported registers. > > I had a look at the Allwinner mdio driver. There is no indication suggesting > what the real hardware would return when trying to access unsupported registers, > and the Ethernet controller datasheet is not public. These are PHY accesses over MDIO bus. Ethernet controller should not care about content of this operations. But on qemu side, it is implemented as part of Ethernet controller emulation... Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent other EEE related operations. But may be actual phy_read_mmd() went wrong. It is a combination of simple phy_read/write to different registers. > For xtensa: > > MDIO_PCS_EEE_ABLE = 0x0014 > > I didn't try to find out what that means. These will be interpreted as the PHY supports 1000KX and 1000T EEE modes. Starting from this point all EEE read write operations will be allowed. > qemu did not report attempts to access unsupported registers. Hm. What is the best way to proceed? Remove genphy_c45_read_eee_abilities() out of genphy_read_abilities() and let add it to PHYs known to support it? Or go deeper and fix QEMU if needed?
On 2/24/23 12:02, Oleksij Rempel wrote: [ ... ] >> >> For cubieboard: >> >> MDIO_PCS_EEE_ABLE = 0x0000 >> >> qemu reports attempts to access unsupported registers. >> >> I had a look at the Allwinner mdio driver. There is no indication suggesting >> what the real hardware would return when trying to access unsupported registers, >> and the Ethernet controller datasheet is not public. > > These are PHY accesses over MDIO bus. Ethernet controller should not > care about content of this operations. But on qemu side, it is implemented as > part of Ethernet controller emulation... > > Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent > other EEE related operations. But may be actual phy_read_mmd() went > wrong. It is a combination of simple phy_read/write to different > registers. > Adding MDD read/write support in qemu doesn't help. Something else in your patch prevents the PHY from coming up. After reverting your patch, I see sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready in the log. This is missing with your patch in place. Anyway, the key difference is not really the qemu emulation, but the added unconditional call to genphy_c45_write_eee_adv() in your patch. If you look closely into that function, you may notice that the 'changed' variable is never set to 0. diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index 3813b86689d0..fee514b96ab1 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); */ int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv) { - int val, changed; + int val, changed = 0; if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) { val = linkmode_to_mii_eee_cap1_t(adv); fixes the problem, both for cubieboard and xtensa. Guenter
On Fri, Feb 24, 2023 at 04:09:40PM -0800, Guenter Roeck wrote: > On 2/24/23 12:02, Oleksij Rempel wrote: > [ ... ] > > > > > > For cubieboard: > > > > > > MDIO_PCS_EEE_ABLE = 0x0000 > > > > > > qemu reports attempts to access unsupported registers. > > > > > > I had a look at the Allwinner mdio driver. There is no indication suggesting > > > what the real hardware would return when trying to access unsupported registers, > > > and the Ethernet controller datasheet is not public. > > > > These are PHY accesses over MDIO bus. Ethernet controller should not > > care about content of this operations. But on qemu side, it is implemented as > > part of Ethernet controller emulation... > > > > Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent > > other EEE related operations. But may be actual phy_read_mmd() went > > wrong. It is a combination of simple phy_read/write to different > > registers. > > > > Adding MDD read/write support in qemu doesn't help. Something else in your patch > prevents the PHY from coming up. After reverting your patch, I see > > sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off > IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready > > in the log. This is missing with your patch in place. > > Anyway, the key difference is not really the qemu emulation, but the added > unconditional call to genphy_c45_write_eee_adv() in your patch. If you look > closely into that function, you may notice that the 'changed' variable is > never set to 0. > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index 3813b86689d0..fee514b96ab1 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); > */ > int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv) > { > - int val, changed; > + int val, changed = 0; > > if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) { > val = linkmode_to_mii_eee_cap1_t(adv); > > fixes the problem, both for cubieboard and xtensa. Good point! Thx for finding it! Do you wont to send the fix against net?
On 2/24/23 22:08, Oleksij Rempel wrote: > On Fri, Feb 24, 2023 at 04:09:40PM -0800, Guenter Roeck wrote: >> On 2/24/23 12:02, Oleksij Rempel wrote: >> [ ... ] >>>> >>>> For cubieboard: >>>> >>>> MDIO_PCS_EEE_ABLE = 0x0000 >>>> >>>> qemu reports attempts to access unsupported registers. >>>> >>>> I had a look at the Allwinner mdio driver. There is no indication suggesting >>>> what the real hardware would return when trying to access unsupported registers, >>>> and the Ethernet controller datasheet is not public. >>> >>> These are PHY accesses over MDIO bus. Ethernet controller should not >>> care about content of this operations. But on qemu side, it is implemented as >>> part of Ethernet controller emulation... >>> >>> Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent >>> other EEE related operations. But may be actual phy_read_mmd() went >>> wrong. It is a combination of simple phy_read/write to different >>> registers. >>> >> >> Adding MDD read/write support in qemu doesn't help. Something else in your patch >> prevents the PHY from coming up. After reverting your patch, I see >> >> sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off >> IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready >> >> in the log. This is missing with your patch in place. >> >> Anyway, the key difference is not really the qemu emulation, but the added >> unconditional call to genphy_c45_write_eee_adv() in your patch. If you look >> closely into that function, you may notice that the 'changed' variable is >> never set to 0. >> >> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >> index 3813b86689d0..fee514b96ab1 100644 >> --- a/drivers/net/phy/phy-c45.c >> +++ b/drivers/net/phy/phy-c45.c >> @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); >> */ >> int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv) >> { >> - int val, changed; >> + int val, changed = 0; >> >> if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) { >> val = linkmode_to_mii_eee_cap1_t(adv); >> >> fixes the problem, both for cubieboard and xtensa. > > Good point! Thx for finding it! > > Do you wont to send the fix against net? No, please go ahead and do it. Guenter
> > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > > index f595acd0a895..67dac9f0e71d 100644 > > --- a/drivers/net/phy/phy-c45.c > > +++ b/drivers/net/phy/phy-c45.c > > @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev) > > * (Register 3.20) > > */ > > val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); > > + printk("MDIO_PCS_EEE_ABLE = 0x%04x", val); > > if (val < 0) > > return val; > > > > For cubieboard: > > MDIO_PCS_EEE_ABLE = 0x0000 > > qemu reports attempts to access unsupported registers. MDIO is a serial bus with two lines, clock driven by the bus master and data. There is a pull up on the data line, so if the device does not respond to a read request, you get 0xffff. That value is all i've ever seen a real PHY do when asked to read a register which does not exist. So i would say QEMU could be better emulate this. The code actually looks for the value 0xffff and then decides that EEE is not supporting in the PHY. The value of 0x0 is probably being interpreted as meaning EEE is supported, but none of the link modes, 10Mbps, 100Mbps etc support EEE. I would say it is then legitimate to read/write other EEE registers, so long as those writes take into account that no link modes are actually supported. Reading the other messages in this thread, a bug has been found in the patches. But i would also say QEMU could do better. Andrew
On 2/27/23 05:08, Andrew Lunn wrote: >>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >>> index f595acd0a895..67dac9f0e71d 100644 >>> --- a/drivers/net/phy/phy-c45.c >>> +++ b/drivers/net/phy/phy-c45.c >>> @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev) >>> * (Register 3.20) >>> */ >>> val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); >>> + printk("MDIO_PCS_EEE_ABLE = 0x%04x", val); >>> if (val < 0) >>> return val; >>> >> >> For cubieboard: >> >> MDIO_PCS_EEE_ABLE = 0x0000 >> >> qemu reports attempts to access unsupported registers. > > MDIO is a serial bus with two lines, clock driven by the bus master > and data. There is a pull up on the data line, so if the device does > not respond to a read request, you get 0xffff. That value is all i've > ever seen a real PHY do when asked to read a register which does not > exist. So i would say QEMU could be better emulate this. > > The code actually looks for the value 0xffff and then decides that EEE > is not supporting in the PHY. > > The value of 0x0 is probably being interpreted as meaning EEE is > supported, but none of the link modes, 10Mbps, 100Mbps etc support > EEE. I would say it is then legitimate to read/write other EEE > registers, so long as those writes take into account that no link > modes are actually supported. > > Reading the other messages in this thread, a bug has been found in the > patches. But i would also say QEMU could do better. > Sure, it could. Always. That is why I checked the qemu code and actually tried to implement some of the EEE handling, only to realize that it didn't help. The emulated PHY does support EEE and would return either 0x0001 or 0x0003 depending on the underlying hardware. However, returning that and returning/ accepting reasonable values for other EEE registers didn't make a difference due to the kernel bug. Thanks, Guenter
Letting regzbot know about the fix: #regzbot fixed-by: 972074ea8840 On Fri, Feb 24, 2023 at 09:20:04AM -0800, Guenter Roeck wrote: > Copying regzbot. > > #regzbot ^introduced 9b01c885be36 > #regzbot title Network interface initialization failures on xtensa, arm:cubieboard > #regzbot ignore-activity > > On Thu, Feb 23, 2023 at 08:16:06PM -0800, Guenter Roeck wrote: > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote: > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote: > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv(). > > > > > > > > It should work as before except write operation to the EEE adv registers > > > > will be done only if some EEE abilities was detected. > > > > > > > > If some driver will have a regression, related driver should provide own > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example. > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > > > > > This patch causes network interface failures with all my xtensa qemu > > > emulations. Reverting it fixes the problem. Bisect log is attached > > > for reference. > > > > > > > Also affected are arm:cubieboard emulations, with same symptom. > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch > > fixes the problem. > > > > Guenter
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 66a4e62009bb..8d927c5e3bf8 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2231,7 +2231,10 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed) { int err; - if (genphy_config_eee_advert(phydev)) + err = genphy_c45_write_eee_adv(phydev, phydev->supported_eee); + if (err < 0) + return err; + else if (err) changed = true; err = genphy_setup_master_slave(phydev); @@ -2653,6 +2656,11 @@ int genphy_read_abilities(struct phy_device *phydev) phydev->supported, val & ESTATUS_1000_XFULL); } + /* This is optional functionality. If not supported, we may get an error + * which should be ignored. + */ + genphy_c45_read_eee_abilities(phydev); + return 0; } EXPORT_SYMBOL(genphy_read_abilities);