From kbaidarov@ru.mvista.com Tue Dec 4 06:30:33 2007 Received: with ECARTIS (v1.0.0; list kdb); Tue, 04 Dec 2007 06:30:44 -0800 (PST) Received: from mail.dev.rtsoft.ru (rtsoft2.corbina.net [85.21.88.2] (may be forged)) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with SMTP id lB4EUTYk032717 for ; Tue, 4 Dec 2007 06:30:31 -0800 Received: (qmail 30223 invoked from network); 4 Dec 2007 14:30:39 -0000 Received: from unknown (HELO localhost.localdomain) (192.168.1.195) by mail.dev.rtsoft.ru with SMTP; 4 Dec 2007 14:30:39 -0000 Date: Tue, 4 Dec 2007 17:30:40 +0300 From: Konstantin Baydarov To: Aaron Young Cc: ayoung@cthulhu.engr.sgi.com (Aaron Young), jlan@sgi.com (Jay Lan), kdb@oss.sgi.com Subject: Re: [PATCH] Add support for USB Keyboard attached to UHCI Message-ID: <20071204173040.4c5d93ff@localhost.localdomain> In-Reply-To: <200711302011.lAUKB65B10139641@kluge.engr.sgi.com> References: <20071130224628.51152a03@localhost.localdomain> <200711302011.lAUKB65B10139641@kluge.engr.sgi.com> X-Mailer: Sylpheed-Claws 2.6.0 (GTK+ 2.8.19; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8bit X-archive-position: 1309 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: kbaidarov@ru.mvista.com Precedence: bulk X-list: kdb On Fri, 30 Nov 2007 12:11:05 -0800 (PST) Aaron Young wrote: > > > > On Fri, 30 Nov 2007 08:43:07 -0800 (PST) > > Aaron Young 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 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. From ayoung@engr.sgi.com Thu Dec 6 08:02:10 2007 Received: with ECARTIS (v1.0.0; list kdb); Thu, 06 Dec 2007 08:02:20 -0800 (PST) Received: from kluge.engr.sgi.com (kluge.engr.sgi.com [192.102.96.102]) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with ESMTP id lB6G271N024018 for ; Thu, 6 Dec 2007 08:02:10 -0800 Received: from kluge.engr.sgi.com (localhost [127.0.0.1]) by kluge.engr.sgi.com (SGI-8.12.5/8.12.5) with ESMTP id lB6G2HIW629779; Thu, 6 Dec 2007 08:02:18 -0800 (PST) Received: (from ayoung@localhost) by kluge.engr.sgi.com (SGI-8.12.5/8.12.5/Submit) id lB6G2HRa621341; Thu, 6 Dec 2007 08:02:17 -0800 (PST) From: Aaron Young Message-Id: <200712061602.lB6G2HRa621341@kluge.engr.sgi.com> Subject: Re: [PATCH] Add support for USB Keyboard attached to UHCI To: kbaidarov@ru.mvista.com (Konstantin Baydarov) Date: Thu, 6 Dec 2007 08:02:16 -0800 (PST) Cc: ayoung@cthulhu.engr.sgi.com (Aaron Young), jlan@sgi.com (Jay Lan), kdb@oss.sgi.com In-Reply-To: <20071204173040.4c5d93ff@localhost.localdomain> from "Konstantin Baydarov" at Dec 04, 2007 05:30:40 PM X-Mailer: ELM [version 2.5 PL6] MIME-Version: 1.0 Content-type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit X-archive-position: 1310 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: ayoung@engr.sgi.com Precedence: bulk X-list: kdb I guess I'm good with it. Couple of nits: 1. The routine kdb_uhci_keyboard_urb() is really not uhci specific and so could be renamed to simply kdb_keyboard_urb(). Or kdb_get_urb() or kdb_get_urb_index() perhaps to be more descriptive. kdb_uhci_keyboard_urb() sounds like it should return a BOOLEAN. 2. I don't understand this comment and code: > + /* 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); I might not be following the diffs properly, but it seems like you should only be calling kdb_usb_keyboard_detach() for urbs which have already been attached via kdb_usb_keyboard_attach(). Is this the case? Once Jay gets all these patches integrated, I'll have to go back and retest OHCI and EHCI (on ia64) to make sure things still work okay. Jay, at this point, since there are several patches now built on top of the EHCI patch(es), I think it would be best to take the EHCI patch(es) and the UHCI patches. We can fix any bugs (if any) in subsequent patches. IF we were to redo the EHCI patches now (because of bugs or whatever), it could render these dependent patches as needing to be redone too. JMO... -Aaron > > On Fri, 30 Nov 2007 12:11:05 -0800 (PST) > Aaron Young wrote: > > > > > > > On Fri, 30 Nov 2007 08:43:07 -0800 (PST) > > > Aaron Young 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 > > 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. From jlan@sgi.com Thu Dec 6 12:02:58 2007 Received: with ECARTIS (v1.0.0; list kdb); Thu, 06 Dec 2007 12:03:07 -0800 (PST) Received: from kluge.engr.sgi.com (kluge.engr.sgi.com [192.102.96.102]) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with ESMTP id lB6K2uta021020 for ; Thu, 6 Dec 2007 12:02:58 -0800 Received: from [150.166.8.78] (aware.engr.sgi.com [150.166.8.78]) by kluge.engr.sgi.com (SGI-8.12.5/8.12.5) with ESMTP id lB6K33IW674045; Thu, 6 Dec 2007 12:03:04 -0800 (PST) Message-ID: <47585605.8020300@sgi.com> Date: Thu, 06 Dec 2007 12:05:25 -0800 From: Jay Lan User-Agent: Thunderbird 2.0.0.6 (X11/20070801) MIME-Version: 1.0 To: Konstantin Baydarov CC: Aaron Young , kdb@oss.sgi.com Subject: Re: [PATCH] Add support for USB Keyboard attached to UHCI References: <20071130224628.51152a03@localhost.localdomain> <200711302011.lAUKB65B10139641@kluge.engr.sgi.com> <20071204173040.4c5d93ff@localhost.localdomain> In-Reply-To: <20071204173040.4c5d93ff@localhost.localdomain> Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1311 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb 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 wrote: > >>> On Fri, 30 Nov 2007 08:43:07 -0800 (PST) >>> Aaron Young 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 > > 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. From jlan@sgi.com Thu Dec 6 12:41:28 2007 Received: with ECARTIS (v1.0.0; list kdb); Thu, 06 Dec 2007 12:41:35 -0800 (PST) Received: from kluge.engr.sgi.com (kluge.engr.sgi.com [192.102.96.102]) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with ESMTP id lB6KfOKQ028672 for ; Thu, 6 Dec 2007 12:41:28 -0800 Received: from [150.166.8.78] (aware.engr.sgi.com [150.166.8.78]) by kluge.engr.sgi.com (SGI-8.12.5/8.12.5) with ESMTP id lB6KfVIW684870; Thu, 6 Dec 2007 12:41:31 -0800 (PST) Message-ID: <47585F0A.5020002@sgi.com> Date: Thu, 06 Dec 2007 12:43:54 -0800 From: Jay Lan User-Agent: Thunderbird 2.0.0.6 (X11/20070801) MIME-Version: 1.0 To: Konstantin Baydarov CC: Aaron Young , kdb@oss.sgi.com Subject: Re: [PATCH] Add support for USB Keyboard attached to UHCI References: <20071130224628.51152a03@localhost.localdomain> <200711302011.lAUKB65B10139641@kluge.engr.sgi.com> <20071204173040.4c5d93ff@localhost.localdomain> <47585605.8020300@sgi.com> In-Reply-To: <47585605.8020300@sgi.com> Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1312 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb Jay Lan wrote: > Hi Konstantin, > > There is one compilation error and one link error on IA64. See below. > > Konstantin Baydarov wrote: [excessive text deleted] >> 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. Well, exporting the symbol did not solve the link problem. :( The kdb_unlink_uhci_qh() is called from arch/ia64/kdb/kdba_io.c, which is linked statically. So, this kdb_unlink_uhci_qh() routine needs to be a build-in as well. Thanks, - jay > > 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 >> >> 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. --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe. From jlan@sgi.com Mon Dec 10 13:58:49 2007 Received: with ECARTIS (v1.0.0; list kdb); Mon, 10 Dec 2007 13:59:05 -0800 (PST) Received: from cthulhu.engr.sgi.com (cthulhu.engr.sgi.com [192.26.80.2]) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with ESMTP id lBALwmqK014795 for ; Mon, 10 Dec 2007 13:58:49 -0800 Received: from [134.15.0.170] (mtv-vpn-sw-corp-0-170.corp.sgi.com [134.15.0.170]) by cthulhu.engr.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id lBAGHhlL010810; Mon, 10 Dec 2007 08:17:43 -0800 Message-ID: <475D66A4.9080906@sgi.com> Date: Mon, 10 Dec 2007 08:17:40 -0800 From: Jay Lan User-Agent: Thunderbird 1.5 (X11/20060317) MIME-Version: 1.0 To: KDB Subject: [ANNOUNCE] The USB ehci/uhci support for KDB 2.6.24-rc4 X-Enigmail-Version: 0.94.0.0 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1314 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb Hi, I uploaded the EHCI patch and UHCI patch to the download area at oss.sgi.com for those who want to explore USB ehci and uhci support in kdb. USB_EHCI_support-v2.bz2, from Aaron Young (ayoung@sgi.com) USB_UHCI_support-v2.bz2, from Konstantin Baydarov (kbaidarov@ru.mvista.com) These two files can be applied on top of the kdb 2.6.24-rc4 patchset. I will upload newer versions, when available. Note that the ehci patch should be applied first. Many thanks to their hard work! Caveats: - The ehci patch has not been tested on x86_64 nor i386 platforms. - The uhci patch will have compilation and link errors on ia64. Enjoy! Thanks, - jay --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe. From jlan@sgi.com Mon Dec 10 13:58:50 2007 Received: with ECARTIS (v1.0.0; list kdb); Mon, 10 Dec 2007 13:59:02 -0800 (PST) Received: from cthulhu.engr.sgi.com (cthulhu.engr.sgi.com [192.26.80.2]) by oss.sgi.com (8.12.11.20060308/8.12.10/SuSE Linux 0.7) with ESMTP id lBALwmqM014795 for ; Mon, 10 Dec 2007 13:58:49 -0800 Received: from [134.15.0.170] (mtv-vpn-sw-corp-0-170.corp.sgi.com [134.15.0.170]) by cthulhu.engr.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id lBAG1ElL009712; Mon, 10 Dec 2007 08:01:14 -0800 Message-ID: <475D62C8.3050208@sgi.com> Date: Mon, 10 Dec 2007 08:01:12 -0800 From: Jay Lan User-Agent: Thunderbird 1.5 (X11/20060317) MIME-Version: 1.0 To: Aaron Young CC: Konstantin Baydarov , Aaron Young , kdb@oss.sgi.com Subject: Re: [PATCH] Add support for USB Keyboard attached to UHCI References: <200712061602.lB6G2HRa621341@kluge.engr.sgi.com> In-Reply-To: <200712061602.lB6G2HRa621341@kluge.engr.sgi.com> X-Enigmail-Version: 0.94.0.0 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1313 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb Aaron Young wrote: > > > > I guess I'm good with it. Couple of nits: > > 1. The routine kdb_uhci_keyboard_urb() is really not uhci specific > and so could be renamed to simply kdb_keyboard_urb(). Or > kdb_get_urb() or kdb_get_urb_index() perhaps to be more descriptive. > kdb_uhci_keyboard_urb() sounds like it should return a BOOLEAN. > > 2. I don't understand this comment and code: > >> + /* 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); > > I might not be following the diffs properly, but it seems like you > should only be calling kdb_usb_keyboard_detach() for urbs which have > already been attached via kdb_usb_keyboard_attach(). Is this the case? > > > Once Jay gets all these patches integrated, I'll have to go > back and retest OHCI and EHCI (on ia64) to make sure things still > work okay. Jay, at this point, since there are several patches now > built on top of the EHCI patch(es), I think it would be best to take > the EHCI patch(es) and the UHCI patches. We can fix any bugs (if any) in > subsequent patches. IF we were to redo the EHCI patches now (because of > bugs or whatever), it could render these dependent patches as needing to > be redone too. JMO... I uploaded the EHCI patch and UHCI patch to the download area at oss.sgi.com for those who want to explore USB ehci and uhci support in kdb. USB_EHCI_support-v2.bz2 USB_UHCI_support-v2.bz2 These two files can be applied on top of the kdb 2.6.24-rc4 patchset. Note that the ehci patch should be applied first. Caveats: - The ehci patch has not been tested on x86_64 nor i386 platforms. - The uhci patch will have compilation and link errors on ia64. Please feedback, success or failure, to this list. Thanks, - jay > > -Aaron > --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe. From matt@linuxbox.com Mon Dec 17 18:36:55 2007 Received: with ECARTIS (v1.0.0; list kdb); Mon, 17 Dec 2007 18:37:01 -0800 (PST) Received: from cuda.sgi.com (cuda2.sgi.com [192.48.168.29]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id lBI2aoSt013076 for ; Mon, 17 Dec 2007 18:36:55 -0800 X-ASG-Debug-ID: 1197945421-175900770000-sLlkUa X-Barracuda-URL: http://cuda.sgi.com:80/cgi-bin/mark.cgi Received: from aa.linuxbox.com (localhost [127.0.0.1]) by cuda.sgi.com (Spam Firewall) with ESMTP id 3FDEE4947E1 for ; Mon, 17 Dec 2007 18:37:01 -0800 (PST) Received: from aa.linuxbox.com (aa.linuxbox.com [134.215.213.37]) by cuda.sgi.com with ESMTP id Se4hzPBsdCuRuzAf for ; Mon, 17 Dec 2007 18:37:01 -0800 (PST) Received: from trosper.private.linuxbox.com (trosper.private.linuxbox.com [10.1.1.45]) by aa.linuxbox.com (8.13.1/8.13.1/SuSE Linux 0.7) with ESMTP id lBI2awWA018805 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL) for ; Mon, 17 Dec 2007 21:36:59 -0500 Received: by trosper.private.linuxbox.com (Postfix, from userid 65534) id E742AC0A3881; Mon, 17 Dec 2007 21:36:58 -0500 (EST) Received: from secant.private.linuxbox.com (unknown [10.1.1.217]) by trosper.private.linuxbox.com (Postfix) with ESMTP id 29BA0C084BAF for ; Mon, 17 Dec 2007 21:36:57 -0500 (EST) Message-ID: <4767323D.5040703@linuxbox.com> Date: Mon, 17 Dec 2007 21:36:45 -0500 From: Matt Benjamin User-Agent: Thunderbird 2.0.0.9 (X11/20071031) MIME-Version: 1.0 To: kdb@oss.sgi.com X-ASG-Orig-Subj: Re: [ANNOUNCE] The USB ehci/uhci for KDB Subject: Re: [ANNOUNCE] The USB ehci/uhci for KDB X-Enigmail-Version: 0.95.5 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0.2 (aa.linuxbox.com [10.1.1.1]); Mon, 17 Dec 2007 21:36:59 -0500 (EST) X-Barracuda-Connect: aa.linuxbox.com[134.215.213.37] X-Barracuda-Start-Time: 1197945422 X-Barracuda-Bayes: INNOCENT GLOBAL 0.0000 1.0000 -2.0210 X-Barracuda-Virus-Scanned: by cuda.sgi.com at sgi.com X-Barracuda-Spam-Score: -1.52 X-Barracuda-Spam-Status: No, SCORE=-1.52 using per-user scores of TAG_LEVEL=2.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.1, rules version 3.1.36931 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M BODY: Custom Rule 7568M X-archive-position: 1315 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: matt@linuxbox.com Precedence: bulk X-list: kdb -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 I just wanted to let folks know that I 1. got this working on x86 (x86_64, Dell Vostro 200) 1.a after moving some static function definitions around to deal with issues mentioned by Jay 1.b and after hacking around an apparent bug in get_usb_char (kdba_io_64.c) The proximate issue is that in (at least) the case where we (I think) we got no keypress, i has the value KDB_USB_NUM_KEYBOARDS, which provokes a trap when we call urb_complete. Thanks for getting this in, KDB developers. I absolutely needed a KDB on these boxes, and this arrived just in time. Matt - -- Matt Benjamin The Linux Box 206 South Fifth Ave. Suite 150 Ann Arbor, MI 48104 http://linuxbox.com tel. 734-761-4689 fax. 734-769-8938 cel. 734-216-5309 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFHZzI9JiSUUSaRdSURCJgQAJ0RnqkqtjKhPF7MCz92V3rp2UUYdQCcDnf5 mE8QFCMHNi4FLbhDadMZDiA= =Km7G -----END PGP SIGNATURE----- --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe. From jlan@sgi.com Tue Dec 18 11:16:24 2007 Received: with ECARTIS (v1.0.0; list kdb); Tue, 18 Dec 2007 11:16:30 -0800 (PST) Received: from cthulhu.engr.sgi.com (cthulhu.engr.sgi.com [192.26.80.2]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id lBIJGMri030956 for ; Tue, 18 Dec 2007 11:16:24 -0800 Received: from [134.15.0.134] (mtv-vpn-sw-corp-0-134.corp.sgi.com [134.15.0.134]) by cthulhu.engr.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id lBIJGVgZ016276; Tue, 18 Dec 2007 11:16:32 -0800 Message-ID: <47681C8F.9080706@sgi.com> Date: Tue, 18 Dec 2007 11:16:31 -0800 From: Jay Lan User-Agent: Thunderbird 1.5 (X11/20060317) MIME-Version: 1.0 To: Matt Benjamin CC: kdb@oss.sgi.com Subject: Re: [ANNOUNCE] The USB ehci/uhci for KDB References: <4767323D.5040703@linuxbox.com> In-Reply-To: <4767323D.5040703@linuxbox.com> X-Enigmail-Version: 0.94.0.0 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1316 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb Matt Benjamin wrote: Hi Matt, > I just wanted to let folks know that I > > 1. got this working on x86 (x86_64, Dell Vostro 200) Great! > 1.a after moving some static function definitions around to deal with > issues mentioned by Jay What kind of problem was that? I did not have problem building on x86_64? My problem was on ia64. > 1.b and after hacking around an apparent bug in get_usb_char > (kdba_io_64.c) I assume it is in arch/x86/kdba_io_64.c? Can you share your hacking with us and what problem did it fix? Maybe you hit something Konstantin did not see. Thanks, - jay > > The proximate issue is that in (at least) the case where we (I think) we > got no keypress, i has the value KDB_USB_NUM_KEYBOARDS, which provokes a > trap when we call urb_complete. > > Thanks for getting this in, KDB developers. I absolutely needed a KDB > on these boxes, and this arrived just in time. > > Matt > > -- > > Matt Benjamin > > The Linux Box > 206 South Fifth Ave. Suite 150 > Ann Arbor, MI 48104 > > http://linuxbox.com > > tel. 734-761-4689 > fax. 734-769-8938 > cel. 734-216-5309 > --------------------------- 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. From jlan@sgi.com Tue Dec 18 11:22:09 2007 Received: with ECARTIS (v1.0.0; list kdb); Tue, 18 Dec 2007 11:22:14 -0800 (PST) Received: from cthulhu.engr.sgi.com (cthulhu.engr.sgi.com [192.26.80.2]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id lBIJM6sI031239 for ; Tue, 18 Dec 2007 11:22:08 -0800 Received: from [134.15.0.134] (mtv-vpn-sw-corp-0-134.corp.sgi.com [134.15.0.134]) by cthulhu.engr.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id lBIJMGgZ017073; Tue, 18 Dec 2007 11:22:17 -0800 Message-ID: <47681DE6.6030409@sgi.com> Date: Tue, 18 Dec 2007 11:22:14 -0800 From: Jay Lan User-Agent: Thunderbird 1.5 (X11/20060317) MIME-Version: 1.0 To: Matt Benjamin CC: kdb@oss.sgi.com Subject: Re: [ANNOUNCE] The USB ehci/uhci for KDB References: <4767323D.5040703@linuxbox.com> <47681C8F.9080706@sgi.com> In-Reply-To: <47681C8F.9080706@sgi.com> X-Enigmail-Version: 0.94.0.0 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-archive-position: 1317 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: jlan@sgi.com Precedence: bulk X-list: kdb Jay Lan wrote: > Matt Benjamin wrote: > > Hi Matt, > >> I just wanted to let folks know that I >> >> 1. got this working on x86 (x86_64, Dell Vostro 200) > > Great! > >> 1.a after moving some static function definitions around to deal with >> issues mentioned by Jay > > What kind of problem was that? I did not have problem building on > x86_64? My problem was on ia64. Ooops, i meant to cancel the message, but pressed a wrong button. I did have a compilation error on x86_64 on the uhci patch i put at oss.sgi.com. I will fix it. I would expect the uhci patch to work at least at x86_64. Thanks, - jay > >> 1.b and after hacking around an apparent bug in get_usb_char >> (kdba_io_64.c) > > I assume it is in arch/x86/kdba_io_64.c? Can you share your hacking > with us and what problem did it fix? Maybe you hit something > Konstantin did not see. > > Thanks, > - jay > > >> The proximate issue is that in (at least) the case where we (I think) we >> got no keypress, i has the value KDB_USB_NUM_KEYBOARDS, which provokes a >> trap when we call urb_complete. >> >> Thanks for getting this in, KDB developers. I absolutely needed a KDB >> on these boxes, and this arrived just in time. >> >> Matt >> >> -- >> >> Matt Benjamin >> >> The Linux Box >> 206 South Fifth Ave. Suite 150 >> Ann Arbor, MI 48104 >> >> http://linuxbox.com >> >> tel. 734-761-4689 >> fax. 734-769-8938 >> cel. 734-216-5309 >> > --------------------------- > 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. From matt@linuxbox.com Tue Dec 18 11:35:55 2007 Received: with ECARTIS (v1.0.0; list kdb); Tue, 18 Dec 2007 11:36:04 -0800 (PST) Received: from cuda.sgi.com (cuda1.sgi.com [192.48.168.28]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id lBIJZp2A032026 for ; Tue, 18 Dec 2007 11:35:55 -0800 X-ASG-Debug-ID: 1198006562-33e100180000-sLlkUa X-Barracuda-URL: http://cuda.sgi.com:80/cgi-bin/mark.cgi Received: from aa.linuxbox.com (localhost [127.0.0.1]) by cuda.sgi.com (Spam Firewall) with ESMTP id CA4781185FC7; Tue, 18 Dec 2007 11:36:02 -0800 (PST) Received: from aa.linuxbox.com (aa.linuxbox.com [134.215.213.37]) by cuda.sgi.com with ESMTP id UJ2B05cG8ZZx7ZrL; Tue, 18 Dec 2007 11:36:02 -0800 (PST) Received: from trosper.private.linuxbox.com (trosper.private.linuxbox.com [10.1.1.45]) by aa.linuxbox.com (8.13.1/8.13.1/SuSE Linux 0.7) with ESMTP id lBIJa01t030763 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Tue, 18 Dec 2007 14:36:00 -0500 Received: by trosper.private.linuxbox.com (Postfix, from userid 65534) id 0E9E2C0A38E3; Tue, 18 Dec 2007 14:36:00 -0500 (EST) Received: from secant.private.linuxbox.com (unknown [10.1.1.217]) by trosper.private.linuxbox.com (Postfix) with ESMTP id AC7F6C0A38BD; Tue, 18 Dec 2007 14:35:54 -0500 (EST) Message-ID: <47682114.5000909@linuxbox.com> Date: Tue, 18 Dec 2007 14:35:48 -0500 From: Matt Benjamin User-Agent: Thunderbird 2.0.0.9 (X11/20071031) MIME-Version: 1.0 To: Jay Lan Cc: kdb@oss.sgi.com X-ASG-Orig-Subj: Re: [ANNOUNCE] The USB ehci/uhci for KDB Subject: Re: [ANNOUNCE] The USB ehci/uhci for KDB References: <4767323D.5040703@linuxbox.com> <47681C8F.9080706@sgi.com> In-Reply-To: <47681C8F.9080706@sgi.com> X-Enigmail-Version: 0.95.5 Content-type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0.2 (aa.linuxbox.com [10.1.1.1]); Tue, 18 Dec 2007 14:36:00 -0500 (EST) X-Barracuda-Connect: aa.linuxbox.com[134.215.213.37] X-Barracuda-Start-Time: 1198006563 X-Barracuda-Bayes: INNOCENT GLOBAL 0.0000 1.0000 -2.0210 X-Barracuda-Virus-Scanned: by cuda.sgi.com at sgi.com X-Barracuda-Spam-Score: -2.02 X-Barracuda-Spam-Status: No, SCORE=-2.02 using per-user scores of TAG_LEVEL=2.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests= X-Barracuda-Spam-Report: Code version 3.1, rules version 3.1.36999 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-archive-position: 1318 X-ecartis-version: Ecartis v1.0.0 Sender: kdb-bounce@oss.sgi.com Errors-to: kdb-bounce@oss.sgi.com X-original-sender: matt@linuxbox.com Precedence: bulk X-list: kdb -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 My hack was too lame to post, I thought--I set i=0 before the indirect call to urb_complete. :) I didn't sort all the cases out, but on some exits from the loop for i to KDB_USB_NUM_KEYBOARDS, i has a value corresponding to a non-present keyboard--so the attempt to get a urb_complete function pointer (last statement in get_usb_char) from that element in the keyboard array is illegal. Matt Jay Lan wrote: > Matt Benjamin wrote: > > Hi Matt, > >> I just wanted to let folks know that I >> >> 1. got this working on x86 (x86_64, Dell Vostro 200) > > Great! > >> 1.a after moving some static function definitions around to deal with >> issues mentioned by Jay > > What kind of problem was that? I did not have problem building on > x86_64? My problem was on ia64. > >> 1.b and after hacking around an apparent bug in get_usb_char >> (kdba_io_64.c) > > I assume it is in arch/x86/kdba_io_64.c? Can you share your hacking > with us and what problem did it fix? Maybe you hit something > Konstantin did not see. > > Thanks, > - jay > > >> The proximate issue is that in (at least) the case where we (I think) we >> got no keypress, i has the value KDB_USB_NUM_KEYBOARDS, which provokes a >> trap when we call urb_complete. >> >> Thanks for getting this in, KDB developers. I absolutely needed a KDB >> on these boxes, and this arrived just in time. >> >> Matt >> >> -- >> >> Matt Benjamin >> >> The Linux Box >> 206 South Fifth Ave. Suite 150 >> Ann Arbor, MI 48104 >> >> http://linuxbox.com >> >> tel. 734-761-4689 >> fax. 734-769-8938 >> cel. 734-216-5309 >> > --------------------------- > 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. - -- Matt Benjamin The Linux Box 206 South Fifth Ave. Suite 150 Ann Arbor, MI 48104 http://linuxbox.com tel. 734-761-4689 fax. 734-769-8938 cel. 734-216-5309 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFHaCEUJiSUUSaRdSURCBHTAKCJ+7+evuNbK8yE/My1L03rRynR6gCdFVwU lziIdv1mm1ZzXOwoL9N2wPs= =zVo+ -----END PGP SIGNATURE----- --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.