Re: [PATCH] xHCI: Fix wrong usage of macro TRB_TYPE

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

 



On Tue, May 04, 2010 at 02:13:02AM +0800, Andiry Xu wrote:
> >From 0ca06b58e18b64ac5fff72953209300bab3f4dbd Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Mon, 3 May 2010 19:04:45 +0800
> Subject: [PATCH] xHCI: Fix wrong usage of macro TRB_TYPE
> 
> Macro TRB_TYPE is misused in some places. Fix the wrong usage.

Wow, what was I smoking?  I guess I haven't seen this bug because the
find_trb_seg() and xhci_find_new_dequeue_state() cases would have to be
triggered by a canceled TD that wrapped around the last segment to the
first segment.  And the slow path of the bulk transfer completion would
rarely be hit unless there was a short packet on a TRB just past the
last segment.  Thanks for catching this!

Greg, this should be queued for the stable kernels too.

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>

> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
>  drivers/usb/host/xhci-ring.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index b648efa..225a5b1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -365,7 +365,8 @@ static struct xhci_segment *find_trb_seg(
>  	while (cur_seg->trbs > trb ||
>  			&cur_seg->trbs[TRBS_PER_SEGMENT - 1] < trb) {
>  		generic_trb = &cur_seg->trbs[TRBS_PER_SEGMENT - 1].generic;
> -		if (TRB_TYPE(generic_trb->field[3]) == TRB_LINK &&
> +		if ((generic_trb->field[3] & TRB_TYPE_BITMASK) ==
> +				TRB_TYPE(TRB_LINK) &&
>  				(generic_trb->field[3] & LINK_TOGGLE))
>  			*cycle_state = ~(*cycle_state) & 0x1;
>  		cur_seg = cur_seg->next;
> @@ -430,7 +431,7 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>  		BUG();
>  
>  	trb = &state->new_deq_ptr->generic;
> -	if (TRB_TYPE(trb->field[3]) == TRB_LINK &&
> +	if ((trb->field[3] & TRB_TYPE_BITMASK) == TRB_TYPE(TRB_LINK) &&
>  				(trb->field[3] & LINK_TOGGLE))
>  		state->new_cycle_state = ~(state->new_cycle_state) & 0x1;
>  	next_trb(xhci, ep_ring, &state->new_deq_seg, &state->new_deq_ptr);
> @@ -1471,8 +1472,10 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>  			for (cur_trb = ep_ring->dequeue, cur_seg = ep_ring->deq_seg;
>  					cur_trb != event_trb;
>  					next_trb(xhci, ep_ring, &cur_seg, &cur_trb)) {
> -				if (TRB_TYPE(cur_trb->generic.field[3]) != TRB_TR_NOOP &&
> -						TRB_TYPE(cur_trb->generic.field[3]) != TRB_LINK)
> +				if ((cur_trb->generic.field[3] &
> +				 TRB_TYPE_BITMASK) != TRB_TYPE(TRB_TR_NOOP) &&
> +				    (cur_trb->generic.field[3] &
> +				 TRB_TYPE_BITMASK) != TRB_TYPE(TRB_LINK))
>  					td->urb->actual_length +=
>  						TRB_LEN(cur_trb->generic.field[2]);
>  			}
> -- 
> 1.6.3.3
> 
> 
> 
--
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