linux-kernel - Re: [PATCH v3 3/6] asus-wmi: Implement TUF laptop keyboard power states

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 09 Aug 2022 20:58:12 +1200
From:   Luke Jones <luke@...nes.dev>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Hans de Goede <hdegoede@...hat.com>,
        Barnabás Pőcze <pobrn@...tonmail.com>,
        Pavel Machek <pavel@....cz>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/6] asus-wmi: Implement TUF laptop keyboard power
 states

Hi Andy,

On Tue, Aug 9 2022 at 10:52:21 +0200, Andy Shevchenko 
<andy.shevchenko@...il.com> wrote:
> On Tue, Aug 9, 2022 at 4:51 AM Luke D. Jones <luke@...nes.dev> wrote:
>> 
>>  Adds support for setting various power states of TUF keyboards.
>>  These states are combinations of:
>>  - boot, set if a boot animation is shown on keyboard
>>  - awake, set if the keyboard LEDs are visible while laptop is on
>>  - sleep, set if an animation is displayed while the laptop is 
>> suspended
>>  - keyboard (unknown effect)
>> 
>>  Adds two sysfs attributes to asus-nb-wmi:
>>  - keyboard_rgb_state
>>  - keyboard_rgb_state_index
> 
> ...
> 
>>                  until the keyboard_rgb_save attribute is set 
>> (write-only):
>>                          * 0 - slow
>>                          * 1 - medium
>>  -                       * 2 - fast
>>  \ No newline at end of file
> 
> ^^^
> 
>>  +                       * 2 - fast
>>  +
>>  +What:          /sys/devices/platform/<platform>/keyboard_rgb_state
>>  +Date:          Aug 2022
>>  +KernelVersion: 6.1
>>  +Contact:       "Luke Jones" <luke@...nes.dev>
>>  +Description:
>>  +               Set some RGB keyboard power states (write-only).
>>  +
>>  +               The accepted input is "boot awake sleep keyboard", 
>> where "n n n n n"
>>  +               options are:
>>  +                       * save - 0 or 1, if 0 then settings are not 
>> retained on boot
>>  +                       * boot - 0 or 1, controls if a boot 
>> animation is shown
>>  +                       * awake - 0 or 1, controls if the keyboard 
>> LED are on during awake
>>  +                       * sleep - 0 or 1, controls if a suspended 
>> animation is shown.
>>  +                                               This is only active 
>> if the AC is connected.
>>  +                       * keyboard - 0 or 1, unknown what effect 
>> this really has
>>  \ No newline at end of file
> 
> ^^^
> 
> Be sure of not repeating the same mistake. I.e. add a newline as 
> suggested.
> 
> 
>>  +       flags = 0;
> 
> Seems you ignored my comment here...
> 
>>  +       if (sscanf(buf, "%hhd %hhd %hhd %hhd %hhd", &save, &boot, 
>> &awake, &sleep, &keyboard) != 5
>>  +               return -EINVAL;
> 
>>  +       save = save == 0 ? 0x0100 : 0x0000;
> 
> ...and here...
> 
>>  +       if (boot)
>>  +               flags |= 0x02;
>>  +       if (awake)
>>  +               flags |= 0x08;
>>  +       if (sleep)
>>  +               flags |= 0x20;
>>  +       if (keyboard)
>>  +               flags |= 0x80;
> 
> ...and here.
> 

Umm... I know for sure I fixed all those. I must have screwed up a git 
rebase :(
I'll fix again.