Re: [RFC PATCH v2 1/2] can: add tx/rx LED trigger support

[Fabio Baltieri <fabio.baltieri@gmail.com>]

Hi Wolfgang,

On Tue, Apr 24, 2012 at 08:46:29AM +0200, Wolfgang Grandegger wrote:
> I still think that the blinking support should go to the timer class to
> avoid duplicated code. Any good reason against? Apart from that the
> patches look good.
> 
> Wolfgang.

I can see you point and I considered your note about adding the
one-shot-blink function to the led-class framework (sorry for not
mentioning it in my first post).  Still, I ended up with this code for
a couple of reasons:

- I think that the led_blink_set function is primarily used to configure
  leds with hardware blinking (like i2c led drivers).  While it would be
  possible to extend the function to get one-shot behavior and always
  fallback on software blink, I think that that's out of the purpose of
  the led-class, which should just translate on-off requests to
  underlaying hardware.

- I think that different drivers may want to obtain different on-off
  behavior depending on the application.  For example in the ide-disk
  case the user expects to see a steady-on LED on constant activity,
  and that's how it's implemented, while in this case the on-if-up
  keep-blinking-on-activity off-if-down makes much more sense. So I think
  that even if a generic blink function were available, people would
  still be using custom functions because they want to fine tune the
  behavior for the application.  Also, maybe a function too generic may
  impact on performance in critical paths.

- in this case, it looks to me like the implementation is as optimized
  as it can be, in the sense that the hot-path does really only some
  essential check and engage the timer and the timer function itself is
  really short.  Also the final blinking effect is nice IMO :-)

Regards,
Fabio