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@xxxxxxxxxx> 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@xxxxxxxxxx> > > +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.
