Patchwork [U-Boot,3/3] usb_storage: Fix EHCI "out of buffer pointers" with CD-ROM

login
register
mail settings
Submitter Kyle Moffett
Date Dec. 20, 2011, 5:41 p.m.
Message ID <1324402874-15400-4-git-send-email-Kyle.D.Moffett@boeing.com>
Download mbox | patch
Permalink /patch/132477/
State Superseded
Headers show

Comments

Kyle Moffett - Dec. 20, 2011, 5:41 p.m.
When performing large bulk reads from a CD or DVD using the U-Boot
usb_storage driver, it generates requests of up to 20 blocks at a time.

With a standard 512-byte block size, that is 10240 bytes and within the
limit of U-Boot's EHCI driver (maximum 5 pages at 4k per page).

Unfortunately CD-ROM media has a 2048-byte blocksize, resulting in a
maximum transfer size of 40960 bytes, which does not fit.

Since the EHCI specification is impossibly obtuse and far beyond my
comprehension, I chose to dynamically compute the limit based on the
blocksize.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
---
 common/usb_storage.c |   45 +++++++++++++++++++++++++--------------------
 1 files changed, 25 insertions(+), 20 deletions(-)
Mike Frysinger - Dec. 20, 2011, 6:20 p.m.
On Tuesday 20 December 2011 12:41:14 Kyle Moffett wrote:
> +	/*
> +	 * 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;

spaces around those operators
-mike
Kyle Moffett - Dec. 20, 2011, 6:24 p.m.
On Dec 20, 2011, at 13:20, Mike Frysinger wrote:
> On Tuesday 20 December 2011 12:41:14 Kyle Moffett wrote:
>> +	/*
>> +	 * 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;
> 
> spaces around those operators

Fixed, thanks for the catch!

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
Mike Frysinger - Dec. 20, 2011, 7:01 p.m.
On Tuesday 20 December 2011 13:24:25 Moffett, Kyle D wrote:
> On Dec 20, 2011, at 13:20, Mike Frysinger wrote:
> > On Tuesday 20 December 2011 12:41:14 Kyle Moffett wrote:
> >> +	/*
> >> +	 * 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;
> > 
> > spaces around those operators
> 
> Fixed, thanks for the catch!

you're too cheery for this list.  i fear you may not last long.  hopefully 
you'll prove me wrong.
-mike

Patch

diff --git a/common/usb_storage.c b/common/usb_storage.c
index d9a2585..2720b8d 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -151,6 +151,7 @@  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 */
@@ -1041,14 +1042,13 @@  static void usb_bin_fixup(struct usb_device_descriptor descriptor,
 }
 #endif /* CONFIG_USB_BIN_FIXUP */
 
-#define USB_MAX_READ_BLK 20
-
 unsigned long usb_stor_read(int device, unsigned long blknr,
 			    unsigned long blkcnt, void *buffer)
 {
 	unsigned long start, blks, buf_addr;
 	unsigned short smallblks;
 	struct usb_device *dev;
+	struct us_data *ss;
 	int retry, i;
 	ccb *srb = &usb_ccb;
 
@@ -1066,13 +1066,14 @@  unsigned long usb_stor_read(int device, unsigned long blknr,
 		if (dev->devnum == usb_dev_desc[device].target)
 			break;
 	}
+	ss = (struct us_data *)dev->privptr;
 
 	usb_disable_asynch(1); /* asynch transfer not allowed */
 	srb->lun = usb_dev_desc[device].lun;
 	buf_addr = (unsigned long)buffer;
 	start = blknr;
 	blks = blkcnt;
-	if (usb_test_unit_ready(srb, (struct us_data *)dev->privptr)) {
+	if (usb_test_unit_ready(srb, ss)) {
 		printf("Device NOT ready\n   Request Sense returned %02X %02X"
 		       " %02X\n", srb->sense_buf[2], srb->sense_buf[12],
 		       srb->sense_buf[13]);
@@ -1086,19 +1087,18 @@  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 > USB_MAX_READ_BLK)
-			smallblks = USB_MAX_READ_BLK;
+		if (blks > ss->max_xfer_blk)
+			smallblks = ss->max_xfer_blk;
 		else
 			smallblks = (unsigned short) blks;
 retry_it:
-		if (smallblks == USB_MAX_READ_BLK)
+		if (smallblks == ss->max_xfer_blk)
 			usb_show_progress();
 		srb->datalen = usb_dev_desc[device].blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (usb_read_10(srb, (struct us_data *)dev->privptr, start,
-		    smallblks)) {
+		if (usb_read_10(srb, ss, start, smallblks)) {
 			USB_STOR_PRINTF("Read ERROR\n");
-			usb_request_sense(srb, (struct us_data *)dev->privptr);
+			usb_request_sense(srb, ss);
 			if (retry--)
 				goto retry_it;
 			blkcnt -= blks;
@@ -1113,19 +1113,18 @@  retry_it:
 			start, smallblks, buf_addr);
 
 	usb_disable_asynch(0); /* asynch transfer allowed */
-	if (blkcnt >= USB_MAX_READ_BLK)
+	if (blkcnt >= ss->max_xfer_blk)
 		debug("\n");
 	return blkcnt;
 }
 
-#define USB_MAX_WRITE_BLK 20
-
 unsigned long usb_stor_write(int device, unsigned long blknr,
 				unsigned long blkcnt, const void *buffer)
 {
 	unsigned long start, blks, buf_addr;
 	unsigned short smallblks;
 	struct usb_device *dev;
+	struct us_data *ss;
 	int retry, i;
 	ccb *srb = &usb_ccb;
 
@@ -1143,6 +1142,7 @@  unsigned long usb_stor_write(int device, unsigned long blknr,
 		if (dev->devnum == usb_dev_desc[device].target)
 			break;
 	}
+	ss = (struct us_data *)dev->privptr;
 
 	usb_disable_asynch(1); /* asynch transfer not allowed */
 
@@ -1150,7 +1150,7 @@  unsigned long usb_stor_write(int device, unsigned long blknr,
 	buf_addr = (unsigned long)buffer;
 	start = blknr;
 	blks = blkcnt;
-	if (usb_test_unit_ready(srb, (struct us_data *)dev->privptr)) {
+	if (usb_test_unit_ready(srb, ss)) {
 		printf("Device NOT ready\n   Request Sense returned %02X %02X"
 		       " %02X\n", srb->sense_buf[2], srb->sense_buf[12],
 			srb->sense_buf[13]);
@@ -1166,19 +1166,18 @@  unsigned long usb_stor_write(int device, unsigned long blknr,
 		 */
 		retry = 2;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (blks > USB_MAX_WRITE_BLK)
-			smallblks = USB_MAX_WRITE_BLK;
+		if (blks > ss->max_xfer_blk)
+			smallblks = ss->max_xfer_blk;
 		else
 			smallblks = (unsigned short) blks;
 retry_it:
-		if (smallblks == USB_MAX_WRITE_BLK)
+		if (smallblks == ss->max_xfer_blk)
 			usb_show_progress();
 		srb->datalen = usb_dev_desc[device].blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (usb_write_10(srb, (struct us_data *)dev->privptr, start,
-		    smallblks)) {
+		if (usb_write_10(srb, ss, start, smallblks)) {
 			USB_STOR_PRINTF("Write ERROR\n");
-			usb_request_sense(srb, (struct us_data *)dev->privptr);
+			usb_request_sense(srb, ss);
 			if (retry--)
 				goto retry_it;
 			blkcnt -= blks;
@@ -1193,7 +1192,7 @@  retry_it:
 			start, smallblks, buf_addr);
 
 	usb_disable_asynch(0); /* asynch transfer allowed */
-	if (blkcnt >= USB_MAX_WRITE_BLK)
+	if (blkcnt >= ss->max_xfer_blk)
 		debug("\n");
 	return blkcnt;
 
@@ -1419,6 +1418,12 @@  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);