linux-kernel - Re: [PATCH v3] HID: lg-g15 - Add support for Logitech G13.

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] [day] [month] [year] [list]

Message-ID: <a6ea0b5d-7586-4529-bf91-d8b966aa986e@kernel.org>
Date: Wed, 10 Sep 2025 13:09:10 +0200
From: Hans de Goede <hansg@...nel.org>
To: "Leo L. Schwab" <ewhac@...ac.org>
Cc: Kate Hsuan <hpa@...hat.com>, Jiri Kosina <jikos@...nel.org>,
 Benjamin Tissoires <bentiss@...nel.org>, linux-input@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] HID: lg-g15 - Add support for Logitech G13.

Hi,

On 10-Sep-25 7:52 AM, Leo L. Schwab wrote:
> On Mon, Sep 08, 2025 at 11:08:29PM +0200, Hans de Goede wrote:
>> There are 2 improvements which I would like to see:
>>
>> 1. When the backlight is turned on through the button, you
>> should pass g15_led->brightness to the notify() call rather
>> then LED_FULL. GNOME will show an OSD with the new brightness
>> value shown as a mini progress bar similar to how it shows
>> speaker volume when doing mute/unmute. This mini progress
>> bar should show the actual brightness being restored, not
>> always full brightness.
>>
> 	If g15_led->brightness is subsequently changed, should a new
> notify() call also be made with that new brightness, i.e. should
> `hw_brightness_changed` be made to track `brightness`?

No, hw_brightness_changed only track changes done independently
by the hw. sysfs writes should not call notify().

> Indeed, it looks
> like you do this in `lg_g15_leds_changed_work()`.

That is for the original G15 and G15-v2, where the buttion
cycles through a couple of backlight levels (IIRC). That work
only gets queued when we receive a button press notification
and then it *reads* the new brightness from the keyboard and
uses that for the notify().

That work does not get queued/used for normal sysfs writes.

>> 2. ATM if the backlight is turned off on the G13 when
>> the driver loads and then one of the buttons gets pressed
>> then a notify() will happen because the led_cdev.hw_brightness_changed
>> value of -1 will be different from the value of 0 in the
>> input-report. This notify will lead to an unwanted OSD
>> notification in GNOME, so this needs to be fixed.
>> IMHO the best fix would be to use:
>>
>> 	hid_hw_raw_request(..., HID_INPUT_REPORT, HID_REQ_GET_REPORT);
>>
>> at probe to get the input-report so that the driver will
>> actually now the backlight state at probe() time without
>> needing to wait for the first time the input-report is send.
>>
> 	Will give this a try.
> 
>> I'll wait for your G13 support to land first and then
>> rebase the G510 patch on top.
>>
> 	Roger that.

Regards,

Hans