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 935602FB995 for ; Thu, 25 Sep 2025 11:57:01 +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=1758801423; cv=none; b=aPR0rmqLpkGqWK+j6pd+mZGijJhTn2iG5Q32xnJXN/FMm6FHr3Pi9wlesJgs/jDklyKRm9UI3wCmsYeyF5coH4hdxvkQcEGleUQzgmSXWzYeXdg6rEn+tNg3jKRRgrvPHp4zP1EG1gzymqggqnLcAXXj9xizbzAD7ZVLvbudRlI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758801423; c=relaxed/simple; bh=V6MTDiVySTY0FtRDwbSPAeskWfvP6UybS3p7cJ7SG+c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=HTitz27zsq+/Mz/USrTmqRLPPAEU4v2n3jtr6prBc5EIsxV0fhHCUXUukK2fnNfJM2zb48XPIwFrA/OTu7EB+fnYzEWH5RrF2eOTXzMf00VSs9XAiloxw3gYURHO54KZJ9zzcwcsMPBgaU8XI3tuIMq9BjPn4yd5mQPPHBVAm/A= 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=GXjyMDoh; 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="GXjyMDoh" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id CD0A94E40DD3; Thu, 25 Sep 2025 11:56:59 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 920F16062C; Thu, 25 Sep 2025 11:56:59 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 990C7102F17C2; Thu, 25 Sep 2025 13:56:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1758801417; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:content-language:in-reply-to:references; bh=aETJ2zYw/6HsvY9Zf2t3GTjzejHx+Y6hnBsuvecmf/k=; b=GXjyMDohYRGVoA/jxxP+Zh/xhFZkXEw47idV2kkxCw4BsGWQCQQpWMx3vX5TDqxj9lCsTk qE4zU9kk6HI3rTWGvibKSdaxk0hi20iX+8mDs656ftEn3KudwCsJZh80+kw4zv8NU19C+c gcFWBMQp9Xvevo9GamLy26ktiXq8jcWVvd1rm8sSW4V0GV43nVd3cFz0YvWJRoRuFm27gh CGd/jii+CU7Xf9etVZ0b3pymhMQ0SroYPtG99j2sICWkx8k6/AMlX6TinkAWlONweTlHIw JS5o2Y9WmCO/tth3lpUwH/y+uTpltg4Ciu0AholB4azScehG9yO5lyfizaoWxA== Message-ID: <4822b6a5-5b40-47bb-8bff-7a3cc91f93c8@bootlin.com> Date: Thu, 25 Sep 2025 17:26:01 +0530 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 RFC net-next 0/9] net: stmmac: experimental 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; format=flowed Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 Hi Russell, On 24/09/2025 23:47, 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. Thanks for that. I'll give this a test on socfpga next week, as I don't have access to the HW right now. It may not be the best platform to test this on, as it has a lynx PCS and no internal PCS :/ Maxime