From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E0ABCCAC5AE for ; Wed, 24 Sep 2025 19:31:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=wWHuMNffMGutQbJvBKG/MZveRAosfgTjgtTC/tzZqkI=; b=y4rJ1GpHdrz/Riw3J4XV433/7W QA+BIAhyGdE0YMalhywcEPTmb+qwWuY9HMqDUz9SOrD16R7+8TIOvdE4DNYjQ1QkWaFTxxxu57sgv wVZK/mu8dYRSRtFqfCE6aMHhRdlS4fgy4wnkCANfVTW9JIYphE6IZEHYDlu7k9KTMnjI3en1rNpis XGsKErmS4g79Anq6N7rs4jeEvSrO2/kM5kjyPuHCM/vt+L+Ng4taI/SdBYa0VdaWUtKba7WnJqftE hZfnXVgCC+OvPmBhh72ye5qIazGG4kU8GacjMXb9fUqf1i2PIqqPiO6ryQlRoDW5hHTm8wk11S7Cd 8MTIA3DQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1v1VDM-000000035WE-1Cw3; Wed, 24 Sep 2025 19:31:44 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1v1VDI-000000035SF-44XN for linux-arm-kernel@lists.infradead.org; Wed, 24 Sep 2025 19:31:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=wWHuMNffMGutQbJvBKG/MZveRAosfgTjgtTC/tzZqkI=; b=b5/u/dXh0JBJyMqjvQCaQFkHk+ 8Ksv2CPwkCAHaWyXWet8FTyq9FgPyo7XewgPvma6VXyppo3tezvBghJZfbT9HetuKxM795x5Kn7sr hIwXirxpW1Br4vDkZdC0CkWR6PKd2tEIxzEtktNIuD7WKO7pXP4kAfvgqHmmkTra4W+m4smtqCSuK XCOL9BxcPLVKGW785xU3zvkHu3JWuKs4AbGatRATYJYy4dHehqcVXwlroC7HjAXdeuwBz+1c2MDRq 4FJmYWRFEXRXE1ABa/Jzw5zSJ3zIMTfFp8EaqrW37NtkfPs0PSG0jrfnENMkugEBKVbMTyULQoSIV Sb37R4Rg==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:34686) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1v1VCp-000000001DY-2osw; Wed, 24 Sep 2025 20:31:11 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.98.2) (envelope-from ) id 1v1VCd-000000007PA-3Edy; Wed, 24 Sep 2025 20:30:59 +0100 Date: Wed, 24 Sep 2025 20:30:59 +0100 From: "Russell King (Oracle)" To: Jacob Keller Cc: Andrew Lunn , Heiner Kallweit , 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 , 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 Chevallier , 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 Subject: Re: [PATCH RFC net-next 0/9] net: stmmac: experimental PCS conversion Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250924_123141_009302_9460B3EE X-CRM114-Status: GOOD ( 42.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Sep 24, 2025 at 12:13:18PM -0700, Jacob Keller wrote: > > > On 9/24/2025 11:17 AM, 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. > > > > I'm no expert with this hardware or driver, but all of your explanations > seem reasonable to me. > > I'd guess the real step is to try and get this tested against the > variety of hardware supported by stmmac? Yes please, that would be very helpful, as I don't want to regress anyone's setup. I'm hoping that this series is going to be the low- risk change. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!