Re: [RFC PATCH v4 1/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04 devices

From: Werner Sembach
Date: Mon Oct 07 2024 - 12:18:08 EST



Am 04.10.24 um 16:46 schrieb Ilpo Järvinen:
On Fri, 4 Oct 2024, Werner Sembach wrote:
Am 03.10.24 um 12:54 schrieb Ilpo Järvinen:
On Wed, 2 Oct 2024, Werner Sembach wrote:
Am 02.10.24 um 11:52 schrieb Ilpo Järvinen:
On Tue, 1 Oct 2024, Werner Sembach wrote:

The TUXEDO Sirius 16 Gen1 and TUXEDO Sirius 16 Gen2 devices have a
per-key
controllable RGB keyboard backlight. The firmware API for it is
implemented
via WMI.

To make the backlight userspace configurable this driver emulates a
LampArray HID device and translates the input from hidraw to the
corresponding WMI calls. This is a new approach as the leds subsystem
lacks
a suitable UAPI for per-key keyboard backlights, and like this no new
UAPI
needs to be established.

v2: Integrated Armins feedback and fixed kernel test robot warnings.
v3: Fixed borked subject line of v2.
v4: Remove unrequired WMI mutex.
Move device checking from probe to init.
Fix device checking working exactly reverse as it should.
Fix null pointer dereference because, hdev->driver_data !=
hdev->dev.driver_data.

Co-developed-by: Christoffer Sandberg <cs@xxxxxxxxx>
Signed-off-by: Christoffer Sandberg <cs@xxxxxxxxx>
Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
Link:
https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@xxxxxxxxxxxxxxxxxxx/
---
That why i choose the rather generic names of just the input and output
length
because there is no semantic connection between the wmi methods in
tuxedo_nb04_wmi_8_b_in_80_b_out and tuxedo_nb04_wmi_496_b_in_80_b_out
respectively that would make for a good name.
So the only valuable characters are prefix + 8/496/80 the rest doesn't
really tell much despite all its characters :-). Details like which of the
numbers is in/out and that the numbers are in bytes could IMO be left to
struct's comment without loss of much information value.

tuxedo_nb04_wmi_8_80 kinda looks strange to me, what about
tuxedo_nb04_wmi_8_in_80_out? but that's on 4 chars shorter.
Perhaps just tuxedo_nb04_wmi_8in_80out ?
ok

I can see you like to use underscores a lot so I can understand if that
feels a step too far :-) (no offence meant).