public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Qiang Ma <maqianga@uniontech.com>
Cc: hdegoede@redhat.com, inux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Input: atkbd - fix LED state at suspend/resume
Date: Fri, 26 Jul 2024 14:57:03 -0700	[thread overview]
Message-ID: <ZqQbr8aZnaYi20Dp@google.com> (raw)
In-Reply-To: <20240726102730.24836-1-maqianga@uniontech.com>

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

  parent reply	other threads:[~2024-07-26 21:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-26 10:27 [PATCH] Input: atkbd - fix LED state at suspend/resume Qiang Ma
2024-07-26 10:41 ` Qiang Ma
2024-07-26 10:43 ` Qiang Ma
2024-07-26 14:43 ` Christophe JAILLET
2024-07-30  2:36   ` Qiang Ma
2024-07-26 21:57 ` Dmitry Torokhov [this message]
2024-07-26 22:01   ` Dmitry Torokhov
2024-07-30  2:17   ` Qiang Ma
     [not found]   ` <20240730101708.50fa6734@john-PC>
2024-08-02  7:36     ` Qiang Ma
2024-08-05  7:51   ` Qiang Ma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZqQbr8aZnaYi20Dp@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=inux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maqianga@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox