Re: [PATCH v2 6/7] dt-bindings: net: realtek: Add property to configure LED mode

[Matthias Kaehlcke]

Hi Florian,

On Wed, Jul 03, 2019 at 02:37:47PM -0700, Florian Fainelli wrote:
> On 7/3/19 12:37 PM, Matthias Kaehlcke wrote:
> > The LED behavior of some Realtek PHYs is configurable. Add the
> > property 'realtek,led-modes' to specify the configuration of the
> > LEDs.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> > ---
> > Changes in v2:
> > - patch added to the series
> > ---
> >  .../devicetree/bindings/net/realtek.txt         |  9 +++++++++
> >  include/dt-bindings/net/realtek.h               | 17 +++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >  create mode 100644 include/dt-bindings/net/realtek.h
> > 
> > diff --git a/Documentation/devicetree/bindings/net/realtek.txt b/Documentation/devicetree/bindings/net/realtek.txt
> > index 71d386c78269..40b0d6f9ee21 100644
> > --- a/Documentation/devicetree/bindings/net/realtek.txt
> > +++ b/Documentation/devicetree/bindings/net/realtek.txt
> > @@ -9,6 +9,12 @@ Optional properties:
> >  
> >  	SSC is only available on some Realtek PHYs (e.g. RTL8211E).
> >  
> > +- realtek,led-modes: LED mode configuration.
> > +
> > +	A 0..3 element vector, with each element configuring the operating
> > +	mode of an LED. Omitted LEDs are turned off. Allowed values are
> > +	defined in "include/dt-bindings/net/realtek.h".
> 
> This should probably be made more general and we should define LED modes
> that makes sense regardless of the PHY device, introduce a set of
> generic functions for validating and then add new function pointer for
> setting the LED configuration to the PHY driver. This would allow to be
> more future proof where each PHY driver could expose standard LEDs class
> devices to user-space, and it would also allow facilities like: ethtool
> -p to plug into that.
> 
> Right now, each driver invents its own way of configuring LEDs, that
> does not scale, and there is not really a good reason for that other
> than reviewing drivers in isolation and therefore making it harder to
> extract the commonality. Yes, I realize that since you are the latest
> person submitting something in that area, you are being selected :)

I see the merit of your proposal to come up with a generic mechanism
to configure Ethernet LEDs, however I can't justify spending much of
my work time on this. If it is deemed useful I'm happy to send another
version of the current patchset that addresses the reviewer's comments,
but if the implementation of a generic LED configuration interface is
a requirement I will have to abandon at least the LED configuration
part of this series.