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