From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="fidtktW6" Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4718A9C; Wed, 13 Dec 2023 12:23:09 -0800 (PST) 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=dENc2lwzRLHtPKhKvuKHXjHSGuomugQdTgIHyUkfNP0=; b=fidtktW6ZbPyKmdXpJNpVbMtIU SuBrlY8SgZjuCBH6zYr9ay3u0fefJagEFwhKZ9bOaHzDhAqAYE+o8Vj6wZNvWcYxWgpf3HesRBcQ3 HUPAOamNh9wfrrJh1GVpeN2CsNCbuS+eL4yy6MUluZkaidUQ31Vbzb3NeGDPIFC1CYsexhMIjPEEZ 1NcGS6J1W9UHN996xASg3HRJpctvkfTAbuH/1G2WdCNsWqqSCy+cETLG3u7sDzo3F9MxPl221Kxyb v6Lx/pKkL8e5f+KtGlrcFsrOoPjA52pbI6mou1AKqHhjxSnQK1pieGa9b94z0EtlWcqTtpThpLMrY QGmcIKlA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:47676) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1rDVkz-0000YT-1A; Wed, 13 Dec 2023 20:23:01 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1rDVkz-0001ow-GJ; Wed, 13 Dec 2023 20:23:01 +0000 Date: Wed, 13 Dec 2023 20:23:01 +0000 From: "Russell King (Oracle)" To: Daniel Golle Cc: Andrew Lunn , Maxime Chevallier , Heiner Kallweit , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net] net: phy: skip LED triggers on PHYs on SFP modules Message-ID: References: <102a9dce38bdf00215735d04cd4704458273ad9c.1702339354.git.daniel@makrotopia.org> <20231212153512.67a7a35b@device.home> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: Russell King (Oracle) On Wed, Dec 13, 2023 at 07:01:29PM +0000, Daniel Golle wrote: > On Wed, Dec 13, 2023 at 03:27:28PM +0000, Russell King (Oracle) wrote: > > No, and we should _not_ mess around with the "LED" configuration on > > PHYs on SFPs. It's possible that the LED output is wired to the LOS > > pin on the module, and messing around with the configuration of that > > would be asking for trouble. > > > > In any case, I thought we didn't drive the LED configuration on PHYs > > where the LED configuration isn't described by firmware - and as the > > PHY on SFP modules would never be described by firmware, hooking > > such a PHY up to the LED framework sounds like a waste of resources > > to me. > > This was exactly my line of thought when posting the patch, however, > Maxime correctly pointed out that the issue with locking and also > what the patch prevents is registration of LED *triggers* rather than > the PHY-controlled LEDs themselves. And having the triggers available > is desirable even beyond the hardware offloaded case (which is probably > the aspect we both were dealing with the most recently and hence had in > mind). It is common to control another system SoC GPIO driven LED(s) > representing the link status and rx/tx traffic, for example. > > So better we get to the core of it and fix the locking issue > (for example by registering LED trigger asynchronously using > delayed_work)... I don't think a delayed work solves anything. Well, it solves the registration problem, but when the PHY is removed, you have to _synchronously_ cancel the delayed work, which could result in it waiting on the called work function to complete. If that called work function is waiting for the lock which we're holding on the remove path, then we're still in a deadlock-prone situation. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!