Re: enable/disable in kbd drivers

Thanks for responding, Max.

On Mar 29 at 09:08 -0800, Maksim Yevmenkin wrote:
On 3/24/07, Timothy Bourke <timbob@xxxxxxxxxxx> wrote in part:
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.

sorry for the delay. i'm not sure about this patch. basically, i do
not think that keyboard should be disabled if it is released from
kbdmux. it is perfectly fine, imo, to have two (or more) active
keyboards attached to the system as long as only one of them is
primary. it is possible to use /dev/kbdX interface to talk to
non-primary keyboard(s) directly.

It seems that, for the extant drivers:
* enable only increments kb_active,
* disable only decrements kb_active.

With the printf suggested above, one can see that repeating the
commands:
kbdcontrol -A usb0 < /dev/console
kbdcontrol -a usb0 < /dev/console
continually increases kb_active.

Even with the patch, detaching any of the current kbd drivers will not
deactivate them (reduce kb_active to zero) because they all call
enable (increment kb_active) when initialized. Which answers one of
the other questions.

Alternative (not serious) suggestion: since the disable hook seems
never to be called, it might as well be removed from the
keyboard_switch...

Tim.

Attachment: pgpUIpK4nCfpU.pgp
Description: PGP signature

Relevant Pages

  • Re: posix-cpu-timers revamp
    ... This patch breaks the shared utime/stime/sched_time ... The check_process_timersroutine bases its computations on the ... shared structure, removing two loops through the threads. ... This is the record for the group leader. ...
    (Linux-Kernel)
  • [PATCH] sata_mv: add 6042 (Gen 2E) support
    ... See the patch below. ... I hope that someone at Marvell or EMC or whereever picks ... + * This routine simply redirects to the general purpose routine ... + * if command is not DMA. ...
    (Linux-Kernel)
  • Re: [PATCH] 2.6 SGI Altix I/O code reorganization
    ... Doing it for the general sn_pci_init routine would let us ... get rid of the check for ia64_platform_isin one of the routines, ... It will be great to avoid spinning this big patch. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: [2.6.18 patch] fix mem_write return value (was: Re: bug report: mem_write)
    ... The point is that in the beginning of the routine, "copied" is set to 0, ... The attached patch should fix it. ... it anytime except when ptrace would allow it. ... size_t count, loff_t *ppos) ...
    (Linux-Kernel)