[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei]

This patch added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support. For supporting the USB interrupt pipe, the globe urb_priv is moved to purb in ed struct. Now, we can process several urbs at one time. The interrupt pipe support codes are ported from Linux kernel 2.4.

Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
---
 drivers/usb_ohci.c |  310 +++++++++++++++++++++++++++++++++++++++++++---------
 drivers/usb_ohci.h |   12 ++-
 2 files changed, 267 insertions(+), 55 deletions(-)

diff --git a/drivers/usb_ohci.c b/drivers/usb_ohci.c
index 70cb6a3..459c809 100644
--- a/drivers/usb_ohci.c
+++ b/drivers/usb_ohci.c
@@ -1,5 +1,11 @@
 /*
- * URB OHCI HCD (Host Controller Driver) for USB on the AT91RM9200.
+ * URB OHCI HCD (Host Controller Driver) for USB on the AT91RM9200 and PCI bus.
+ *
+ * Interrupt support is added. Now, it has been tested
+ * on ULI1575 chip and works well with USB keyboard.
+ *
+ * (C) Copyright 2007
+ * Zhang Wei, Freescale Semiconductor, Inc. <wei.zhang@freescale.com>
  *
  * (C) Copyright 2003
  * Gary Jennejohn, DENX Software Engineering <gj@denx.de>
@@ -35,7 +41,7 @@
  * 1 - you MUST define LITTLEENDIAN in the configuration file for the
  *     board or this driver will NOT work!
  * 2 - this driver is intended for use with USB Mass Storage Devices
- *     (BBB) ONLY. There is NO support for Interrupt or Isochronous pipes!
+ *     (BBB) and USB keyboard. There is NO support for Isochronous pipes!
  * 3 - when running on a PQFP208 AT91RM9200, define CONFIG_AT91C_PQFP_UHPBUG
  *     to activate workaround for bug #41 or this driver will NOT work!
  */
@@ -56,6 +62,8 @@
 # include <asm/arch/pxa-regs.h>
 #elif defined(CONFIG_MPC5200)
 # include <mpc5xxx.h>
+#elif defined(CONFIG_PCI_OHCI)
+# include <pci.h>
 #endif
 
 #include <malloc.h>
@@ -66,6 +74,7 @@
     defined(CONFIG_S3C2400) || \
     defined(CONFIG_S3C2410) || \
     defined(CONFIG_440EP) || \
+    defined(CONFIG_PCI_OHCI) || \
     defined(CONFIG_MPC5200)
 # define OHCI_USE_NPS		/* force NoPowerSwitching mode */
 #endif
@@ -79,12 +88,19 @@
 #define OHCI_CONTROL_INIT \
 	(OHCI_CTRL_CBSR & 0x3) | OHCI_CTRL_IE | OHCI_CTRL_PLE
 
-#define readl(a) (*((vu_long *)(a)))
-#define writel(a, b) (*((vu_long *)(b)) = ((vu_long)a))
+#define readl(a) m32_swap(*((vu_long *)(a)))
+#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))
 
 #define min_t(type,x,y) ({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
 
-#undef DEBUG
+#ifdef CONFIG_PCI_OHCI
+static struct pci_device_id ohci_pci_ids[] = {
+	{0x10b9, 0x5237},	/* ULI1575 PCI OHCI module ids */
+	/* Please add supported PCI OHCI controller ids here */
+	{0, 0}
+};
+#endif
+
 #ifdef DEBUG
 #define dbg(format, arg...) printf("DEBUG: " format "\n", ## arg)
 #else
@@ -114,15 +130,11 @@ struct ohci_hcca ghcca[1];
 struct ohci_hcca *phcca;
 /* this allocates EDs for all possible endpoints */
 struct ohci_device ohci_dev;
-/* urb_priv */
-urb_priv_t urb_priv;
 /* RHSC flag */
 int got_rhsc;
 /* device which was disconnected */
 struct usb_device *devgone;
 
-/* flag guarding URB transation */
-int urb_finished = 0;
 
 
 /*-------------------------------------------------------------------------*/
@@ -177,6 +189,7 @@ static void urb_free_priv (urb_priv_t * urb)
 			}
 		}
 	}
+	free(urb);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -187,11 +200,10 @@ static int sohci_get_current_frame_number (struct usb_device * dev);
 /* debug| print the main components of an URB
  * small: 0) header + data packets 1) just header */
 
-static void pkt_print (struct usb_device * dev, unsigned long pipe, void * buffer,
+static void pkt_print (urb_priv_t *purb, struct usb_device * dev,
+	unsigned long pipe, void * buffer,
 	int transfer_len, struct devrequest * setup, char * str, int small)
 {
-	urb_priv_t * purb = &urb_priv;
-
 	dbg("%s URB:[%4x] dev:%2d,ep:%2d-%c,type:%s,len:%d/%d stat:%#lx",
 			str,
 			sohci_get_current_frame_number (dev),
@@ -200,7 +212,7 @@ static void pkt_print (struct usb_device * dev, unsigned long pipe, void * buffe
 			usb_pipeout (pipe)? 'O': 'I',
 			usb_pipetype (pipe) < 2? (usb_pipeint (pipe)? "INTR": "ISOC"):
 				(usb_pipecontrol (pipe)? "CTRL": "BULK"),
-			purb->actual_length,
+			(purb ? purb->actual_length : 0),
 			transfer_len, dev->status);
 #ifdef	OHCI_VERBOSE_DEBUG
 	if (!small) {
@@ -214,10 +226,11 @@ static void pkt_print (struct usb_device * dev, unsigned long pipe, void * buffe
 		}
 		if (transfer_len > 0 && buffer) {
 			printf (__FILE__ ": data(%d/%d):",
-				purb->actual_length,
+				(purb ? purb->actual_length : 0),
 				transfer_len);
 			len = usb_pipeout (pipe)?
-					transfer_len: purb->actual_length;
+					transfer_len:
+					(purb ? purb->actual_length : 0);
 			for (i = 0; i < 16 && i < len; i++)
 				printf (" %02x", ((__u8 *) buffer) [i]);
 			printf ("%s\n", i < len? "...": "");
@@ -413,13 +426,17 @@ static void ohci_dump (ohci_t *controller, int verbose)
 
 /* get a transfer request */
 
-int sohci_submit_job(struct usb_device *dev, unsigned long pipe, void *buffer,
-		int transfer_len, struct devrequest *setup, int interval)
+int sohci_submit_job(urb_priv_t *urb, struct devrequest *setup)
 {
 	ohci_t *ohci;
 	ed_t * ed;
-	urb_priv_t *purb_priv;
+	urb_priv_t *purb_priv = urb;
 	int i, size = 0;
+	struct usb_device *dev = urb->dev;
+	unsigned long pipe = urb->pipe;
+	void *buffer = urb->transfer_buffer;
+	int transfer_len = urb->transfer_buffer_length;
+	int interval = urb->interval;
 
 	ohci = &gohci;
 
@@ -430,18 +447,11 @@ int sohci_submit_job(struct usb_device *dev, unsigned long pipe, void *buffer,
 		return -1;
 	}
 
-	/* if we have an unfinished URB from previous transaction let's
-	 * fail and scream as quickly as possible so as not to corrupt
-	 * further communication */
-	if (!urb_finished) {
-		err("sohci_submit_job: URB NOT FINISHED");
-		return -1;
-	}
 	/* we're about to begin a new transaction here so mark the URB unfinished */
-	urb_finished = 0;
+	urb->finished = 0;
 
 	/* every endpoint has a ed, locate and fill it */
-	if (!(ed = ep_add_ed (dev, pipe))) {
+	if (!(ed = ep_add_ed (dev, pipe, interval, 1))) {
 		err("sohci_submit_job: ENOMEM");
 		return -1;
 	}
@@ -455,13 +465,17 @@ int sohci_submit_job(struct usb_device *dev, unsigned long pipe, void *buffer,
 			size = (transfer_len == 0)? 2:
 						(transfer_len - 1) / 4096 + 3;
 			break;
+		case PIPE_INTERRUPT: /* 1 TD */
+			size = 1;
+			break;
 	}
 
+	ed->purb = urb;
+
 	if (size >= (N_URB_TD - 1)) {
 		err("need %d TDs, only have %d", size, N_URB_TD);
 		return -1;
 	}
-	purb_priv = &urb_priv;
 	purb_priv->pipe = pipe;
 
 	/* fill the private part of the URB */
@@ -497,6 +511,40 @@ int sohci_submit_job(struct usb_device *dev, unsigned long pipe, void *buffer,
 	return 0;
 }
 
+static inline int sohci_return_job(struct ohci *hc, urb_priv_t *urb)
+{
+	struct ohci_regs *regs = hc->regs;
+
+	switch (usb_pipetype (urb->pipe)) {
+	case PIPE_INTERRUPT:
+		/* implicitly requeued */
+		if (urb->dev->irq_handle &&
+				(urb->dev->irq_act_len = urb->actual_length)) {
+			writel (OHCI_INTR_WDH, &regs->intrenable);
+			readl (&regs->intrenable); /* PCI posting flush */
+			urb->dev->irq_handle(urb->dev);
+			writel (OHCI_INTR_WDH, &regs->intrdisable);
+			readl (&regs->intrdisable); /* PCI posting flush */
+		}
+		urb->actual_length = 0;
+		td_submit_job (
+				urb->dev,
+				urb->pipe,
+				urb->transfer_buffer,
+				urb->transfer_buffer_length,
+				NULL,
+				urb,
+				urb->interval);
+		break;
+	case PIPE_CONTROL:
+	case PIPE_BULK:
+		break;
+	default:
+		return 0;
+	}
+	return 1;
+}
+
 /*-------------------------------------------------------------------------*/
 
 #ifdef DEBUG
@@ -514,13 +562,73 @@ static int sohci_get_current_frame_number (struct usb_device *usb_dev)
  * ED handling functions
  *-------------------------------------------------------------------------*/
 
+/* search for the right branch to insert an interrupt ed into the int tree
+ * do some load ballancing;
+ * returns the branch and
+ * sets the interval to interval = 2^integer (ld (interval)) */
+
+static int ep_int_ballance (ohci_t * ohci, int interval, int load)
+{
+	int i, branch = 0;
+
+	/* search for the least loaded interrupt endpoint
+	 * branch of all 32 branches
+	 */
+	for (i = 0; i < 32; i++)
+		if (ohci->ohci_int_load [branch] > ohci->ohci_int_load [i])
+			branch = i;
+
+	branch = branch % interval;
+	for (i = branch; i < 32; i += interval)
+		ohci->ohci_int_load [i] += load;
+
+	return branch;
+}
+
+/*-------------------------------------------------------------------------*/
+
+/*  2^int( ld (inter)) */
+
+static int ep_2_n_interval (int inter)
+{
+	int i;
+	for (i = 0; ((inter >> i) > 1 ) && (i < 5); i++);
+	return 1 << i;
+}
+
+/*-------------------------------------------------------------------------*/
+
+/* the int tree is a binary tree
+ * in order to process it sequentially the indexes of the branches have to be mapped
+ * the mapping reverses the bits of a word of num_bits length */
+
+static int ep_rev (int num_bits, int word)
+{
+	int i, wout = 0;
+
+	for (i = 0; i < num_bits; i++)
+		wout |= (((word >> i) & 1) << (num_bits - i - 1));
+	return wout;
+}
+
+/*-------------------------------------------------------------------------*
+ * ED handling functions
+ *-------------------------------------------------------------------------*/
+
 /* link an ed into one of the HC chains */
 
 static int ep_link (ohci_t *ohci, ed_t *edi)
 {
 	volatile ed_t *ed = edi;
+	int int_branch;
+	int i;
+	int inter;
+	int interval;
+	int load;
+	__u32 * ed_p;
 
 	ed->state = ED_OPER;
+	ed->int_interval = 0;
 
 	switch (ed->type) {
 	case PIPE_CONTROL:
@@ -554,12 +662,49 @@ static int ep_link (ohci_t *ohci, ed_t *edi)
 		}
 		ohci->ed_bulktail = edi;
 		break;
+
+	case PIPE_INTERRUPT:
+		load = ed->int_load;
+		interval = ep_2_n_interval (ed->int_period);
+		ed->int_interval = interval;
+		int_branch = ep_int_ballance (ohci, interval, load);
+		ed->int_branch = int_branch;
+
+		for (i = 0; i < ep_rev (6, interval); i += inter) {
+			inter = 1;
+			for (ed_p = &(ohci->hcca->int_table[ep_rev (5, i) + int_branch]);
+				(*ed_p != 0) && (((ed_t *)ed_p)->int_interval >= interval);
+				ed_p = &(((ed_t *)ed_p)->hwNextED))
+					inter = ep_rev (6, ((ed_t *)ed_p)->int_interval);
+			ed->hwNextED = *ed_p;
+			*ed_p = m32_swap(ed);
+		}
+		break;
 	}
 	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
 
+/* scan the periodic table to find and unlink this ED */
+static void periodic_unlink ( struct ohci *ohci, volatile struct ed *ed,
+		unsigned index, unsigned period)
+{
+	for (; index < NUM_INTS; index += period) {
+		__u32	*ed_p = &ohci->hcca->int_table [index];
+
+		/* ED might have been unlinked through another path */
+		while (*ed_p != 0) {
+			if (((struct ed *)m32_swap (ed_p)) == ed) {
+				*ed_p = ed->hwNextED;
+				break;
+			}
+			ed_p = & (((struct ed *)m32_swap (ed_p))->hwNextED);
+		}
+	}
+}
+
+
 /* unlink an ed from one of the HC chains.
  * just the link to the ed is unlinked.
  * the link from the ed still points to another operational ed or 0
@@ -568,6 +713,7 @@ static int ep_link (ohci_t *ohci, ed_t *edi)
 static int ep_unlink (ohci_t *ohci, ed_t *edi)
 {
 	volatile ed_t *ed = edi;
+	int i;
 
 	ed->hwINFO |= m32_swap (OHCI_ED_SKIP);
 
@@ -605,6 +751,12 @@ static int ep_unlink (ohci_t *ohci, ed_t *edi)
 			((ed_t *)m32_swap (*((__u32 *)&ed->hwNextED)))->ed_prev = ed->ed_prev;
 		}
 		break;
+
+	case PIPE_INTERRUPT:
+		periodic_unlink (ohci, ed, 0, 1);
+		for (i = ed->int_branch; i < 32; i += ed->int_interval)
+		    ohci->ohci_int_load[i] -= ed->int_load;
+		break;
 	}
 	ed->state = ED_UNLINK;
 	return 0;
@@ -621,7 +773,8 @@ static int ep_unlink (ohci_t *ohci, ed_t *edi)
  * info fields are setted anyway even though most of them should not
  * change
  */
-static ed_t * ep_add_ed (struct usb_device *usb_dev, unsigned long pipe)
+static ed_t * ep_add_ed (struct usb_device *usb_dev, unsigned long pipe,
+		int interval, int load)
 {
 	td_t *td;
 	ed_t *ed_ret;
@@ -654,6 +807,11 @@ static ed_t * ep_add_ed (struct usb_device *usb_dev, unsigned long pipe)
 			| usb_pipeslow (pipe) << 13
 			| usb_maxpacket (usb_dev, pipe) << 16);
 
+	if (ed->type == PIPE_INTERRUPT && ed->state == ED_UNLINK) {
+		ed->int_period = interval;
+		ed->int_load = load;
+	}
+
 	return ed_ret;
 }
 
@@ -768,6 +926,13 @@ static void td_submit_job (struct usb_device *dev, unsigned long pipe, void *buf
 		if (!ohci->sleeping)
 			writel (OHCI_CLF, &ohci->regs->cmdstatus); /* start Control list */
 		break;
+
+	case PIPE_INTERRUPT:
+		info = usb_pipeout (urb->pipe)?
+			TD_CC | TD_DP_OUT | toggle:
+			TD_CC | TD_R | TD_DP_IN | toggle;
+		td_fill (ohci, info, data, data_len, dev, cnt++, urb);
+		break;
 	}
 	if (urb->length != cnt)
 		dbg("TD LENGTH %d != CNT %d", urb->length, cnt);
@@ -783,7 +948,7 @@ static void td_submit_job (struct usb_device *dev, unsigned long pipe, void *buf
 static void dl_transfer_length(td_t * td)
 {
 	__u32 tdINFO, tdBE, tdCBP;
-	urb_priv_t *lurb_priv = &urb_priv;
+	urb_priv_t *lurb_priv = td->ed->purb;
 
 	tdINFO = m32_swap (td->hwINFO);
 	tdBE   = m32_swap (td->hwBE);
@@ -820,7 +985,7 @@ static td_t * dl_reverse_done_list (ohci_t *ohci)
 		td_list = (td_t *)td_list_hc;
 
 		if (TD_CC_GET (m32_swap (td_list->hwINFO))) {
-			lurb_priv = &urb_priv;
+			lurb_priv = td_list->ed->purb;
 			dbg(" USB-error/status: %x : %p",
 					TD_CC_GET (m32_swap (td_list->hwINFO)), td_list);
 			if (td_list->ed->hwHeadP & m32_swap (0x1)) {
@@ -860,10 +1025,10 @@ static int dl_done_list (ohci_t *ohci, td_t *td_list)
 	while (td_list) {
 		td_list_next = td_list->next_dl_td;
 
-		lurb_priv = &urb_priv;
 		tdINFO = m32_swap (td_list->hwINFO);
 
 		ed = td_list->ed;
+		lurb_priv = ed->purb;
 
 		dl_transfer_length(td_list);
 
@@ -882,14 +1047,16 @@ static int dl_done_list (ohci_t *ohci, td_t *td_list)
 			    (lurb_priv->state != URB_DEL))
 #else
 			if ((ed->state & (ED_OPER | ED_UNLINK)))
-				urb_finished = 1;
+#endif
+				lurb_priv->finished = sohci_return_job(ohci,
+						lurb_priv);
 			else
 				dbg("dl_done_list: strange.., ED state %x, ed->state\n");
 		} else
 			dbg("dl_done_list: processing TD %x, len %x\n", lurb_priv->td_cnt,
 				lurb_priv->length);
-#endif
-		if (ed->state != ED_NEW) {
+		if (ed->state != ED_NEW &&
+			  (usb_pipetype (lurb_priv->pipe) != PIPE_INTERRUPT)) {
 			edHeadP = m32_swap (ed->hwHeadP) & 0xfffffff0;
 			edTailP = m32_swap (ed->hwTailP);
 
@@ -1063,8 +1230,7 @@ static int ohci_submit_rh_msg(struct usb_device *dev, unsigned long pipe,
 	__u16 wLength;
 
 #ifdef DEBUG
-urb_priv.actual_length = 0;
-pkt_print(dev, pipe, buffer, transfer_len, cmd, "SUB(rh)", usb_pipein(pipe));
+pkt_print(NULL, dev, pipe, buffer, transfer_len, cmd, "SUB(rh)", usb_pipein(pipe));
 #else
 	wait_ms(1);
 #endif
@@ -1279,9 +1445,7 @@ pkt_print(dev, pipe, buffer, transfer_len, cmd, "SUB(rh)", usb_pipein(pipe));
 	dev->status = stat;
 
 #ifdef DEBUG
-	if (transfer_len)
-		urb_priv.actual_length = transfer_len;
-	pkt_print(dev, pipe, buffer, transfer_len, cmd, "RET(rh)", 0/*usb_pipein(pipe)*/);
+	pkt_print(NULL, dev, pipe, buffer, transfer_len, cmd, "RET(rh)", 0/*usb_pipein(pipe)*/);
 #else
 	wait_ms(1);
 #endif
@@ -1299,6 +1463,16 @@ int submit_common_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 	int stat = 0;
 	int maxsize = usb_maxpacket(dev, pipe);
 	int timeout;
+	urb_priv_t *urb;
+
+	urb = malloc(sizeof(urb_priv_t));
+	memset(urb, 0, sizeof(urb_priv_t));
+
+	urb->dev = dev;
+	urb->pipe = pipe;
+	urb->transfer_buffer = buffer;
+	urb->transfer_buffer_length = transfer_len;
+	urb->interval = interval;
 
 	/* device pulled? Shortcut the action. */
 	if (devgone == dev) {
@@ -1307,8 +1481,8 @@ int submit_common_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 	}
 
 #ifdef DEBUG
-	urb_priv.actual_length = 0;
-	pkt_print(dev, pipe, buffer, transfer_len, setup, "SUB", usb_pipein(pipe));
+	urb->actual_length = 0;
+	pkt_print(urb, dev, pipe, buffer, transfer_len, setup, "SUB", usb_pipein(pipe));
 #else
 	wait_ms(1);
 #endif
@@ -1318,7 +1492,7 @@ int submit_common_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 		return -1;
 	}
 
-	if (sohci_submit_job(dev, pipe, buffer, transfer_len, setup, interval) < 0) {
+	if (sohci_submit_job(urb, setup) < 0) {
 		err("sohci_submit_job failed");
 		return -1;
 	}
@@ -1353,18 +1527,20 @@ int submit_common_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 		 * finished we need to re-iterate this loop so as
 		 * hc_interrupt() gets called again as there needs to be some
 		 * more TD's to process still */
-		if ((stat >= 0) && (stat != 0xff) && (urb_finished)) {
+		if ((stat >= 0) && (stat != 0xff) && (urb->finished)) {
 			/* 0xff is returned for an SF-interrupt */
 			break;
 		}
 
 		if (--timeout) {
 			wait_ms(1);
+			if (!urb->finished)
+				dbg("\%");
+
 		} else {
 			err("CTL:TIMEOUT ");
 			dbg("submit_common_msg: TO status %x\n", stat);
-			stat = USB_ST_CRC_ERR;
-			urb_finished = 1;
+			urb->finished = 1;
 			stat = USB_ST_CRC_ERR;
 			break;
 		}
@@ -1374,13 +1550,14 @@ int submit_common_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 	dev->act_len = transfer_len;
 
 #ifdef DEBUG
-	pkt_print(dev, pipe, buffer, transfer_len, setup, "RET(ctlr)", usb_pipein(pipe));
+	pkt_print(urb, dev, pipe, buffer, transfer_len, setup, "RET(ctlr)", usb_pipein(pipe));
 #else
 	wait_ms(1);
 #endif
 
 	/* free TDs in urb_priv */
-	urb_free_priv (&urb_priv);
+	if (usb_pipetype (pipe) != PIPE_INTERRUPT)
+		urb_free_priv (urb);
 	return 0;
 }
 
@@ -1399,8 +1576,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	info("submit_control_msg");
 #ifdef DEBUG
-	urb_priv.actual_length = 0;
-	pkt_print(dev, pipe, buffer, transfer_len, setup, "SUB", usb_pipein(pipe));
+	pkt_print(NULL, dev, pipe, buffer, transfer_len, setup, "SUB", usb_pipein(pipe));
 #else
 	wait_ms(1);
 #endif
@@ -1423,7 +1599,8 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 		int transfer_len, int interval)
 {
 	info("submit_int_msg");
-	return -1;
+	return submit_common_msg(dev, pipe, buffer, transfer_len, NULL,
+			interval);
 }
 
 /*-------------------------------------------------------------------------*
@@ -1537,6 +1714,12 @@ static int hc_start (ohci_t * ohci)
 
 /*-------------------------------------------------------------------------*/
 
+/* Poll USB interrupt. */
+void usb_event_poll(void)
+{
+	hc_interrupt();
+}
+
 /* an interrupt happens */
 
 static int hc_interrupt (void)
@@ -1587,8 +1770,10 @@ static int hc_interrupt (void)
 	if (ints & OHCI_INTR_WDH) {
 		wait_ms(1);
 		writel (OHCI_INTR_WDH, &regs->intrdisable);
+		(void)readl (&regs->intrdisable); /* flush */
 		stat = dl_done_list (&gohci, dl_reverse_done_list (&gohci));
 		writel (OHCI_INTR_WDH, &regs->intrenable);
+		(void)readl (&regs->intrdisable); /* flush */
 	}
 
 	if (ints & OHCI_INTR_SO) {
@@ -1634,6 +1819,9 @@ static char ohci_inited = 0;
 
 int usb_lowlevel_init(void)
 {
+#ifdef CONFIG_PCI_OHCI
+	pci_dev_t pdev;
+#endif
 
 #ifdef CFG_USB_OHCI_CPU_INIT
 	/* cpu dependant init */
@@ -1647,7 +1835,6 @@ int usb_lowlevel_init(void)
 		return -1;
 #endif
 	memset (&gohci, 0, sizeof (ohci_t));
-	memset (&urb_priv, 0, sizeof (urb_priv_t));
 
 	/* align the storage */
 	if ((__u32)&ghcca[0] & 0xff) {
@@ -1673,7 +1860,25 @@ int usb_lowlevel_init(void)
 	gohci.disabled = 1;
 	gohci.sleeping = 0;
 	gohci.irq = -1;
+#ifdef CONFIG_PCI_OHCI
+	pdev = pci_find_devices(ohci_pci_ids, 0);
+
+	if (pdev != -1) {
+		u16 vid, did;
+		u32 base;
+		pci_read_config_word(pdev, PCI_VENDOR_ID, &vid);
+		pci_read_config_word(pdev, PCI_DEVICE_ID, &did);
+		printf("OHCI pci controller (%04x, %04x) found @(%d:%d:%d)\n",
+				vid, did, (pdev >> 16) & 0xff,
+				(pdev >> 11) & 0x1f, (pdev >> 8) & 0x7);
+		pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &base);
+		printf("OHCI regs address 0x%08x\n", base);
+		gohci.regs = (struct ohci_regs *)base;
+	} else
+		return -1;
+#else
 	gohci.regs = (struct ohci_regs *)CFG_USB_OHCI_REGS_BASE;
+#endif
 
 	gohci.flags = 0;
 	gohci.slot_name = CFG_USB_OHCI_SLOT_NAME;
@@ -1716,7 +1921,6 @@ int usb_lowlevel_init(void)
 	ohci_dump (&gohci, 1);
 #else
 	wait_ms(1);
-	urb_finished = 1;
 #endif
 	ohci_inited = 1;
 	return 0;
diff --git a/drivers/usb_ohci.h b/drivers/usb_ohci.h
index 95fbc44..64fa614 100644
--- a/drivers/usb_ohci.h
+++ b/drivers/usb_ohci.h
@@ -64,7 +64,8 @@ struct ed {
 	struct ed *ed_rm_list;
 
 	struct usb_device *usb_dev;
-	__u32 unused[3];
+	void *purb;
+	__u32 unused[2];
 } __attribute((aligned(16)));
 typedef struct ed ed_t;
 
@@ -349,9 +350,14 @@ typedef struct
 	ed_t *ed;
 	__u16 length;	/* number of tds associated with this request */
 	__u16 td_cnt;	/* number of tds already serviced */
+	struct usb_device *dev;
 	int   state;
 	unsigned long pipe;
+	void *transfer_buffer;
+	int transfer_buffer_length;
+	int interval;
 	int actual_length;
+	int finished;
 	td_t *td[N_URB_TD];	/* list pointer to all corresponding TDs associated with this request */
 } urb_priv_t;
 #define URB_DEL 1
@@ -375,6 +381,7 @@ typedef struct ohci {
 
 	struct ohci_regs *regs; /* OHCI controller's memory */
 
+	int ohci_int_load[32];	 /* load of the 32 Interrupt Chains (for load balancing)*/
 	ed_t *ed_rm_list[2];	 /* lists of all endpoints to be removed */
 	ed_t *ed_bulktail;	 /* last endpoint of bulk list */
 	ed_t *ed_controltail;	 /* last endpoint of control list */
@@ -397,7 +404,8 @@ struct ohci_device {
 /* endpoint */
 static int ep_link(ohci_t * ohci, ed_t * ed);
 static int ep_unlink(ohci_t * ohci, ed_t * ed);
-static ed_t * ep_add_ed(struct usb_device * usb_dev, unsigned long pipe);
+static ed_t * ep_add_ed(struct usb_device * usb_dev, unsigned long pipe,
+		int interval, int load);
 
 /*-------------------------------------------------------------------------*/
 
-- 
1.5.1

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Zhang Wei,

I have some minor issues with your this patch.

Zhang Wei <wei.zhang@freescale.com> writes:

> This patch added USB PCI-OHCI chips support, interrupt pipe support
> and usb event poll support. For supporting the USB interrupt pipe, the
> globe urb_priv is moved to purb in ed struct. Now, we can process
> several urbs at one time. The interrupt pipe support codes are ported
> from Linux kernel 2.4.
>
> Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
> ---
>  drivers/usb_ohci.c | 310
> +++++++++++++++++++++++++++++++++++++++++++---------
>  drivers/usb_ohci.h |   12 ++-
>  2 files changed, 267 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/usb_ohci.c b/drivers/usb_ohci.c
> index 70cb6a3..459c809 100644
> --- a/drivers/usb_ohci.c
> +++ b/drivers/usb_ohci.c
> @@ -1,5 +1,11 @@
>  /*
> - * URB OHCI HCD (Host Controller Driver) for USB on the AT91RM9200.
> + * URB OHCI HCD (Host Controller Driver) for USB on the AT91RM9200
> and PCI bus.
> + *
> + * Interrupt support is added. Now, it has been tested
> + * on ULI1575 chip and works well with USB keyboard.
> + *
> + * (C) Copyright 2007
> + * Zhang Wei, Freescale Semiconductor, Inc. <wei.zhang@freescale.com>
>   *
>   * (C) Copyright 2003
>   * Gary Jennejohn, DENX Software Engineering <gj@denx.de>
> @@ -35,7 +41,7 @@
>   * 1 - you MUST define LITTLEENDIAN in the configuration file for the
>   *     board or this driver will NOT work!
>   * 2 - this driver is intended for use with USB Mass Storage Devices
> - * (BBB) ONLY. There is NO support for Interrupt or Isochronous
> pipes!
> + * (BBB) and USB keyboard. There is NO support for Isochronous pipes!
>   * 3 - when running on a PQFP208 AT91RM9200, define
> CONFIG_AT91C_PQFP_UHPBUG
>   * to activate workaround for bug #41 or this driver will NOT work!
>   */
> @@ -56,6 +62,8 @@
>  # include <asm/arch/pxa-regs.h>
>  #elif defined(CONFIG_MPC5200)
>  # include <mpc5xxx.h>
> +#elif defined(CONFIG_PCI_OHCI)
> +# include <pci.h>
>  #endif
>  
>  #include <malloc.h>
> @@ -66,6 +74,7 @@
>      defined(CONFIG_S3C2400) || \
>      defined(CONFIG_S3C2410) || \
>      defined(CONFIG_440EP) || \
> +    defined(CONFIG_PCI_OHCI) || \
>      defined(CONFIG_MPC5200)
>  # define OHCI_USE_NPS		/* force NoPowerSwitching mode */
>  #endif
> @@ -79,12 +88,19 @@
>  #define OHCI_CONTROL_INIT \
>  	(OHCI_CTRL_CBSR & 0x3) | OHCI_CTRL_IE | OHCI_CTRL_PLE
>  
> -#define readl(a) (*((vu_long *)(a)))
> -#define writel(a, b) (*((vu_long *)(b)) = ((vu_long)a))
> +#define readl(a) m32_swap(*((vu_long *)(a)))
> +#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))

Can you explain why this is needed? I'm well aware that the endianness
handling in the USB Code has become pretty awfull, so I'm trying to at
least understand why what is needed.

>  #define min_t(type,x,y) ({ type __x = (x); type __y = (y); __x < __y
> ? __x: __y; })
>  
> -#undef DEBUG
> +#ifdef CONFIG_PCI_OHCI
> +static struct pci_device_id ohci_pci_ids[] = {
> +	{0x10b9, 0x5237},	/* ULI1575 PCI OHCI module ids */
> +	/* Please add supported PCI OHCI controller ids here */
> +	{0, 0}
> +};
> +#endif
> +
>  #ifdef DEBUG
>  #define dbg(format, arg...) printf("DEBUG: " format "\n", ## arg)
>  #else
> @@ -114,15 +130,11 @@ struct ohci_hcca ghcca[1];
>  struct ohci_hcca *phcca;
>  /* this allocates EDs for all possible endpoints */
>  struct ohci_device ohci_dev;
> -/* urb_priv */
> -urb_priv_t urb_priv;
>  /* RHSC flag */
>  int got_rhsc;
>  /* device which was disconnected */
>  struct usb_device *devgone;
>  
> -/* flag guarding URB transation */
> -int urb_finished = 0;

Can you explain why this flag is not needed anymore? We had a discussion
about this some time ago and agreed that is should not be removed unless
we understand exactly why it was put there.

>  /*-------------------------------------------------------------------------*/
> @@ -177,6 +189,7 @@ static void urb_free_priv (urb_priv_t * urb)
>  			}
>  		}
>  	}
> +	free(urb);
>  }
>  
>  /*-------------------------------------------------------------------------*/
> @@ -187,11 +200,10 @@ static int sohci_get_current_frame_number
> (struct usb_device * dev);
>  /* debug| print the main components of an URB
>   * small: 0) header + data packets 1) just header */
>  
> -static void pkt_print (struct usb_device * dev, unsigned long pipe,
> void * buffer,
> +static void pkt_print (urb_priv_t *purb, struct usb_device * dev,
> +	unsigned long pipe, void * buffer,
>  	int transfer_len, struct devrequest * setup, char * str, int
> small)
>  {
> -	urb_priv_t * purb = &urb_priv;
> -
>  	dbg("%s URB:[%4x] dev:%2d,ep:%2d-%c,type:%s,len:%d/%d
> stat:%#lx",
>  			str,
>  			sohci_get_current_frame_number (dev),
> @@ -200,7 +212,7 @@ static void pkt_print (struct usb_device * dev,
> unsigned long pipe, void * buffe
>  			usb_pipeout (pipe)? 'O': 'I',
>  			usb_pipetype (pipe) < 2? (usb_pipeint (pipe)?
> "INTR": "ISOC"):
>  				(usb_pipecontrol (pipe)? "CTRL":
> "BULK"),
> -			purb->actual_length,
> +			(purb ? purb->actual_length : 0),
>  			transfer_len, dev->status);
>  #ifdef	OHCI_VERBOSE_DEBUG
>  	if (!small) {
> @@ -214,10 +226,11 @@ static void pkt_print (struct usb_device * dev,
> unsigned long pipe, void * buffe
>  		}
>  		if (transfer_len > 0 && buffer) {
>  			printf (__FILE__ ": data(%d/%d):",
> -				purb->actual_length,
> +				(purb ? purb->actual_length : 0),
>  				transfer_len);
>  			len = usb_pipeout (pipe)?
> - transfer_len: purb->actual_length;
> +					transfer_len:
> + (purb ? purb->actual_length : 0);
>  			for (i = 0; i < 16 && i < len; i++)
>  				printf (" %02x", ((__u8 *) buffer)
> [i]);
>  			printf ("%s\n", i < len? "...": "");
> @@ -413,13 +426,17 @@ static void ohci_dump (ohci_t *controller, int
> verbose)
>  
>  /* get a transfer request */
>  
> -int sohci_submit_job(struct usb_device *dev, unsigned long pipe, void
> *buffer,
> - int transfer_len, struct devrequest *setup, int interval)
> +int sohci_submit_job(urb_priv_t *urb, struct devrequest *setup)
>  {
>  	ohci_t *ohci;
>  	ed_t * ed;
> -	urb_priv_t *purb_priv;
> +	urb_priv_t *purb_priv = urb;
>  	int i, size = 0;
> +	struct usb_device *dev = urb->dev;
> +	unsigned long pipe = urb->pipe;
> +	void *buffer = urb->transfer_buffer;
> +	int transfer_len = urb->transfer_buffer_length;
> +	int interval = urb->interval;
>  
>  	ohci = &gohci;
>  
> @@ -430,18 +447,11 @@ int sohci_submit_job(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  		return -1;
>  	}
>  
> - /* if we have an unfinished URB from previous transaction let's
> - * fail and scream as quickly as possible so as not to corrupt
> -	 * further communication */
> -	if (!urb_finished) {
> -		err("sohci_submit_job: URB NOT FINISHED");
> -		return -1;
> -	}
>  	/* we're about to begin a new transaction here so mark the URB
> unfinished */
> -	urb_finished = 0;
> +	urb->finished = 0;
>  
>  	/* every endpoint has a ed, locate and fill it */
> -	if (!(ed = ep_add_ed (dev, pipe))) {
> +	if (!(ed = ep_add_ed (dev, pipe, interval, 1))) {
>  		err("sohci_submit_job: ENOMEM");
>  		return -1;
>  	}
> @@ -455,13 +465,17 @@ int sohci_submit_job(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  			size = (transfer_len == 0)? 2:
>  						(transfer_len - 1) /
> 4096 + 3;
>  			break;
> +		case PIPE_INTERRUPT: /* 1 TD */
> +			size = 1;
> +			break;
>  	}
>  
> +	ed->purb = urb;
> +
>  	if (size >= (N_URB_TD - 1)) {
>  		err("need %d TDs, only have %d", size, N_URB_TD);
>  		return -1;
>  	}
> -	purb_priv = &urb_priv;
>  	purb_priv->pipe = pipe;
>  
>  	/* fill the private part of the URB */
> @@ -497,6 +511,40 @@ int sohci_submit_job(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  	return 0;
>  }
>  
> +static inline int sohci_return_job(struct ohci *hc, urb_priv_t *urb)
> +{
> +	struct ohci_regs *regs = hc->regs;
> +
> +	switch (usb_pipetype (urb->pipe)) {
> +	case PIPE_INTERRUPT:
> +		/* implicitly requeued */
> +		if (urb->dev->irq_handle &&
> + (urb->dev->irq_act_len = urb->actual_length)) {
> +			writel (OHCI_INTR_WDH, &regs->intrenable);
> + readl (&regs->intrenable); /* PCI posting flush */
> +			urb->dev->irq_handle(urb->dev);
> +			writel (OHCI_INTR_WDH, &regs->intrdisable);
> + readl (&regs->intrdisable); /* PCI posting flush */
> +		}
> +		urb->actual_length = 0;
> +		td_submit_job (
> +				urb->dev,
> +				urb->pipe,
> +				urb->transfer_buffer,
> +				urb->transfer_buffer_length,
> +				NULL,
> +				urb,
> +				urb->interval);
> +		break;
> +	case PIPE_CONTROL:
> +	case PIPE_BULK:
> +		break;
> +	default:
> +		return 0;
> +	}
> +	return 1;
> +}
> +
>  /*-------------------------------------------------------------------------*/
>  
>  #ifdef DEBUG
> @@ -514,13 +562,73 @@ static int sohci_get_current_frame_number
> (struct usb_device *usb_dev)
>   * ED handling functions
>   *-------------------------------------------------------------------------*/
>  
> +/* search for the right branch to insert an interrupt ed into the int
> tree
> + * do some load ballancing;
> + * returns the branch and
> + * sets the interval to interval = 2^integer (ld (interval)) */
> +
> +static int ep_int_ballance (ohci_t * ohci, int interval, int load)
> +{
> +	int i, branch = 0;
> +
> +	/* search for the least loaded interrupt endpoint
> +	 * branch of all 32 branches
> +	 */
> +	for (i = 0; i < 32; i++)
> + if (ohci->ohci_int_load [branch] > ohci->ohci_int_load [i])
> +			branch = i;
> +
> +	branch = branch % interval;
> +	for (i = branch; i < 32; i += interval)
> +		ohci->ohci_int_load [i] += load;
> +
> +	return branch;
> +}
> +
> +/*-------------------------------------------------------------------------*/
> +
> +/*  2^int( ld (inter)) */
> +
> +static int ep_2_n_interval (int inter)
> +{
> +	int i;
> +	for (i = 0; ((inter >> i) > 1 ) && (i < 5); i++);
> +	return 1 << i;
> +}
> +
> +/*-------------------------------------------------------------------------*/
> +
> +/* the int tree is a binary tree
> + * in order to process it sequentially the indexes of the branches
> have to be mapped
> + * the mapping reverses the bits of a word of num_bits length */
> +
> +static int ep_rev (int num_bits, int word)
> +{
> +	int i, wout = 0;
> +
> +	for (i = 0; i < num_bits; i++)
> +		wout |= (((word >> i) & 1) << (num_bits - i - 1));
> +	return wout;
> +}
> +
> +/*-------------------------------------------------------------------------*
> + * ED handling functions
> +
> *-------------------------------------------------------------------------*/
> +
>  /* link an ed into one of the HC chains */
>  
>  static int ep_link (ohci_t *ohci, ed_t *edi)
>  {
>  	volatile ed_t *ed = edi;
> +	int int_branch;
> +	int i;
> +	int inter;
> +	int interval;
> +	int load;
> +	__u32 * ed_p;
>  
>  	ed->state = ED_OPER;
> +	ed->int_interval = 0;
>  
>  	switch (ed->type) {
>  	case PIPE_CONTROL:
> @@ -554,12 +662,49 @@ static int ep_link (ohci_t *ohci, ed_t *edi)
>  		}
>  		ohci->ed_bulktail = edi;
>  		break;
> +
> +	case PIPE_INTERRUPT:
> +		load = ed->int_load;
> +		interval = ep_2_n_interval (ed->int_period);
> +		ed->int_interval = interval;
> +		int_branch = ep_int_ballance (ohci, interval, load);
> +		ed->int_branch = int_branch;
> +
> +		for (i = 0; i < ep_rev (6, interval); i += inter) {
> +			inter = 1;
> + for (ed_p = &(ohci->hcca->int_table[ep_rev (5, i) + int_branch]);
> + (*ed_p != 0) && (((ed_t *)ed_p)->int_interval >= interval);
> +				ed_p = &(((ed_t *)ed_p)->hwNextED))
> + inter = ep_rev (6, ((ed_t *)ed_p)->int_interval);
> +			ed->hwNextED = *ed_p;
> +			*ed_p = m32_swap(ed);
> +		}
> +		break;
>  	}
>  	return 0;
>  }
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/* scan the periodic table to find and unlink this ED */
> +static void periodic_unlink ( struct ohci *ohci, volatile struct ed
> *ed,
> +		unsigned index, unsigned period)
> +{
> +	for (; index < NUM_INTS; index += period) {
> +		__u32	*ed_p = &ohci->hcca->int_table [index];
> +
> + /* ED might have been unlinked through another path */
> +		while (*ed_p != 0) {
> +			if (((struct ed *)m32_swap (ed_p)) == ed) {
> +				*ed_p = ed->hwNextED;
> +				break;
> +			}
> + ed_p = & (((struct ed *)m32_swap (ed_p))->hwNextED);
> +		}
> +	}
> +}
> +
> +
>  /* unlink an ed from one of the HC chains.
>   * just the link to the ed is unlinked.
>   * the link from the ed still points to another operational ed or 0
> @@ -568,6 +713,7 @@ static int ep_link (ohci_t *ohci, ed_t *edi)
>  static int ep_unlink (ohci_t *ohci, ed_t *edi)
>  {
>  	volatile ed_t *ed = edi;
> +	int i;
>  
>  	ed->hwINFO |= m32_swap (OHCI_ED_SKIP);
>  
> @@ -605,6 +751,12 @@ static int ep_unlink (ohci_t *ohci, ed_t *edi)
>  			((ed_t *)m32_swap (*((__u32
> *)&ed->hwNextED)))->ed_prev = ed->ed_prev;
>  		}
>  		break;
> +
> +	case PIPE_INTERRUPT:
> +		periodic_unlink (ohci, ed, 0, 1);
> + for (i = ed->int_branch; i < 32; i += ed->int_interval)
> +		    ohci->ohci_int_load[i] -= ed->int_load;
> +		break;
>  	}
>  	ed->state = ED_UNLINK;
>  	return 0;
> @@ -621,7 +773,8 @@ static int ep_unlink (ohci_t *ohci, ed_t *edi)
>   * info fields are setted anyway even though most of them should not
>   * change
>   */
> -static ed_t * ep_add_ed (struct usb_device *usb_dev, unsigned long
> pipe)
> +static ed_t * ep_add_ed (struct usb_device *usb_dev, unsigned long
> pipe,
> +		int interval, int load)
>  {
>  	td_t *td;
>  	ed_t *ed_ret;
> @@ -654,6 +807,11 @@ static ed_t * ep_add_ed (struct usb_device
> *usb_dev, unsigned long pipe)
>  			| usb_pipeslow (pipe) << 13
>  			| usb_maxpacket (usb_dev, pipe) << 16);
>  
> +	if (ed->type == PIPE_INTERRUPT && ed->state == ED_UNLINK) {
> +		ed->int_period = interval;
> +		ed->int_load = load;
> +	}
> +
>  	return ed_ret;
>  }
>  
> @@ -768,6 +926,13 @@ static void td_submit_job (struct usb_device
> *dev, unsigned long pipe, void *buf
>  		if (!ohci->sleeping)
>  			writel (OHCI_CLF, &ohci->regs->cmdstatus); /*
> start Control list */
>  		break;
> +
> +	case PIPE_INTERRUPT:
> +		info = usb_pipeout (urb->pipe)?
> +			TD_CC | TD_DP_OUT | toggle:
> +			TD_CC | TD_R | TD_DP_IN | toggle;
> + td_fill (ohci, info, data, data_len, dev, cnt++, urb);
> +		break;
>  	}
>  	if (urb->length != cnt)
>  		dbg("TD LENGTH %d != CNT %d", urb->length, cnt);
> @@ -783,7 +948,7 @@ static void td_submit_job (struct usb_device *dev,
> unsigned long pipe, void *buf
>  static void dl_transfer_length(td_t * td)
>  {
>  	__u32 tdINFO, tdBE, tdCBP;
> -	urb_priv_t *lurb_priv = &urb_priv;
> +	urb_priv_t *lurb_priv = td->ed->purb;
>  
>  	tdINFO = m32_swap (td->hwINFO);
>  	tdBE   = m32_swap (td->hwBE);
> @@ -820,7 +985,7 @@ static td_t * dl_reverse_done_list (ohci_t *ohci)
>  		td_list = (td_t *)td_list_hc;
>  
>  		if (TD_CC_GET (m32_swap (td_list->hwINFO))) {
> -			lurb_priv = &urb_priv;
> +			lurb_priv = td_list->ed->purb;
>  			dbg(" USB-error/status: %x : %p",
>  					TD_CC_GET (m32_swap
> (td_list->hwINFO)), td_list);
>  			if (td_list->ed->hwHeadP & m32_swap (0x1)) {
> @@ -860,10 +1025,10 @@ static int dl_done_list (ohci_t *ohci, td_t
> *td_list)
>  	while (td_list) {
>  		td_list_next = td_list->next_dl_td;
>  
> -		lurb_priv = &urb_priv;
>  		tdINFO = m32_swap (td_list->hwINFO);
>  
>  		ed = td_list->ed;
> +		lurb_priv = ed->purb;
>  
>  		dl_transfer_length(td_list);
>  
> @@ -882,14 +1047,16 @@ static int dl_done_list (ohci_t *ohci, td_t
> *td_list)
>  			    (lurb_priv->state != URB_DEL))
>  #else
>  			if ((ed->state & (ED_OPER | ED_UNLINK)))
> -				urb_finished = 1;
> +#endif
> + lurb_priv->finished = sohci_return_job(ohci,
> +						lurb_priv);
>  			else
>  				dbg("dl_done_list: strange.., ED state
> %x, ed->state\n");
>  		} else
>  			dbg("dl_done_list: processing TD %x, len
> %x\n", lurb_priv->td_cnt,
>  				lurb_priv->length);
> -#endif
> -		if (ed->state != ED_NEW) {
> +		if (ed->state != ED_NEW &&
> + (usb_pipetype (lurb_priv->pipe) != PIPE_INTERRUPT)) {
>  			edHeadP = m32_swap (ed->hwHeadP) & 0xfffffff0;
>  			edTailP = m32_swap (ed->hwTailP);
>  
> @@ -1063,8 +1230,7 @@ static int ohci_submit_rh_msg(struct usb_device
> *dev, unsigned long pipe,
>  	__u16 wLength;
>  
>  #ifdef DEBUG
> -urb_priv.actual_length = 0;
> -pkt_print(dev, pipe, buffer, transfer_len, cmd, "SUB(rh)",
> usb_pipein(pipe));
> +pkt_print(NULL, dev, pipe, buffer, transfer_len, cmd, "SUB(rh)",
> usb_pipein(pipe));
>  #else
>  	wait_ms(1);
>  #endif
> @@ -1279,9 +1445,7 @@ pkt_print(dev, pipe, buffer, transfer_len, cmd,
> "SUB(rh)", usb_pipein(pipe));
>  	dev->status = stat;
>  
>  #ifdef DEBUG
> -	if (transfer_len)
> -		urb_priv.actual_length = transfer_len;
> - pkt_print(dev, pipe, buffer, transfer_len, cmd, "RET(rh)",
> 0/*usb_pipein(pipe)*/);
> + pkt_print(NULL, dev, pipe, buffer, transfer_len, cmd, "RET(rh)",
> 0/*usb_pipein(pipe)*/);
>  #else
>  	wait_ms(1);
>  #endif
> @@ -1299,6 +1463,16 @@ int submit_common_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  	int stat = 0;
>  	int maxsize = usb_maxpacket(dev, pipe);
>  	int timeout;
> +	urb_priv_t *urb;
> +
> +	urb = malloc(sizeof(urb_priv_t));
> +	memset(urb, 0, sizeof(urb_priv_t));
> +
> +	urb->dev = dev;
> +	urb->pipe = pipe;
> +	urb->transfer_buffer = buffer;
> +	urb->transfer_buffer_length = transfer_len;
> +	urb->interval = interval;
>  
>  	/* device pulled? Shortcut the action. */
>  	if (devgone == dev) {
> @@ -1307,8 +1481,8 @@ int submit_common_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  	}
>  
>  #ifdef DEBUG
> -	urb_priv.actual_length = 0;
> - pkt_print(dev, pipe, buffer, transfer_len, setup, "SUB",
> usb_pipein(pipe));
> +	urb->actual_length = 0;
> + pkt_print(urb, dev, pipe, buffer, transfer_len, setup, "SUB",
> usb_pipein(pipe));
>  #else
>  	wait_ms(1);
>  #endif
> @@ -1318,7 +1492,7 @@ int submit_common_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  		return -1;
>  	}
>  
> - if (sohci_submit_job(dev, pipe, buffer, transfer_len, setup,
> interval) < 0) {
> +	if (sohci_submit_job(urb, setup) < 0) {
>  		err("sohci_submit_job failed");
>  		return -1;
>  	}
> @@ -1353,18 +1527,20 @@ int submit_common_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  		 * finished we need to re-iterate this loop so as
>  		 * hc_interrupt() gets called again as there needs to
> be some
>  		 * more TD's to process still */
> - if ((stat >= 0) && (stat != 0xff) && (urb_finished)) {
> + if ((stat >= 0) && (stat != 0xff) && (urb->finished)) {
>  			/* 0xff is returned for an SF-interrupt */
>  			break;
>  		}
>  
>  		if (--timeout) {
>  			wait_ms(1);
> +			if (!urb->finished)
> +				dbg("\%");
> +
>  		} else {
>  			err("CTL:TIMEOUT ");
>  			dbg("submit_common_msg: TO status %x\n",
> stat);
> -			stat = USB_ST_CRC_ERR;
> -			urb_finished = 1;
> +			urb->finished = 1;
>  			stat = USB_ST_CRC_ERR;
>  			break;
>  		}
> @@ -1374,13 +1550,14 @@ int submit_common_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  	dev->act_len = transfer_len;
>  
>  #ifdef DEBUG
> - pkt_print(dev, pipe, buffer, transfer_len, setup, "RET(ctlr)",
> usb_pipein(pipe));
> + pkt_print(urb, dev, pipe, buffer, transfer_len, setup, "RET(ctlr)",
> usb_pipein(pipe));
>  #else
>  	wait_ms(1);
>  #endif
>  
>  	/* free TDs in urb_priv */
> -	urb_free_priv (&urb_priv);
> +	if (usb_pipetype (pipe) != PIPE_INTERRUPT)
> +		urb_free_priv (urb);
>  	return 0;
>  }
>  
> @@ -1399,8 +1576,7 @@ int submit_control_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  
>  	info("submit_control_msg");
>  #ifdef DEBUG
> -	urb_priv.actual_length = 0;
> - pkt_print(dev, pipe, buffer, transfer_len, setup, "SUB",
> usb_pipein(pipe));
> + pkt_print(NULL, dev, pipe, buffer, transfer_len, setup, "SUB",
> usb_pipein(pipe));
>  #else
>  	wait_ms(1);
>  #endif
> @@ -1423,7 +1599,8 @@ int submit_int_msg(struct usb_device *dev,
> unsigned long pipe, void *buffer,
>  		int transfer_len, int interval)
>  {
>  	info("submit_int_msg");
> -	return -1;
> + return submit_common_msg(dev, pipe, buffer, transfer_len, NULL,
> +			interval);
>  }
>  
>  /*-------------------------------------------------------------------------*
> @@ -1537,6 +1714,12 @@ static int hc_start (ohci_t * ohci)
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/* Poll USB interrupt. */
> +void usb_event_poll(void)
> +{
> +	hc_interrupt();
> +}
> +
>  /* an interrupt happens */
>  
>  static int hc_interrupt (void)
> @@ -1587,8 +1770,10 @@ static int hc_interrupt (void)
>  	if (ints & OHCI_INTR_WDH) {
>  		wait_ms(1);
>  		writel (OHCI_INTR_WDH, &regs->intrdisable);
> +		(void)readl (&regs->intrdisable); /* flush */
>  		stat = dl_done_list (&gohci, dl_reverse_done_list
> (&gohci));
>  		writel (OHCI_INTR_WDH, &regs->intrenable);
> +		(void)readl (&regs->intrdisable); /* flush */
>  	}
>  
>  	if (ints & OHCI_INTR_SO) {
> @@ -1634,6 +1819,9 @@ static char ohci_inited = 0;
>  
>  int usb_lowlevel_init(void)
>  {
> +#ifdef CONFIG_PCI_OHCI
> +	pci_dev_t pdev;
> +#endif
>  
>  #ifdef CFG_USB_OHCI_CPU_INIT
>  	/* cpu dependant init */
> @@ -1647,7 +1835,6 @@ int usb_lowlevel_init(void)
>  		return -1;
>  #endif
>  	memset (&gohci, 0, sizeof (ohci_t));
> -	memset (&urb_priv, 0, sizeof (urb_priv_t));
>  
>  	/* align the storage */
>  	if ((__u32)&ghcca[0] & 0xff) {
> @@ -1673,7 +1860,25 @@ int usb_lowlevel_init(void)
>  	gohci.disabled = 1;
>  	gohci.sleeping = 0;
>  	gohci.irq = -1;
> +#ifdef CONFIG_PCI_OHCI
> +	pdev = pci_find_devices(ohci_pci_ids, 0);
> +
> +	if (pdev != -1) {
> +		u16 vid, did;
> +		u32 base;
> +		pci_read_config_word(pdev, PCI_VENDOR_ID, &vid);
> +		pci_read_config_word(pdev, PCI_DEVICE_ID, &did);
> + printf("OHCI pci controller (%04x, %04x) found @(%d:%d:%d)\n",
> +				vid, did, (pdev >> 16) & 0xff,
> + (pdev >> 11) & 0x1f, (pdev >> 8) & 0x7);
> + pci_read_config_dword(pdev, PCI_BASE_ADDRESS_0, &base);
> +		printf("OHCI regs address 0x%08x\n", base);
> +		gohci.regs = (struct ohci_regs *)base;
> +	} else
> +		return -1;
> +#else
>  	gohci.regs = (struct ohci_regs *)CFG_USB_OHCI_REGS_BASE;
> +#endif
>  
>  	gohci.flags = 0;
>  	gohci.slot_name = CFG_USB_OHCI_SLOT_NAME;
> @@ -1716,7 +1921,6 @@ int usb_lowlevel_init(void)
>  	ohci_dump (&gohci, 1);
>  #else
>  	wait_ms(1);
> -	urb_finished = 1;
>  #endif
>  	ohci_inited = 1;
>  	return 0;
> diff --git a/drivers/usb_ohci.h b/drivers/usb_ohci.h
> index 95fbc44..64fa614 100644
> --- a/drivers/usb_ohci.h
> +++ b/drivers/usb_ohci.h
> @@ -64,7 +64,8 @@ struct ed {
>  	struct ed *ed_rm_list;
>  
>  	struct usb_device *usb_dev;
> -	__u32 unused[3];
> +	void *purb;
> +	__u32 unused[2];
>  } __attribute((aligned(16)));
>  typedef struct ed ed_t;
>  
> @@ -349,9 +350,14 @@ typedef struct
>  	ed_t *ed;
>  	__u16 length; /* number of tds associated with this request */
>  	__u16 td_cnt;	/* number of tds already serviced */
> +	struct usb_device *dev;
>  	int   state;
>  	unsigned long pipe;
> +	void *transfer_buffer;
> +	int transfer_buffer_length;
> +	int interval;
>  	int actual_length;
> +	int finished;
>  	td_t *td[N_URB_TD]; /* list pointer to all corresponding TDs
> associated with this request */
>  } urb_priv_t;
>  #define URB_DEL 1
> @@ -375,6 +381,7 @@ typedef struct ohci {
>  
>  	struct ohci_regs *regs; /* OHCI controller's memory */
>  
> + int ohci_int_load[32]; /* load of the 32 Interrupt Chains (for load
> balancing)*/
>  	ed_t *ed_rm_list[2]; /* lists of all endpoints to be removed
> */
>  	ed_t *ed_bulktail;	 /* last endpoint of bulk list */
>  	ed_t *ed_controltail;	 /* last endpoint of control list */
> @@ -397,7 +404,8 @@ struct ohci_device {
>  /* endpoint */
>  static int ep_link(ohci_t * ohci, ed_t * ed);
>  static int ep_unlink(ohci_t * ohci, ed_t * ed);
> -static ed_t * ep_add_ed(struct usb_device * usb_dev, unsigned long
> pipe);
> +static ed_t * ep_add_ed(struct usb_device * usb_dev, unsigned long
> pipe,
> +		int interval, int load);
>  
>  /*-------------------------------------------------------------------------*/

Best regards

Markus Klotzb?cher

--
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Markus,

Thanks for your feedback, please see my inline comments: 

> -----Original Message-----
> From: Markus Klotzb?cher [mailto:mk at denx.de] 
> 
> Zhang Wei,
> 
> I have some minor issues with your this patch.
> 
> Zhang Wei <wei.zhang@freescale.com> writes:
> 
<...>
> >  
> > -#define readl(a) (*((vu_long *)(a)))
> > -#define writel(a, b) (*((vu_long *)(b)) = ((vu_long)a))
> > +#define readl(a) m32_swap(*((vu_long *)(a)))
> > +#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))
> 
> Can you explain why this is needed? I'm well aware that the endianness
> handling in the USB Code has become pretty awfull, so I'm trying to at
> least understand why what is needed.
> 

Yes, the endian is a really awfull problem. :) If you do not swap the value, you will get or put a wrong endian data in big-endian boards, such as most powerpc boards. I've printed the debug information with the register value by using readl() in our MPC8641HPCN board (big-endian), no swap, the register value is endianness different than linux kernel usb-driver. It can not work. So, in x86 or the other little-endian platforms, the swap is no need, but in big-endian platforms, the swap is must be.


> >  #define min_t(type,x,y) ({ type __x = (x); type __y = (y); 
> __x < __y
> > ? __x: __y; })
> >  
> > -#undef DEBUG
> > +#ifdef CONFIG_PCI_OHCI
> > +static struct pci_device_id ohci_pci_ids[] = {
> > +	{0x10b9, 0x5237},	/* ULI1575 PCI OHCI module ids */
> > +	/* Please add supported PCI OHCI controller ids here */
> > +	{0, 0}
> > +};
> > +#endif
> > +
> >  #ifdef DEBUG
> >  #define dbg(format, arg...) printf("DEBUG: " format "\n", ## arg)
> >  #else
> > @@ -114,15 +130,11 @@ struct ohci_hcca ghcca[1];
> >  struct ohci_hcca *phcca;
> >  /* this allocates EDs for all possible endpoints */
> >  struct ohci_device ohci_dev;
> > -/* urb_priv */
> > -urb_priv_t urb_priv;
> >  /* RHSC flag */
> >  int got_rhsc;
> >  /* device which was disconnected */
> >  struct usb_device *devgone;
> >  
> > -/* flag guarding URB transation */
> > -int urb_finished = 0;
> 
> Can you explain why this flag is not needed anymore? We had a 
> discussion
> about this some time ago and agreed that is should not be 
> removed unless
> we understand exactly why it was put there.
> 

You misunderstands it. :) I do not remove it. I just move it to the individual urb structure.
I'll explain it. If no interrupt pipe support, a global urb_finished flag is ok, only one urb should be processed in the same time. But if the interrupt pipe support is added, it's difference. The interrupt pipe is an 'endless' urb, there are more than one urb should be processed in the same time. When the interrupt pipe is in processing, the other urb such as ctrl urb is also should be processed. So I move the urb_finished flag to individual urb structure: urb->finished.

Such as below:

> >  	/* we're about to begin a new transaction here so mark the URB
> > unfinished */
> > -	urb_finished = 0;
> > +	urb->finished = 0;
> >  
> >  	/* every endpoint has a ed, locate and fill it */
> > -	if (!(ed = ep_add_ed (dev, pipe))) {
> > +	if (!(ed = ep_add_ed (dev, pipe, interval, 1))) {
> >  		err("sohci_submit_job: ENOMEM");
> >  		return -1;
> >  	}

Best Regards,
Zhang Wei

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Hi Zhang Wei,

Thanks for your comments.

"Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:

> <...>
>> >  
>> > -#define readl(a) (*((vu_long *)(a)))
>> > -#define writel(a, b) (*((vu_long *)(b)) = ((vu_long)a))
>> > +#define readl(a) m32_swap(*((vu_long *)(a)))
>> > +#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))
>> 
>> Can you explain why this is needed? I'm well aware that the
> endianness
>> handling in the USB Code has become pretty awfull, so I'm trying to
> at
>> least understand why what is needed.
>> 
>
> Yes, the endian is a really awfull problem. :) If you do not swap the
> value, you will get or put a wrong endian data in big-endian boards,
> such as most powerpc boards. I've printed the debug information with
> the register value by using readl() in our MPC8641HPCN board
> (big-endian), no swap, the register value is endianness different than
> linux kernel usb-driver. It can not work. So, in x86 or the other
> little-endian platforms, the swap is no need, but in big-endian
> platforms, the swap is must be.

So this BE CPU and LE Controller again. The only problem I see with your
fix is that because you _always_ do byteswapping for register accesses
on BE CPUs, this would break support for the case BE CPU and BE OHCI
Controller (but which doesn't exist in U-Boot so far).

No need to change this now, because I'm going to clean this up soon, but
I'd appreciate your help in testing once this is done.

>
>> >  #define min_t(type,x,y) ({ type __x = (x); type __y = (y); 
>> __x < __y
>> > ? __x: __y; })
>> >  
>> > -#undef DEBUG
>> > +#ifdef CONFIG_PCI_OHCI
>> > +static struct pci_device_id ohci_pci_ids[] = {
>> > +	{0x10b9, 0x5237},	/* ULI1575 PCI OHCI module ids */
>> > +	/* Please add supported PCI OHCI controller ids here */
>> > +	{0, 0}
>> > +};
>> > +#endif
>> > +
>> >  #ifdef DEBUG
>> > #define dbg(format, arg...) printf("DEBUG: " format "\n", ## arg)
>> >  #else
>> > @@ -114,15 +130,11 @@ struct ohci_hcca ghcca[1];
>> >  struct ohci_hcca *phcca;
>> >  /* this allocates EDs for all possible endpoints */
>> >  struct ohci_device ohci_dev;
>> > -/* urb_priv */
>> > -urb_priv_t urb_priv;
>> >  /* RHSC flag */
>> >  int got_rhsc;
>> >  /* device which was disconnected */
>> >  struct usb_device *devgone;
>> >  
>> > -/* flag guarding URB transation */
>> > -int urb_finished = 0;
>> 
>> Can you explain why this flag is not needed anymore? We had a 
>> discussion
>> about this some time ago and agreed that is should not be 
>> removed unless
>> we understand exactly why it was put there.
>> 
>
> You misunderstands it. :) I do not remove it. I just move it to the
> individual urb structure.
> I'll explain it. If no interrupt pipe support, a global urb_finished
> flag is ok, only one urb should be processed in the same time. But if
> the interrupt pipe support is added, it's difference. The interrupt
> pipe is an 'endless' urb, there are more than one urb should be
> processed in the same time. When the interrupt pipe is in processing,
> the other urb such as ctrl urb is also should be processed. So I move
> the urb_finished flag to individual urb structure: urb->finished.
>
> Such as below:
>
>> > /* we're about to begin a new transaction here so mark the URB
>> > unfinished */
>> > -	urb_finished = 0;
>> > +	urb->finished = 0;
>> >  
>> >  	/* every endpoint has a ed, locate and fill it */
>> > -	if (!(ed = ep_add_ed (dev, pipe))) {
>> > +	if (!(ed = ep_add_ed (dev, pipe, interval, 1))) {
>> >  		err("sohci_submit_job: ENOMEM");
>> >  		return -1;

I get it, thanks for explaining.

Best regards

Markus

--
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Markus, 

Thanks! Please see my inline comments:

> -----Original Message-----
> From: Markus Klotzb?cher [mailto:mk at denx.de] 
> Hi Zhang Wei,
> 
> Thanks for your comments.
> 
> "Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:
> 
> > <...>
> >> >  
> >> > -#define readl(a) (*((vu_long *)(a)))
> >> > -#define writel(a, b) (*((vu_long *)(b)) = ((vu_long)a))
> >> > +#define readl(a) m32_swap(*((vu_long *)(a)))
> >> > +#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))
> >> 
> >> Can you explain why this is needed? I'm well aware that the
> > endianness
> >> handling in the USB Code has become pretty awfull, so I'm trying to
> > at
> >> least understand why what is needed.
> >> 
> >
> > Yes, the endian is a really awfull problem. :) If you do 
> not swap the
> > value, you will get or put a wrong endian data in big-endian boards,
> > such as most powerpc boards. I've printed the debug information with
> > the register value by using readl() in our MPC8641HPCN board
> > (big-endian), no swap, the register value is endianness 
> different than
> > linux kernel usb-driver. It can not work. So, in x86 or the other
> > little-endian platforms, the swap is no need, but in big-endian
> > platforms, the swap is must be.
> 
> So this BE CPU and LE Controller again. The only problem I 
> see with your
> fix is that because you _always_ do byteswapping for register accesses
> on BE CPUs, this would break support for the case BE CPU and BE OHCI
> Controller (but which doesn't exist in U-Boot so far).
> 
> No need to change this now, because I'm going to clean this 
> up soon, but
> I'd appreciate your help in testing once this is done.
> 

I'm using m32_swap(), it is not _always_ do. You can see following codes in usb_ohci.c:

#if defined(CONFIG_440EP) || defined(CONFIG_MPC5200)
# define m16_swap(x) (x)
# define m32_swap(x) (x)
#else
# define m16_swap(x) swap_16(x)
# define m32_swap(x) swap_32(x)
#endif

And the swap_32() is also defined in the file include/usb.h:

#ifdef LITTLEENDIAN
# define swap_16(x) (x)
# define swap_32(x) (x)
#else
# define swap_16(x) __swap_16(x)
# define swap_32(x) __swap_32(x)
#endif /* LITTLEENDIAN */

So It _only_ works on BE platforms.

Best Regards,
Zhang Wei

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Hi Zhang Wei,

"Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:

>> So this BE CPU and LE Controller again. The only problem I 
>> see with your
>> fix is that because you _always_ do byteswapping for register accesses
>> on BE CPUs, this would break support for the case BE CPU and BE OHCI
>> Controller (but which doesn't exist in U-Boot so far).
>> 
>> No need to change this now, because I'm going to clean this 
>> up soon, but
>> I'd appreciate your help in testing once this is done.
>> 
>
> I'm using m32_swap(), it is not _always_ do. You can see following
> codes in usb_ohci.c:

I see, you're right. Still, this mess needs to be cleaned up. I'll start
a new thread.

Best regards

Markus

--
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Markus, 

Thanks!
When you clean up, I can help you test it in our BE boards.

Best Regards,
Zhang Wei

> 
> I see, you're right. Still, this mess needs to be cleaned up. 
> I'll start
> a new thread.
> 
> Best regards
> 
> Markus
> 
> --
> DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
> Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
> 

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Markus,

How about your codes cleaning and this patch?

Cheers!
Zhang Wei
 
> > 
> > I see, you're right. Still, this mess needs to be cleaned up. 
> > I'll start
> > a new thread.
> > 
> > Best regards
> > 
> > Markus
> > 
> > --
> > DENX Software Engineering GmbH, HRB 165235 Munich, CEO: 
> Wolfgang Denk
> > Office:  Kirchenstr. 5,       D-82194 Groebenzell,          
>   Germany
> > 
> 

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Dear Zhang Wei,

"Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:

> How about your codes cleaning and this patch?

I'm sorry this is taking longer. It's in the upper region of my todo
list. Please be patient.

Viele Gr??e / Best regards

Markus Klotzb?cher

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Dear Zhang Wei,

"Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:

> How about your codes cleaning and this patch?

I finally made it, sorry it took so long. Can you test the code with PCI
OHCI? I guess you will have to update you board configuration (add
CFG_OHCI_SWAP_REG_ACCESS).

Comments? Thanks!

Viele Gr??e / Best regards

Markus Klotzb?cher

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Jon Loeliger]

On Wed, 2007-06-06 at 05:52, Markus Klotzb??cher wrote:
> Dear Zhang Wei,
> 
> "Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:
> 
> > How about your codes cleaning and this patch?
> 
> I finally made it, sorry it took so long. Can you test the code with PCI
> OHCI? I guess you will have to update you board configuration (add
> CFG_OHCI_SWAP_REG_ACCESS).
> 
> Comments? Thanks!

Excellent!  We'll pull that tree and do some testing on it!

Thanks,
jdl

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Markus,

Thanks so much!

Jason help us to test it. He find a bug in your patch "USB/OHCI: endianness cleanup in the generic ohci driver" as below:

-#define readl(a) m32_swap(*((vu_long *)(a)))
-#define writel(a, b) (*((vu_long *)(b)) = m32_swap((vu_long)a))
+/*
+ * e.g. PCI controllers need this
+ */
+#ifdef CFG_OHCI_SWAP_REG_ACCESS
+# define readl(a) __swap_16(*((vu_long *)(a)))
+# define writel(a, b) (*((vu_long *)(b)) = __swap_32((vu_long)a))

readl() should be __swap_32() not __swap_16(). He has already committed a patch to fix it.

Besides that, usb works well.

Cheers!
Wei.

> -----Original Message-----
> From: Markus Klotzb?cher [mailto:mk at denx.de] 
> Sent: Wednesday, June 06, 2007 6:53 PM
> To: Zhang Wei-r63237
> Cc: u-boot-users at lists.sourceforge.net; Loeliger 
> Jon-LOELIGER; Wang Haiying-r54964; wd at denx.de
> Subject: Re: [PATCH 2/3] Added USB PCI-OHCI chips 
> support,interrupt pipe support and usb event poll support.
> 
> Dear Zhang Wei,
> 
> "Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:
> 
> > How about your codes cleaning and this patch?
> 
> I finally made it, sorry it took so long. Can you test the 
> code with PCI
> OHCI? I guess you will have to update you board configuration (add
> CFG_OHCI_SWAP_REG_ACCESS).
> 
> Comments? Thanks!
> 
> Viele Gr??e / Best regards
> 
> Markus Klotzb?cher
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> 

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Markus Klotzbücher]

Hi Wei,

"Zhang Wei-r63237" <Wei.Zhang@freescale.com> writes:

> Thanks so much!
>
> Jason help us to test it. He find a bug in your patch "USB/OHCI:
> endianness cleanup in the generic ohci driver" as below:

Uhh, stupid bug. Thanks for testing.

> Besides that, usb works well.

Great!

Best regards

Markus Klotzb?cher

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Robert Delien]

Hello Zhang,

> This patch added USB PCI-OHCI chips support, interrupt pipe support
> and usb event poll support. For supporting the USB interrupt pipe,
> the globe urb_priv is moved to purb in ed struct. Now, we can
> process several urbs at one time. The interrupt pipe support codes
> are ported from Linux kernel 2.4.

Two questions/remarks:
1. Why are you defining two macros, called 'readl' and 'writel', while both
of these are available from asm/io.h on all architectures?
2. In 'usb_lowlevel_int' the register base address isn't translated from
PCI address space to processor address space.

With kind regards,

      Robert.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20070703/a6bd59da/attachment.htm 

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Robert,

Thanks!

> From: Robert Delien [mailto:robert.delien at nxp.com] 
> Hello Zhang,
> Two questions/remarks:
> 1. Why are you defining two macros, called 'readl' and 'writel', while
both of these are available from asm/io.h on all architectures?

Yes, but here they're special macros for USB register access. You can
refer to the newest usb-git tree of Markus.

> 2. In 'usb_lowlevel_int' the register base address isn't translated
from PCI address space to processor address space.

Does current codes not run well on your system? It's in U-boot now. The
address in the PCI bar is processor address.

Cheers!
Wei.

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Robert Delien]

Hello Zhang,

Thank you for your quick response!

> > 1. Why are you defining two macros, called 'readl' and 'writel', while
> > both of these are available from asm/io.h on all architectures?
>
> Yes, but here they're special macros for USB register access. You can
> refer to the newest usb-git tree of Markus.

I see; Endianity is a bit tricky here. Though the similar name appeared a
bit ambiguous to me.

> > 2. In 'usb_lowlevel_int' the register base address isn't translated
> > from PCI address space to processor address space.
>
> Does current codes not run well on your system? It's in U-boot now. The
> address in the PCI bar is processor address.

No, unfortunately not, but I didn't expect it to work right away ;-).

I am currently using Markus' u-boot-usb.git repository which is the cleaned
up version, as I understand. Our platform is a MIPS32 (4Kec) based asic
running in Little Endian mode with a proprietary PCI bus bridge and a quite
common ISP1564 OHCI USB controller. Our PCI bus is running properly: We've
been using a RTL8139 Ethernet card under U-Boot successfully for a long
time.

The PCI BARs configure the address decoders of a PCI device and thus hold
the devices address in PCI space. If your platform uses a non translating
PCI bus bridge that address will be indentical to the address in the
processors address space. But some platforms use translating bus bridges,
such as the Hawk on the PowerPC platform. The RealTek RTL8139 Ethernet
driver (drivers/rtl8139.c) uses 'pci_mem_to_phys' (from pci.h) to translate
the address read from BAR1 to an address in processor space.

An extra complication is in the memory architecture of the MIPS: On the
MIPS the CPU always uses virtual addresses, even when the MMU is off. So,
we have to translate a physical register address to a virtual address in
the uncached memory segment. The RTL8139 driver seems to do this correctly
for the MIPS platform, but I didn't yet take the time to figure out how.
For the time being a 'base |= 0xA0000000' suffices.

I'm still not 100% confident in the Endianity because we are running our
MIPS in Little Endian mode, but that hasn't got my focus right now because
I assume you have thought about this a lot longer than I did :-).

With kind regards,

      Robert.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20070703/a5b49a20/attachment.htm 

[U-Boot-Users] [PATCH 2/3] Added USB PCI-OHCI chips support, interrupt pipe support and usb event poll support.

[Zhang Wei-r63237]

Hi, Robert,

> 
> > > 1. Why are you defining two macros, called 'readl' and 
> 'writel', while
> > > both of these are available from asm/io.h on all architectures?
> > 
> > Yes, but here they're special macros for USB register 
> access. You can
> > refer to the newest usb-git tree of Markus.
> 
> I see; Endianity is a bit tricky here. Though the similar 
> name appeared a bit ambiguous to me.

Yes, they can be changed to usb_readl and usb_writel.

> 
> > > 2. In 'usb_lowlevel_int' the register base address isn't 
> translated
> > > from PCI address space to processor address space.
> > 
> > Does current codes not run well on your system? It's in 
> U-boot now. The
> > address in the PCI bar is processor address.
> 
> No, unfortunately not, but I didn't expect it to work right away ;-).
> 
> I am currently using Markus' u-boot-usb.git repository which 
> is the cleaned up version, as I understand. Our platform is a 
> MIPS32 (4Kec) based asic running in Little Endian mode with a 
> proprietary PCI bus bridge and a quite common ISP1564 OHCI 
> USB controller. Our PCI bus is running properly: We've been 
> using a RTL8139 Ethernet card under U-Boot successfully for a 
> long time.
> 
> The PCI BARs configure the address decoders of a PCI device 
> and thus hold the devices address in PCI space. If your 
> platform uses a non translating PCI bus bridge that address 
> will be indentical to the address in the processors address 
> space. But some platforms use translating bus bridges, such 
> as the Hawk on the PowerPC platform. The RealTek RTL8139 
> Ethernet driver (drivers/rtl8139.c) uses 'pci_mem_to_phys' 
> (from pci.h) to translate the address read from BAR1 to an 
> address in processor space.
> 
> An extra complication is in the memory architecture of the 
> MIPS: On the MIPS the CPU always uses virtual addresses, even 
> when the MMU is off. So, we have to translate a physical 
> register address to a virtual address in the uncached memory 
> segment. The RTL8139 driver seems to do this correctly for 
> the MIPS platform, but I didn't yet take the time to figure 
> out how. For the time being a 'base |= 0xA0000000' suffices.
> 
> I'm still not 100% confident in the Endianity because we are 
> running our MIPS in Little Endian mode, but that hasn't got 
> my focus right now because I assume you have thought about 
> this a lot longer than I did :-).
> 
> With kind regards,
> 

You are right! I'm bound to powerpc only. :) I think you can send a
patch to fix it.

Thanks!
Wei.