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)