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

[Marek Vasut]

Dear Stefan Herbrechtsmeier,

> Am 04.07.2012 08:57, schrieb Schneider, Kolja:
> >> 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 at denx.de>
> >>>> Signed-off-by: Stefan Herbrechtsmeier <stefan at 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?
> 
> Yes, you can use up to five 4096 byte buffers.
> 
> >   - 	I think I once stumbled across some comment that said as much as 
the
> >   blocks
> > 	
> > 	always having to be aligned anyway?
> 
> The buffers must be aligned to a 4096 byte page. This means that you
> have to use the first and last buffer to align your data to the next or
> previous 4096 byte page boundary.

All right, I managed to replicate the issue. This (or similar) doesn't work for 
you, right:

usb read 0x42000004 0x0 0x400

The proper solution would be to introduce a bounce buffer for such unaligned 
transfers. A proper, generic bounce buffer that can be configured to bounce on 
specified boundaries and warns about performance penalties.

Best regards,
Marek Vasut