> > +/* Expose sysfs for every blink to be configurable from userspace */ > > +DEFINE_OFFLOAD_TRIGGER(blink_tx, BLINK_TX); > > +DEFINE_OFFLOAD_TRIGGER(blink_rx, BLINK_RX); > > +DEFINE_OFFLOAD_TRIGGER(keep_link_10m, KEEP_LINK_10M); > > +DEFINE_OFFLOAD_TRIGGER(keep_link_100m, KEEP_LINK_100M); > > +DEFINE_OFFLOAD_TRIGGER(keep_link_1000m, KEEP_LINK_1000M); You might get warnings about CamelCase, but i suggest keep_link_10M, keep_link_100M and keep_link_1000M. These are megabits, not millibits. > > +DEFINE_OFFLOAD_TRIGGER(keep_half_duplex, KEEP_HALF_DUPLEX); > > +DEFINE_OFFLOAD_TRIGGER(keep_full_duplex, KEEP_FULL_DUPLEX); What does keep mean in this context? > > +DEFINE_OFFLOAD_TRIGGER(option_linkup_over, OPTION_LINKUP_OVER); > > +DEFINE_OFFLOAD_TRIGGER(option_power_on_reset, OPTION_POWER_ON_RESET); > > +DEFINE_OFFLOAD_TRIGGER(option_blink_2hz, OPTION_BLINK_2HZ); > > +DEFINE_OFFLOAD_TRIGGER(option_blink_4hz, OPTION_BLINK_4HZ); > > +DEFINE_OFFLOAD_TRIGGER(option_blink_8hz, OPTION_BLINK_8HZ); > > This is very strange. Is option_blink_2hz a trigger on itself? Or just > an option for another trigger? It seems that it is an option, so that I > can set something like > blink_tx,option_blink_2hz > and the LED will blink on tx activity with frequency 2 Hz... If that is > so, I think you are misnaming your macros or something, since you are > defining option_blink_2hz as a trigger with > DEFINE_OFFLOAD_TRIGGER Yes, i already said this needs handling differently. The 2Hz, 4Hz and 8Hz naturally fit the delay_on, delay_of sysfs attributes. Andrew