public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "dmitry.torokhov" <dmitry.torokhov@gmail.com>
To: lianzhi chang <changlianzhi@uniontech.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	jirislaby <jirislaby@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	282827961 <282827961@qq.com>, kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v20] tty: Fix the keyboard led light display problem
Date: Mon, 13 Dec 2021 21:40:25 -0800	[thread overview]
Message-ID: <YbguSZFyWGUt+Nwh@google.com> (raw)
In-Reply-To: <tencent_404C1E8253D7D34255D5026C@qq.com>

On Tue, Dec 14, 2021 at 10:06:38AM +0800, lianzhi chang wrote:
> > On Mon, Dec 13, 2021 at 02:12:44PM +0800, lianzhi chang wrote:
> > > > Use the "ctrl+alt+Fn" key combination to switch the system from tty to
> > > > +#define KDGKBLEDCTL  0x4B73  /* set whether to allow control of keyboard lights */
> > > > +#define KDSKBLEDCTL  0x4B74  /* get whether the keyboard light is currently allowed to be set */
> > >
> > > What userspace code is going to use these new ioctls?
> > >
> > > I still don't understand the problem that this is supposed to be
> > > solving.  How can we have never had a problem until now with regards to
> > > LED settings on keyboards?  What commit caused this to change?  Has it
> > > always been broken for 30 years and no one noticed?
> > 
> > Yes, it's been going on since forever (I guess at least 2.6 where input
> > core was introduced) and nobody really cared as very few people bounce
> > between graphical environment and VTs _and_ use CapsLock or NumLock
> > _and_ have keyboards with these LEDs).
> > 
> > Now, there is a couple-line solution that was in v19, lianzhi chang just
> > needed to drop condition that was being introduced in
> > vt_set_leds_compute_shiftstate(). That would ensure that proper state of
> > LEDs is restored on every VT switch. It also might have resulted in
> > LEDs switching momentarily off before turning on according to the
> > graphical desktop preferences, but I do not thing this is a big issue.
> 
> As you said, in the V19 version, there is a judgment condition in
>  vt_set_leds_compute_shiftstate(). If you delete it, there will be 
> some minor flaws. I always want to solve it perfectly. This may 
> be my selfish intention, but my ability is lacking, which has led 
> to iterating 21 versions.
> 
> > 
> > Now we are building this new infra with new ioctls, etc, for miniscule
> > gain of avoiding this blink. I do not believe this is worth it.
> If the current solution is not worth it, or the solution optimized for v19
> is easier to accept, I will go back to the v19 code to submit.

My recommendation would be to land the adjusted v19 (the one without the
conditional) that fixes original issue of LED state not being restored
on VT switch, and then as a followup, and if you so inclined, offer the
patch that introduces special ioctls to notify keyboard driver that
it should avoid touching LED state completely for a given VT, so that we
can discuss it separately and decide if such functionality is
needed/wanted.

Thanks.

-- 
Dmitry

      reply	other threads:[~2021-12-14  5:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13  6:12 [PATCH v20] tty: Fix the keyboard led light display problem lianzhi chang
2021-12-13 12:15 ` Andy Shevchenko
2021-12-13 12:41   ` Joe Perches
2021-12-13 19:41     ` Dmitry Torokhov
2021-12-13 20:06       ` Andy Shevchenko
2021-12-13 21:38         ` Dmitry Torokhov
2021-12-13 13:20 ` Dan Carpenter
2021-12-13 13:36 ` Greg KH
2021-12-13 19:39   ` Dmitry Torokhov
2021-12-14  2:06     ` lianzhi chang
2021-12-14  5:40       ` dmitry.torokhov [this message]

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=YbguSZFyWGUt+Nwh@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=282827961@qq.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=changlianzhi@uniontech.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.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