lkml.org 
[lkml]   [2021]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: lets settle the LED `function` property regarding the netdev trigger
On Sun, 3 Oct 2021 20:56:23 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Fri, Oct 01, 2021 at 02:36:01PM +0200, Marek Behún wrote:
> > Hello Pavel, Jacek, Rob and others,
> >
> > I'd like to settle DT binding for the LED function property regarding
> > the netdev LED trigger.
> >
> > Currently we have, in include/dt-bindings/leds/common.h, the following
> > functions defined that could be interpreted as request to enable netdev
> > trigger on given LEDs:
> > activity
> > lan
> > rx tx
> > wan
> > wlan
> >
> > The "activity" function was originally meant to imply the CPU
> > activity trigger, while "rx" and "tx" are AFAIK meant as UART indicators
> > (tty LED trigger), see
> > https://lore.kernel.org/linux-leds/20190609190803.14815-27-jacek.anaszewski@gmail.com/
> >
> > The netdev trigger supports different settings:
> > - indicate link
> > - blink on rx, blink on tx, blink on both
> >
> > The current scheme does not allow for implying these.
> >
> > I therefore propose that when a LED has a network device handle in the
> > trigger-sources property, the "rx", "tx" and "activity" functions
> > should also imply netdev trigger (with the corresponding setting).
> > A "link" function should be added, also implying netdev trigger.
> >
> > What about if a LED is meant by the device vendor to indicate both link
> > (on) and activity (blink)?
> > The function property is currently a string. This could be changed to
> > array of strings, and then we can have
> > function = "link", "activity";
> > Since the function property is also used for composing LED classdev
> > names, I think only the first member should be used for that.
> >
> > This would allow for ethernet LEDs with names
> > ethphy-0:green:link
> > ethphy-0:yellow:activity
> > to be controlled by netdev trigger in a specific setting without the
> > need to set the trigger in /sys/class/leds.
>
> Hi Marek
>
> There is no real standardization here. Which means PHYs differ a lot
> in what they can do. We need to strike a balance between over
> simplifying and only supporting a very small set of PHY LED features,
> and allowing great flexibility and having each PHY implement its own
> specific features and having little in common.
>
> I think your current proposal is currently on the too simple side.
>
> One common feature is that there are multiple modes for indicating
> link, which take into account the link speed. Look at for example
> include/dt-bindings/net/microchip-lan78xx.h
>
> #define LAN78XX_LINK_ACTIVITY 0
> #define LAN78XX_LINK_1000_ACTIVITY 1
> #define LAN78XX_LINK_100_ACTIVITY 2
> #define LAN78XX_LINK_10_ACTIVITY 3
> #define LAN78XX_LINK_100_1000_ACTIVITY 4
> #define LAN78XX_LINK_10_1000_ACTIVITY 5
> #define LAN78XX_LINK_10_100_ACTIVITY 6
>
> And:
>
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK10 0x0010
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK100 0x0020
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK10X 0x0030
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK1000 0x0040
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK10_0 0x0050
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK100X 0x0060
> intel-xway.c:#define XWAY_MMD_LEDxL_BLINKS_LINK10XX 0x0070
>
> Marvell PHYs have similar LINK modes which can either be one specific
> speed, or a combination of speeds.
>
> This is a common enough feature, and a frequently used feature, we
> need to support it. We also need to forward looking. We should not
> limit ourselves to 10/100/1G. We have 3 PHY drivers which support
> 2.5G, 5G and 10G. 25G and 40G are standardized so are likely to come
> along at some point.
>
> One way we could support this is:
>
> function = "link100", "link1G", "activity";
>
> for LAN78XX_LINK_100_1000_ACTIVITY, etc.
>
> Andrew

Hello Andrew,

I am aware of this, and in fact am working on a proposal for an
extension of netdev LED extension, to support the different link
modes. (And also to support for multi-color LEDs.)

But I am not entirely sure whether these different link modes should be
also definable via device-tree. Are there devices with ethernet LEDs
dedicated for a specific speed? (i.e. the manufacturer says in the
documentation of the device, or perhaps on the device's case, that this
LED shows 100M/1000M link, and that other LED is shows 10M link?)
If so, that this should be specified in the devicetree, IMO. But are
such devices common?

And what about multi-color LEDs? There are ethernet ports where one LED
is red-green, and so can generate red, green, and yellow color. Should
device tree also define which color indicates which mode?

Marek

\
 
 \ /
  Last update: 2021-10-03 21:27    [W:1.000 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site