Hi Sarah, Thanks for your correction and suggestion. I am sorry for that I don't give a very clear description for FSE patch. Let me to explain it again with a special scenario. 1. There are two TDs in td_list that will be transferred. 2. The first TD was transferred successfully reported by an transfer event with COMP_SUCCESS. The xHCI driver called finish_td() to remove the first TD from the td_list and increase the ep_ring->dequeue and ep_ring->deq_seg to the next TD. But the HW internal ep ring dequeue maintained by xHC may stay at the last TRB of the first TD before start the next transfer for the second TD. 3. The stop endpoint command run just after the first TD being transferred successfully. It means that an FSE(CC=COMP_STOP_INVAL) queued after the transfer event(CC=COMP_SUCCESS) for the first TD. The FSE uses the HW internal ep ring dequeue which point to the last TRB of the first TD. And the last TRB of the first TD is the event_trb. 4. Then driver called handle_tx_event() again to process the FSE. It will try to find out the event_seg which include the event_trb of the FSE by the line: event_seg = trb_in_td(ep_ring->deq_seg, ep_ring->dequeue, td->last_trb, event_dma) It will fail with NULL return value because that the ep_ring->deq_seg and ep_ring->dequeue move to the second TD. i.e. you can't find out the last TRB of the first TD in the second TD. And this make the driver go to an wrong way with "return -ESHUTDWON". Another complicate case will occur when there an Link TD between the two TDs. So I think we need an patch for these cases like that: 1. Record the TD as prev_td in ep_ring before delete it from the td_list. 2. Find the FSE event_trb in the prev_td or between the prev_td and cur_td for it maybe a link TD. 3. Just jump over the FSE in the event ring because the prev transfer event have been processed. That's all my think about the FSE. It's too hard to figure out them. :( BTW: I will refine the other patches and commit them later. -----Original Message----- From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] Sent: Tuesday, May 10, 2011 12:33 AM To: Xu, Andiry Cc: linux-usb@xxxxxxxxxxxxxxx; He, Alex Subject: Re: [RFC 0/9] xHCI 1.0 patchset On Thu, May 05, 2011 at 09:59:18AM -0700, Sarah Sharp wrote: > On Thu, May 05, 2011 at 06:13:47PM +0800, Andiry Xu wrote: > > Hi Sarah, > > > > This is xHCI 1.0 compliance patchset based on your for-usb-next branch. > > It includes some new field setting and error handlers defined by xHCI > > 1.0 spec, some clarifications for ambiguous descriptions and new > > features. > > > > The Block Event Interrupt feature is used to reduce the interrupt rate > > and can improve the system performance for isoc transfer. > > Wonderful, thank you! I'll look over your patches and get back to you > soon. Hi Andiry and Alex, Thanks for pushing these patches. It's getting very close to the 2.6.40 merge window, and I'm a bit concerned about pushing some of the larger patches this late in the 2.6.39 release cycle. I'm going to push to Greg a subset of this series that seems fairly straight forward and didn't have much controversy during the review process. Here's what I'll push: [RFC 1/9] xHCI 1.0: Setup Stage TRB Transfer Type flag [RFC 2/9] xHCI 1.0: Control endpoint average TRB length field set [RFC 3/9] xHCI 1.0: Isoch endpoint CErr field set [RFC 4/9] xHCI 1.0: Block Interrupts for Isoch transfer [RFC 5/9] xHCI 1.0: TT_THINK_TIME set [PATCH 9/9] xHCI 1.0: Max Exit Latency Too Large Error I'd like to take a longer look at the sixth patch, "xHCI 1.0: Soft Retry mechanism" because it will change the current hardware behavior and it's a larger chunk of code. The seventh and eighth patches have already been reviewed and just need to be fixed. I'm actually not really sure what to make of the "xHCI 1.0: Force Stopped Event(FSE)" patch. Your patch description doesn't tell me why you need this, and it doesn't very clearly explain what you're trying to do. I think that it's going to effect the cancellation code, is that correct? If so, you need to give some details on how it will effect the cancellation code, because that code is fairly hairy. The patch also seems to be thrown together rather fast, if the number of spelling mistakes are any indication. Has it been throughly tested? Sarah Sharp -- 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
