On Fri, Oct 11, 2002, Dan Streetman <[EMAIL PROTECTED]> wrote: > On Fri, 11 Oct 2002, Johannes Erdfelt wrote: > >On Fri, Oct 11, 2002, Dan Streetman <[EMAIL PROTECTED]> wrote: > >> -for places where a td->link was written, added check to make sure HC > >> hadn't already wrote that td->link back to the td's qh->element. > > > >This sounds racy. > > I think it actually fixes a race. There are only 2 places, in in > uhci_append_queued_urb and uhci_delete_queued_urb. My mental picture of > the race is this: > > QH1 -(element)-> TD -> <whatever...QH or TERM> > > So, let's say the HC is currently processing the TD. It finishes, reads > TD->link, and writes it back into QH->element. Then, TD->link is updated > by the HCD. But, TD is already out of the frame list! So whatever was > put there really should be in QH->element, and whatever's in QH->element > shouldn't be there. > > You can check the 2 places (which do essentially the same thing) in the > patch to see if you agree. They both have a comment "If the HC..."
Oh, that's not a problem. We'll be removing that finished URB (QH + TD's) very shortly and we'll put the new URB that just lost the race into the schedule. It'll get serviced. So there is a race, but it's a self fixing one. The HC could also retire the last TD in between setting td->link and updating qh->element as well. The HC caches the entire TD for the duration of the packet being sent to the device. That's a lot of CPU cycles. I'm just hesitant of reading/writing shared fields when we know that we don't own it exclusively. > >> -changed lowspeed control TDs from depth to breadth. > > > >Why for? This will guarantee that it will take (# of TD's * 1 ms) time > >to complete. > > Well, mainly to make the code common...but also, making it depth opens the > possibility (for a large transfer) of hogging the bus, and, will most > lowspeed devices really be able to send more than 1 packet per frame? It's control, it's allowed to hog the bus. I don't see the difference between transferring x packets in one frame versus 1 packet in x frames. I know that there are low speed devices that can send multiple packets per frame. The only disadvantage I can see is if a device has to NAK. We could use that packet to do something more useful. I don't think it's a problem because the most common usage of low speed control is enumeration time. I'd rather not slow that down anymore than it needs to be. > Anyway I can change it back, or Greg can update the patch. I can fix it. > >> -added FIXME to uhci_delete_queued_urb to indicate killing an active URB > >> may confuse the device, especially for control transfers, if some data has > >> already beed transferred. I'm pretty sure this assumption is correct...? > > > >Yes. The device will expect the rest of the DATA phase and the STATUS > >phase. > > > >Maybe the correct thing to do here is always do the STATUS phase and let > >the device bitch that it wasn't valid? > > Yeah, this on is tricky...probably some devices would stall the pipe if > the control (or other type) was aborted mid-transfer... It won't stall the control transfer we aborted, but it would stall the next one which might not be what people expect. Of course, if the driver needs to unlink a control URB that's not completed, it's usually because the device is constantly NAK'ing and the status phase may not work either. Maybe the change in direction (that is caused by the status phase) will cause it to fail a bit more gracefully? I don't really have a good idea on what to do. Seems kinda hopeless to me. JE ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
