lkml.org 
[lkml]   [2022]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 2/6] asus-wmi: Implement TUF laptop keyboard LED modes
    From
    Date
    Hi,

    On Tue, 2022-08-09 at 11:22 +0200, Andy Shevchenko wrote:
    > On Tue, Aug 9, 2022 at 4:51 AM Luke D. Jones <luke@ljones.dev> wrote:
    > >
    > > Adds support for changing the laptop keyboard LED modes. These
    > > are visible effects such as static, rainbow, pulsing, colour
    > > cycles.
    > >
    > > These sysfs attributes are added to asus-nb-wmi:
    > > - keyboard_rgb_save
    > > - keyboard_rgb_mode
    > > - keyboard_rgb_speed
    >
    > ...
    >
    > > +What:          /sys/devices/platform/<platform>/keyboard_rgb_speed
    > > +Date:          Aug 2022
    > > +KernelVersion: 6.1
    > > +Contact:       "Luke Jones" <luke@ljones.dev>
    > > +Description:
    > > +               Set the speed of the selected RGB effect, the speed
    > > will not apply
    > > +               until the keyboard_rgb_save attribute is set
    > > (write-only):
    > > +                       * 0 - slow
    > > +                       * 1 - medium
    > > +                       * 2 - fast
    >
    > > \ No newline at end of file
    >
    > ^^^
    >
    > ...
    >
    > > +       u8 save;
    > > +       int err;
    >
    > > +
    > > +       struct asus_wmi *asus = dev_get_drvdata(device);
    > > +       struct led_classdev *cdev = &asus-
    > > >keyboard_rgb_led.dev.led_cdev;
    >
    > No blank line in the definition block and try to keep "the longest
    > line first", a.k.a. reversed xmas tree ordering.
    >
    > ...
    >
    > > +       u8 mode;
    > > +
    > > +       struct asus_wmi *asus = dev_get_drvdata(device);
    >
    > Ditto.
    >
    > I would really recommend you spending some time to read the existing
    > code (better recent one) and look for the common patterns.
    >
    > ...
    >
    > > +       /* These are the known usable modes across all TUF/ROG */
    > > +       if (mode >= 12 || mode == 10)
    >
    > The second condition was different in previous versions. Or am I
    > confused by another patch series?
    >

    It's a mistake on my part..

    > > +         asus->keyboard_rgb_led.mode = 10;
    > > +       else
    > > +         asus->keyboard_rgb_led.mode = mode;
    >
    > ...
    >
    > > +
    > > +
    >
    > Single blank line is enough.
    >
    > ...
    >
    > > -               struct mc_subled *mc_led_info = asus-
    > > >keyboard_rgb_mode.subled_info;
    > > -               struct led_classdev_mc *mc_cdev = &asus-
    > > >keyboard_rgb_mode.dev;
    > > +               struct mc_subled *mc_led_info = asus-
    > > >keyboard_rgb_led.subled_info;
    > > +               struct led_classdev_mc *mc_cdev = &asus-
    > > >keyboard_rgb_led.dev;
    >
    > Not sure why this change happened...

    I thought I wrote this in changelog. Either way, it is meant to be in
    first patch. git can be damned frustrating to use.

    I will try to fix all these little things and let the series bake at
    least a week before next version.

    Kind regards,
    Luke.


    \
     
     \ /
      Last update: 2022-08-09 11:31    [W:9.536 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site