From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [78.32.30.218]) (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 9412D33DED9; Wed, 25 Mar 2026 09:55:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=78.32.30.218 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774432560; cv=none; b=iK9buLMZHXmNsj/Xzk46LgyvYRqVjihYanTTEEHETMCmDc7NYP8nUdDzDEG0Ko/QQNUvMSZ2s8rgNvqQXSD/rptZVzAzXwLWRyI7/B0fbboXeG42WEAscmYjeYWkgNZgDP/mIyt2ityhAUkklzAW/FVWYTqMno4utmmDBrdAKCo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774432560; c=relaxed/simple; bh=eGcO8nmKMF0kgmNqDIPpvsCqlsO5XrI1/W32NJH1C+g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=D3aL7QJ9mU9+GXs7Def11fQimocBy1G20cBuLljhY9R8pP/FQeVx7/xQRp2TOMS/UvHIWhm3qTP+9moBQhot7fUfs3YSIpnLL4FY53KgHKOJeWADkiwOm8ssQgtVlQ7w4owKWD+eHWNz6x662FS0WzXUHMX0EwuCfCXpHZPP3Bo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk; spf=none smtp.mailfrom=armlinux.org.uk; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b=Z8x233+7; arc=none smtp.client-ip=78.32.30.218 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="Z8x233+7" 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-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To: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=MrD/JzDrD0dZqQ/BKgHkwNPVPT9uW7xqNUxn9RdOSf0=; b=Z8x233+7/xEqjXlOVVGBNMEXlx 4PiPESHRrXr707uK287cImoqPmojVVBPJjpiVHLP8K9o5bXrO+huTt6l5UZtjgdow0hyYQyewI6qB dT32Wx231olUTVkjaMDCdoE0HxdSh2PQBNWYNqzhPUpbLfVZa9mGfdcGMV6l99/nbaqi3JuWsCE/O p65OUFIr0EM7afi/FmlgJMmDaIqpLw0LAdkUkZv4NoWGSyxcYpeFInUdc/MoOof2CTfOowrMacfuq gDu3gemEl3fmL1dH19WvcRcuwPUBqM40rzNdpubjfZ82DGouFaBdNzPUIFR3/uSAiM2CSbcVfvIKb 0QuMRIrA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:54494) 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 1w5Kxf-0000000039n-2tj3; Wed, 25 Mar 2026 09:55:39 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.98.2) (envelope-from ) id 1w5Kxc-000000005wp-1R1H; Wed, 25 Mar 2026 09:55:36 +0000 Date: Wed, 25 Mar 2026 09:55:36 +0000 From: "Russell King (Oracle)" To: Linus Walleij Cc: Christophe Roullier , Andrew Lunn , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Alexandre Torgue , antonio.borneo@foss.st.com, Maxime Chevallier , Vladimir Oltean , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] net: stmmac: fix pinctrl management during suspend/resume Message-ID: References: <20260313105718.359614-1-christophe.roullier@foss.st.com> <20260313105718.359614-2-christophe.roullier@foss.st.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: Russell King (Oracle) On Thu, Mar 19, 2026 at 10:43:25AM +0100, Linus Walleij wrote: > On Tue, Mar 17, 2026 at 2:11 PM Russell King (Oracle) > wrote: > > I'm trying to get an answer on this, because the original patch > > description here says: > > > > | In the deepest low-power modes, the pinctrl configuration is > > | lost and is never restored if the interface is down. > > > > stmmac uses the "default" pinctrl state at probe time. This commit > > says that is lost over suspend/resume - which to me sounds like > > a pinctrl driver bug, because on resume, the pinctrl driver is not > > ensuring that the pinctrl state is restored to whatever it was when > > the suspend happened (whether the driver explicitly changed it or > > not.) > > > > To put it another way... > > > > On entry to probe for a non-pinctrl driver, if DT describes a default > > pinctrl state, that state will be selected by core code. > > > > On suspend, the driver is free to select another state if it so wishes, > > or do nothing (e.g. it's unaware of pinctrl.) > > > > On resume, the pinctrl layer, what is expected to happen. Surely, it > > is reasonable for a pinctrl unaware driver, or at least a driver which > > has _not_ changed the pinctrl state to expect that the default pinctrl > > state is still in effect when its resume function is called - and if > > that is not the case, then there's a bug here. > > > > Another way to put it... > > > > We shouldn't be expecting device drivers to have to mess with pinctrl > > e.g. switching to a sleep state and then back to a default state just > > to have pinctrl settings restored to a functional state on resume. > > OK I see your point, the pinctrl hardware state should not change > behind the back of the driver, and if it does, and that is worked > around by these calls to restore the state using the pinctrl PM > helpers become a messy quirk. > > - Selecting those states to reconfigure pins into special modes > during sleep is OK. > > - Selecting those states to restore the hardware state inside the > pin controller itself is NOT OK. > > If this is done for the latter reason, you're right of course, there is > a bug in the pin controller. Certainly it is expected to maintain its > own state over a suspend/resume cycle. Thanks for the clarification. So, going back to the original patch: "In the deepest low-power modes, the pinctrl configuration is lost and is never restored if the interface is down. This commit ensures that the pinctrl state is set in all cases." This commit message seems to be suggesting that the switch to sleep mode and back to default mode is to ensure that the pinctrl driver for this platform correctly re-sets the pinctrl state back to the default state - which suggests that this patch is the wrong approach and there is a bug somewhere in the pin controller layer. That said, for platforms that do provide a sleep state for stmmac, it makes sense to switch the pinctrl to that state even when the netif is down. So, I think there's actually two separate issues: 1. the pin controller not restoring the state that was set at suspend. 2. not switching to sleep state when supported when the netif is down. and fixing (2) will mask (1) which will be bad. So, I think (1) needs to be fixed first while we have an easy-to-test case, and my suggested replacement patch is a better approach for (2). -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!