mbox series

[RESEND,0/5] dt-bindings: support Ethernet devices as LED triggers

Message ID 20220505135512.3486-1-zajec5@gmail.com (mailing list archive)
Headers show
Series dt-bindings: support Ethernet devices as LED triggers | expand

Message

Rafał Miłecki May 5, 2022, 1:55 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

Some LEDs are designed to represent a state of another device. That may
be USB port, Ethernet interface, CPU, hard drive and more.

We already have support for LEDs that are designed to indicate USB port
(e.g. light on when USB device gets connected). There is DT binding for
that and Linux implementation in USB trigger.

This patchset adds support for describing LEDs that should react to
Ethernet interface status. That is commonly used in routers. They often
have LED to display state and activity of selected physical port. It's
also common to have multiple LEDs, each reacting to a specific link
speed.

Patch 5/5 is proof of concept and is not meant to be applied yet.

Rafał Miłecki (5):
  dt-bindings: net: add bitfield defines for Ethernet speeds
  dt-bindings: net: allow Ethernet devices as LED triggers
  dt-bindings: leds: add Ethernet triggered LEDs to example
  ARM: dts: BCM5301X: Add triggers for Luxul XWR-1200 network LEDs
  leds: trigger: netdev: support DT "trigger-sources" property

 .../devicetree/bindings/leds/common.yaml      | 21 +++++++++++++++
 .../bindings/net/ethernet-controller.yaml     |  3 +++
 arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts | 22 +++++++++++----
 drivers/leds/trigger/ledtrig-netdev.c         | 26 ++++++++++++++++++
 include/dt-bindings/net/eth.h                 | 27 +++++++++++++++++++
 5 files changed, 94 insertions(+), 5 deletions(-)
 create mode 100644 include/dt-bindings/net/eth.h

Comments

Christian Marangi May 5, 2022, 2:02 p.m. UTC | #1
On Thu, May 05, 2022 at 03:55:07PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Some LEDs are designed to represent a state of another device. That may
> be USB port, Ethernet interface, CPU, hard drive and more.
> 
> We already have support for LEDs that are designed to indicate USB port
> (e.g. light on when USB device gets connected). There is DT binding for
> that and Linux implementation in USB trigger.
> 
> This patchset adds support for describing LEDs that should react to
> Ethernet interface status. That is commonly used in routers. They often
> have LED to display state and activity of selected physical port. It's
> also common to have multiple LEDs, each reacting to a specific link
> speed.
>

I notice this is specific to ethernet speed... I wonder if we should
expand this also to other thing like duplex state or even rx/tx.

> Patch 5/5 is proof of concept and is not meant to be applied yet.
> 
> Rafał Miłecki (5):
>   dt-bindings: net: add bitfield defines for Ethernet speeds
>   dt-bindings: net: allow Ethernet devices as LED triggers
>   dt-bindings: leds: add Ethernet triggered LEDs to example
>   ARM: dts: BCM5301X: Add triggers for Luxul XWR-1200 network LEDs
>   leds: trigger: netdev: support DT "trigger-sources" property
> 
>  .../devicetree/bindings/leds/common.yaml      | 21 +++++++++++++++
>  .../bindings/net/ethernet-controller.yaml     |  3 +++
>  arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts | 22 +++++++++++----
>  drivers/leds/trigger/ledtrig-netdev.c         | 26 ++++++++++++++++++
>  include/dt-bindings/net/eth.h                 | 27 +++++++++++++++++++
>  5 files changed, 94 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/net/eth.h
> 
> -- 
> 2.34.1
>
Rafał Miłecki May 5, 2022, 2:21 p.m. UTC | #2
On 5.05.2022 16:02, Ansuel Smith wrote:
> On Thu, May 05, 2022 at 03:55:07PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Some LEDs are designed to represent a state of another device. That may
>> be USB port, Ethernet interface, CPU, hard drive and more.
>>
>> We already have support for LEDs that are designed to indicate USB port
>> (e.g. light on when USB device gets connected). There is DT binding for
>> that and Linux implementation in USB trigger.
>>
>> This patchset adds support for describing LEDs that should react to
>> Ethernet interface status. That is commonly used in routers. They often
>> have LED to display state and activity of selected physical port. It's
>> also common to have multiple LEDs, each reacting to a specific link
>> speed.
>>
> 
> I notice this is specific to ethernet speed... I wonder if we should
> expand this also to other thing like duplex state or even rx/tx.

I didn't see any router with separated Rx/Tx LEDs, but it still sounds
like a valid case.

We could add flags for that in proposed field like:
trigger-sources = <&port (SPEED_1000 | LINK | TX)>;

Or add separated field for non-speed flags like:
trigger-sources = <&port SPEED_1000 (LINK | TX)>;

Let's see what DT experts say about it.
Christian Marangi May 5, 2022, 2:30 p.m. UTC | #3
On Thu, May 05, 2022 at 04:21:33PM +0200, Rafał Miłecki wrote:
> On 5.05.2022 16:02, Ansuel Smith wrote:
> > On Thu, May 05, 2022 at 03:55:07PM +0200, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Some LEDs are designed to represent a state of another device. That may
> > > be USB port, Ethernet interface, CPU, hard drive and more.
> > > 
> > > We already have support for LEDs that are designed to indicate USB port
> > > (e.g. light on when USB device gets connected). There is DT binding for
> > > that and Linux implementation in USB trigger.
> > > 
> > > This patchset adds support for describing LEDs that should react to
> > > Ethernet interface status. That is commonly used in routers. They often
> > > have LED to display state and activity of selected physical port. It's
> > > also common to have multiple LEDs, each reacting to a specific link
> > > speed.
> > > 
> > 
> > I notice this is specific to ethernet speed... I wonder if we should
> > expand this also to other thing like duplex state or even rx/tx.
> 
> I didn't see any router with separated Rx/Tx LEDs, but it still sounds
> like a valid case.
>

Not a normal configuration but it's doable. For qca8k you can really set
the led to do whatever you want.

> We could add flags for that in proposed field like:
> trigger-sources = <&port (SPEED_1000 | LINK | TX)>;
> 
> Or add separated field for non-speed flags like:
> trigger-sources = <&port SPEED_1000 (LINK | TX)>;
> 
> Let's see what DT experts say about it.