RE: [PATCH] usb: wusbcore: fix short transfers

2013-12-10 Thread David Laight
 From: Thomas Pugliese
 Sent: 09 December 2013 19:11
 To: linux-usb@vger.kernel.org
 Cc: gre...@linuxfoundation.org; Thomas Pugliese
 Subject: [PATCH] usb: wusbcore: fix short transfers
 
 If a URB is broken up into multiple transfer segments and a short
 transfer occurs in any segment other than the last, the URB will
 currently get stuck in the driver forever.  This patch adds a check for
 a short transfer and cleans up any pending segments so the URB can
 complete properly.

I thought that generic code ensured that this never happens.
I thought that only xhci supports arbitrary fragmentation.

David



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


RE: [PATCH] usb: wusbcore: fix short transfers

2013-12-10 Thread Thomas Pugliese


On Tue, 10 Dec 2013, David Laight wrote:

  From: Thomas Pugliese
  Sent: 09 December 2013 19:11
  To: linux-usb@vger.kernel.org
  Cc: gre...@linuxfoundation.org; Thomas Pugliese
  Subject: [PATCH] usb: wusbcore: fix short transfers
  
  If a URB is broken up into multiple transfer segments and a short
  transfer occurs in any segment other than the last, the URB will
  currently get stuck in the driver forever.  This patch adds a check for
  a short transfer and cleans up any pending segments so the URB can
  complete properly.
 
 I thought that generic code ensured that this never happens.
 I thought that only xhci supports arbitrary fragmentation.
 
   David
 
 
 
 

Hi David, 
WUSB actually does support arbitrary fragmentation but that is not the 
type of fragmentation that is addressed with this fix.  The HWA can break 
up large URBs into smaller transfer segments that are not visible to the 
USB core.  These segments are similar to TRBs in xHCI.  If a large read 
request was broken up into multiple transfer segments and the read 
completed early due to a short transfer, the tracking data for the 
remaining segments would not be cleaned up and the URB would stay in the 
driver forever.  This patch solves that problem.

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


[PATCH] usb: wusbcore: fix short transfers

2013-12-09 Thread Thomas Pugliese
If a URB is broken up into multiple transfer segments and a short 
transfer occurs in any segment other than the last, the URB will 
currently get stuck in the driver forever.  This patch adds a check for 
a short transfer and cleans up any pending segments so the URB can 
complete properly.

Signed-off-by: Thomas Pugliese thomas.pugli...@gmail.com
---
 drivers/usb/wusbcore/wa-xfer.c |  128 +++-
 1 file changed, 74 insertions(+), 54 deletions(-)

diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c
index a88b8c6..673ad80 100644
--- a/drivers/usb/wusbcore/wa-xfer.c
+++ b/drivers/usb/wusbcore/wa-xfer.c
@@ -1993,7 +1993,7 @@ static int wa_xfer_status_to_errno(u8 status)
  * the xfer will complete cleanly.
  */
 static void wa_complete_remaining_xfer_segs(struct wa_xfer *xfer,
-   struct wa_seg *incoming_seg)
+   struct wa_seg *incoming_seg, enum wa_seg_status status)
 {
int index;
struct wa_rpipe *rpipe = xfer-ep-hcpriv;
@@ -2015,7 +2015,7 @@ static void wa_complete_remaining_xfer_segs(struct 
wa_xfer *xfer,
 */
case WA_SEG_DELAYED:
xfer-segs_done++;
-   current_seg-status = incoming_seg-status;
+   current_seg-status = status;
break;
case WA_SEG_ABORTED:
break;
@@ -2028,6 +2028,58 @@ static void wa_complete_remaining_xfer_segs(struct 
wa_xfer *xfer,
}
 }
 
+/* Populate the wa-buf_in_urb based on the current transfer state. */
+static int wa_populate_buf_in_urb(struct wahc *wa, struct wa_xfer *xfer,
+   unsigned int seg_idx, unsigned int bytes_transferred)
+{
+   int result = 0;
+   struct wa_seg *seg = xfer-seg[seg_idx];
+
+   BUG_ON(wa-buf_in_urb-status == -EINPROGRESS);
+   /* this should always be 0 before a resubmit. */
+   wa-buf_in_urb-num_mapped_sgs  = 0;
+
+   if (xfer-is_dma) {
+   wa-buf_in_urb-transfer_dma = xfer-urb-transfer_dma
+   + (seg_idx * xfer-seg_size);
+   wa-buf_in_urb-transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+   wa-buf_in_urb-transfer_buffer = NULL;
+   wa-buf_in_urb-sg = NULL;
+   wa-buf_in_urb-num_sgs = 0;
+   } else {
+   /* do buffer or SG processing. */
+   wa-buf_in_urb-transfer_flags = ~URB_NO_TRANSFER_DMA_MAP;
+
+   if (xfer-urb-transfer_buffer) {
+   wa-buf_in_urb-transfer_buffer =
+   xfer-urb-transfer_buffer
+   + (seg_idx * xfer-seg_size);
+   wa-buf_in_urb-sg = NULL;
+   wa-buf_in_urb-num_sgs = 0;
+   } else {
+   /* allocate an SG list to store seg_size bytes
+   and copy the subset of the xfer-urb-sg
+   that matches the buffer subset we are
+   about to read. */
+   wa-buf_in_urb-sg = wa_xfer_create_subset_sg(
+   xfer-urb-sg,
+   seg_idx * xfer-seg_size,
+   bytes_transferred,
+   (wa-buf_in_urb-num_sgs));
+
+   if (!(wa-buf_in_urb-sg)) {
+   wa-buf_in_urb-num_sgs = 0;
+   result = -ENOMEM;
+   }
+   wa-buf_in_urb-transfer_buffer = NULL;
+   }
+   }
+   wa-buf_in_urb-transfer_buffer_length = bytes_transferred;
+   wa-buf_in_urb-context = seg;
+
+   return result;
+}
+
 /*
  * Process a xfer result completion message
  *
@@ -2041,12 +2093,13 @@ static void wa_xfer_result_chew(struct wahc *wa, struct 
wa_xfer *xfer,
int result;
struct device *dev = wa-usb_iface-dev;
unsigned long flags;
-   u8 seg_idx;
+   unsigned int seg_idx;
struct wa_seg *seg;
struct wa_rpipe *rpipe;
unsigned done = 0;
u8 usb_status;
unsigned rpipe_ready = 0;
+   unsigned bytes_transferred = le32_to_cpu(xfer_result-dwTransferLength);
 
spin_lock_irqsave(xfer-lock, flags);
seg_idx = xfer_result-bTransferSegment  0x7f;
@@ -2079,66 +2132,33 @@ static void wa_xfer_result_chew(struct wahc *wa, struct 
wa_xfer *xfer,
/* FIXME: we ignore warnings, tally them for stats */
if (usb_status  0x40)  /* Warning?... */
usb_status = 0; /* ... pass */
+   /*
+* If the last segment bit is set, complete the remaining segments.
+* When the current segment is completed, either in wa_buf_in_cb for
+* transfers with data or below for no data, the xfer will complete.
+*/
+   if (xfer_result-bTransferSegment  0x80)
+