lkml.org 
[lkml]   [2021]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v20] tty: Fix the keyboard led light display problem
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
> desktop or switch the system from desktop to tty. After the switch is
> completed, it is found that the state of the keyboard lock is
> inconsistent with the state of the keyboard Led light.The reasons are
> as follows:
>
> * The desktop environment (Xorg and other services) is bound to a tty
> (assuming it is tty1), and the kb->kbdmode attribute value of tty1
> will be set to VC_OFF. According to the current code logic, in the
> desktop environment, the values of ledstate and kb->ledflagstate
> of tty1 will not be modified anymore, so they are always 0.
>
> * When switching between each tty, the final value of ledstate set by
> the previous tty is compared with the kb->ledflagstate value of the
> current tty to determine whether to set the state of the keyboard
> light. The process of switching between desktop and tty is also the
> process of switching between tty1 and other ttys. There are two
> situations:
>
> - (1) In the desktop environment, tty1 will not set the ledstate,
> which will cause when switching from the desktop to other ttys,
> if the desktop lights up the keyboard's led, after the switch is
> completed, the keyboard's led light will always be on;
>
> - (2) When switching from another tty to the desktop, this
> mechanism will trigger tty1 to set the led state. If other tty
> lights up the led of the keyboard before switching to the desktop,
> the led will be forcibly turned off. This situation should
> be avoided.
>
> * Current patch explanation:When VT is switched,the keyboard LED
> status will be set to the current tty saved value;the value of
> kb->kbdledctl can be used to confirm whether the current VT
> can change the status of the keyboard led light;kb->kbdledctl
> is a new addition,you can use ioctl get or set.
>
> Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

I believe the original issue hasn't been reported by above mentioned
CIs/people. If you want to give a credit, use changelog for that.

...

> +/**
> + * vt_do_kdskbledctl

Description?

> + * @console: the console to use
> + * @arg: the requested mode
> + *
> + * Whether to allow the current vt to change the
> + * keyboard light
> + */

...

> + struct kbd_struct *kb = &kbd_table[console];
> + int ret = 0;
> + unsigned long flags;

Slightly better to read:

struct kbd_struct *kb = &kbd_table[console];
unsigned long flags;
int ret = 0;

...

> +int vt_do_kdgkbledctl(unsigned int console)
> +{
> + struct kbd_struct *kb = &kbd_table[console];

+ Blank line here. Checkpatch should complain.

> + /* This is a spot read so needs no locking */
> + switch (kb->kbdledctl) {
> + case VC_LEDCTL_ON:
> + return K_LEDCTL_ON;
> + case VC_LEDCTL_OFF:
> + return K_LEDCTL_OFF;

> + }

Indentation issue.
Consider adding default case.

> +}

...

> + unsigned char kbdledctl:1; /*Whether to allow to control the led of the keyboard */

Missed space in the comment.

--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2021-12-13 13:16    [W:0.336 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site