[U-Boot] [PATCH] usb_storage: fix ehci driver max transfer size

Schneider, Kolja
2012-07-04T06:57:25Z

> Am 03.07.2012 20:10, schrieb Marek Vasut:
> >> The commit 5dd95cf93dfffa1d19a1928990852aac9f55b9d9 'usb_storage:
> >> Fix EHCI "out of buffer pointers" with CD-ROM' introduce a bug in
> >> usb_storage as it wrongly assumes that every transfer can use 4k
> >> per qt_buffer. This is wrong if the start address of the data
> >> is not 4k aligned and leads to 'EHCI timed out on TD' messages
> >> because of 'out of buffer pointers' in ehci_td_buffer function.
> >>
> >> Cc: Marek Vasut <marex@denx.de>
> >> Signed-off-by: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
> > Ok, first I have to admit I broke my promise to look into this ASAP, sorry
> about
> > it :-(
> No problem, as long as we get it into the next release. ;-)
> >
> > Just curious, but shouldn't it be ((4096 * 5) / dev_desc->blk_sz) - 1 ?
> No, because the first blk need to be aligned with the 4096. In worst
> case the blk is at the end of the 4096 range. If we assume that the blk
> is aligned to blk_sz we can change it to ((4096 * 4) / dev_desc->blk_sz)
> + 1. I skip the last blk (+ 1) because with 4096 aligned first blk we
> unaligned the next transfer and add extra short packages to each ehci
> transfer.
>
> If we want to maximise the usage we need to calculate the max_xfer_blk
> depending on the start address of the first blk.
>

I admit to not totally getting it. However, there are two things that come to my mind:
- Doesn't the EHCI Specification mention exactly five buffers that can/should/must
be used?
- I think I once stumbled across some comment that said as much as the blocks
always having to be aligned anyway?

:-) Kolja

> >> ---
> >> common/usb_storage.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/common/usb_storage.c b/common/usb_storage.c
> >> index faad237..612a553 100644
> >> --- a/common/usb_storage.c
> >> +++ b/common/usb_storage.c
> >> @@ -1416,10 +1416,10 @@ int usb_stor_get_info(struct usb_device
> *dev,
> >> struct us_data *ss, USB_STOR_PRINTF("partype: %d\n", dev_desc-
> >part_type);
> >>
> >> /*
> >> - * The U-Boot EHCI driver cannot handle more than 4096 * 5 bytes in
> a
> >> - * transfer without running itself out of qt_buffers.
> >> + * The U-Boot EHCI driver cannot handle more than 4096 * 4 bytes in
> a
> >> + * unaligned transfer without running itself out of qt_buffers.
> >> */
> >> - ss->max_xfer_blk = (4096 * 5) / dev_desc->blksz;
> >> + ss->max_xfer_blk = (4096 * 4) / dev_desc->blksz;
> >>
> >> init_part(dev_desc);
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot