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
