> From: Sarah Sharp > On Mon, Dec 16, 2013 at 03:20:28PM -0000, David Laight wrote: > > > I know all these difference clearly, inc_deq() is indeed a common > > > function for different rings, but lasr_trb() & last_trb_on_last_seg() > > > inside it use different condition to determine the last trb in an > > > event ring and an non-event ring; and sorry, i still not find why last > > > trb in an event ring skipped by H/W according to the current logic. > > > > > > Thanks! > > > > > > > Read the specs and the code. > > State your objections clearly. > > Based on my analysis, this patch will produce correct behavior with the > event ring: > > http://marc.info/?l=linux-usb&m=138697807827548&w=2 > > Am I missing something? > -/* Is this TRB a link TRB or was the last TRB the last TRB in this event ring > - * segment? I.e. would the updated event TRB pointer step off the end of the > - * event seg? > +/* Is this TRB a link TRB or was the last TRB in this event ring segment? > + * I.e. would the updated event TRB pointer step off the end of the event > + * seg? > */ > static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring, > struct xhci_segment *seg, union xhci_trb *trb) > { > if (ring == xhci->event_ring) > - return trb == &seg->trbs[TRBS_PER_SEGMENT]; > + return trb == &seg->trbs[TRBS_PER_SEGMENT - 1]; > else > return TRB_TYPE_LINK_LE32(trb->link.control); > } It would be extremely confusing for the above function to behave differently for event and other rings. Actually last_trb() could just be: return trb == seg->last_trb; provided an appropriate 'last_trb' field was set. If we assume that there are no empty ring segments (ie adjacent LINK TRBs), and that it is never actually called with a pointer to a link trb. Then the inc_deq() code could just be: ring->deq_updates++; ring->num_trbs_free++; if (last_trb(xhci, ring, ring->deq_seg, ++ring->dequeue)) { ring->deq_seg = ring->deq_seg->next; ring->dequeue = ring->deq_seg->trbs; if (ring->deq_seg == ring->first_seg) /* not sure of name */ ring->cycle_state ^= 1; } David -- 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
