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? > + 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... > - asus->keyboard_rgb_mode.save = 1; > - asus->keyboard_rgb_mode.mode = 0; > - asus->keyboard_rgb_mode.speed = 0xeb; > + asus->keyboard_rgb_led.save = 1; > + asus->keyboard_rgb_led.mode = 0; > + asus->keyboard_rgb_led.speed = 0xeb; ...and this. Is it some kind of renaming? Can you split it to another patch if it was initially like that? -- With Best Regards, Andy Shevchenko
