Re: [PATCH net-next 5/5] igc: Export LEDs

From: "Marek Behún" <kabel@kernel.org>
To: Pavel Machek <pavel@ucw.cz>, andrew@lunn.ch
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	Michael Walle <michael@walle.cc>,
	anthony.l.nguyen@intel.com, bigeasy@linutronix.de,
	davem@davemloft.net, dvorax.fuxbrumer@linux.intel.com,
	f.fainelli@gmail.com, jacek.anaszewski@gmail.com,
	kuba@kernel.org, kurt@linutronix.de, linux-leds@vger.kernel.org,
	netdev@vger.kernel.org, sasha.neftin@intel.com,
	vinicius.gomes@intel.com, vitaly.lifshits@intel.com
Subject: Re: [PATCH net-next 5/5] igc: Export LEDs
Date: Wed, 25 Aug 2021 17:26:13 +0200	[thread overview]
Message-ID: <20210825172613.71b62113@dellmb> (raw)
In-Reply-To: <20210817190241.GA15389@amd>

On Tue, 17 Aug 2021 21:02:42 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2021-08-10 22:53:53, Marek Behún wrote:
> > On Tue, 10 Aug 2021 21:53:35 +0200
> > Pavel Machek <pavel@ucw.cz> wrote:
> >   
> > > > Pavel, one point of the discussion is that in this case the LED
> > > > is controlled by MAC, not PHY. So the question is whether we
> > > > want to do "ethmacN" (in addition to "ethphyN").    
> > > 
> > > Sorry, I missed that. I guess that yes, ethmacX is okay, too.
> > > 
> > > Even better would be to find common term that could be used for
> > > both ethmacN and ethphyN and just use that. (Except that we want
> > > to avoid ethX). Maybe "ethportX" would be suitable?  
> > 
> > See
> >   https://lore.kernel.org/linux-leds/YQAlPrF2uu3Gr+0d@lunn.ch/
> > and
> >   https://lore.kernel.org/linux-leds/20210727172828.1529c764@thinkpad/
> >  
> 
> Ok, I guess I'd preffer all LEDs corresponding to one port to be
> grouped, but that may be hard to do.

Hi Pavel (and Andrew),

The thing is that normally we are creating LED classdevs when the
parent device is probed. In this case when the PHY is probed. But we
will know the corresponding MAC only once the PHY is attached to it's
network interface.

Also, a PHY may be theoretically attached to multiple different
interfaces during it's lifetime. I guess there isn't many boards
currently that have such a configuration wired (maybe none), but
kernel's API is prepared for this.

So we really can't group them under one parent device, unless:

- we create LED classdevs only after PHY is attached (which will make
  us unable to blink the LEDs when the PHY is not attached yet) and
  destroy them when PHY is detached. I find this solution wrong - the
  LEDs will be unavailable for example if the MAC driver fails to probe
  for some reason

- or we add a mechanism to reprobe LEDs upon request (which also seems
  a rather unsatisfactory solution to me...)

- maybe some other solution?

Marek