On Mon, 14 Sep 2015, Igor Kotrasinski wrote:
> transfer() schedules a rescan for transfers larger than
> maxpacket, which is wrong for transfers that are multiples
> of maxpacket.
>
> Rewrite to fix and clarify packet multiple / remainder
> transfer logic.
>
> Signed-off-by: Igor Kotrasinski <i.kotrasinsk@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index da38475..3fdcbda 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1288,6 +1288,7 @@ top:
> /* if there's no request queued, the device is NAKing; return */
> list_for_each_entry(req, &ep->queue, queue) {
> unsigned host_len, dev_len, len;
> + unsigned rem;
> int is_short, to_host;
> int rescan = 0;
>
> @@ -1320,12 +1321,17 @@ top:
> if (len == 0)
> break;
>
> - /* use an extra pass for the final short packet */
> - if (len > ep->ep.maxpacket) {
> - rescan = 1;
> - len -= (len % ep->ep.maxpacket);
> + /* send multiple of maxpacket first, then remainder */
> + rem = (len % ep->ep.maxpacket);
> + if (len - rem > 0) {
That looks really strange, particularly since len and rem are both
unsigned. Just say "if (len > rem)". Or even "if (len >=
ep->ep.maxpacket)", which is equivalent and is closer to the existing
code.
> + is_short = 0;
> + len -= rem;
> + if (rem > 0)
> + rescan = 1;
> + } else { /* rem > 0 */
> + is_short = 1;
> + len = rem;
This line is unnecessary. If len is not > rem then they must be equal
already.
> }
> - is_short = (len % ep->ep.maxpacket) != 0;
>
> len = dummy_perform_transfer(urb, req, len);
Alan Stern
--
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
