linux-kernel - Re: [PATCH 0/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04

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: <et3cv7i2lhsjoq26toweh4uv72yo34u3wqrj3q2urfnx2bhiq3@fdtkag4bcekh>
Date: Fri, 27 Sep 2024 18:08:52 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Werner Sembach <wse@...edocomputers.com>
Cc: dri-devel@...ts.freedesktop.org, hdegoede@...hat.com, jelle@...aa.nl, 
	jikos@...nel.org, lee@...nel.org, linux-input@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org, miguel.ojeda.sandonis@...il.com, 
	ojeda@...nel.org, onitake@...il.com, pavel@....cz
Subject: Re: [PATCH 0/1] platform/x86/tuxedo: Add virtual LampArray for
 TUXEDO NB04

On Sep 26 2024, Werner Sembach wrote:
> Hi,
> took some time but now a first working draft of the suggested new way of
> handling per-key RGB keyboard backlights is finished. See:
> https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@tuxedocomputers.com/
> First time for me sending a whole new driver to the LKML, so please excuse
> mistakes I might have made.
> 
> Known bugs:
> - The device has a lightbar which is currently not implemented and
>   therefore stuck to blue once the first backlight control command is send.
> 
> What is still missing:
> - The leds fallback
> - Lightbar control
> 
> Some general noob questions:
> 
> Initially I though it would be nice to have 2 modules, one jsut being the
> wmi initialization and utility stuff and one just being the backlight logic
> stuff, being loaded automatically via module_alias, but that would still
> require me to create the virtual hid device during the wmi_ab probe, and
> that already needs the ll_driver, so i guess I have to do it statically
> like i did now?
> Or in other words: I would have liked to have a module dependency graph
> like this:
>     tuxedo_nb04_lamp_array depends on tuxedo_nb04_platform (combining *_wmi_init and *_wmi_utility)
> but if i currently split it into modules i would get this:
>     tuxedo_nb04_wmi_ab_init dpends on tuxedo_nb04_wmi_ab_lamp_array depends on tuxedo_nb04_wmi_utility

On more general question to you: how much confident are you about your
LampArray implementation?

If you still need to add/fix stuff in it, I would advise you to have a
simple HID device, with bare minimum functionality, and then add the
LampArray functionality on top through HID-BPF. This way you can fix
LampArray out of band with the kernel, while having a more stable kernel
module. This should be possible with v6.11+.

Another solution is to still have your wmi-to-hid module, and then a
HID kernel module in drivers/hid that supports LampArray.

But I would strongly suggest while you are figuring out the userspace
part to stick to HID-BPF, and then once you are happy we can move to a
full kernel module.

Cheers,
Benjamin

> 
> Currently after creating the virtual hdev in the wmi init probe function I
> have to keep track of it and manually destroy it during the wmi init
> remove. Can this be automated devm_kzalloc-style?
> 
> Kind regards,
> Werner Sembach
> 
>