[PATCH 5/5] xhci: Simpify processing LINK TRBs when writing trb

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

 



The ring write pointer has to be left pointing at any LINK TRB
when all the TRB for a TD have been added.
This is currently acheived by advancing past a LINK in prepare_ring()
and conditonally by queue_trb() (actually inc_enq) after writing the TRB
if the caller passed 'more_trbs_coming = true'.

Instead just advance the pointer past a LINK TRB in queue_trb() before
writing the required entry. Invert ring->cycle_state when the new
ring segment is the first one.

The value saved in 'td->last_trb' is now incorrect (it could refer to
a LINK TRB). Correct by saving 'ring->enqueue - 1' after queue_trb()
has been called for the last fragment.
Done via a static inline function since ity is very implementation
specific.
It doesn't matter if td->first_trb points to a LINK, it will be skipped.

Remove the now-unused more_trbs_coming parameter to queue_trb().

The code never generates LINK TRB pointing to LINK TRB - so the loops
in the old code are not needed. Similarly inc_enq() has never been
called for event rings - so those checks aren't needed.

Signed-off-by: David Laight <david.laight@xxxxxxxxxx>
---
 drivers/usb/host/xhci-ring.c | 211 ++++++++++++-------------------------------
 drivers/usb/host/xhci.h      |   7 ++
 2 files changed, 64 insertions(+), 154 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0d0bd7f..639704e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -117,12 +117,6 @@ static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
 		return TRB_TYPE_LINK_LE32(trb->link.control);
 }
 
-static int enqueue_is_link_trb(struct xhci_ring *ring)
-{
-	struct xhci_link_trb *link = &ring->enqueue->link;
-	return TRB_TYPE_LINK_LE32(link->control);
-}
-
 union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
 {
 	/* Enqueue pointer can be left pointing to the link TRB,
@@ -187,81 +181,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
 }
 
 /*
- * See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs.  That would be dumb and this would loop.
- *
- * If we've just enqueued a TRB that is in the middle of a TD (meaning the
- * chain bit is set), then set the chain bit in all the following link TRBs.
- * If we've enqueued the last TRB in a TD, make sure the following link TRBs
- * have their chain bit cleared (so that each Link TRB is a separate TD).
- *
- * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
- * set, but other sections talk about dealing with the chain bit set.  This was
- * fixed in the 0.96 specification errata, but we have to assume that all 0.95
- * xHCI hardware can't handle the chain bit being cleared on a link TRB.
- *
- * @more_trbs_coming:	Will you enqueue more TRBs before calling
- *			prepare_transfer()?
- */
-static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
-			bool more_trbs_coming)
-{
-	u32 chain;
-	union xhci_trb *next;
-
-	chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
-	/* If this is not event ring, there is one less usable TRB */
-	if (ring->type != TYPE_EVENT &&
-			!last_trb(xhci, ring, ring->enq_seg, ring->enqueue))
-		ring->num_trbs_free--;
-	next = ++(ring->enqueue);
-
-	ring->enq_updates++;
-	/* Update the dequeue pointer further if that was a link TRB or we're at
-	 * the end of an event ring segment (which doesn't have link TRBS)
-	 */
-	while (last_trb(xhci, ring, ring->enq_seg, next)) {
-		if (ring->type != TYPE_EVENT) {
-			/*
-			 * If the caller doesn't plan on enqueueing more
-			 * TDs before ringing the doorbell, then we
-			 * don't want to give the link TRB to the
-			 * hardware just yet.  We'll give the link TRB
-			 * back in prepare_ring() just before we enqueue
-			 * the TD at the top of the ring.
-			 */
-			if (!chain && !more_trbs_coming)
-				break;
-
-			/* If we're not dealing with 0.95 hardware or
-			 * isoc rings on AMD 0.96 host,
-			 * carry over the chain bit of the previous TRB
-			 * (which may mean the chain bit is cleared).
-			 */
-			if (!(ring->type == TYPE_ISOC &&
-					(xhci->quirks & XHCI_AMD_0x96_HOST))
-						&& !xhci_link_trb_quirk(xhci)) {
-				next->link.control &=
-					cpu_to_le32(~TRB_CHAIN);
-				next->link.control |=
-					cpu_to_le32(chain);
-			}
-			/* Give this link TRB to the hardware */
-			wmb();
-			next->link.control ^= cpu_to_le32(TRB_CYCLE);
-
-			/* Toggle the cycle bit after the last ring segment. */
-			if (last_trb_on_last_seg(xhci, ring, ring->enq_seg, next)) {
-				ring->cycle_state = (ring->cycle_state ? 0 : 1);
-			}
-		}
-		ring->enq_seg = ring->enq_seg->next;
-		ring->enqueue = ring->enq_seg->trbs;
-		next = ring->enqueue;
-	}
-}
-
-/*
  * Check to see if there's room to enqueue num_trbs on the ring and make sure
  * enqueue pointer will not advance into dequeue segment. See rules above.
  */
@@ -2894,28 +2813,59 @@ irqreturn_t xhci_msi_irq(int irq, void *hcd)
 /*
  * Generic function for queueing a TRB on a ring.
  * The caller must have checked to make sure there's room on the ring.
- *
- * @more_trbs_coming:	Will you enqueue more TRBs before calling
- *			prepare_transfer()?
  */
 static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
-		bool more_trbs_coming,
 		u32 field1, u32 field2, u32 field3, u32 field4)
 {
-	struct xhci_generic_trb *trb;
+	union xhci_trb *trb;
+
+	trb = ring->enqueue;
+
+	/* Don't overwrite a link TRB, just advance past it */
+	if (TRB_TYPE_LINK_LE32(trb->link.control)) {
+		/* If we're not dealing with 0.95 hardware or isoc rings
+		 * on AMD 0.96 host, copy the chain bit from the previous TRB.
+		 * (For 0.95 the chain bit is always set.)
+		 */
+		__le32 link_control = trb->link.control;
+		if (!xhci_link_trb_quirk(xhci) &&
+				!(ring->type == TYPE_ISOC &&
+				 (xhci->quirks & XHCI_AMD_0x96_HOST))) {
+			link_control &= cpu_to_le32(~TRB_CHAIN);
+			link_control |= trb[-1].link.control &
+					cpu_to_le32(TRB_CHAIN);
+		}
+
+		if (trb != ring->enqueue_first)
+			link_control ^= cpu_to_le32(TRB_CYCLE);
+		trb->link.control = link_control;
+
+		/* Advance to the next ring segment */
+		ring->enq_seg = ring->enq_seg->next;
+		ring->enqueue = ring->enq_seg->trbs;
+		trb = ring->enqueue;
+
+		/* Toggle the cycle bit after the last ring segment. */
+		if (ring->enq_seg == ring->first_seg)
+			ring->cycle_state ^= TRB_CYCLE;
+	}
 
-	trb = &ring->enqueue->generic;
 
 	/* Give the controller ownership of all but the first TRB */
 	field4 |= ring->cycle_state;
-	if (trb == &ring->enqueue_first->generic)
+	if (trb == ring->enqueue_first)
 		field4 ^= TRB_CYCLE;
 
-	trb->field[0] = cpu_to_le32(field1);
-	trb->field[1] = cpu_to_le32(field2);
-	trb->field[2] = cpu_to_le32(field3);
-	trb->field[3] = cpu_to_le32(field4);
-	inc_enq(xhci, ring, more_trbs_coming);
+	trb->generic.field[0] = cpu_to_le32(field1);
+	trb->generic.field[1] = cpu_to_le32(field2);
+	trb->generic.field[2] = cpu_to_le32(field3);
+	trb->generic.field[3] = cpu_to_le32(field4);
+
+	ring->num_trbs_free--;
+	ring->enq_updates++;
+
+	/* Leave enqueue pointing to the next entry - which might be a LINK */
+	ring->enqueue = trb + 1;
 }
 
 /*
@@ -3031,36 +2981,7 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
 		}
 	}
 
-	if (enqueue_is_link_trb(ep_ring)) {
-		struct xhci_ring *ring = ep_ring;
-		union xhci_trb *next;
-
-		next = ring->enqueue;
-
-		while (last_trb(xhci, ring, ring->enq_seg, next)) {
-			/* If we're not dealing with 0.95 hardware or isoc rings
-			 * on AMD 0.96 host, clear the chain bit.
-			 */
-			if (!xhci_link_trb_quirk(xhci) &&
-					!(ring->type == TYPE_ISOC &&
-					 (xhci->quirks & XHCI_AMD_0x96_HOST)))
-				next->link.control &= cpu_to_le32(~TRB_CHAIN);
-			else
-				next->link.control |= cpu_to_le32(TRB_CHAIN);
-
-			/* If LINK follows a NOP, invert cycle */
-			if (next != ep_ring->enqueue_first)
-				next->link.control ^= cpu_to_le32(TRB_CYCLE);
-
-			/* Toggle the cycle bit after the last ring segment. */
-			if (last_trb_on_last_seg(xhci, ring, ring->enq_seg, next)) {
-				ring->cycle_state = (ring->cycle_state ? 0 : 1);
-			}
-			ring->enq_seg = ring->enq_seg->next;
-			ring->enqueue = ring->enq_seg->trbs;
-			next = ring->enqueue;
-		}
-	}
+	/* queue_trb() will advance past a LINK TRB. */
 
 	return 0;
 }
@@ -3281,7 +3202,6 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	int trb_buff_len, this_sg_len, running_total;
 	unsigned int total_packet_count;
 	u64 addr;
-	bool more_trbs_coming;
 
 	ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
 	if (!ep_ring)
@@ -3332,7 +3252,6 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			field |= TRB_CHAIN;
 		} else {
 			/* FIXME - add check for ZERO_PACKET flag before this */
-			td->last_trb = ep_ring->enqueue;
 			field |= TRB_IOC;
 		}
 
@@ -3362,11 +3281,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			remainder |
 			TRB_INTR_TARGET(0);
 
-		if (num_trbs > 1)
-			more_trbs_coming = true;
-		else
-			more_trbs_coming = false;
-		queue_trb(xhci, ep_ring, more_trbs_coming,
+		queue_trb(xhci, ep_ring,
 				lower_32_bits(addr),
 				upper_32_bits(addr),
 				length_field,
@@ -3396,6 +3311,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			trb_buff_len =
 				urb->transfer_buffer_length - running_total;
 	} while (running_total < urb->transfer_buffer_length);
+	td->last_trb = xhci_last_written_trb(ep_ring);
 
 	check_trb_math(urb, num_trbs, running_total);
 	giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
@@ -3410,7 +3326,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	struct urb_priv *urb_priv;
 	struct xhci_td *td;
 	int num_trbs;
-	bool more_trbs_coming;
 	u32 field, length_field;
 
 	int running_total, trb_buff_len, ret;
@@ -3473,7 +3388,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			field |= TRB_CHAIN;
 		} else {
 			/* FIXME - add check for ZERO_PACKET flag before this */
-			td->last_trb = ep_ring->enqueue;
 			field |= TRB_IOC;
 		}
 
@@ -3495,11 +3409,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			remainder |
 			TRB_INTR_TARGET(0);
 
-		if (num_trbs > 1)
-			more_trbs_coming = true;
-		else
-			more_trbs_coming = false;
-		queue_trb(xhci, ep_ring, more_trbs_coming,
+		queue_trb(xhci, ep_ring,
 				lower_32_bits(addr),
 				upper_32_bits(addr),
 				length_field,
@@ -3513,6 +3423,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		if (trb_buff_len > TRB_MAX_BUFF_SIZE)
 			trb_buff_len = TRB_MAX_BUFF_SIZE;
 	} while (running_total < urb->transfer_buffer_length);
+	td->last_trb = xhci_last_written_trb(ep_ring);
 
 	check_trb_math(urb, num_trbs, running_total);
 	giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
@@ -3576,7 +3487,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		}
 	}
 
-	queue_trb(xhci, ep_ring, true,
+	queue_trb(xhci, ep_ring,
 		  setup->bRequestType | setup->bRequest << 8 | le16_to_cpu(setup->wValue) << 16,
 		  le16_to_cpu(setup->wIndex) | le16_to_cpu(setup->wLength) << 16,
 		  TRB_LEN(8) | TRB_INTR_TARGET(0),
@@ -3596,29 +3507,29 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	if (urb->transfer_buffer_length > 0) {
 		if (setup->bRequestType & USB_DIR_IN)
 			field |= TRB_DIR_IN;
-		queue_trb(xhci, ep_ring, true,
+		queue_trb(xhci, ep_ring,
 				lower_32_bits(urb->transfer_dma),
 				upper_32_bits(urb->transfer_dma),
 				length_field,
 				field);
 	}
 
-	/* Save the DMA address of the last TRB in the TD */
-	td->last_trb = ep_ring->enqueue;
-
 	/* Queue status TRB - see Table 7 and sections 4.11.2.2 and 6.4.1.2.3 */
 	/* If the device sent data, the status stage is an OUT transfer */
 	if (urb->transfer_buffer_length > 0 && setup->bRequestType & USB_DIR_IN)
 		field = 0;
 	else
 		field = TRB_DIR_IN;
-	queue_trb(xhci, ep_ring, false,
+	queue_trb(xhci, ep_ring,
 			0,
 			0,
 			TRB_INTR_TARGET(0),
 			/* Event on completion */
 			field | TRB_IOC | TRB_TYPE(TRB_STATUS));
 
+	/* Save the DMA address of the last TRB in the TD */
+	td->last_trb = xhci_last_written_trb(ep_ring);
+
 	giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
 	return 0;
 }
@@ -3710,7 +3621,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	int running_total, trb_buff_len, td_len, td_remain_len, ret;
 	u64 start_addr, addr;
 	int i, j;
-	bool more_trbs_coming = true;
 
 	ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
 
@@ -3782,7 +3692,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			if (j < trbs_per_td - 1) {
 				field |= TRB_CHAIN;
 			} else {
-				td->last_trb = ep_ring->enqueue;
 				field |= TRB_IOC;
 				if (xhci->hci_version == 0x100 &&
 						!(xhci->quirks &
@@ -3791,12 +3700,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 					if (i < num_tds - 1)
 						field |= TRB_BEI;
 				}
-				/*
-				 * We want to advance past LINK TRBs until the
-				 * very last TRB.
-				 */
-				if (i == num_tds - 1)
-					more_trbs_coming = false;
 			}
 
 			/* Calculate TRB length */
@@ -3819,7 +3722,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 				remainder |
 				TRB_INTR_TARGET(0);
 
-			queue_trb(xhci, ep_ring, more_trbs_coming,
+			queue_trb(xhci, ep_ring,
 				lower_32_bits(addr),
 				upper_32_bits(addr),
 				length_field,
@@ -3829,6 +3732,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 			addr += trb_buff_len;
 			td_remain_len -= trb_buff_len;
 		}
+		td->last_trb = xhci_last_written_trb(ep_ring);
 	}
 
 	if (xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs == 0) {
@@ -3938,8 +3842,7 @@ static int queue_command(struct xhci_hcd *xhci, u32 field1, u32 field2,
 					"unfailable commands failed.\n");
 		return ret;
 	}
-	queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3,
-			field4);
+	queue_trb(xhci, xhci->cmd_ring, field1, field2, field3, field4);
 	/* The 'first' TRB might be a LINK TRB... */
 	giveback_first_trb(xhci, 0, 0, xhci->cmd_ring);
 	return 0;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6973c56..7c6ae8f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1620,6 +1620,13 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd *xhci)
 	return xhci->quirks & XHCI_LINK_TRB_QUIRK;
 }
 
+static inline union xhci_trb *xhci_last_written_trb(struct xhci_ring *ring)
+{
+	/* The last trb that contains a real request */
+	return ring->enqueue - 1;
+}
+
+
 /* xHCI debugging */
 void xhci_print_ir_set(struct xhci_hcd *xhci, int set_num);
 void xhci_print_registers(struct xhci_hcd *xhci);
-- 
1.8.1.2



--
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