----- Original Message ----- > From: "CAI Qian" <[email protected]> > To: "Sarah Sharp" <[email protected]> > Cc: [email protected] > Sent: Friday, January 11, 2013 10:58:36 AM > Subject: [PATCH 3.0.x] xHCI: Fix TD Size calculation on 1.0 hosts. > > Hi Sarah, > > Please see if you could provide an ACK/NAK here, so Greg could > apply/drop > this to the stable release. Ping Sarah, this is still missing from the latest stable-3.0.y hence pending on your review. It is still applied and complied fine there, and this back-port patch just made some context changes.
Regards, CAI Qian > > Thanks, > CAI Qian > > From 4525c0a10dff7ad3669763c28016c7daffc3900e Mon Sep 17 00:00:00 > 2001 > From: Sarah Sharp <[email protected]> > Date: Thu, 25 Oct 2012 15:56:40 -0700 > Subject: [PATCH] xHCI: Fix TD Size calculation on 1.0 hosts. > > The xHCI 1.0 specification made a change to the TD Size field in > TRBs. > The value is now the number of packets that remain to be sent in the > TD, > not including this TRB. The TD Size value for the last TRB in a TD > must > always be zero. > > The xHCI function xhci_v1_0_td_remainder() attempts to calculate > this, > but it gets it wrong. First, it erroneously reuses the old > xhci_td_remainder function, which will right shift the value by 10. > The > xHCI 1.0 spec as of June 2011 says nothing about right shifting by > 10. > Second, it does not set the TD size for the last TRB in a TD to zero. > > Third, it uses roundup instead of DIV_ROUND_UP. The total packet > count > is supposed to be the total number of bytes in this TD, divided by > the > max packet size, rounded up. DIV_ROUND_UP is the right function to > use > in that case. > > With the old code, a TD on an endpoint with max packet size 1024 > would > be set up like so: > TRB 1, TRB length = 600 bytes, TD size = 0 > TRB 1, TRB length = 200 bytes, TD size = 0 > TRB 1, TRB length = 100 bytes, TD size = 0 > > With the new code, the TD would be set up like this: > TRB 1, TRB length = 600 bytes, TD size = 1 > TRB 1, TRB length = 200 bytes, TD size = 1 > TRB 1, TRB length = 100 bytes, TD size = 0 > > This commit should be backported to kernels as old as 3.0, that > contain > the commit 4da6e6f247a2601ab9f1e63424e4d944ed4124f3 "xhci 1.0: Update > TD > size field format." > > Signed-off-by: Sarah Sharp <[email protected]> > Reported-by: Chintan Mehta <[email protected]> > Reported-by: Shimmer Huang <[email protected]> > Tested-by: Bhavik Kothari <[email protected]> > Tested-by: Shimmer Huang <[email protected]> > Signed-off-by: CAI Qian <[email protected]> > > diff --git a/drivers/usb/host/xhci-ring.c > b/drivers/usb/host/xhci-ring.c > index 1a38281..f14ad43 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -3005,11 +3005,11 @@ static u32 xhci_td_remainder(unsigned int > remainder) > } > > /* > - * For xHCI 1.0 host controllers, TD size is the number of packets > remaining in > - * the TD (*not* including this TRB). > + * For xHCI 1.0 host controllers, TD size is the number of max > packet sized > + * packets remaining in the TD (*not* including this TRB). > * > * Total TD packet count = total_packet_count = > - * roundup(TD size in bytes / wMaxPacketSize) > + * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize) > * > * Packets transferred up to and including this TRB = > packets_transferred = > * rounddown(total bytes transferred including this TRB / > wMaxPacketSize) > @@ -3017,15 +3017,16 @@ static u32 xhci_td_remainder(unsigned int > remainder) > * TD size = total_packet_count - packets_transferred > * > * It must fit in bits 21:17, so it can't be bigger than 31. > + * The last TRB in a TD must have the TD size set to zero. > */ > - > static u32 xhci_v1_0_td_remainder(int running_total, int > trb_buff_len, > - unsigned int total_packet_count, struct urb *urb) > + unsigned int total_packet_count, struct urb *urb, > + unsigned int num_trbs_left) > { > int packets_transferred; > > /* One TRB with a zero-length data packet. */ > - if (running_total == 0 && trb_buff_len == 0) > + if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == > 0)) > return 0; > > /* All the TRB queueing functions don't count the current TRB in > @@ -3034,7 +3035,9 @@ static u32 xhci_v1_0_td_remainder(int > running_total, int trb_buff_len, > packets_transferred = (running_total + trb_buff_len) / > le16_to_cpu(urb->ep->desc.wMaxPacketSize); > > - return xhci_td_remainder(total_packet_count - packets_transferred); > + if ((total_packet_count - packets_transferred) > 31) > + return 31 << 17; > + return (total_packet_count - packets_transferred) << 17; > } > > static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags, > @@ -3061,7 +3064,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd > *xhci, gfp_t mem_flags, > > num_trbs = count_sg_trbs_needed(xhci, urb); > num_sgs = urb->num_mapped_sgs; > - total_packet_count = roundup(urb->transfer_buffer_length, > + total_packet_count = DIV_ROUND_UP(urb->transfer_buffer_length, > le16_to_cpu(urb->ep->desc.wMaxPacketSize)); > > trb_buff_len = prepare_transfer(xhci, xhci->devs[slot_id], > @@ -3151,7 +3154,8 @@ static int queue_bulk_sg_tx(struct xhci_hcd > *xhci, gfp_t mem_flags, > running_total); > } else { > remainder = xhci_v1_0_td_remainder(running_total, > - trb_buff_len, total_packet_count, urb); > + trb_buff_len, total_packet_count, urb, > + num_trbs - 1); > } > length_field = TRB_LEN(trb_buff_len) | > remainder | > @@ -3314,7 +3318,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, > gfp_t mem_flags, > running_total); > } else { > remainder = xhci_v1_0_td_remainder(running_total, > - trb_buff_len, total_packet_count, urb); > + trb_buff_len, total_packet_count, urb, > + num_trbs - 1); > } > length_field = TRB_LEN(trb_buff_len) | > remainder | > @@ -3589,7 +3594,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd > *xhci, gfp_t mem_flags, > addr = start_addr + urb->iso_frame_desc[i].offset; > td_len = urb->iso_frame_desc[i].length; > td_remain_len = td_len; > - total_packet_count = roundup(td_len, > + total_packet_count = DIV_ROUND_UP(td_len, > le16_to_cpu(urb->ep->desc.wMaxPacketSize)); > /* A zero-length transfer still involves at least one packet. */ > if (total_packet_count == 0) > @@ -3669,7 +3674,8 @@ static int xhci_queue_isoc_tx(struct xhci_hcd > *xhci, gfp_t mem_flags, > } else { > remainder = xhci_v1_0_td_remainder( > running_total, trb_buff_len, > - total_packet_count, urb); > + total_packet_count, urb, > + (trbs_per_td - j - 1)); > } > length_field = TRB_LEN(trb_buff_len) | > remainder | > -- > To unsubscribe from this list: send the line "unsubscribe stable" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
