linux-kernel - Future handling of complex RGB devices on Linux v2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <247b5dcd-fda8-45a7-9896-eabc46568281@tuxedocomputers.com>
Date: Wed, 21 Feb 2024 12:12:38 +0100
From: Werner Sembach <wse@...edocomputers.com>
To: Hans de Goede <hdegoede@...hat.com>, Pavel Machek <pavel@....cz>
Cc: Lee Jones <lee@...nel.org>, jikos@...nel.org,
 linux-kernel@...r.kernel.org, Jelle van der Waa <jelle@...aa.nl>,
 Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 linux-input@...r.kernel.org, ojeda@...nel.org, linux-leds@...r.kernel.org
Subject: Future handling of complex RGB devices on Linux v2

Hi,

so after more feedback from the OpenRGB maintainers I came up with an even more 
generic proposal: 
https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869

Copy pasting the relevant part:

 >Another, yet more generic, approach:
 >
 >```
 >get-device-info ioctl returning:
 >{
 >    char name[64]                /* Device model name / identifier */
 >    enum device_type            /* e.g. keyboard, mouse, lightbar, etc. */
 >    char firmware_version_string[64]    /* if known to the driver, empty 
otherwise */
 >    char serial_number[64]            /* if known to the driver, empty 
otherwise */
 >    enum supported_commands[128]        /* comands supported by the firmware */
 >}
 >
 >evaluate-set-command ioctl taking:
 >{
 >    enum command                /* one of supported_commands */
 >    union data
 >    {
 >        char raw[3072],
 >        {
 >            <input struct for command 0>
 >        },
 >        {
 >            <input struct for command 1>
 >        },
 >        ...
 >    }
 >}
 >
 >evaluate-get-command ioctl taking:
 >{
 >    enum command                /* one of supported_commands */
 >    union data
 >    {
 >        char raw[3072],
 >        {
 >            <input struct for command 0>
 >        },
 >        {
 >            <input struct for command 1>
 >        },
 >        ...
 >    }
 >}
 >and returning:
 >{
 >    union data
 >    {
 >        char raw[3072],
 >        {
 >            <return struct for command 0>    /* not every command might have 
one */
 >        },
 >        {
 >            <return struct for command 1>    /* not every command might have 
one */
 >        },
 >        ...
 >    }
 >}
 >```
 >
 >- char name[64] still includes, if know to the driver, information about 
physical or even printed layout.
 >- differentiation between evaluate-set-command and evaluate-get-command is 
mainly there for performance optimization for direct mode (for 
evaluate-set-command the kernel does not have to copy anything back to userspace)
 >- commands without a return struct must not be used with evaluate-get-command
 >- the input struct might be empty for very simple commands (or "int unused" to 
not confuse the compiler if neccessary)
 >
 >Now is the question: How does userspace know which commands takes/returns 
which structs? Define them in one big header file (as struct 
clevo_set_breathing_mode_1_input, struct tongfang_set_breathing_mode_1_input, 
etc.), or somehow dynamicaly? I'm warming up to Hans suggestion to just do it 
statically, unlike my suggestion yesterday.
 >
 >Min/Max values are documented in the header file (if not implied by variable 
type). With different max value -> different command, e.g. 
clevo_set_breathing_mode_1 for devices with speed from 0 to 7 and 
clevo_set_breathing_mode_2 for devices with speed from 1 to 10.

But at this point it is almost a generic interface that can be used to expose 
anything to userspace, looping back to the sanitized-wmiraw idea that was 
floating around earlier.

So a new approach (Please correct me if there is already something similar I'm 
not aware of):

New subsystem "Platform Device Commands" (short platdevcom) (I'm open for better 
name suggestions):

- Registers /sys/class/platdevcom/platdevcom[0-9]* (similar to hidraw)
- Has get-device-info ioctl, evaluate-set-command ioctl, and 
evaluate-get-command ioctl as described above
- device_type enum entries for rgb would be for example rgbleds_keyboard, 
rgbleds_mouse, etc.

On a high level this subsystem can be used to expose any platform functionality 
to userspace that doesn't fit another subsystem in a central location. This 
could be for example a nearly 1 to 1 sanitized mapping to wmi calls. Or writing 
a specific EC register to control OEM BIOS features like flexi charging (only 
charge battery to specific percentage to extend the live).

However I am aware that this is hardly an api. So Maybe it's best to just fall 
back on extending the leds subsystem with the deactivate command, and from there 
just implement the few rgb devices that are not hidraw as misc devices in a per 
OEM fasion without a unified api.