Message-ID: <20211014135844.440e4e19@dellmb>
Date:   Thu, 14 Oct 2021 13:58:44 +0200
From:   Marek Behún <kabel@...nel.org>
To:     Alexander Dahl <ada@...rsis.com>
Cc:     Pavel Machek <pavel@....cz>, devicetree@...r.kernel.org,
        linux-leds@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        robh+dt@...nel.org, Jacek Anaszewski <jacek.anaszewski@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH 2/3] dt-bindings: leds: Add `excludes` property

On Thu, 14 Oct 2021 13:30:39 +0200
Alexander Dahl <ada@...rsis.com> wrote:

> Hei hei,
> 
> Am Thu, Oct 14, 2021 at 12:43:09PM +0200 schrieb Marek Behún:
> > On Thu, 14 Oct 2021 12:29:18 +0200
> > Pavel Machek <pavel@....cz> wrote:
> >   
> > > Hi!
> > >   
> > > > Some RJ-45 connectors have LEDs wired in the following way:
> > > > 
> > > >          LED1
> > > >       +--|>|--+
> > > >       |       |
> > > >   A---+--|<|--+---B
> > > >          LED2
> > > > 
> > > > With + on A and - on B, LED1 is ON and LED2 is OFF. Inverting
> > > > the polarity turns LED1 OFF and LED2 ON.
> > > > 
> > > > So these LEDs exclude each other.
> > > > 
> > > > Add new `excludes` property to the LED binding. The property is
> > > > a phandle-array to all the other LEDs that are excluded by this
> > > > LED.    
> > > 
> > > I don't think this belongs to the LED binding.
> > > 
> > > This is controller limitation, and the driver handling the
> > > controller needs to know about it... so it does not need to learn
> > > that from the LED binding.  
> > 
> > It's not necessarily a controller limitation, rather a limitation of
> > the board (or ethernet connector, in the case of LEDs on an ethernet
> > connector).  
> 
> Such LEDs are not limited to PHYs or ethernet connectors.  There is
> hardware with such dual color LEDs connected to GPIO pins (either
> directly to the SoC or through some GPIO expander like an 74hc595
> shift register).  That mail points to such hardware:
> 
> https://www.spinics.net/lists/linux-leds/msg11847.html
> 
> I asked about how this can be modelled back in 2019 and it was also
> discussed last year:
> 
> https://www.spinics.net/lists/linux-leds/msg11665.html
> https://lore.kernel.org/linux-leds/2315048.uTtSMl1LR1@ada/
> 
> The "solution" back when I first asked was treating them as ordinary
> GPIO-LEDs and ignore the "exclusion topic" which means in practice the
> LED goes OFF if both pins are ON (high) at the same time, which works
> well enough in practice.
> 
> > But I guess we could instead document this property in the ethernet
> > PHY controller binding for a given PHY.  
> 
> Because such LEDs are not restricted to ethernet PHYs, but can also be
> used with GPIOs from the hardware point of view, I would not put it
> there.
> 
> Furthermore I would not view this as a restriction of the gpio-leds
> controller, but it's a property of the LEDs itself or the way they are
> wired to the board.
> 
> This could (or should as Pavel suggested back in 2019) be put to a new
> driver, at least for the GPIO case, but it would need some kind of new
> binding anyways.  With that in mind I consider the proposed binding to
> be well comprehensible for a human reader/writer.
> 
> I'm sorry, I did not have leisure time to implement such a driver yet.
> Breadboard hardware for that still waiting in the drawer. :-/

That's why I think we need the `excludes` property.

On the sw side, it should work like this:
$ cd /sys/class/leds
$ echo 1 >LED1/brightness
$ cat LED1/brightness LED2/brightness
1
0
$ echo 1 >LED2/brightness
$ cat LED1/brightness LED2/brightness
0
1

The drivers could also implement brightness_hw_changed for these LEDs.

Marek

Powered by blists - more mailing lists