Re: [PATCH net] net: phy: skip LED triggers on PHYs on SFP modules

[Maxime Chevallier]

Hi Andrew, Russell,

On Wed, 13 Dec 2023 15:27:28 +0000
"Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote:

> On Wed, Dec 13, 2023 at 10:08:25AM +0100, Andrew Lunn wrote:
> > On Tue, Dec 12, 2023 at 03:35:12PM +0100, Maxime Chevallier wrote:  
> > > Hi Daniel
> > > 
> > > On Tue, 12 Dec 2023 00:05:35 +0000
> > > Daniel Golle <daniel@xxxxxxxxxxxxxx> wrote:
> > >   
> > > > Calling led_trigger_register() when attaching a PHY located on an SFP
> > > > module potentially (and practically) leads into a deadlock.
> > > > Fix this by not calling led_trigger_register() for PHYs localted on SFP
> > > > modules as such modules actually never got any LEDs.  
> > > 
> > > While I don't have a fix for this issue, I think your justification
> > > isn't good. This isn't about having LEDs on the module or not, but
> > > rather the PHY triggering LED events for LEDS that can be located
> > > somewhere else on the system (like the front pannel of a switch).  
> > 
> > SFP LEDs are very unlikely to be on the front panel, since there is no
> > such pins on the SFP cage.
> > 
> > Russell, in your collection of SFPs do you have any with LEDs?  
> 
> 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.
> 

So it looks to me that the Daniel's patch does make sense then, even
without considering the underlying locking issue ?

Sorry for my misunderstanding of the LED driving that started this
discussion :/

Maxime