On Mon, Nov 08, 2021 at 03:17:33PM +0100, Andrew Lunn wrote: > On Mon, Nov 08, 2021 at 01:24:58AM +0100, Ansuel Smith wrote: > > Add Offload Trigger for PHY Activity. This special trigger is used to > > configure and expose the different HW trigger that are provided by the > > PHY. Each offload trigger can be configured by sysfs and on trigger > > activation the offload mode is enabled. > > > > This currently implement these hw triggers: > > - blink_tx: Blink LED on tx packet receive > > - blink_rx: Blink LED on rx packet receive > > - blink_collision: Blink LED on collision detection > > When did you last see a collision? Do you really have a 1/2 duplex > link? Just because the PHY can, does not mean we should support > it. Lets restrict this to the most useful modes. > Ok will drop this. In my case (qca8k) I also never see a device using it so I agree on the fact that should be dropped. > > - link_10m: Keep LED on with 10m link speed > > - link_100m: Keep LED on with 100m link speed > > - link_1000m: Keep LED on with 1000m link speed > > - half_duplex: Keep LED on with half duplex link > > - full_duplex: Keep LED on with full duplex link > > - linkup_over: Keep LED on with link speed and blink on rx/tx traffic > > - power_on_reset: Keep LED on with switch reset > > > - blink_2hz: Set blink speed at 2hz for every blink event > > - blink_4hz: Set blink speed at 4hz for every blink event > > - blink_8hz: Set blink speed at 8hz for every blink event > > These seems like attributes, not blink modes. They need to be > specified somehow differently, or not at all. Do we really need them? > Sorry I didn't update the commit. In sysfs they are exposed as option like the power_on_reset and linkup_over. So they are option on how the LED behave on the event. > > - blink_auto: Set blink speed at 2hz for 10m link speed, > > 4hz for 100m and 8hz for 1000m > > Another attribute, and one i've not seen any other PHY do. > Yes we can consider dropping this but I think the other 3 should be keeped. > Andrew -- Ansuel
