On Fri, 30 Sep 2011, Paul Zimmerman wrote:
> The Synopsys USB device controller requires all OUT transfer request
> lengths to be aligned to max packet size. The mass storage gadgets do
> not meet this requirement for Super Speed. The gadgets already have a
> function which performs this alignment for CBW packets, so use it for
> data packets too.
>
> The alternative would be to implement bounce buffers in the DWC3
> driver, but that could have a significant impact on performance.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> Alan, Felipe,
>
> I remeber there was a lot of discussion on the list around this
> or a similar issue, was a conclusion ever reached? Is this patch
> acceptable?
>
> drivers/usb/gadget/f_mass_storage.c | 6 ++----
> drivers/usb/gadget/file_storage.c | 6 ++----
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index 562d29e..f7ae1e8 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -958,8 +958,7 @@ static int do_write(struct fsg_common *common)
> * equal to the buffer size, which is divisible by
> * the bulk-out maxpacket size.
> */
> - bh->outreq->length = amount;
> - bh->bulk_out_intended_length = amount;
> + set_bulk_out_req_length(common, bh, amount);
> bh->outreq->short_not_ok = 1;
> if (!start_out_transfer(common, bh))
> /* Dunno what to do if common->fsg is NULL */
> @@ -1612,8 +1611,7 @@ static int throw_away_data(struct fsg_common *common)
> * equal to the buffer size, which is divisible by
> * the bulk-out maxpacket size.
> */
> - bh->outreq->length = amount;
> - bh->bulk_out_intended_length = amount;
> + set_bulk_out_req_length(common, bh, amount);
> bh->outreq->short_not_ok = 1;
> if (!start_out_transfer(common, bh))
> /* Dunno what to do if common->fsg is NULL */
> diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
> index 5590f00..5d28e79 100644
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -1325,8 +1325,7 @@ static int do_write(struct fsg_dev *fsg)
> * equal to the buffer size, which is divisible by
> * the bulk-out maxpacket size.
> */
> - bh->outreq->length = bh->bulk_out_intended_length =
> - amount;
> + set_bulk_out_req_length(fsg, bh, amount);
> bh->outreq->short_not_ok = 1;
> start_transfer(fsg, fsg->bulk_out, bh->outreq,
> &bh->outreq_busy, &bh->state);
> @@ -1961,8 +1960,7 @@ static int throw_away_data(struct fsg_dev *fsg)
> * equal to the buffer size, which is divisible by
> * the bulk-out maxpacket size.
> */
> - bh->outreq->length = bh->bulk_out_intended_length =
> - amount;
> + set_bulk_out_req_length(fsg, bh, amount);
> bh->outreq->short_not_ok = 1;
> start_transfer(fsg, fsg->bulk_out, bh->outreq,
> &bh->outreq_busy, &bh->state);
>
Hmmm, I don't remember the full story. Probably it could be tracked
down in the email archives.
For the record, I have a local patch in my own tree (never submitted to
Greg) that includes what you have written plus a little more, although
I didn't bother to change f_mass_storage.c along with file_storage.c.
It is given below. Differences from the current state of the source
file are at least partly explained by the fact that I haven't updated
my repository since kernel.org went down.
Alan Stern
Index: usb-3.1/drivers/usb/gadget/file_storage.c
===================================================================
--- usb-3.1.orig/drivers/usb/gadget/file_storage.c
+++ usb-3.1/drivers/usb/gadget/file_storage.c
@@ -1334,9 +1334,7 @@ static int do_write(struct fsg_dev *fsg)
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
- bh->outreq->length = bh->bulk_out_intended_length =
- amount;
- bh->outreq->short_not_ok = 1;
+ set_bulk_out_req_length(fsg, bh, amount);
start_transfer(fsg, fsg->bulk_out, bh->outreq,
&bh->outreq_busy, &bh->state);
fsg->next_buffhd_to_fill = bh->next;
@@ -1369,6 +1367,11 @@ static int do_write(struct fsg_dev *fsg)
amount = curlun->file_length - file_offset;
}
+ /* Don't accept excess data. The spec doesn't say
+ * what to do in this case. We'll ignore the error.
+ */
+ amount = min(amount, bh->bulk_out_intended_length);
+
/* Don't write a partial block */
amount = round_down(amount, curlun->blksize);
if (amount == 0)
@@ -1408,7 +1411,7 @@ static int do_write(struct fsg_dev *fsg)
empty_write:
/* Did the host decide to stop early? */
- if (bh->outreq->actual != bh->outreq->length) {
+ if (bh->outreq->actual < bh->bulk_out_intended_length) {
fsg->short_packet_received = 1;
break;
}
@@ -1952,7 +1955,7 @@ static int throw_away_data(struct fsg_de
fsg->next_buffhd_to_drain = bh->next;
/* A short packet or an error ends everything */
- if (bh->outreq->actual != bh->outreq->length ||
+ if (bh->outreq->actual < bh->bulk_out_intended_length ||
bh->outreq->status != 0) {
raise_exception(fsg, FSG_STATE_ABORT_BULK_OUT);
return -EINTR;
@@ -1970,9 +1973,7 @@ static int throw_away_data(struct fsg_de
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
- bh->outreq->length = bh->bulk_out_intended_length =
- amount;
- bh->outreq->short_not_ok = 1;
+ set_bulk_out_req_length(fsg, bh, amount);
start_transfer(fsg, fsg->bulk_out, bh->outreq,
&bh->outreq_busy, &bh->state);
fsg->next_buffhd_to_fill = bh->next;
@@ -2651,7 +2652,6 @@ static int get_next_command(struct fsg_d
/* Queue a request to read a Bulk-only CBW */
set_bulk_out_req_length(fsg, bh, USB_BULK_CB_WRAP_LEN);
- bh->outreq->short_not_ok = 1;
start_transfer(fsg, fsg->bulk_out, bh->outreq,
&bh->outreq_busy, &bh->state);
--
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
