Re: [PATCH v2 2/3] usb: gadget: file_storage: Make CD-ROM emulation work with Mac OS-X

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

 



On 03/22/2011 05:13 PM, ext Alan Stern wrote:
On Tue, 22 Mar 2011, Roger Quadros wrote:

However, if host asked for data more than the TOC length then residue will be
greater than zero and this will result in zero padded transfers.

Not a big issue but should we be fixing it?

one way could be to set fsg->residue to actual TOC length. in do_read_toc().

The current behavior is required by the the Bulk-Only Transport
specification.  The spec says (section 6.7.2, case (4) or (5)):

If the device intends to send less data than the host indicated, then:
     The device shall send the intended data.
	The device may send fill data to pad up to a total of
	dCBWDataTransferLength.
	If the device actually transfers less data than the host indicated,
	then:
	    The device may end the transfer with a short packet.
	    The device shall STALL the Bulk-In pipe.

You're loading the mass-storage gadget with the "stall=n" module
parameter, right?  Therefore the driver is not allowed to halt the

Yes i'm loading it with "stall=n".

Bulk-In endpoint, and therefore the driver is not allowed to send less
data than the host indicated, which means the data has to be padded.

Thanks. i got it now.


On the other hand, I don't think any implementations would get upset if
we simply ended the transfer with a short packet instead of adhering
strictly to the spec.

The patch below should do what you want.  I haven't tested it.

I tried your patch with the CD-ROM implementation and it works perfectly. I do not see the unnecessary zero padded transfers any more.

Do you think we should have this patch in? with the risk of not strictly adhering to spec for cases where controller cannot stall?

Maybe the term "controller cannot stall" itself does not adhere to bulk-only transport spec :).

  static int throw_away_data(struct fsg_common *common)
  {
  	struct fsg_buffhd	*bh;
@@ -1702,6 +1671,10 @@ static int finish_reply(struct fsg_commo
  		if (common->data_size == 0) {
  			/* Nothing to send */

+		/* Don't know what to do if common->fsg is NULL */
+		} else if (!common->fsg) {
+			rc = -EIO;
+
  		/* If there's no residue, simply send the last buffer */
  		} else if (common->residue == 0) {
  			bh->inreq->zero = 0;
@@ -1710,24 +1683,19 @@ static int finish_reply(struct fsg_commo
  			common->next_buffhd_to_fill = bh->next;

  		/*
-		 * For Bulk-only, if we're allowed to stall then send the
-		 * short packet and halt the bulk-in endpoint.  If we can't
-		 * stall, pad out the remaining data with 0's.
+		 * For Bulk-only, mark the end of the data with a short
+		 * packet.  If we are allowed to stall, halt the bulk-in
+		 * endpoint.  (Note: This violates the Bulk-Only Transport
+		 * specification, which requires us to pad the data if we

violates the spec only if we are not allowed to stall (i.e. stall=n) right?

+		 * don't halt the endpoint.  Presumably nobody will mind.)
  		 */

--
regards,
-roger
--
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