linux-kernel - Re: [PATCH v3 1/1] HP: wmi: added support for 4 zone keyboard rgb

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: <bb74a97e-3f25-4d56-9733-46e5325c47e7@redhat.com>
Date: Thu, 11 Jul 2024 17:17:23 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Carlos Ferreira <carlosmiguelferreira.2003@...il.com>
Cc: platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] HP: wmi: added support for 4 zone keyboard rgb

Hi Ilpo,

Thank you for reviewing this.

On 7/11/24 10:41 AM, Ilpo Järvinen wrote:
> On Sun, 7 Jul 2024, Carlos Ferreira wrote:
> 
>> This driver adds supports for 4 zone keyboard rgb on omen laptops
>> and maps the wmi backlight toggle event to KEY_KBDILLUMTOGGLE.
>> For the backlight, it uses the multicolor led api.
>>
>> Tested on the HP Omen 15-en1001np.
>>
>> Signed-off-by: Carlos Ferreira <carlosmiguelferreira.2003@...il.com>
>> ---
>> Changes in v3:
>>  - Moved to the multicolor led api
>>  - Mapped the wmi backlight toggle event to KEY_KBDILLUMTOGGLE
>>  - Some other minor changes
>> Changes in v2:
>>  - Rearranged code to remove forward declarations
>>  - Changed from sprintf() to sysfs_emit()
>>  - Fixed some identation and coding style problems
>>  - Switched from manual bit manipulation to GENMASK(x, y) + FIELD_PREP(XX, )
>>  - #define'ed magic constants
>> ---
>>  drivers/platform/x86/hp/hp-wmi.c | 248 +++++++++++++++++++++++++++++--
>>  1 file changed, 239 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
>> index 5fa553023842..5eae47961f76 100644
>> --- a/drivers/platform/x86/hp/hp-wmi.c
>> +++ b/drivers/platform/x86/hp/hp-wmi.c
>> @@ -14,6 +14,8 @@
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>  
>>  #include <linux/kernel.h>
>> +#include <linux/led-class-multicolor.h>
>> +#include <linux/leds.h>
>>  #include <linux/module.h>
>>  #include <linux/init.h>
>>  #include <linux/slab.h>
>> @@ -24,6 +26,7 @@
>>  #include <linux/platform_profile.h>
>>  #include <linux/hwmon.h>
>>  #include <linux/acpi.h>
>> +#include <linux/bits.h>
> 
> You need to add #include <linux/bitfield.h> as LKP found out. The 
> MODULE_DESCRIPTION() ones you can ignore as LKP unfortunately spams some 
> kernel-wide problems to all patches even if they're entirely unrelated
> to the patch in question.

Actually the LKP / kernel test robot email with the MODULE_DESCRIPTION()
warnings does contain useful, new errors. The problem though is that
the bot prefixes new (not seen on builds without this patch) messages
with '>>' making them show up as quoted (replied-to) text making the
actual new errors harder to spot :|   The 2 new actual errors in the
email with all the MODULE_DESCRIPTION warnings are:

ERROR: modpost: "devm_led_classdev_multicolor_unregister" [drivers/platform/x86/hp/hp-wmi.ko] undefined!
ERROR: modpost: "devm_led_classdev_multicolor_register_ext" [drivers/platform/x86/hp/hp-wmi.ko] undefined!

I agree with all the other review remarks and I'll do my own review
soon.

Regards,

Hans