On Fri, 30 Nov 2007 12:11:05 -0800 (PST) Aaron Young <[EMAIL PROTECTED]> wrote:
> > > > On Fri, 30 Nov 2007 08:43:07 -0800 (PST) > > Aaron Young <[EMAIL PROTECTED]> 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 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 */ +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.
