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

[Luke Jones <luke@ljones.dev>]

Hi Andy,

On Tue, Aug 9 2022 at 10:52:21 +0200, Andy Shevchenko 
<andy.shevchenko@gmail.com> wrote:
> On Tue, Aug 9, 2022 at 4:51 AM Luke D. Jones <luke@ljones.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@ljones.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.