On Friday 16 June 2017 17:46:58 Kai-Heng Feng wrote: > On Fri, Jun 16, 2017 at 3:52 PM, Pali Rohár <pali.rohar@xxxxxxxxx> wrote: > > Should not this check to be rather: > > > > (kbd_token_bits != 0 && (kbd_token_bits & BIT(KBD_LED_OFF_TOKEN)) != BIT(KBD_LED_OFF_TOKEN)) > > > > To express that we have at least one token at it is different from > > KBD_LED_OFF_TOKEN token? > > Yes, this expresses the intention more clearly. I'll use it instead. Err... second part of condition is wrong. It should be: (kbd_token_bits & ~BIT(KBD_LED_OFF_TOKEN)) (Remove off token bit and check that there is some other bit set too) > > > >> kbd_led_present = true; > >> } > >> > > > > And more important, there are three ways how to control keyboard > > backlight level: > > > > 1) Via SMBIOS token > > 2) Via SMBIOS call 4/11/0x2 (arg2, byte0) > > 3) Via SMBIOS call 4/11/0x2 (arg3, byte2) > > > > You are adding special case when only one SMBIOS toekn OFF is present > > which belongs to 1). > > > > Therefore there should be same check for 2) and 3) that there are more > > the one option to set... > > I am not familiar with SMBIOS call. > Can you point out where 2) and 3) functions are? See function kbd_led_level_set() (which calls kbd_set_level()). Also see comment above function kbd_get_info(). For 2) there is kbd_mode_levels_count and for 3) there is kbd_info.levels. -- Pali Rohár pali.rohar@xxxxxxxxx
