[PATCH] Add support for USB Keyboard attached to UHCI

Konstantin Baydarov kbaidarov at ru.mvista.com
Tue Dec 4 06:30:40 PST 2007


On Fri, 30 Nov 2007 12:11:05 -0800 (PST)
Aaron Young <ayoung at cthulhu.engr.sgi.com> wrote:

> > 
> > On Fri, 30 Nov 2007 08:43:07 -0800 (PST)
> > Aaron Young <ayoung at cthulhu.engr.sgi.com> wrote:
> > 
> > > > 
> > > > 
> > > > 
> > > >  Looking better! More comments:
> > > > 
> > > > 1. I don't understand the need for the call to
> > > >    kdb_uhci_keyboard_clear(). It's only called if
> > > >    kdb_usb_keyboard_attach() fails and in that case we
> > > >    didn't add the urb to kdb_usb_kbds[]. So, I don't
> > > >    see the need to clear it out of kdb_usb_kbds[]...
> > > >    (I'm probably missing something here).
> >  - Now I call kdb_uhci_keyboard_clear() if usb_submit_urb() failed,
> > that is called after kdb_usb_keyboard_attach().
> 
>     Why not just call kdb_usb_keyboard_detach()?
  Got rid from kdb_uhci_keyboard_clear().

> 
> > 
> > > > 
> > > > 2. I probably would have made kdb_uhci_submit_urb() return
> > > >    the newly created kdb_urb (extra arg) on success and not
> > > > call kdb_usb_keyboard_attach() directly from within. Then you
> > > > can just make a single call to kdb_usb_keyboard_attach() for
> > > > all three cases (OHCI, EHCI and UHCI) out of hid_probe(). But,
> > > > not a *BIG* deal...
> >  - We should first fill kdb_usb_kbds[] before call of
> > usb_submit_urb() because uhci_urb_enqueue() checks kdb_usb_kbds[]
> > to find out if KDB URB is enqueued. So kdb_usb_keyboard_attach()
> > should be called in the "middle" of the function
> > kdb_uhci_submit_urb().
> 
>    OK..
> 
> > 
> > > > 
> > > > 3. We'll have to get updates to ia64 kdba_io.c as well,
> > > > otherwise it will result in compile errors on ia64. i.e.
> > > > kdb_usb_keyboard_attach() will have a number of args mismatch
> > > > and all the routines you added to kdba_io_32.c and kdba_io_64.c
> > > > will not be there.
> >  - Can't find any ia64 file...
> 
>   linux/arch/ia64/kdb/kdba_io.c
  Added arch/ia64/kdb/kdba_io.c.

> 
>   Maybe it's part of a different KDB patch or something? Jay?
> 
> > > > 
> > > >  Does hotplug/hotunplug of the keyboards work?
> > > >  Does it correctly remove the KDB URBs on hotunplug?
> >  - kdb_usb_keyboard_detach() is never called on my PC. So I tested
> > only 1 case: attaching keyboard after kernel boot - works OK.
> 
>    Should get called out of hid_disconnect() when a
>    keyboard is unplugged. If you are using a usb1.1
>    hub, it may not work - I've seen this problem with
>    USB1.1 hub. MIght want to try plugging/unplugging the keyboard
>    directly into the chassis port if so.
  Fixed hotplug/hotunplug. The problem was in my patch - UHCI URB should be unlinked from endpoin queue.

> 
>    BTW - do you have a USB2.0 hub? The EHCI KDB code has yet to
>    be tested on x86 and I'd be curious if it worked or not (hope it
> does)...
> 
>    -Aaron
  No, I don't have USB2.0 hub.

  Aaron, here is incremental patch against my last patch (http://oss.sgi.com/archives/kdb/2007-11/msg00039.html):

Index: linux-2.6.24-rc2/arch/ia64/kdb/kdba_io.c
===================================================================
--- linux-2.6.24-rc2.orig/arch/ia64/kdb/kdba_io.c
+++ linux-2.6.24-rc2/arch/ia64/kdb/kdba_io.c
@@ -65,12 +65,120 @@ static unsigned char kdb_usb_keycode[256
 	150,158,159,128,136,177,178,176,142,152,173,140
 };
 
+/* Check if URB is managed by KDB code */
+int kdb_uhci_keyboard_urb(struct urb *urb)
+{
+	int i;
+
+	for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
+		if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].urb == urb)
+			return i;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_urb);
+
+/* Check if UHCI QH is managed by KDB code */
+int kdb_uhci_keyboard_check_uhci_qh(struct uhci_qh *qh)
+{
+	int i;
+
+	for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
+		if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh)
+			return i;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_check_uhci_qh);
+
+/* Set UHCI QH using URB pointer */
+int kdb_uhci_keyboard_set_qh(struct urb *urb, struct uhci_qh *qh)
+{
+	int i;
+
+	i = kdb_uhci_keyboard_urb(urb);
+	if (i != -1)
+		kdb_usb_kbds[i].qh = qh;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_qh);
+
+/* Get UHCI QH using URB pointer */
+struct uhci_qh *kdb_uhci_keyboard_get_qh(struct urb *urb)
+{
+	int i;
+
+	i = kdb_uhci_keyboard_urb(urb);
+	if (i != -1)
+		return kdb_usb_kbds[i].qh;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_qh);
+
+/* Set UHCI hid_event using URB pointer */
+int kdb_uhci_keyboard_set_hid_event(struct urb *urb, int hid_event)
+{
+	int i;
+
+	i = kdb_uhci_keyboard_urb(urb);
+	if (i != -1)
+		kdb_usb_kbds[i].kdb_hid_event = hid_event;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_hid_event);
+
+/* Get UHCI hid_event using URB pointer */
+int kdb_uhci_keyboard_get_hid_event(struct urb *urb)
+{
+	int i;
+
+	i = kdb_uhci_keyboard_urb(urb);
+	if (i != -1)
+		return kdb_usb_kbds[i].kdb_hid_event;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_hid_event);
+
+/* Set UHCI hid_event using UHCI QH pointer */
+int kdb_uhci_keyboard_set_hid_event_qh(struct uhci_qh *qh, int hid_event)
+{
+	int i;
+
+	for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
+		if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh){
+			kdb_usb_kbds[i].kdb_hid_event = hid_event;
+			return i;
+		}
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_set_hid_event_qh);
+
+/* Get UHCI hid_event using UHCI QH pointer */
+int kdb_uhci_keyboard_get_hid_event_qh(struct uhci_qh *qh)
+{
+	int i;
+
+	for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
+		if (kdb_usb_kbds[i].urb && kdb_usb_kbds[i].qh == qh)
+			return kdb_usb_kbds[i].kdb_hid_event;
+	}
+
+	return -1;
+}
+EXPORT_SYMBOL_GPL (kdb_uhci_keyboard_get_hid_event_qh);
+
 /*
  * kdb_usb_keyboard_attach()
  * Attach a USB keyboard to kdb.
  */
 int
-kdb_usb_keyboard_attach(struct urb *urb, unsigned char *buffer, void *poll_func)
+kdb_usb_keyboard_attach(struct urb *urb, unsigned char *buffer,
+			void *poll_func, void *compl_func, struct urb *hid_urb)
 {
 	int	i;
 	int	rc = -1;
@@ -92,6 +200,8 @@ kdb_usb_keyboard_attach(struct urb *urb,
 		kdb_usb_kbds[i].urb = urb;
 		kdb_usb_kbds[i].buffer = buffer;
 		kdb_usb_kbds[i].poll_func = poll_func;
+		kdb_usb_kbds[i].urb_complete = compl_func;
+		kdb_usb_kbds[i].hid_urb = hid_urb;
 
 		rc = 0;	/* success */
 
@@ -111,6 +221,7 @@ kdb_usb_keyboard_detach(struct urb *urb)
 {
 	int	i;
 	int	rc = -1;
+	extern void kdb_unlink_uhci_qh(struct urb *urb, struct uhci_qh *qh);
 
 	if (kdb_no_usb)
 		return 0;
@@ -122,14 +233,21 @@ kdb_usb_keyboard_detach(struct urb *urb)
 	 */
 
 	for (i = 0; i < KDB_USB_NUM_KEYBOARDS; i++) {
-		if (kdb_usb_kbds[i].urb != urb)
-			continue;
+		if (kdb_usb_kbds[i].qh) {
+			/* UHCI keyboard */
+			if (kdb_usb_kbds[i].hid_urb != urb)
+	                        continue;
+			kdb_unlink_uhci_qh(kdb_usb_kbds[i].urb, kdb_usb_kbds[i].qh);
+		} else
+			if (kdb_usb_kbds[i].urb != urb)
+				continue;
 
 		/* found it, clear the index */
 		kdb_usb_kbds[i].urb = NULL;
 		kdb_usb_kbds[i].buffer = NULL;
 		kdb_usb_kbds[i].poll_func = NULL;
 		kdb_usb_kbds[i].caps_lock = 0;
+		kdb_usb_kbds[i].hid_urb = NULL;
 
 		rc = 0;	/* success */
 
@@ -152,6 +270,9 @@ get_usb_char(void)
 	int	ret;
 	unsigned char keycode, spec;
 	extern u_short plain_map[], shift_map[], ctrl_map[];
+	int     ret_key = -1, i2, j, max;
+
+	ret = -1;
 
 	if (kdb_no_usb)
 		return -1;
@@ -179,15 +300,24 @@ get_usb_char(void)
 		if (ret < 0) /* error or no characters, try the next kbd */
 			continue;
 
+		/* If 2 keys was pressed simultaneously,
+		 * both keycodes will be in buffer.
+		 * Last pressed key will be last non
+		 * zero byte.
+		 */
+		for (j=0; j<4; j++){
+			if (!kdb_usb_kbds[i].buffer[2+j])
+				break;
+		}
+		/* Last pressed key */
+		max = j + 1;
+
 		spec = kdb_usb_kbds[i].buffer[0];
-		keycode = kdb_usb_kbds[i].buffer[2];
+		keycode = kdb_usb_kbds[i].buffer[max];
 		kdb_usb_kbds[i].buffer[0] = (char)0;
 		kdb_usb_kbds[i].buffer[2] = (char)0;
 
-		if(kdb_usb_kbds[i].buffer[3]) {
-			kdb_usb_kbds[i].buffer[3] = (char)0;
-			continue;
-		}
+		ret_key = -1;
 
 		/* A normal key is pressed, decode it */
 		if(keycode)
@@ -199,10 +329,12 @@ get_usb_char(void)
 			{
 			case 0x2:
 			case 0x20: /* Shift */
-				return shift_map[keycode];
+				ret_key = shift_map[keycode];
+				break;
 			case 0x1:
 			case 0x10: /* Ctrl */
-				return ctrl_map[keycode];
+				ret_key = ctrl_map[keycode];
+				break;
 			case 0x4:
 			case 0x40: /* Alt */
 				break;
@@ -211,31 +343,47 @@ get_usb_char(void)
 			switch(keycode)
 			{
 			case 0x1C: /* Enter */
-				return 13;
+				ret_key = 13;
+				break
 
 			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.


More information about the kdb mailing list