FYI: ACPI 'sleep 1' resets atkbd keycodes
Subsystems (2)
This may be just a minor issue:
I had to use the setkeycodes utility to enable some extra keys (that weren't mapped by kernel's atkbd tables).
After waking from sleep 1, those keys were reset. That is, I had to use 'setkeycodes' again to customize the tables again.
IMHO the way kernel works now is correct. It should *not* have extra code just to handle that. Just make sure anybody that alters his kbd tables puts some extra script to recover the tables after an ACPI wake.
This should be more like a note to Linux distributors.
Have a nice day.
-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
Quoted 10 lines · P. Christeas
This may be just a minor issue:
I had to use the setkeycodes utility to enable some extra keys (that weren't mapped by kernel's atkbd tables).
After waking from sleep 1, those keys were reset. That is, I had to use 'setkeycodes' again to customize the tables again.IMHO the way kernel works now is correct. It should *not* have extra code just to handle that. Just make sure anybody that alters his kbd tables puts some extra script to recover the tables after an ACPI wake.
This should be more like a note to Linux distributors.
Hmm, I, on the other hand don't think it's correct. Because the kernel has extra code to delete the tables on wake, which is wrong. Thanks for noticing.
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR
Quoted 6 lines · P. Christeas
This may be just a minor issue:
I had to use the setkeycodes utility to enable some extra keys (that weren't mapped by kernel's atkbd tables).
After waking from sleep 1, those keys were reset. That is, I had to use 'setkeycodes' again to customize the tables again.
Quoted 6 lines
IMHO the way kernel works now is correct. It should *not* have extra code just to handle that. Just make sure anybody that alters his kbd tables puts some extra script to recover the tables after an ACPI wake.
This should be more like a note to Linux distributors.Have a nice day.
Patch attached, please test. It'll make it into 2.6.3, with some luck even 2.6.2.
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR
[email protected], 2004-01-25 12:58:24+01:00, [email protected]
input: Bail out in atkbd.c if scancode set is changed, don't
reinitialize scancode map. This is even more anoying than a new keyboard device in the unlikely case of set change.
atkbd.c | 37 ++++++------------------------------- 1 files changed, 6 insertions(+), 31 deletions(-)
drivers/input/keyboard/atkbd.c +6 -31
@@ -743,43 +743,18 @@struct serio_dev *dev = serio->dev;int i; -if (!dev) { -printk (KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n" ); -return -1 ; - } +if (!dev) { +printk (KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n" ); +return -1 ; + }if (atkbd->write) {if (atkbd_probe (atkbd))return -1 ; - - atkbd->set =atkbd_set_3 (atkbd); +if (atkbd->set !=atkbd_set_3 (atkbd)) +return -1 ;atkbd_enable (atkbd); - }else { - atkbd->set =2 ; - atkbd->id =0xab00 ; } - -/* -* Here we probably should check if the keyboard has the same set that -* it had before and bail out if it's different. But this will most likely -* cause new keyboard device be created... and for the user it will look -* like keyboard is lost -*/ - -if (atkbd->translated) { -for (i =0 ; i <128 ; i++) { - atkbd->keycode[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]]; - atkbd->keycode[i |0x80 ] = atkbd_set2_keycode[atkbd_unxlate_table[i] |0x80 ]; - } - }else if (atkbd->set ==2 ) { -memcpy (atkbd->keycode, atkbd_set2_keycode,sizeof (atkbd->keycode)); - }else { -memcpy (atkbd->keycode, atkbd_set3_keycode,sizeof (atkbd->keycode)); - } - -for (i =0 ; i <512 ; i++) -if (atkbd->keycode[i] && atkbd->keycode[i] <255 ) -set_bit (atkbd->keycode[i], atkbd->dev.keybit);return 0 ; }
Quoted 3 lines
Patch attached, please test. It'll make it into 2.6.3, with some luck even 2.6.2.
Your patch works fine for me. (shame that I had to reboot, though)
Many thanks for your time.
Quoted 13 lines · Vojtech Pavlik
Quoted 12 lines · P. Christeas
This may be just a minor issue:
I had to use the setkeycodes utility to enable some extra keys (that weren't mapped by kernel's atkbd tables).
After waking from sleep 1, those keys were reset. That is, I had to use 'setkeycodes' again to customize the tables again.IMHO the way kernel works now is correct. It should *not* have extra code just to handle that. Just make sure anybody that alters his kbd tables puts some extra script to recover the tables after an ACPI wake.
This should be more like a note to Linux distributors.
Quoted 4 lines
Quoted 1 line
Have a nice day.
Patch attached, please test. It'll make it into 2.6.3, with some luck even 2.6.2.
I feel a little uneasy about killing the input device if set changes after resume. That would cause a new input device created but for user it would look like keyboard is lost or am I missing someting?
What about the patch below which would reset keyboard to the default keymap only if set changes?
Signature · 164 lines
Dmitry =================================================================== [email protected], 2004-02-02 23:33:52-05:00, [email protected] Input: atkbd - load default keymap on resume only if set is changed atkbd.c | 95 ++++++++++++++++++++++++++++++++++------------------------------ 1 files changed, 51 insertions(+), 44 deletions(-) =================================================================== diff -Nru a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c --- a/drivers/input/keyboard/atkbd.c Mon Feb 2 23:35:33 2004 +++ b/drivers/input/keyboard/atkbd.c Mon Feb 2 23:35:33 2004 @@ -619,6 +619,38 @@ kfree(atkbd); } +static void atkbd_set_name(struct atkbd *atkbd) +{ + if (atkbd->set == 4) { + sprintf(atkbd->name, "AT Set 2 Extended keyboard"); + } else + sprintf(atkbd->name, "AT %s Set %d keyboard", + atkbd->translated ? "Translated" : "Raw", atkbd->set); +} + +/* + * Installs default keymap based on the detected mode (set) + */ +static void atkbd_install_keymap(struct atkbd *atkbd) +{ + int i; + + if (atkbd->translated) { + for (i = 0; i < 128; i++) { + atkbd->keycode[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]]; + atkbd->keycode[i | 0x80] = atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80]; + } + } else if (atkbd->set == 2) { + memcpy(atkbd->keycode, atkbd_set2_keycode, sizeof(atkbd->keycode)); + } else { + memcpy(atkbd->keycode, atkbd_set3_keycode, sizeof(atkbd->keycode)); + } + + for (i = 0; i < 512; i++) + if (atkbd->keycode[i] && atkbd->keycode[i] < 255) + set_bit(atkbd->keycode[i], atkbd->dev.keybit); +} + /* * atkbd_connect() is called when the serio module finds and interface * that isn't handled yet by an appropriate device driver. We check if @@ -629,7 +661,6 @@ static void atkbd_connect(struct serio *serio, struct serio_dev *dev) { struct atkbd *atkbd; - int i; if (!(atkbd = kmalloc(sizeof(struct atkbd), GFP_KERNEL))) return; @@ -696,26 +727,12 @@ atkbd->id = 0xab00; } - if (atkbd->set == 4) { + if (atkbd->set == 4) atkbd->dev.ledbit[0] |= BIT(LED_COMPOSE) | BIT(LED_SUSPEND) | BIT(LED_SLEEP) | BIT(LED_MUTE) | BIT(LED_MISC); - sprintf(atkbd->name, "AT Set 2 Extended keyboard"); - } else - sprintf(atkbd->name, "AT %s Set %d keyboard", - atkbd->translated ? "Translated" : "Raw", atkbd->set); + atkbd_set_name(atkbd); sprintf(atkbd->phys, "%s/input0", serio->phys); - if (atkbd->translated) { - for (i = 0; i < 128; i++) { - atkbd->keycode[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]]; - atkbd->keycode[i | 0x80] = atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80]; - } - } else if (atkbd->set == 2) { - memcpy(atkbd->keycode, atkbd_set2_keycode, sizeof(atkbd->keycode)); - } else { - memcpy(atkbd->keycode, atkbd_set3_keycode, sizeof(atkbd->keycode)); - } - atkbd->dev.name = atkbd->name; atkbd->dev.phys = atkbd->phys; atkbd->dev.id.bustype = BUS_I8042; @@ -723,9 +740,7 @@ atkbd->dev.id.product = atkbd->translated ? 1 : atkbd->set; atkbd->dev.id.version = atkbd->id; - for (i = 0; i < 512; i++) - if (atkbd->keycode[i] && atkbd->keycode[i] < 255) - set_bit(atkbd->keycode[i], atkbd->dev.keybit); + atkbd_install_keymap(atkbd); input_register_device(&atkbd->dev); @@ -741,12 +756,12 @@ { struct atkbd *atkbd = serio->private; struct serio_dev *dev = serio->dev; - int i; + int old_set = atkbd->set; - if (!dev) { - printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n"); - return -1; - } + if (!dev) { + printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n"); + return -1; + } if (atkbd->write) { if (atkbd_probe(atkbd)) @@ -759,27 +774,19 @@ atkbd->id = 0xab00; } - /* - * Here we probably should check if the keyboard has the same set that - * it had before and bail out if it's different. But this will most likely - * cause new keyboard device be created... and for the user it will look - * like keyboard is lost - */ + if (atkbd->set != old_set) { + /* + * ok, we woke up with a different keyboard attached. Alhtough we could just + * signal failure it's not very nice as it will most likely cause new keyboard + * device be created... and for the user it will look like keyboard is lost. + */ + atkbd_set_name(atkbd); - if (atkbd->translated) { - for (i = 0; i < 128; i++) { - atkbd->keycode[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]]; - atkbd->keycode[i | 0x80] = atkbd_set2_keycode[atkbd_unxlate_table[i] | 0x80]; - } - } else if (atkbd->set == 2) { - memcpy(atkbd->keycode, atkbd_set2_keycode, sizeof(atkbd->keycode)); - } else { - memcpy(atkbd->keycode, atkbd_set3_keycode, sizeof(atkbd->keycode)); - } + printk(KERN_INFO "atkbd: new %s detected at %s, loading default keymap\n", + atkbd->name, atkbd->name); - for (i = 0; i < 512; i++) - if (atkbd->keycode[i] && atkbd->keycode[i] < 255) - set_bit(atkbd->keycode[i], atkbd->dev.keybit); + atkbd_install_keymap(atkbd); + } return 0; }
Quoted 3 lines · Dmitry Torokhov
I feel a little uneasy about killing the input device if set changes after resume. That would cause a new input device created but for user it would look like keyboard is lost or am I missing someting?
The console will automatically and immediately grab the new keyboard and everything will continue to work, even X, because it uses the console for input.
Later, when X uses the event interface, it'll get a notification that the keyboard was removed and added again, which will cause it to open the new one, so all will be OK, too.
Quoted 2 lines
What about the patch below which would reset keyboard to the default keymap only if set changes?
I don't think there is a problem with just reconnecting. It's extremely unlikely that the set can change, too, because everyone uses translated set 2, and that's the lowest common denominator.
Unless you specified extra arguments on the kernel command line, you can't get any other set.
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR
In article <[email protected]>,
P. Christeas <[email protected]> wrote:
| This may be just a minor issue: | I had to use the setkeycodes utility to enable some extra keys (that weren't | mapped by kernel's atkbd tables). | After waking from sleep 1, those keys were reset. That is, I had to use | 'setkeycodes' again to customize the tables again. | | IMHO the way kernel works now is correct. It should *not* have extra code just | to handle that. Just make sure anybody that alters his kbd tables puts some | extra script to recover the tables after an ACPI wake. | This should be more like a note to Linux distributors.
I'm not totally sure I agree that the kernel should save this information. With all the things the kernel does save, keyboard might be a good thing. Consider someone with a REALLY hacked layout, like modified dvorak or some of the keyboards for the access challenged. Now think "can't use the keyboard enough to type in the ...ing commands to do the load."
If we can carry the code for two competing disfunctional ACPI implementations and a used-to-work but we broke it for some machines APM, we can surely add a pair om memcpy lines to eliminate at least one "oh-shit moment."
Obviously my opinion only, some breakage due to BIOS misfeatures, contents may settle in shipping, etc.
Signature · 3 lines
bill davidsen <[email protected]> CTO, TMR Associates, Inc Doing interesting things with little computers since 1979.