> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
