Michal Sojka wrote:
Hi,

I tried to rewrite USB 2.0 isochronous transfer support and here is the patch against 2.6.0-test7.

Cool! That's needed some attention, I'll have a look at this. And I've really been hoping someone would help out in this area.


I tested this code with our measuring device and I was able to do high-speed high-bandwidth transfer (IN transactions) continuously for very long time. In my opinion there are still some things to do on this code, but because I'm quite new to Linux kernel programming I would like to hear some comments.

It already looks better than what was there before ... :)


You did the right thing by adding new data structures to handle
ISO.  Not that I've had a chance yet to look at what you added;
but it clearly needed that kind of attention.

Did you think much about how to make the split ISO behave?
I've basically thought that, other than TT scheduling, it
ought to be shaped roughly like high speed ISO -- better
data structures and all.


There are some questions included in the source and I don't know the answers. These are mainly:

  1. How to claim and disclaim bandwidth pro iso transfer? Should I
     call usb_claimb_bandwidth for each urb?

Bandwidth must be allocated per-endpoint. URBs are irrelevant, except that if the endpoint has no URBs queued, there's no point in reserving the endpoint's bandwidth any longer. There was already code to do that -- though like all the ISO code, I've counted it as questionable.


  2. Performance of scan_periodic. I thing that the loop in
     scan_periodic may block interrupts for long time and it is better
     don't restart the loop when micro frame counter changes during
     processing the periodic list.

It shouldn't _need_ to be a "long time"; as a rule there shouldn't be more than a handful of entries that completed since the last IRQ. And only entries that completed should have any work involved.

I seem to remember adding the restart logic to make sure the scan
behaved right, since entries could be removed from (or added to!)
the periodic schedule whenever the driver lock was dropped.

On the other hand it could sometimes be correct to wait for the next
completion IRQ, and scan then.


  3. Reentrancy of scan_periodic. When I started studying of ehci isoc
     code, I thought that scan_periodic should be reentrant because of
     dropping of ehci lock when giving urb back to the driver. Original
     version wasn’t reentrant, because of variable ehci->next_uframe
     which wasn’t updated before returning urb. When I added updating
     of this variable it behaves better. After this I started with
     deeper rewriting of code and now it seems, that is not necessary
     to update this variable.

OK, I'll see how that looks. That was related to the rescan logic.



Again, thanks.


- Dave



Regards


Michal Sojka






-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to