Messages in this thread | | | Date | Mon, 13 Dec 2021 14:15:24 +0200 | From | Andy Shevchenko <> | Subject | Re: [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
| |