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

[Luke Jones]

Hi Andy,

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