On Tuesday 22 March 2005 2:05 am, Karsten Wiese wrote:
> > On Sunday 20 March 2005 4:06 am, Karsten Wiese wrote:
> > > Without Hunk 1 ehci sitd's "Buffer Pointer (Page 1)" initialisation is 
> > > wrong,
> > 
> > This still doesn't seem quite right.  In fact "buf1" initialization in
> > sitd_sched_init() looks seriously borked.  I don't see how resizing to
> > u32 could help ... the relevant field in the sITD is five bits, not 16!
> 
> handling of those five lsbits "TRANSACTION POSITION" & "TRANSACTION COUNT" 
> (you mean those?) 
> isn't touched by the u16 to u32 change.

Good point.  It just changes how I as thinking of the usage of buf1;
better match for the current code, I guess.


> its bits [31:12] where u32 instead of u16 really helps.
> with u16, bits [31:16] of packet->buf1 don't exist physically, thusly
> compiler must think they are 0. 
> so we only set dma address correctly with u32 in sitd_patch()'s line:
>       sitd->hw_buf [1] = cpu_to_le32 (uf->buf1);
> 
> > And sitd_patch() does the wrong stuff too.

The thing that was really wrong was handlig of the buffer-crossing case.
It wasn't properly detected, and the 64bit-dma wasn't setup right either.
(Not that anything turns on 64bit DMA just now.)


> > So what I'll do is post a revised patch merging all your split ISO fixes,
> > with sitd_sched_init() and sitd_patch() updated to handle sitd->hw_buf[1]
> > better ... and not just growing this field to 32 bits.
> 
> looking forward to check the right stuff out. 

Well, this looks right to me, but as you know I don't have a good way to
reproduce it.  See if this version behaves for you.

- Dave

This contains patches to the EHCI driver for the full speed isochronous
transfer support:

    - Fixes a cut'n'paste error in the logic to check for microframe
      scheduling collisions with a given transaction translator.
      The error caused full speed iso tds to be treated in part as
      if they were high speed iso tds, which could oops.

    - Correctly rounds 188-byte OUT transfers to fit into a single
      packet ... no split transfers needed.  Resolves the "buzz" in
      some audio OUT streams.

    - The sitd->hw_buf[1] and sitd->hw_buf_hi[1] fields were not
      correctly initialized, affecting transfers which crossed 4K
      boundaries.  Also helps resolve some "buzz" sounds.

    - The split transaction state flag doesn't indicate an error, and
      it should be ignored when deciding if (IN) transfers had errors.

Signed-off-by: Karsten Wiese <[EMAIL PROTECTED]>
Signed-off-by: David Brownell <[EMAIL PROTECTED]>

--- 1.61/drivers/usb/host/ehci-sched.c	2005-03-21 02:27:12 -08:00
+++ edited/drivers/usb/host/ehci-sched.c	2005-03-22 11:24:16 -08:00
@@ -208,7 +208,7 @@
 				here = here.qh->qh_next;
 				continue;
 			case Q_TYPE_SITD:
-				if (same_tt (dev, here.itd->urb->dev)) {
+				if (same_tt (dev, here.sitd->urb->dev)) {
 					u16		mask;
 
 					mask = le32_to_cpu (here.sitd
@@ -218,7 +218,7 @@
 					if (mask & uf_mask)
 						break;
 				}
-				type = Q_NEXT_TYPE (here.qh->hw_next);
+				type = Q_NEXT_TYPE (here.sitd->hw_next);
 				here = here.sitd->sitd_next;
 				continue;
 			// case Q_TYPE_FSTN:
@@ -1504,18 +1504,17 @@
 
 		/* might need to cross a buffer page within a td */
 		packet->bufp = buf;
-		buf += length;
-		packet->buf1 = buf & ~0x0fff;
+		packet->buf1 = (buf + length) & ~0x0fff;
 		if (packet->buf1 != (buf & ~(u64)0x0fff))
 			packet->cross = 1;
 
 		/* OUT uses multiple start-splits */ 
 		if (stream->bEndpointAddress & USB_DIR_IN)
 			continue;
-		length = 1 + (length / 188);
-		packet->buf1 |= length;
+		length = (length + 187) / 188;
 		if (length > 1) /* BEGIN vs ALL */
-			packet->buf1 |= 1 << 3;
+			length |= 1 << 3;
+		packet->buf1 |= length;
 	}
 }
 
@@ -1610,10 +1609,9 @@
 	sitd->hw_buf_hi [0] = cpu_to_le32 (bufp >> 32);
 
 	sitd->hw_buf [1] = cpu_to_le32 (uf->buf1);
-	if (uf->cross) {
+	if (uf->cross)
 		bufp += 4096;
-		sitd->hw_buf_hi [1] = cpu_to_le32 (bufp >> 32);
-	}
+	sitd->hw_buf_hi [1] = cpu_to_le32 (bufp >> 32);
 	sitd->index = index;
 }
 
@@ -1697,7 +1695,7 @@
 /*-------------------------------------------------------------------------*/
 
 #define	SITD_ERRS (SITD_STS_ERR | SITD_STS_DBE | SITD_STS_BABBLE \
-			| SITD_STS_XACT | SITD_STS_MMF | SITD_STS_STS)
+	       			| SITD_STS_XACT | SITD_STS_MMF)
 
 static unsigned
 sitd_complete (
--- 1.58/drivers/usb/host/ehci.h	2005-03-21 02:27:12 -08:00
+++ edited/drivers/usb/host/ehci.h	2005-03-22 11:23:47 -08:00
@@ -431,7 +431,7 @@
 	__le32			transaction;	/* itd->hw_transaction[i] |= */
 	u8			cross;		/* buf crosses pages */
 	/* for full speed OUT splits */
-	u16			buf1;
+	u32			buf1;
 };
 
 /* temporary schedule data for packets from iso urbs (both speeds)

Reply via email to