From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] usb_storage: fix ehci driver max transfer size
Date: Sat, 7 Jul 2012 23:58:48 +0200 [thread overview]
Message-ID: <201207072358.49043.marex@denx.de> (raw)
In-Reply-To: <4FF3F7C7.6040209@herbrechtsmeier.net>
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@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?
>
> 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
next prev parent reply other threads:[~2012-07-07 21:58 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-18 18:22 [U-Boot] [PATCH] usb_storage: fix ehci driver max transfer size Stefan Herbrechtsmeier
2012-06-18 18:57 ` Marek Vasut
2012-07-03 18:10 ` Marek Vasut
2012-07-04 6:32 ` Stefan Herbrechtsmeier
2012-07-04 6:57 ` Schneider, Kolja
2012-07-04 7:59 ` Stefan Herbrechtsmeier
2012-07-07 21:58 ` Marek Vasut [this message]
2012-07-08 11:08 ` Stefan Herbrechtsmeier
2012-07-08 18:58 ` Marek Vasut
2012-07-09 19:52 ` [U-Boot] [PATCH v2] " Stefan Herbrechtsmeier
2012-07-10 2:12 ` Marek Vasut
2012-07-10 6:53 ` Stefan Herbrechtsmeier
2012-07-10 7:22 ` Marek Vasut
2012-07-10 17:59 ` Stefan Herbrechtsmeier
2012-07-10 18:02 ` Marek Vasut
2012-07-14 22:23 ` Ilya Yanok
2012-07-15 8:14 ` Marek Vasut
2012-07-15 8:41 ` Ilya Yanok
2012-07-15 9:51 ` Marek Vasut
2012-07-15 14:48 ` Ilya Yanok
2012-07-15 9:50 ` Marek Vasut
2012-07-18 12:50 ` Marek Vasut
2012-07-18 15:46 ` Stefan Herbrechtsmeier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201207072358.49043.marex@denx.de \
--to=marex@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox