[E3-hacking] [PATCH v2] LEDS: Add output invertion option to backlight trigger

[Janusz Krzysztofik]

Ralph Corderoy wrote:
> Hi Janusz,
> 
>>     struct led_classdev *led = n->led;
>>     struct fb_event *fb_event = data;
>>     int *blank = fb_event->data;
>>     int new_status = *blank ? BLANK : UNBLANK;
>>   
>>     switch (event) {
>>     case FB_EVENT_BLANK :
>>         if (new_status == n->old_status)
>>             break;
>>   
>>         if ((n->old_status == UNBLANK) ^ n->invert) {
>>             n->brightness = led->brightness;
>>             led_set_brightness(led, LED_OFF);
>>         } else {
>>             led_set_brightness(led, n->brightness);
>>         }
>>   
>>         n->old_status = new_status;
>>   
>>         break;
>>     }
>>   
>>     return 0;
> 
> This is a general question rather than a comment on the patch.  My gcc
> for x86 with -O3 produces code that sets up led, blank, and new_status
> before testing if event == FB_EVENT_BLANK.  Clearly, none of that work
> is needed if the condition is false.
> 
> Is it assumed in kernel coding that the compiler is clever enough not to
> do this, or that the extra work doesn't matter?


Hmm, I've never been wondering about it, but I guess the former is more 
likely. Anyways, this is rather a common practice in kernel coding.


BTW, regarding your previous comment on declaring the newly added 
unsigned .invert, as well as the already present int .old_space as 
booleans in order to conserve space:

1. I use sscanf for assigning a user provided value to .invert, so its
    type must be one of those supported by sscanf. I could sscanf to a
    local variable and than copy the value, but I'm not sure if this
    would still conserve space.
2. To introduce a change that is not related directly to my patch
    subject matter (ie. change type of the .old_status), I would have to
    do it with a separate patch.

Cheers,
Janusz


> Cheers,
> Ralph.
> 
> 
> _______________________________________________
> e3-hacking mailing list
> e3-hacking at earth.li
> http://www.earth.li/cgi-bin/mailman/listinfo/e3-hacking
>