Hi!
> Okay, so the netdev trigger offers modes `link`, `rx`, `tx`.
> You can enable/disable either of these (via separate sysfs files). `rx`
> and `tx` blink the LED, `link` turns the LED on if the interface is
> linked.
I wonder if people really need separate rx and tx, but... this sounds
reasonable.
> The phy_led_trigger subsystem works differently. Instead of registering
> one trigger (like netdev) it registers one trigger per PHY device and
> per speed. So for a PHY with name XYZ and supported speeds 1Gbps,
> 100Mbps, 10Mbps it registers 3 triggers:
> XYZ:1Gbps XYZ:100Mbps XYZ:10Mbps
That is not reasonable.
> I propose that at least these HW modes should be available (and
> documented) for ethernet PHY controlled LEDs:
Ok, and which of these will you actually use?
> mode to determine link on:
> - `link`
> mode for activity (these should blink):
> - `activity` (both rx and tx), `rx`, `tx`
> mode for link (on) and activity (blink)
> - `link/activity`, maybe `link/rx` and `link/tx`
> mode for every supported speed:
> - `1Gbps`, `100Mbps`, `10Mbps`, ...
> mode for every supported cable type:
> - `copper`, `fiber`, ... (are there others?)
That's ... way too many options.
Can we do it like netdev trigger? link? yes/no. rx? yes/no. tx? yes/no.
If displaying link only for certain speeds is useful, have link_min
and link_max, specifying values in Mbps? Default would be link_min ==
0, and link_max = 25000, so it would react on any link speed.
Is mode for cable type really useful? Then we should have link_fiber?
yes/no. link_copper? yes/no.
> mode that allows the user to determine link speed
> - `speed` (or maybe `linkspeed` ?)
> - on some Marvell PHYs the speed can be determined by how fast
> the LED is blinking (ie. 1Gbps blinks with default blinking
> frequency, 100Mbps with half blinking frequeny of 1Gbps, 10Mbps
> of half blinking frequency of 100Mbps)
> - on other Marvell PHYs this is instead:
> 1Gpbs blinks 3 times, pause, 3 times, pause, ...
> 100Mpbs blinks 2 times, pause, 2 times, pause, ...
> 10Mpbs blinks 1 time, pause, 1 time, pause, ...
> - we don't need to differentiate these modes with different names,
> because the important thing is just that this mode allows the
> user to determine the speed from how the LED blinks
I'd be very careful. Userspace should know what they are asking
for. I'd propose simply ignoring this feature.
> mode to just force blinking - `blink`
We already have different support for blinking in LED subsystem. Lets use that.
Best regards,
Pavel
