| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <OSZPR01MB7019B9CA4E027DA525477D6EAAF6A@OSZPR01MB7019.jpnprd01.prod.outlook.com> Date: Fri, 17 Oct 2025 17:06:01 +0000 From: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com> To: Russell King <linux@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com> CC: Abhishek Chauhan <quic_abchauha@...cinc.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>, Alexis Lothore <alexis.lothore@...tlin.com>, Andrew Lunn <andrew+netdev@...n.ch>, Boon Khai Ng <boon.khai.ng@...era.com>, Choong Yong Liang <yong.liang.choong@...ux.intel.com>, Daniel Machon <daniel.machon@...rochip.com>, "David S. Miller" <davem@...emloft.net>, Drew Fustini <dfustini@...storrent.com>, Emil Renner Berthing <emil.renner.berthing@...onical.com>, Eric Dumazet <edumazet@...gle.com>, Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>, Furong Xu <0x1207@...il.com>, Inochi Amaoto <inochiama@...il.com>, Jacob Keller <jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>, "Jan Petrous (OSS)" <jan.petrous@....nxp.com>, Jisheng Zhang <jszhang@...nel.org>, Kees Cook <kees@...nel.org>, Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>, Ley Foon Tan <leyfoon.tan@...rfivetech.com>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>, "linux-stm32@...md-mailman.stormreply.com" <linux-stm32@...md-mailman.stormreply.com>, Matthew Gerlach <matthew.gerlach@...era.com>, Maxime Chevallier <maxime.chevallier@...tlin.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>, Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Oleksij Rempel <o.rempel@...gutronix.de>, Paolo Abeni <pabeni@...hat.com>, Rohan G Thomas <rohan.g.thomas@...era.com>, Shenwei Wang <shenwei.wang@....com>, Simon Horman <horms@...nel.org>, Song Yoong Siang <yoong.siang.song@...el.com>, Swathi K S <swathi.ks@...sung.com>, Tiezhu Yang <yangtiezhu@...ngson.cn>, Vinod Koul <vkoul@...nel.org>, Vladimir Oltean <olteanv@...il.com>, Vladimir Oltean <vladimir.oltean@....com>, Yu-Chun Lin <eleanor15x@...il.com> Subject: RE: [PATCH net-next v2 00/14] net: stmmac: phylink PCS conversion Hi, > From: Russell King <linux@...linux.org.uk> > Sent: 16 October 2025 15:35 > To: Andrew Lunn <andrew@...n.ch>; Heiner Kallweit <hkallweit1@...il.com> > Cc: Abhishek Chauhan <quic_abchauha@...cinc.com>; Alexandre Torgue > <alexandre.torgue@...s.st.com>; Alexis Lothore > <alexis.lothore@...tlin.com>; Andrew Lunn <andrew+netdev@...n.ch>; Boon > Khai Ng <boon.khai.ng@...era.com>; Choong Yong Liang > <yong.liang.choong@...ux.intel.com>; Daniel Machon > <daniel.machon@...rochip.com>; David S. Miller <davem@...emloft.net>; Drew > Fustini <dfustini@...storrent.com>; Emil Renner Berthing > <emil.renner.berthing@...onical.com>; Eric Dumazet <edumazet@...gle.com>; > Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>; Furong Xu > <0x1207@...il.com>; Inochi Amaoto <inochiama@...il.com>; Jacob Keller > <jacob.e.keller@...el.com>; Jakub Kicinski <kuba@...nel.org>; Jan Petrous > (OSS) <jan.petrous@....nxp.com>; Jisheng Zhang <jszhang@...nel.org>; Kees > Cook <kees@...nel.org>; Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>; > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>; Ley Foon > Tan <leyfoon.tan@...rfivetech.com>; linux-arm-kernel@...ts.infradead.org; > linux-arm-msm@...r.kernel.org; linux-stm32@...md-mailman.stormreply.com; > Matthew Gerlach <matthew.gerlach@...era.com>; Maxime Chevallier > <maxime.chevallier@...tlin.com>; Maxime Coquelin > <mcoquelin.stm32@...il.com>; Michal Swiatkowski > <michal.swiatkowski@...ux.intel.com>; netdev@...r.kernel.org; Oleksij > Rempel <o.rempel@...gutronix.de>; Paolo Abeni <pabeni@...hat.com>; Rohan G > Thomas <rohan.g.thomas@...era.com>; Shenwei Wang <shenwei.wang@....com>; > Simon Horman <horms@...nel.org>; Song Yoong Siang > <yoong.siang.song@...el.com>; Swathi K S <swathi.ks@...sung.com>; Tiezhu > Yang <yangtiezhu@...ngson.cn>; Vinod Koul <vkoul@...nel.org>; Vladimir > Oltean <olteanv@...il.com>; Vladimir Oltean <vladimir.oltean@....com>; Yu- > Chun Lin <eleanor15x@...il.com> > Subject: [PATCH net-next v2 00/14] net: stmmac: phylink PCS conversion > > This series is radical - it takes the brave step of ripping out much of > the existing PCS support code and throwing it all away. > > I have discussed the introduction of the STMMAC_FLAG_HAS_INTEGRATED_PCS > flag with Bartosz Golaszewski, and the conclusion I came to is that this > is to workaround the breakage that I've been going on about concerning the > phylink conversion for the last five or six years. > > The problem is that the stmmac PCS code manipulates the netif carrier > state, which confuses phylink. > > There is a way of testing this out on the Jetson Xavier NX platform as the > "PCS" code paths can be exercised while in RGMII mode - because RGMII also > has in-band status and the status register is shared with SGMII. Testing > this out confirms my long held theory: the interrupt handler manipulates > the netif carrier state before phylink gets a look-in, which means that > the mac_link_up() and mac_link_down() methods are never called, resulting > in the device being non-functional. > > Moreover, on dwmac4 cores, ethtool reports incorrect information - despite > having a full-duplex link, ethtool reports that it is half-dupex. > > Thus, this code is completely broken - anyone using it will not have a > functional platform, and thus it doesn't deserve to live any longer, > especially as it's a thorn in phylink. > > Rip all this out, leaving just the bare bones initialisation in place. > > However, this is not the last of what's broken. We have this hw->ps > integer which is really not descriptive, and the DT property from which it > comes from does little to help understand what's going on. > Putting all the clues together: > > - early configuration of the GMAC configuration register for the > speed. > - setting the SGMII rate adapter layer to take its speed from the > GMAC configuration register. > > Lastly, setting the transmit enable (TE) bit, which is a typo that puts > the nail in the coffin of this code. It should be the transmit > configuration (TC) bit. Given that when the link comes up, phylink will > call mac_link_up() which will overwrite the speed in the GMAC > configuration register, the only part of this that is functional is > changing where the SGMII rate adapter layer gets its speed from, which is > a boolean. > > From what I've found so far, everyone who sets the snps,ps-speed property > which configures this mode also configures a fixed link, so the pre- > configuration is unnecessary - the link will come up anyway. > > So, this series rips that out the preconfiguration as well, and replaces > hw->ps with a boolean hw->reverse_sgmii_enable flag. > > We then move the sole PCS configuration into a phylink_pcs instance, which > configures the PCS control register in the same way as is done during the > probe function. > > Thus, we end up with much easier and simpler conversion to phylink PCS > than previous attempts. > > Even so, this still results in inband mode always being enabled at the > moment in the new .pcs_config() method to reflect what the probe function > was doing. The next stage will be to change that to allow phylink to > correctly configure the PCS. This needs fixing to allow platform glue > maintainers who are currently blocked to progress. > > Please note, however, that this has not been tested with any SGMII > platform. > > I've tried to get as many people into the Cc list with get_maintainers, I > hope that's sufficient to get enough eyeballs on this. > > Changes since RFC: > - new patch (7) to remove RGMII "pcs" mode > - new patch (8) to move reverse "pcs" mode to stmmac_check_pcs_mode() > - new patch (9) to simplify the code moved in the previous patch > - new patch (10) to rename the confusing hw->ps to something more > understandable. > - new patch (11) to shut up inappropriate complaints about > "snps,ps-speed" being invalid. > - new patch (13) to add a MAC .pcs_init method, which will only be > called when core has PCS present. > - modify patch 14 to use this new pcs_init method. > > Despite getting a couple of responses to the RFC series posted in > September, I have had nothing testing this on hardware. I have tested this > on the Jetson Xavier NX, which included trial runs with enabling the RGMII > "pcs" mode, hence the new patches that rip out this mode. I have come to > the conclusion that the only way to get stmmac changes tested is to get > them merged into net-next, thereby forcing people to have to run with > them... and we'll deal with any fallout later. > > Changes since v1: > - added Andrew's reviewed-bys > - added additional comments to patch 3, 11 and 14. > No code changes. > > drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +- > drivers/net/ethernet/stmicro/stmmac/common.h | 5 +- > .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 6 +- > drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 6 +- > .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 65 ++--------------- > ---- > drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 3 +- > drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 66 ++--------------- > ---- > .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 25 +------- > drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 +- > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++ > .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 68 +---------------- > ----- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 24 ++++---- > drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 47 +++++++++++++++ > drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 23 ++++++-- > include/linux/stmmac.h | 1 - > 15 files changed, 104 insertions(+), 245 deletions(-) > Although RZ/V2H doesn't have PCS, I tested this on Renesas RZ/V2H EVK and found no regressions. Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com> Cheers, Prabhakar > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! > > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists