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

[Dmitry Torokhov]

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?

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.

I think we need better understand what is going on here. Maybe post the
entire communication between the kernel and i8042?

Thanks.

-- 
Dmitry