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

[Qiang Ma <maqianga@uniontech.com>]

On Fri, 26 Jul 2024 14:57:03 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.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?
> 
> Thanks.
> 

---
Qiang