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: <64b1c076-f1f7-45a3-900a-dd52ab50cd4e@kernel.org>
Date: Wed, 10 Sep 2025 21:16:45 +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 8:02 PM, Leo L. Schwab wrote:
> On Wed, Sep 10, 2025 at 01:09:10PM +0200, Hans de Goede wrote:
>> 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().
>>
> 	Erm...  So brightness_hw_changed should only sample
> g15_led->brightness on first probe?
> 
> 	What should happen in this case:
> 
> 	* Driver loads, probes G13 backlight's current color, calculates
> 	  brightness to be 50, sets both `brightness` and
> 	  `brightness_hw_changed` sysfs values to 50.
> 	* User presses toggle key; backlight is now off.
> 	  `brightness_hw_changed` now set to 0.
> 	  `brightness` and RGB values remain unchanged.
> 	* User writes to `brightness` sysfs value, setting it to 255.  This
> 	  does *not* turn the backlight back on; `hw_brightness_changed`
> 	  remains unchanged.
> 	* User presses toggle key; backlight is back on, showing the
> 	  original color, but brighter.
> 
> 	What should brightness_hw_changed be updated to, if anything?

Since the driver writes any new values to the G13 and the G13 accepts
those and remembers them even when the backlight is off,
the notify() should be passed g15_led->brightness when an
off -> on transition happens (and 0 or LED_OFF for the on -> off
transition).

Since g15_led->brightness gets initialized by reading the actual
setting from the G13 at probe time and then gets updated on
any successful completion if writing a new brightness value
to the G13, it should always reflect the value which the backlight
will be set at by the G13 after an off -> on transition.

Or am I missing something ?

In case of your example step above then the notify() should
be passed 255 as brightness and that should also be the value
in g15_led->brightness since g15_led->brightness gets set
to the brightness send to the G13 hw on a successful setting
of the report, right ?

Regards,

Hans