enable/disable in kbd drivers

[Timothy Bourke]

I have almost finished a ppbus-based driver for Super Nintendo
controllers. It presents itself as a keyboard to the operating system. I
wanted to start and stop the polling thread via, respectively, the
kbd_enable_t and kbd_disable_t hooks, but these do not seem to be called
properly. Hence this post.

The enable and disable functions are identical across the keyboard
drivers (atkbd, ukbd, vkbd, kbdmux). Enable calls the KBD_ACTIVATE macro
which increments the kb_active count. Disable calls the KBD_DEACTIVATE
macro which decrements the kb_active count. It seems reasonable to
assume that the two functions should be called in pairs.

However, enable is called too many times and disable is never called.

In the kbdmux_ioctl routine:
    KBADDKBD: enable is called via the KBDMUX_ENABLE macro.
    KBRELKBD: does NOT call disable

Taking dev/usb/ukbd.c as an example, the effect can be seen by adding
this line to the ukbd_enable function (after the call to KBD_ACTIVATE):
    printf("ukbd_enable: %d\n", KBD_IS_ACTIVE(kbd));
And similarly for ukbd_disable and then running dmesg or kbdcontrol.

Additionally, each kbd driver calls its own enable function when
attached. For example, in USB_ATTACH(ukbd):
    (*sw->enable)(kbd);
This would appear to be unnecessary for keyboards connected via kbdmux.
I am less certain about those connected directly, but the syscons
sccngetch routine does seems to call enable and disable. Perhaps it
should also call enable when it first starts?

Does the attached patch seem reasonable? It would fix my immediate
problem.

I could write a patch to remove the calls to enable in the driver init
routines but tI don't really understand how syscons works.

Tim.


--- sys/dev/kbdmux/kbdmux.c.orig        Sun Mar 25 11:33:57 2007
+++ sys/dev/kbdmux/kbdmux.c     Sun Mar 25 11:36:46 2007
@@ -132,6 +132,9 @@
 #define        KBDMUX_ENABLE(kbd) \
        (*kbdsw[(kbd)->kb_index]->enable)((kbd))
 
+#define        KBDMUX_DISABLE(kbd) \
+       (*kbdsw[(kbd)->kb_index]->disable)((kbd))
+
 #define        KBDMUX_POLL(kbd, on) \
        (*kbdsw[(kbd)->kb_index]->poll)((kbd), (on))
 
@@ -1029,6 +1032,7 @@
                                break;
 
                if (k != NULL) {
+                       KBDMUX_DISABLE(k->kbd);
                        error = kbd_release(k->kbd, &k->kbd);
                        if (error == 0) {
                                SLIST_REMOVE(&state->ks_kbds, k, kbdmux_kbd, 
next);

pgpAmz8sYqcyU.pgp
Description: PGP signature