linux-kernel - Re: [PATCH] Input: atkbd - fix LED state at suspend/resume

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

Message-ID: <5CC999395DCE8ABD+20240802153624.02dfe6a4@john-PC>
Date: Fri, 2 Aug 2024 15:36:24 +0800
From: Qiang Ma <maqianga@...ontech.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: hdegoede@...hat.com, linux-input@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: atkbd - fix LED state at suspend/resume

On Tue, 30 Jul 2024 10:17:08 +0800
Qiang Ma <maqianga@...ontech.com> wrote:

> On Fri, 26 Jul 2024 14:57:03 -0700
> Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
> 
> Hi Dmitry,
> 
> > Hi Qiang,
> > 
> > On Fri, Jul 26, 2024 at 06:27:30PM +0800, Qiang Ma wrote:  
> > > After we turn on the keyboard CAPSL LED and let the system
> > > suspend, the keyboard LED is not off, and the kernel log is as
> > > follows:
> > > 
> > > [  185.987574] i8042: [44060] ed -> i8042 (kbd-data)
> > > [  185.988057] i8042: [44061] ** <- i8042 (interrupt, 0, 1)
> > > [  185.988067] i8042: [44061] 04 -> i8042 (kbd-data)
> > > [  185.988248] i8042: [44061] ** <- i8042 (interrupt, 0, 1)
> > > 
> > > The log shows that after the command 0xed is sent, the data
> > > sent is 0x04 instead of 0x00.
> > > 
> > > Solution:
> > > Add a bitmap variable ledon in the atkbd structure, and then set
> > > ledon according to code-value in the event, in the atkbd_set_leds
> > > function, first look at the value of lenon, if it is 0, there is
> > > no need to look at the value of dev->led, otherwise, Need to look
> > > at dev->led to determine the keyboard LED on/off.    
> > 
> > I am not sure why duplicating input_dev->led which is supposed to
> > record which LEDs are currently active on an input device would
> > solve the issue. Could you please explain?  
> 
> At this point, the suspend purpose is to turn off the led(calling
> input_dev_toggle(input_dev, false) in function input_dev_suspend()),
> but the dev->led gets the status is on, so it will not turn off the
> led.
> 
> > 
> > The input core is supposed to turn off all LEDs on suspend. This
> > happens in input_dev_toggle() which is called from
> > input_dev_suspend(). It iterates over all LEDs on a device and turns
> > off active ones one by one.
> > 
> > I think what happens here is we are running afoul of the throttling
> > done in atkbd (see atkbd_schedule_event_work), and it does not
> > actually turn off all LEDs in time. But on the other hand
> > atkbd_cleanup() (which is called to suspend the keyboard) calls
> > 
> > 	ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_DEF);
> > 
> > which should turn off everything anyways.  
> 
> Send reset command under the actual test did not turn off the led, get
> it just reset the i8042 register? Did not change the led corresponding
> gpio status?
> 
> > 
> > I think we need better understand what is going on here. Maybe post
> > the entire communication between the kernel and i8042?
> > 

Description of ps/2 keyboard interface on this
website(http://www-ug.eecg.toronto.edu/msl/nios_devices/datasheets/PS2%20Keyboard%20Protocol.htm):

0xF6 (Set Default) - Load default typematic rate/delay (10.9cps /
500ms), key types (all keys typematic/make/break), and scan code set
(2).

It seems that sending this F6 reset command does not mention what
effect it has on the LED.

> > Thanks.
> >   
> 
> ---
> Qiang