All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] usb_storage: fix ehci driver max transfer size
Date: Tue, 10 Jul 2012 19:59:22 +0200	[thread overview]
Message-ID: <bea9a9dcecbaef5a56e13adc1323a98cb22b0091.1341942485.git.stefan@herbrechtsmeier.net> (raw)
In-Reply-To: <1340043748-9261-1-git-send-email-stefan@herbrechtsmeier.net>

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
4096 bytes per qt_buffer. This is wrong if the start address of
the data is not page aligned to 4096 bytes and leads to 'EHCI
timed out on TD' messages because of 'out of buffer pointers'
in ehci_td_buffer function.

The bug appears during load of a fragmented file and
read from or write to an unaligned memory address.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>

---
Changes for v2:
 - Replace fixed worst case calculation with dynamic
   computation based on start address of transfer
Changes for v3:
 - Add parentheses within macro around parameters

 common/usb_storage.c |   38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index faad237..c528979 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -150,12 +150,18 @@ struct us_data {
 	unsigned int	irqpipe;	 	/* pipe for release_irq */
 	unsigned char	irqmaxp;		/* max packed for irq Pipe */
 	unsigned char	irqinterval;		/* Intervall for IRQ Pipe */
-	unsigned long	max_xfer_blk;		/* Max blocks per xfer */
 	ccb		*srb;			/* current srb */
 	trans_reset	transport_reset;	/* reset routine */
 	trans_cmnd	transport;		/* transport routine */
 };
 
+/*
+ * The U-Boot EHCI driver cannot handle more than 5 page aligned buffers
+ * of 4096 bytes in a transfer without running itself out of qt_buffers
+ */
+#define USB_MAX_XFER_BLK(start, blksz) \
+	(((4096 * 5) - ((start) % 4096)) / (blksz))
+
 static struct us_data usb_stor[USB_MAX_STOR_DEV];
 
 
@@ -1041,7 +1047,7 @@ static void usb_bin_fixup(struct usb_device_descriptor descriptor,
 unsigned long usb_stor_read(int device, unsigned long blknr,
 			    unsigned long blkcnt, void *buffer)
 {
-	unsigned long start, blks, buf_addr;
+	unsigned long start, blks, buf_addr, max_xfer_blk;
 	unsigned short smallblks;
 	struct usb_device *dev;
 	struct us_data *ss;
@@ -1083,12 +1089,14 @@ unsigned long usb_stor_read(int device, unsigned long blknr,
 		/* XXX need some comment here */
 		retry = 2;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (blks > ss->max_xfer_blk)
-			smallblks = ss->max_xfer_blk;
+		max_xfer_blk = USB_MAX_XFER_BLK(buf_addr,
+						usb_dev_desc[device].blksz);
+		if (blks > max_xfer_blk)
+			smallblks = (unsigned short) max_xfer_blk;
 		else
 			smallblks = (unsigned short) blks;
 retry_it:
-		if (smallblks == ss->max_xfer_blk)
+		if (smallblks == max_xfer_blk)
 			usb_show_progress();
 		srb->datalen = usb_dev_desc[device].blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
@@ -1109,7 +1117,7 @@ retry_it:
 			start, smallblks, buf_addr);
 
 	usb_disable_asynch(0); /* asynch transfer allowed */
-	if (blkcnt >= ss->max_xfer_blk)
+	if (blkcnt >= max_xfer_blk)
 		debug("\n");
 	return blkcnt;
 }
@@ -1117,7 +1125,7 @@ retry_it:
 unsigned long usb_stor_write(int device, unsigned long blknr,
 				unsigned long blkcnt, const void *buffer)
 {
-	unsigned long start, blks, buf_addr;
+	unsigned long start, blks, buf_addr, max_xfer_blk;
 	unsigned short smallblks;
 	struct usb_device *dev;
 	struct us_data *ss;
@@ -1162,12 +1170,14 @@ unsigned long usb_stor_write(int device, unsigned long blknr,
 		 */
 		retry = 2;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (blks > ss->max_xfer_blk)
-			smallblks = ss->max_xfer_blk;
+		max_xfer_blk = USB_MAX_XFER_BLK(buf_addr,
+						usb_dev_desc[device].blksz);
+		if (blks > max_xfer_blk)
+			smallblks = (unsigned short) max_xfer_blk;
 		else
 			smallblks = (unsigned short) blks;
 retry_it:
-		if (smallblks == ss->max_xfer_blk)
+		if (smallblks == max_xfer_blk)
 			usb_show_progress();
 		srb->datalen = usb_dev_desc[device].blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
@@ -1188,7 +1198,7 @@ retry_it:
 			start, smallblks, buf_addr);
 
 	usb_disable_asynch(0); /* asynch transfer allowed */
-	if (blkcnt >= ss->max_xfer_blk)
+	if (blkcnt >= max_xfer_blk)
 		debug("\n");
 	return blkcnt;
 
@@ -1415,12 +1425,6 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 	USB_STOR_PRINTF(" address %d\n", dev_desc->target);
 	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.
-	 */
-	ss->max_xfer_blk = (4096 * 5) / dev_desc->blksz;
-
 	init_part(dev_desc);
 
 	USB_STOR_PRINTF("partype: %d\n", dev_desc->part_type);
-- 
1.7.9.5

  parent reply	other threads:[~2012-07-10 17:59 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
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 [this message]
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=bea9a9dcecbaef5a56e13adc1323a98cb22b0091.1341942485.git.stefan@herbrechtsmeier.net \
    --to=stefan@herbrechtsmeier.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.