From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A89804400 for ; Fri, 17 Oct 2025 07:05:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760684740; cv=none; b=XsGhHgWdnjwE/X3UaDEmBp3/mo46w0i3fqBTlt9GNvjOmfXcERlKBL2Z79dCvYJU2EcnYqa3OyjekaLgL65izlHEvpVsb3oVaQLi0BZ9JicfZcYHYmTX+vxLbAEVtZoP2Z/DS1fuVbZMyrzfyzRJdXePy2R1JM92u5l5qbYUyAA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760684740; c=relaxed/simple; bh=l9RIq9pv47w7bEV5KWrnvIb9LJMvjqefg4292JUfx3g=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=f/gwcQLY3CLaUk+4XWg0TN+JZc0sWKI+gqth5ltUVFoIRrgfjKdZg5SHX1BaKnLEoz0nmPdYJ6IUHAFitkqGYhNmfXZ4/la2jHEXPrkQOTMmf7+O7C2jXNQ1mN/sT0VxDvfWZ7bAktbeTXTd0vaIY4GP64jYtbFmdTrSYvwxV8E= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=sjVqtIm4; arc=none smtp.client-ip=185.246.85.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="sjVqtIm4" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 9CF034E41115; Fri, 17 Oct 2025 07:05:35 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 651C9606DB; Fri, 17 Oct 2025 07:05:35 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 00B0D102F22FF; Fri, 17 Oct 2025 09:04:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1760684733; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=8Txvygtf95T8nvPnqElNFv3jlBIUlCRN1Ha31LP6OZM=; b=sjVqtIm4RJwQ4q/9IDLKMSW6riYcV/yNt+HFk6E2Z9iRJ7mcel14odvS1qcyV6Zojb2ZL/ UjK86X1GZjeFt8l4tTWqqX5GMkhp2tz//ZFswJJxaec0tbucR/wHOUZPsTXsOTWqF6Hkcy PsT3RCGuLuL3pIzCuxEBS19909xxzsZzs/iU/so1N2Ny/VakwLErGxdMdFvoTDEZfIGP7c 1q7q03jcaA/Jdyzq66uIb9IX6f6YcMZtfsL6xNXl25uo1acubbem9B/PXBj+xHKuu2fXEd CJGlJW9d8NPHglu7w1OISAR7MVpEiXJ497Z1xlPBUvJt0TtWvMd4li2EVhgW5g== Message-ID: <1f9d856f-4cfb-472c-abec-96ce55f4db81@bootlin.com> Date: Fri, 17 Oct 2025 09:04:55 +0200 Precedence: bulk X-Mailing-List: linux-arm-msm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v2 00/14] net: stmmac: phylink PCS conversion To: "Russell King (Oracle)" , Andrew Lunn , Heiner Kallweit Cc: Abhishek Chauhan , Alexandre Torgue , Alexis Lothore , Andrew Lunn , Boon Khai Ng , Choong Yong Liang , Daniel Machon , "David S. Miller" , Drew Fustini , Emil Renner Berthing , Eric Dumazet , Faizal Rahim , Furong Xu <0x1207@gmail.com>, Inochi Amaoto , Jacob Keller , Jakub Kicinski , "Jan Petrous (OSS)" , Jisheng Zhang , Kees Cook , Kunihiko Hayashi , Lad Prabhakar , Ley Foon Tan , linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, Matthew Gerlach , Maxime Coquelin , Michal Swiatkowski , netdev@vger.kernel.org, Oleksij Rempel , Paolo Abeni , Rohan G Thomas , Shenwei Wang , Simon Horman , Song Yoong Siang , Swathi K S , Tiezhu Yang , Vinod Koul , Vladimir Oltean , Vladimir Oltean , Yu-Chun Lin References: From: Maxime Chevallier Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 Hello, On 16/10/2025 16:35, Russell King (Oracle) wrote: > 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. I have tested this on : - dwmac-socfpga w/ Lynx, SGMII and 1000BaseX mode - dwmac-socfpga w/ RGMII mode - stm32-dwmac (on stm32mp157a), w/ RGMII mode I don't have any device available with stmmac using the internal PCS implementation, but at least the 2 platforms above don't regress with this series. Tested-by: Maxime Chevallier Maxime