Messages in this thread | | | Date | Fri, 5 Nov 2021 15:36:55 -0700 | From | Dmitry Torokhov <> | Subject | Re: [PATCH v12] tty: Fix the keyboard led light display problem |
| |
Hi lianzhi On Fri, Nov 05, 2021 at 09:48:16PM +0800, lianzhi chang wrote: > Switching from the desktop environment to the tty environment, > the state of the keyboard led lights and the state of the keyboard > lock are inconsistent. This is because the attribute kb->kbdmode > of the tty bound in the desktop environment (Xorg) is set to > VC_OFF, which causes the ledstate and kb->ledflagstate > values of the bound tty to always be 0, which causes the switch > from the desktop When to the tty environment, the LED light > status is inconsistent with the keyboard lock status. > In order to ensure that the keyboard LED lights are displayed > normally during the VT switching process, when the VT is > switched, the current VT LED configuration is forced to be issued. > > Signed-off-by: lianzhi chang <changlianzhi@uniontech.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v10: > The led state of the input device is no longer synchronized to > ledstate, and the related code is deleted. The current plan is > changed to: when the VT is switched, the LED state saved by the > current VT is forced to be issued. > v11: > Supplement the signature of the collaborator. > > drivers/tty/vt/keyboard.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c > index c7fbbcdcc346..20013c45b979 100644 > --- a/drivers/tty/vt/keyboard.c > +++ b/drivers/tty/vt/keyboard.c > @@ -153,6 +153,7 @@ static int shift_state = 0; > > static unsigned int ledstate = -1U; /* undefined */ > static unsigned char ledioctl; > +static bool vt_switch; > > /* > * Notifier list for console keyboard events > @@ -412,8 +413,12 @@ static void do_compute_shiftstate(void) > /* We still have to export this method to vt.c */ > void vt_set_leds_compute_shiftstate(void) > { > + struct kbd_struct *kb; > unsigned long flags; > > + kb = kbd_table + fg_console; > + if (kb->kbdmode != VC_OFF) > + vt_switch = true;
Could you please add explanation here why it is not racy (I do not think it is as I believe currently executed tasklets can be scheduled).
> set_leds(); > > spin_lock_irqsave(&kbd_event_lock, flags); > @@ -1247,14 +1252,24 @@ void vt_kbd_con_stop(unsigned int console) > */ > static void kbd_bh(struct tasklet_struct *unused) > { > + struct kbd_struct *kb; > unsigned int leds; > unsigned long flags; > > + kb = kbd_table + fg_console; > + if (kb->kbdmode == VC_OFF) > + return;
Why do we need to do this? Won't this stop setting arbitrary LEDs via console ioctl?
> + > spin_lock_irqsave(&led_lock, flags); > leds = getleds(); > leds |= (unsigned int)kbd->lockstate << 8; > spin_unlock_irqrestore(&led_lock, flags); > > + if (vt_switch) { > + ledstate = ~leds; > + vt_switch = false; > + } > + > if (leds != ledstate) { > kbd_propagate_led_state(ledstate, leds); > ledstate = leds; > @@ -1643,6 +1658,8 @@ int __init kbd_init(void) > int i; > int error; > > + vt_switch = false;
No need to explicitly initialize it here, as a static variable it will be automatically initialized to 0 (false).
> + > for (i = 0; i < MAX_NR_CONSOLES; i++) { > kbd_table[i].ledflagstate = kbd_defleds(); > kbd_table[i].default_ledflagstate = kbd_defleds();
Thanks.
-- Dmitry
| |