[PATCH] Add support for USB Keyboard attached to UHCI
Jay Lan
jlan at sgi.com
Thu Dec 6 12:05:25 PST 2007
Hi Konstantin,
There is one compilation error and one link error on IA64. See below.
Konstantin Baydarov wrote:
> On Fri, 30 Nov 2007 12:11:05 -0800 (PST)
> Aaron Young <ayoung at cthulhu.engr.sgi.com> wrote:
>
>>> On Fri, 30 Nov 2007 08:43:07 -0800 (PST)
>>> Aaron Young <ayoung at cthulhu.engr.sgi.com> wrote:
>>>
>>>>>
>>>>>
>>>>> Looking better! More comments:
>>>>>
>>>>> 1. I don't understand the need for the call to
>>>>> kdb_uhci_keyboard_clear(). It's only called if
>>>>> kdb_usb_keyboard_attach() fails and in that case we
>>>>> didn't add the urb to kdb_usb_kbds[]. So, I don't
>>>>> see the need to clear it out of kdb_usb_kbds[]...
>>>>> (I'm probably missing something here).
>>> - Now I call kdb_uhci_keyboard_clear() if usb_submit_urb() failed,
>>> that is called after kdb_usb_keyboard_attach().
>> Why not just call kdb_usb_keyboard_detach()?
> Got rid from kdb_uhci_keyboard_clear().
>
>>>>> 2. I probably would have made kdb_uhci_submit_urb() return
>>>>> the newly created kdb_urb (extra arg) on success and not
>>>>> call kdb_usb_keyboard_attach() directly from within. Then you
>>>>> can just make a single call to kdb_usb_keyboard_attach() for
>>>>> all three cases (OHCI, EHCI and UHCI) out of hid_probe(). But,
>>>>> not a *BIG* deal...
>>> - We should first fill kdb_usb_kbds[] before call of
>>> usb_submit_urb() because uhci_urb_enqueue() checks kdb_usb_kbds[]
>>> to find out if KDB URB is enqueued. So kdb_usb_keyboard_attach()
>>> should be called in the "middle" of the function
>>> kdb_uhci_submit_urb().
>> OK..
>>
>>>>> 3. We'll have to get updates to ia64 kdba_io.c as well,
>>>>> otherwise it will result in compile errors on ia64. i.e.
>>>>> kdb_usb_keyboard_attach() will have a number of args mismatch
>>>>> and all the routines you added to kdba_io_32.c and kdba_io_64.c
>>>>> will not be there.
>>> - Can't find any ia64 file...
>> linux/arch/ia64/kdb/kdba_io.c
> Added arch/ia64/kdb/kdba_io.c.
>
>> Maybe it's part of a different KDB patch or something? Jay?
>>
>>>>> Does hotplug/hotunplug of the keyboards work?
>>>>> Does it correctly remove the KDB URBs on hotunplug?
>>> - kdb_usb_keyboard_detach() is never called on my PC. So I tested
>>> only 1 case: attaching keyboard after kernel boot - works OK.
>> Should get called out of hid_disconnect() when a
>> keyboard is unplugged. If you are using a usb1.1
>> hub, it may not work - I've seen this problem with
>> USB1.1 hub. MIght want to try plugging/unplugging the keyboard
>> directly into the chassis port if so.
> Fixed hotplug/hotunplug. The problem was in my patch - UHCI URB should be unlinked from endpoin queue.
>
>> BTW - do you have a USB2.0 hub? The EHCI KDB code has yet to
>> be tested on x86 and I'd be curious if it worked or not (hope it
>> does)...
>>
>> -Aaron
> No, I don't have USB2.0 hub.
>
> Aaron, here is incremental patch against my last patch (http://oss.sgi.com/archives/kdb/2007-11/msg00039.html):
>
> Index: linux-2.6.24-rc2/arch/ia64/kdb/kdba_io.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/arch/ia64/kdb/kdba_io.c
> +++ linux-2.6.24-rc2/arch/ia64/kdb/kdba_io.c
> @@ -65,12 +65,120 @@ static unsigned char kdb_usb_keycode[256
> 150,158,159,128,136,177,178,176,142,152,173,140
> };
>
> +/* Check if URB is managed by KDB code */
> +int kdb_uhci_keyboard_urb(struct urb *urb)
> +{
> + int i;
> +
> + for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
> + if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].urb == urb)
> + return i;
> + }
> + return -1;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_urb);
> +
> +/* Check if UHCI QH is managed by KDB code */
> +int kdb_uhci_keyboard_check_uhci_qh(struct uhci_qh *qh)
> +{
> + int i;
> +
> + for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
> + if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh)
> + return i;
> + }
> + return -1;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_check_uhci_qh);
> +
> +/* Set UHCI QH using URB pointer */
> +int kdb_uhci_keyboard_set_qh(struct urb *urb, struct uhci_qh *qh)
> +{
> + int i;
> +
> + i = kdb_uhci_keyboard_urb(urb);
> + if (i != -1)
> + kdb_usb_kbds[i].qh = qh;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_qh);
> +
> +/* Get UHCI QH using URB pointer */
> +struct uhci_qh *kdb_uhci_keyboard_get_qh(struct urb *urb)
> +{
> + int i;
> +
> + i = kdb_uhci_keyboard_urb(urb);
> + if (i != -1)
> + return kdb_usb_kbds[i].qh;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_qh);
> +
> +/* Set UHCI hid_event using URB pointer */
> +int kdb_uhci_keyboard_set_hid_event(struct urb *urb, int hid_event)
> +{
> + int i;
> +
> + i = kdb_uhci_keyboard_urb(urb);
> + if (i != -1)
> + kdb_usb_kbds[i].kdb_hid_event = hid_event;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_hid_event);
> +
> +/* Get UHCI hid_event using URB pointer */
> +int kdb_uhci_keyboard_get_hid_event(struct urb *urb)
> +{
> + int i;
> +
> + i = kdb_uhci_keyboard_urb(urb);
> + if (i != -1)
> + return kdb_usb_kbds[i].kdb_hid_event;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_hid_event);
> +
> +/* Set UHCI hid_event using UHCI QH pointer */
> +int kdb_uhci_keyboard_set_hid_event_qh(struct uhci_qh *qh, int hid_event)
> +{
> + int i;
> +
> + for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
> + if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh){
> + kdb_usb_kbds[i].kdb_hid_event = hid_event;
> + return i;
> + }
> + }
> + return -1;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_hid_event_qh);
> +
> +/* Get UHCI hid_event using UHCI QH pointer */
> +int kdb_uhci_keyboard_get_hid_event_qh(struct uhci_qh *qh)
> +{
> + int i;
> +
> + for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
> + if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh)
> + return kdb_usb_kbds[i].kdb_hid_event;
> + }
> +
> + return -1;
> +}
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_hid_event_qh);
> +
> /*
> * kdb_usb_keyboard_attach()
> * Attach a USB keyboard to kdb.
> */
> int
> -kdb_usb_keyboard_attach(struct urb *urb, unsigned char *buffer, void *poll_func)
> +kdb_usb_keyboard_attach(struct urb *urb, unsigned char *buffer,
> + void *poll_func, void *compl_func, struct urb *hid_urb)
> {
> int i;
> int rc = -1;
> @@ -92,6 +200,8 @@ kdb_usb_keyboard_attach(struct urb *urb,
> kdb_usb_kbds[i].urb = urb;
> kdb_usb_kbds[i].buffer = buffer;
> kdb_usb_kbds[i].poll_func = poll_func;
> + kdb_usb_kbds[i].urb_complete = compl_func;
> + kdb_usb_kbds[i].hid_urb = hid_urb;
>
> rc = 0; /* success */
>
> @@ -111,6 +221,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
> {
> int i;
> int rc = -1;
> + extern void kdb_unlink_uhci_qh(struct urb *urb, struct uhci_qh *qh);
>
> if (kdb_no_usb)
> return 0;
> @@ -122,14 +233,21 @@ kdb_usb_keyboard_detach(struct urb *urb)
> */
>
> for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
> - if (kdb_usb_kbds[i].urb != urb)
> - continue;
> + if (kdb_usb_kbds[i].qh) {
> + /* UHCI keyboard */
> + if (kdb_usb_kbds[i].hid_urb != urb)
> + continue;
> + kdb_unlink_uhci_qh(kdb_usb_kbds[i].urb, kdb_usb_kbds[i].qh);
> + } else
> + if (kdb_usb_kbds[i].urb != urb)
> + continue;
>
> /* found it, clear the index */
> kdb_usb_kbds[i].urb = NULL;
> kdb_usb_kbds[i].buffer = NULL;
> kdb_usb_kbds[i].poll_func = NULL;
> kdb_usb_kbds[i].caps_lock = 0;
> + kdb_usb_kbds[i].hid_urb = NULL;
>
> rc = 0; /* success */
>
> @@ -152,6 +270,9 @@ get_usb_char(void)
> int ret;
> unsigned char keycode, spec;
> extern u_short plain_map[], shift_map[], ctrl_map[];
> + int ret_key = -1, i2, j, max;
> +
> + ret = -1;
>
> if (kdb_no_usb)
> return -1;
> @@ -179,15 +300,24 @@ get_usb_char(void)
> if (ret < 0) /* error or no characters, try the next kbd */
> continue;
>
> + /* If 2 keys was pressed simultaneously,
> + * both keycodes will be in buffer.
> + * Last pressed key will be last non
> + * zero byte.
> + */
> + for (j=0; j<4; j++){
> + if (!kdb_usb_kbds[i].buffer[2+j])
> + break;
> + }
> + /* Last pressed key */
> + max = j + 1;
> +
> spec = kdb_usb_kbds[i].buffer[0];
> - keycode = kdb_usb_kbds[i].buffer[2];
> + keycode = kdb_usb_kbds[i].buffer[max];
> kdb_usb_kbds[i].buffer[0] = (char)0;
> kdb_usb_kbds[i].buffer[2] = (char)0;
>
> - if(kdb_usb_kbds[i].buffer[3]) {
> - kdb_usb_kbds[i].buffer[3] = (char)0;
> - continue;
> - }
> + ret_key = -1;
>
> /* A normal key is pressed, decode it */
> if(keycode)
> @@ -199,10 +329,12 @@ get_usb_char(void)
> {
> case 0x2:
> case 0x20: /* Shift */
> - return shift_map[keycode];
> + ret_key = shift_map[keycode];
> + break;
> case 0x1:
> case 0x10: /* Ctrl */
> - return ctrl_map[keycode];
> + ret_key = ctrl_map[keycode];
> + break;
> case 0x4:
> case 0x40: /* Alt */
> break;
> @@ -211,31 +343,47 @@ get_usb_char(void)
> switch(keycode)
> {
> case 0x1C: /* Enter */
> - return 13;
> + ret_key = 13;
> + break
A ';' is missing in the above line.
>
> case 0x3A: /* Capslock */
> kdb_usb_kbds[i].caps_lock = !(kdb_usb_kbds[i].caps_lock);
> break;
> case 0x0E: /* Backspace */
> - return 8;
> + ret_key = 8;
> + break;
> case 0x0F: /* TAB */
> - return 9;
> + ret_key = 9;
> + break;
> case 0x77: /* Pause */
> break ;
> default:
> if(!kdb_usb_kbds[i].caps_lock) {
> - return plain_map[keycode];
> + ret_key = plain_map[keycode];
> + break;
> }
> else {
> - return shift_map[keycode];
> + ret_key = shift_map[keycode];
> + break;
> }
> }
> }
> +
> + /* Key was pressed, return keycode */
> + break;
> }
>
> - /* no chars were returned from any of the USB keyboards */
> + for(i2=0; i2<8; i2++){
> + if (kdb_usb_kbds[i2].buffer)
> + for(j=0; j<8; j++){
> + kdb_usb_kbds[i2].buffer[j] = (char)0;
> + }
> + }
>
> - return -1;
> + if (kdb_usb_kbds[i].urb_complete)
> + (*kdb_usb_kbds[i].urb_complete)((struct urb *)kdb_usb_kbds[i].urb);
> +
> + return ret_key;
> }
> #endif /* CONFIG_KDB_USB */
>
> Index: linux-2.6.24-rc2/drivers/hid/usbhid/hid-core.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/drivers/hid/usbhid/hid-core.c
> +++ linux-2.6.24-rc2/drivers/hid/usbhid/hid-core.c
> @@ -992,13 +992,15 @@ static inline void kdb_usb_fill_int_urb
>
> static int kdb_uhci_submit_urb(struct usb_interface *intf, unsigned int usbhid_bufsize, struct urb *hid_inurb)
> {
> - struct urb *kdb_urb = NULL;
> + struct urb *kdb_urb;
> unsigned char *kdb_buffer;
> dma_addr_t uhci_inbuf_dma;
> int ret = -1;
> extern void * usb_hcd_get_kdb_poll_func(struct usb_device *udev);
> extern void * usb_hcd_get_kdb_completion_func(struct usb_device *udev);
>
> + kdb_urb = NULL;
> + kdb_buffer = NULL;
> if (!(kdb_buffer = usb_buffer_alloc(hid_inurb->dev,
> usbhid_bufsize, GFP_ATOMIC,
> &uhci_inbuf_dma)))
> @@ -1020,6 +1022,7 @@ static int kdb_uhci_submit_urb(struct us
> (kdb_urb)->transfer_dma = uhci_inbuf_dma;
> (kdb_urb)->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>
> +
> ret = kdb_usb_keyboard_attach(kdb_urb, kdb_buffer,
> usb_hcd_get_kdb_poll_func(interface_to_usbdev(intf)),
> usb_hcd_get_kdb_completion_func(interface_to_usbdev(intf)), hid_inurb);
> @@ -1028,14 +1031,24 @@ static int kdb_uhci_submit_urb(struct us
> goto out;
>
> if (usb_submit_urb(kdb_urb, GFP_ATOMIC)){
> - kdb_uhci_keyboard_clear(kdb_urb);
> + /* We don't know exactly if UHCI KDB QH was allocated,
> + * so if kdb_usb_keyboard_detach(hid_inurb) failed
> + * kdb_usb_keyboard_detach(kdb_urb) should be called.
> + */
> + kdb_usb_keyboard_detach(hid_inurb);
> + kdb_usb_keyboard_detach(kdb_urb);
> goto out;
> }
> + /* Remove KDB special URB from endpoin queue to
> + * prevent hang during hid_disconnect().
> + */
> + list_del(&(kdb_urb->urb_list));
>
> ret = 0;
> return ret;
> out:
> /* Some Error Cleanup */
> + ret = -1;
> printk("KDB: Error, UHCI Keyboard HID won't work!\n");
>
> if (kdb_buffer)
> Index: linux-2.6.24-rc2/arch/x86/kdb/kdba_io_32.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/arch/x86/kdb/kdba_io_32.c
> +++ linux-2.6.24-rc2/arch/x86/kdb/kdba_io_32.c
> @@ -79,24 +79,7 @@ int kdb_uhci_keyboard_check_uhci_qh(stru
> }
> return -1;
> }
> -EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_chech_uhci_qh);
> -
> -/* Clear keyboar entry */
> -int kdb_uhci_keyboard_clear(struct urb *urb)
> -{
> - int i;
> -
> - i = kdb_uhci_keyboard_urb(urb);
> - if (i != -1){
> - kdb_usb_kbds[i].urb = NULL;
> - kdb_usb_kbds[i].buffer = NULL;
> - kdb_usb_kbds[i].poll_func = NULL;
> - kdb_usb_kbds[i].urb_complete = NULL;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_clear);
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_check_uhci_qh);
>
> /* Set UHCI QH using URB pointer */
> int kdb_uhci_keyboard_set_qh(struct urb *urb, struct uhci_qh *qh)
> @@ -228,6 +211,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
> {
> int i;
> int rc = -1;
> + extern void kdb_unlink_uhci_qh(struct urb *urb, struct uhci_qh *qh);
>
> if (kdb_no_usb)
> return 0;
> @@ -243,6 +227,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
> /* UHCI keyboard */
> if (kdb_usb_kbds[i].hid_urb != urb)
> continue;
> + kdb_unlink_uhci_qh(kdb_usb_kbds[i].urb, kdb_usb_kbds[i].qh);
> } else
> if (kdb_usb_kbds[i].urb != urb)
> continue;
> Index: linux-2.6.24-rc2/arch/x86/kdb/kdba_io_64.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/arch/x86/kdb/kdba_io_64.c
> +++ linux-2.6.24-rc2/arch/x86/kdb/kdba_io_64.c
> @@ -79,24 +79,7 @@ int kdb_uhci_keyboard_check_uhci_qh(stru
> }
> return -1;
> }
> -EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_chech_uhci_qh);
> -
> -/* Clear keyboar entry */
> -int kdb_uhci_keyboard_clear(struct urb *urb)
> -{
> - int i;
> -
> - i = kdb_uhci_keyboard_urb(urb);
> - if (i != -1){
> - kdb_usb_kbds[i].urb = NULL;
> - kdb_usb_kbds[i].buffer = NULL;
> - kdb_usb_kbds[i].poll_func = NULL;
> - kdb_usb_kbds[i].urb_complete = NULL;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_clear);
> +EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_check_uhci_qh);
>
> /* Set UHCI QH using URB pointer */
> int kdb_uhci_keyboard_set_qh(struct urb *urb, struct uhci_qh *qh)
> @@ -228,6 +211,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
> {
> int i;
> int rc = -1;
> + extern void kdb_unlink_uhci_qh(struct urb *urb, struct uhci_qh *qh);
>
> if (kdb_no_usb)
> return 0;
> @@ -243,6 +227,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
> /* UHCI keyboard */
> if (kdb_usb_kbds[i].hid_urb != urb)
> continue;
> + kdb_unlink_uhci_qh(kdb_usb_kbds[i].urb, kdb_usb_kbds[i].qh);
> } else
> if (kdb_usb_kbds[i].urb != urb)
> continue;
> Index: linux-2.6.24-rc2/drivers/usb/host/uhci-q.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/drivers/usb/host/uhci-q.c
> +++ linux-2.6.24-rc2/drivers/usb/host/uhci-q.c
> @@ -27,7 +27,7 @@
> */
> #ifdef CONFIG_KDB_USB
> /* KDB HID QH, managed by KDB code */
> -extern int kdb_uhci_keyboard_chech_uhci_qh(struct uhci_qh *qh);
> +extern int kdb_uhci_keyboard_check_uhci_qh(struct uhci_qh *qh);
> extern int kdb_uhci_keyboard_set_qh(struct urb *urb, struct uhci_qh *qh);
> extern struct uhci_qh *kdb_uhci_keyboard_get_qh(struct urb *urb);
> extern int kdb_uhci_keyboard_set_hid_event(struct urb *urb, int hid_event);
> @@ -1728,7 +1728,7 @@ static int uhci_advance_check(struct uhc
>
> #ifdef CONFIG_KDB_USB
> /* Don't manage KDB QH */
> - if(kdb_uhci_keyboard_chech_uhci_qh(qh) != -1){
> + if(kdb_uhci_keyboard_check_uhci_qh(qh) != -1){
> ret = 0;
> goto done;
> }
> @@ -1827,7 +1827,7 @@ rescan:
>
> #ifdef CONFIG_KDB_USB
> /* Don't manage KDB QH */
> - if(kdb_uhci_keyboard_chech_uhci_qh(qh) != -1)
> + if(kdb_uhci_keyboard_check_uhci_qh(qh) != -1)
> continue;
> #endif
> if (uhci_advance_check(uhci, qh)) {
> Index: linux-2.6.24-rc2/drivers/usb/host/uhci-hcd.c
> ===================================================================
> --- linux-2.6.24-rc2.orig/drivers/usb/host/uhci-hcd.c
> +++ linux-2.6.24-rc2/drivers/usb/host/uhci-hcd.c
> @@ -434,6 +434,21 @@ static irqreturn_t uhci_irq(struct usb_h
> return IRQ_HANDLED;
> }
>
> +/* Unlink KDB QH from hardware and software scheduler */
Since CONFIG_USB_UHCI_HCD can be a module, the following routine needs
to be exported.
Also, the new routine should be enclosed inside the CONFIG_KDB_USB.
Thanks,
- jay
> +void kdb_unlink_uhci_qh(struct urb *urb, struct uhci_qh *qh)
> +{
> + unsigned long flags;
> + struct uhci_hcd *uhci;
> +
> + uhci = (struct uhci_hcd *) hcd_to_uhci(bus_to_hcd(urb->dev->bus));
> +
> + spin_lock_irqsave(&uhci->lock, flags);
> + unlink_interrupt(NULL, qh);
> + list_del(&(qh->node));
> + spin_unlock_irqrestore(&uhci->lock, flags);
> +
> +}
> +
> #ifdef CONFIG_KDB_USB
> static int
> uhci_kdb_poll_char(struct urb *urb)
> Index: linux-2.6.24-rc2/include/linux/kdb.h
> ===================================================================
> --- linux-2.6.24-rc2.orig/include/linux/kdb.h
> +++ linux-2.6.24-rc2/include/linux/kdb.h
> @@ -144,7 +144,6 @@ extern void smp_kdb_stop(void);
> #include <linux/usb.h>
>
> extern int kdb_uhci_keyboard_urb(struct urb *urb);
> -extern int kdb_uhci_keyboard_clear(struct urb *urb);
> extern int kdb_usb_keyboard_alloc(void);
>
> extern int kdb_usb_keyboard_attach(struct urb *urb, unsigned char *buffer, void *poll_func, void *compl_func, struct urb *hid_urb);
> ---------------------------
> Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.
---------------------------
Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.
More information about the kdb
mailing list