On Wed Jul 28 2021, Heiner Kallweit wrote:

Did we come to any conclusion?

My preliminary r8169 implementation now creates the following LED
names:

Is that implementation somewhere available?

lrwxrwxrwx 1 root root 0 Jul 26 22:50 r8169-led0-0300 -> ../../devices/pci0000:00/0000:00:1d.0/0000:03:00.0/net/enp3s0/r8169-led0-0300
lrwxrwxrwx 1 root root 0 Jul 26 22:50 r8169-led1-0300 -> ../../devices/pci0000:00/0000:00:1d.0/0000:03:00.0/net/enp3s0/r8169-led1-0300
lrwxrwxrwx 1 root root 0 Jul 26 22:50 r8169-led2-0300 -> ../../devices/pci0000:00/0000:00:1d.0/0000:03:00.0/net/enp3s0/r8169-led2-0300

I understood that LEDs should at least be renamed to r8169-0300::link-0
to link-2 Is this correct? Or do we have to wait with any network LED support
for a name discussion outcome?

For the different LED modes I defined private hw triggers (using trigger_type
to make the triggers usable with r8169 LEDs only). The trigger attribute now
looks like this:

[none] link_10_100 link_1000 link_10_100_1000 link_ACT link_10_100_ACT link_1000_ACT link_10_100_1000_ACT

Nice, or? Issue is just that these trigger names really should be made a
standard for all network LEDs. I don't care about the exact naming, important
is just that trigger names are the same, no matter whether it's about a r8169-
or igc- or whatever network chip controlled LEDs.

Your above trigger definitions are OK for the igc as well. Except it
supports up to 2500 Mbit/s. For igc there's also more triggers available
such as filter activity, paused and spd mode.

However, what about the cross LED settings which are common to all LEDs?
The igc has one attribute to control the blink rate of all three LEDs.

And I don't have a good solution for initialization yet. LED mode is whatever
BIOS sets, but initial trigger value is "none". I would have to read the
initial LED control register values, iterate over the triggers to find the
matching one, and call led_trigger_set() to properly set this trigger as
current trigger.

Yes, there needs to be a way to determine the default state.

Thanks,
Kurt