At 11:45 -0800 02-01-15, David Brownell wrote:
>  >     Tx interval
>>  is set to zero for one-shot mode (although it makes no difference if
>>  I set to the interval value from the endpoint).
>
>I think that "one shot interrupts" are a UHCI-ism.  Unlink the
>urb in the completion handler, if you need it to complete only
>one of its transfers.

OK.

>
>>  driver_write()
>>  {
>>    ...
>>     if (tx_urb->status == -EINPROGRESS)
>>     {
>>       wait(tx_sem); /*  hit by reception of -ECONNRESET in tx_completion */
>
>It's not a good idea to access fields of URBs after they've been
>submitted, until the completion handler fires.  Try reworking this
>semaphore so it acts as a mutex ... always grab it before writing,
>and release on an unlink completion.  And then make all other
>completions start your unlink, unless there's still data to transmit.

Done - I'm using down_trylock(write_mutex) to determine whether the 
URB is still 'HCD owned'.

>  >    }
>>     FILL_INT_URB(tx_urb, some data, tx_completion, etc.)
>>     usb_submit_urb(tgx_urb)
>>  ...
>>  }
>>
>>  tx_completion(urb)
>>  {
>>     if (urb->status == -ECONNRESET)
>>     {
>>      wake_up(tx_sem);
>>       return;
>>     }
>>
>>     if (more data to transmit)
>>     {
>>       FILL_INT_URB(tx_urb, more data, etc.); /* HCD will resubmit this */
>
>HCD still owns the urb, you shouldn't be modifying it in this
>handler.  Copy the new data into the existing buffer, up to
>the transfer length you already specified.

As it happens the only field it would change is 
transfer_buffer_length (all the other fields remain unchanged as it 
does reuse the same buffer), but I take the general point...

>  >    }
>>     else
>>     {
>>  #ifdef EVIL_HACK_NO_94
>>       urb->status = -EINPROGRESS; /* Bodge - see b) below */
>
>Yes, this is annoying but necessary in this case.  There's no
>real record that the URB is in the "owned by HCD but inside
>a completion handler" state.   Should get fixed in the 2.5 tree;
>and a fix might get backported.

Fair enough - would be nice if this was documented somewhere though :)

>  > #endif
>>       urb->transfer_flags |= USB_ASYNC_UNLINK;
>>       usb_unlink_urb(urb);
>>     }
>>  }
>>
>>  My main question is whether this approach is correct?
>
>Modulo some issues I mentioned above, I think so ... though I won't
>claim I could verify such a thing at this time.  (No test hardware to
>verify it with :)

Good - that's what I was hoping to hear.

>  > Whilst attempting to debug this on my system (Apple Titanium G4
>>  PowerBook - uses OHCI controller usb-ohci.c) I have observed the
>>  following interesting 'features' of the OHCI driver:
>>
>>  a) Unlike the documentation & usb-uhci.c, it only generates unlink
>>  completion events for asynchronous unlinks (-ECONNRESET) but not for
>>  synchronous unlinks (should be -ENOENT?).  This appears to be a
>>  straight inconsistency between the implementations and is easily
>>  fixed.
>
>Right, that shouldn't happen.  I see code that should return -ENOENT,
>but you're saying it doesn't kick in for you.

Really?  I can see several places that set the status to -ENOENT, but 
mostly the callback is invoked only in the case of -ECONNRESET (there 
are at least three such occurences - just search for -ECONNRESET or 
urb->complete in usb-ohci.c.  True for this file in both 2.4.18pre3 
tree and the 2.5.2 tree I've just rsynced!)

>  > c) Having worked around b) by setting urb->status to -EINPROGRESS
>>  before calling usb_unlink_urb, I never receive a completion with
>>  status -ECONNRESET.  After instrumenting usb-ohci.c I know that the
>>  unlink correctly marks the URB for unlinking and enables the start
>>  frame interrupt.  The start frame interrupt does occur and does
>>  invoke the routine dl_del_list() for processing of the unlink, but it
>>  does not invoke dl_del_urb() on my (being) unlinked URB. Having dealt
>>  with the list, the start frame interrupt is disabled as no further
>>  worked is deemed necessary.  At this point I am at the limit of my
>  > current knowledge....a few simple attempts at tweaking have generally
>>  resulted in a reboot being required :(
>>
>>  The observed behaviour is that my first transmission works, then my
>>  second transmission waits forever for the URB to become unlinked.
>
>Hmm, that shouldn't be happening.

Agreed :)  With the mutex the behaviour of my code alters slightly - 
after timing out the response from each transmission, the application 
attempts to re-send.  The first transmission works (and claims the 
mutex), but subsequent driver writes merely stuff the internal buffer 
and then fail to claim the mutex and so return, expecting the data to 
be transmitted when the completion handler finally invokes...which it 
never does!

I think the problem may be that an interrupt ED has only one TD 
chained off it and so head and tail pointers are the same (thus the 
initial URB freeing loop in dl_del_list never runs).  After some more 
instrumentation, I can see that the endpoint has been unlinked from 
the interrupt chains, it's just that the URB completion that has not 
been invoked.

>  > I am currently attempting to be able to try this on a UHCI based
>  > system for comparison, and reading the OHCI spec for enlightenment,
>  > but would be grateful for any suggestions (or patches for usb-ohci.c)
>>  to try.
>
>Can you use the 2.5.2 tree with your driver?  If so, I'd be interested
>to know whether the "ohci-hcd" driver (which I'll post soon as a patch)
>has the same failure modes for you.

I tried, but the PowerPC/PowerMac drivers (in particular IDE - rather 
crucial!) have not yet been brought up to date (still using request 
structures rather than new bio stuff).  I could probably do all the 
porting for this if I had enough time, but I don't so this will have 
to wait until Paul or Ben get the PowerPC/PowerMac stuff sorted in 
the 2.5 tree...

I have now borrowed an Intel laptop with USB & RedHat 6.2 installed 
- I'll try and build both 2.4.18 and 2.5.2 kernels with my driver on 
that system and see what happens - no idea what USB controller it has 
yet though!

Thanks for the feedback

Stu
-- 
The Janitorium <http://www.janitorium.co.uk/>


_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to