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@earth.li http://www.earth.li/cgi-bin/mailman/listinfo/e3-hacking