Re: [PATCH] usb: gadget: file_storage: don't assume wMaxPacketSize to be 512

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi again,

On Fri, Apr 01, 2011 at 06:34:42PM +0300, Felipe Balbi wrote:
> > The correct approach is to align on the larger of the block size and
> > the maxpacket size (provided one is a multiple of the other, which
> > isn't always true with wireless USB!).  In theory short transfers

How about this instead ? (completely untested, compile tested only)

I still need to be sure if I'm using the correct endpoints on all
locations, I'm a bit confused about do_read_capacity() and do_verify()
will spend more time with this approach if you think it's good enough.
At least we won't be lying to host saying block size is 512 but aligning
on 1024 bytes in SuperSpeed. What do you think ?

diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index a6eacb5..48c296e 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -70,7 +70,7 @@
  * drive, but currently all LUNs have to be the same type.  The CD-ROM
  * emulation includes a single data track and no audio tracks; hence there
  * need be only one backing file per LUN.  Note also that the CD-ROM block
- * length is set to 512 rather than the more common value 2048.
+ * length is set to wMaxPacketSize rather than the more common value 2048.
  *
  * Requirements are modest; only a bulk-in and a bulk-out endpoint are
  * needed (an interrupt-out endpoint is also needed for CBI).  The memory
@@ -1124,6 +1124,7 @@ static int sleep_thread(struct fsg_dev *fsg)
 static int do_read(struct fsg_dev *fsg)
 {
 	struct fsg_lun		*curlun = fsg->curlun;
+	struct usb_ep		*ep = fsg->bulk_in;
 	u32			lba;
 	struct fsg_buffhd	*bh;
 	int			rc;
@@ -1215,7 +1216,7 @@ static int do_read(struct fsg_dev *fsg)
 		} else if (nread < amount) {
 			LDBG(curlun, "partial file read: %d/%u\n",
 					(int) nread, amount);
-			nread -= (nread & 511);	// Round down to a block
+			nread &= ~(ep->maxpacket - 1);	// Round down to a block
 		}
 		file_offset  += nread;
 		amount_left  -= nread;
@@ -1249,6 +1250,7 @@ static int do_read(struct fsg_dev *fsg)
 
 static int do_write(struct fsg_dev *fsg)
 {
+	struct usb_ep		*ep = fsg->bulk_out;
 	struct fsg_lun		*curlun = fsg->curlun;
 	u32			lba;
 	struct fsg_buffhd	*bh;
@@ -1331,7 +1333,7 @@ static int do_write(struct fsg_dev *fsg)
 				curlun->info_valid = 1;
 				continue;
 			}
-			amount -= (amount & 511);
+			amount &= ~(ep->maxpacket - 1);
 			if (amount == 0) {
 
 				/* Why were we were asked to transfer a
@@ -1347,7 +1349,7 @@ static int do_write(struct fsg_dev *fsg)
 			if (amount_left_to_req == 0)
 				get_some_more = 0;
 
-			/* amount is always divisible by 512, hence by
+			/* amount is always divisible by block size, hence by
 			 * the bulk-out maxpacket size */
 			bh->outreq->length = bh->bulk_out_intended_length =
 					amount;
@@ -1402,7 +1404,7 @@ static int do_write(struct fsg_dev *fsg)
 			} else if (nwritten < amount) {
 				LDBG(curlun, "partial file write: %d/%u\n",
 						(int) nwritten, amount);
-				nwritten -= (nwritten & 511);
+				nwritten &= ~(ep->maxpacket - 1);
 						// Round down to a block
 			}
 			file_offset += nwritten;
@@ -1466,6 +1468,7 @@ static void invalidate_sub(struct fsg_lun *curlun)
 static int do_verify(struct fsg_dev *fsg)
 {
 	struct fsg_lun		*curlun = fsg->curlun;
+	struct usb_ep		*ep = fsg->bulk_out;
 	u32			lba;
 	u32			verification_length;
 	struct fsg_buffhd	*bh = fsg->next_buffhd_to_fill;
@@ -1544,7 +1547,7 @@ static int do_verify(struct fsg_dev *fsg)
 		} else if (nread < amount) {
 			LDBG(curlun, "partial file verify: %d/%u\n",
 					(int) nread, amount);
-			nread -= (nread & 511);	// Round down to a sector
+			nread &= ~(ep->maxpacket - 1);	// Round down to a sector
 		}
 		if (nread == 0) {
 			curlun->sense_data = SS_UNRECOVERED_READ_ERROR;
@@ -1650,6 +1653,7 @@ static int do_request_sense(struct fsg_dev *fsg, struct fsg_buffhd *bh)
 static int do_read_capacity(struct fsg_dev *fsg, struct fsg_buffhd *bh)
 {
 	struct fsg_lun	*curlun = fsg->curlun;
+	struct usb_ep	*ep = fsg->bulk_out;
 	u32		lba = get_unaligned_be32(&fsg->cmnd[2]);
 	int		pmi = fsg->cmnd[8];
 	u8		*buf = (u8 *) bh->buf;
@@ -1662,7 +1666,7 @@ static int do_read_capacity(struct fsg_dev *fsg, struct fsg_buffhd *bh)
 
 	put_unaligned_be32(curlun->num_sectors - 1, &buf[0]);
 						/* Max logical block */
-	put_unaligned_be32(512, &buf[4]);	/* Block length */
+	put_unaligned_be32(ep->maxpacket, &buf[4]);	/* Block length */
 	return 8;
 }
 
@@ -1876,6 +1880,7 @@ static int do_read_format_capacities(struct fsg_dev *fsg,
 			struct fsg_buffhd *bh)
 {
 	struct fsg_lun	*curlun = fsg->curlun;
+	struct usb_ep	*ep = fsg->bulk_out;
 	u8		*buf = (u8 *) bh->buf;
 
 	buf[0] = buf[1] = buf[2] = 0;
@@ -1884,7 +1889,7 @@ static int do_read_format_capacities(struct fsg_dev *fsg,
 
 	put_unaligned_be32(curlun->num_sectors, &buf[0]);
 						/* Number of blocks */
-	put_unaligned_be32(512, &buf[4]);	/* Block length */
+	put_unaligned_be32(ep->maxpacket, &buf[4]);	/* Block length */
 	buf[4] = 0x02;				/* Current capacity */
 	return 12;
 }
@@ -2008,7 +2013,7 @@ static int throw_away_data(struct fsg_dev *fsg)
 			amount = min(fsg->usb_amount_left,
 					(u32) mod_data.buflen);
 
-			/* amount is always divisible by 512, hence by
+			/* amount is always divisible by block size, hence by
 			 * the bulk-out maxpacket size */
 			bh->outreq->length = bh->bulk_out_intended_length =
 					amount;

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux