Re: [PATCH] platform/x86: acer-wmi: Add support for Acer Helios 300 RGB keyboard backlight

[Pavel Machek]

Hi!

> > From e65b0ddbf559aa3ca8a7998404e7e67e64b705e9 Mon Sep 17 00:00:00 2001
> > From: JafarAkhondali <jafar.akhoondali@xxxxxxxxx>
> > Date: Fri, 14 May 2021 08:26:47 +0430
> > Subject: [PATCH] platform/x86: acer-wmi: Add support for Acer Helios
> > 300 rgb keyboard backlight
> > 
> > The Acer helios 300 provides gaming functions WMI that is available in
> > Windows, however this was not implemented in Linux. The process of finding
> > the related method was done by decompiling PredatorSense(official Acer
> > gaming functions software for Predator series) and decompiling WQ
> > buffers. This patch provides a gaming interface which will then expose a
> > character device named "acer-gkbbl". This character device accepts 16
> > bytes long config, which is specific for the backlight method. The
> > meaning of each bytes ordered by bit position is as follows:
> > 
> > Bit 0 -> Backlight modes:
> > 1: Breath
> > 2: Neon
> > 3: Wave
> > 4: Shifting
> > 5: Zoom
> > Bit 1 -> Animation Speed: from 1 to 9 ( 1 is slowest, 9 is fastest)
> > Bit 2 -> Brightness from 0 to 100 ( 0 is no backlight, 100 is brightest)
> > Bit 3 -> Unknown. Wave effect uses 8, other modes must use 0
> > Bit 4 -> Animation Direction:
> > 1: Right-to-Left
> > 2: Left-to-Right
> > Bit 5 -> Red Color Selection
> > Bit 6 -> Green Color Selection
> > Bit 7 -> Blue Color Selection
> > Bit 8 -> Currently unknown, or not used in known model
> > Bit 9 -> Currently unknown, or not used in known model
> > Bit 10 -> Currently unknown, or not used in known model
> > Bit 11 -> Currently unknown, or not used in known model
> > Bit 12 -> Currently unknown, or not used in known model
> > Bit 13 -> Currently unknown, or not used in known model
> > Bit 14 -> Currently unknown, or not used in known model
> > Bit 15 -> Currently unknown, or not used in known model
> > 
> > Filling this config is out of scope for the kernel module, and this module
> > only acts as an interface.
> > 
> > Currently, I'm not sure with the method for communicating with user-space,
> > but since leds.h subsystem wouldn't fit for complex actions such as this
> > complex config, I couldn't find any better method than char dev.
> 
> Thank you for your patch, given that there is no existing kernel
> interface which is a good match for the features exported by this
> keyboard I'm fine with just having a raw interface where userspace
> writes GAMING_KBBL_CONFIG_LEN bytes as you suggest.

Keyboard backlight goes through LED interface (so please cc the mailing list) and
no, passing raw bytes to hardware is not an acceptable interface.

> But lets not use a classdev + chardev for this please, you can
> just add a binary write-only sysfs-atrribute under the wmi-dev for
> this with a name like (for example) gaming_kbd_backlight_config
> and then userspace can write to that without needing a class + char
> dev just for this single write.

NAK. We have existing interfaces for this.

Best regards,
								Pavel