Re: [PATCH] ehci: remove old TT sched code

2013-10-17 Thread Dan Streetman
On Wed, Oct 16, 2013 at 5:09 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 16 Oct 2013, Dan Streetman wrote:

 On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern st...@rowland.harvard.edu 
 wrote:
  On Tue, 15 Oct 2013, Dan Streetman wrote:
 

  Until the new scheduler is fixed up to work properly, I prefer to
  keep both options available.

 Ok, no hurry.  With the work you've done on it before and the new
 stuff in usb-next it should be working very well soon :-)

 Dear me, no...  The changes I have made so far amount to perhaps 25% of
 the work needed to make it truly reliable.  It has so many bugs, I
 wouldn't even try to list them all.

Heh, well I guess I wrote it when I was young and coded sloppily ;-)
I'll review it all again and try to help fix the bugs.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ehci: remove old TT sched code

2013-10-17 Thread Alan Stern
On Thu, 17 Oct 2013, Dan Streetman wrote:

 On Wed, Oct 16, 2013 at 5:09 PM, Alan Stern st...@rowland.harvard.edu wrote:
  On Wed, 16 Oct 2013, Dan Streetman wrote:
 
  On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern st...@rowland.harvard.edu 
  wrote:
   On Tue, 15 Oct 2013, Dan Streetman wrote:
  
 
   Until the new scheduler is fixed up to work properly, I prefer to
   keep both options available.
 
  Ok, no hurry.  With the work you've done on it before and the new
  stuff in usb-next it should be working very well soon :-)
 
  Dear me, no...  The changes I have made so far amount to perhaps 25% of
  the work needed to make it truly reliable.  It has so many bugs, I
  wouldn't even try to list them all.
 
 Heh, well I guess I wrote it when I was young and coded sloppily ;-)
 I'll review it all again and try to help fix the bugs.

It's not a matter of sloppy coding but of design.  The main issues are:

We don't handle siTD back link pointers.  Therefore we can't
allow full-speed isochronous IN transfers to be scheduled in
B-uframe 4 or later.

We don't handle FSTN nodes in the schedule.  Therefore we can't
allow full- or low-speed interrupt transfers to be scheduled
in B-uframe 4 or later.  (In principle, that is -- in practice
we do allow uframes 4 and 5, at the cost of having too few
CSPLIT packets.)

We don't compute the full-speed budget according to the USB 
spec.  The spec says that budgets are to be computed in terms
of best-case full-speed bytes, not worst-case microseconds.

If an URB's bandwidth can't be accomodated in the current
schedule, we don't try to rebalance the schedule to make the
bandwidth fit.

We don't obey various restrictions on allowable schedules.
For example, the EHCI spec says that any full-speed isochronous
transfer of length = 588 bytes must be the first one in its
frame, but we don't enforce this.

We reserve an endpoint's bandwidth when the first URB is 
submitted rather than when the alternate setting is installed.

There's a bunch of smaller issues under each of those categories, but 
those are the biggies.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ehci: remove old TT sched code

2013-10-16 Thread Alan Stern
On Tue, 15 Oct 2013, Dan Streetman wrote:

 This removes the old EHCI transaction translator scheduling code,
 leaving only the new (now over 7 years old) scheduling code.
 
 ---
 
 I think it's been long enough to prove the new tt sched
 code works, and we can drop the old stuff.  Alan, Greg,
 and reason to keep the old tt sched code?
 
 This patch is against usb-next branch.

I'm not so sure it's a good idea to do this now.  I rather suspect 
there are people who still do use the old scheduler.

Until the new scheduler is fixed up to work properly, I prefer to
keep both options available.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ehci: remove old TT sched code

2013-10-16 Thread Dan Streetman
On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Tue, 15 Oct 2013, Dan Streetman wrote:

 This removes the old EHCI transaction translator scheduling code,
 leaving only the new (now over 7 years old) scheduling code.

 I'm not so sure it's a good idea to do this now.  I rather suspect
 there are people who still do use the old scheduler.

Really?  I thought the new scheduler works pretty well by now...it's
the default config in at least some, if not most, distros...

 Until the new scheduler is fixed up to work properly, I prefer to
 keep both options available.

Ok, no hurry.  With the work you've done on it before and the new
stuff in usb-next it should be working very well soon :-)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ehci: remove old TT sched code

2013-10-16 Thread Alan Stern
On Wed, 16 Oct 2013, Dan Streetman wrote:

 On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern st...@rowland.harvard.edu 
 wrote:
  On Tue, 15 Oct 2013, Dan Streetman wrote:
 
  This removes the old EHCI transaction translator scheduling code,
  leaving only the new (now over 7 years old) scheduling code.
 
  I'm not so sure it's a good idea to do this now.  I rather suspect
  there are people who still do use the old scheduler.
 
 Really?  I thought the new scheduler works pretty well by now...it's
 the default config in at least some, if not most, distros...

It would be more accurate to say that the new scheduler works sort of 
okay most of the time.  I would hope that most distros make it the 
default; it is more capable than the old scheduler.

Still, some people are very conservative.  And it's possible that some 
situations are handled better by the old scheduler than the new one 
(although I can't think of any offhand).

  Until the new scheduler is fixed up to work properly, I prefer to
  keep both options available.
 
 Ok, no hurry.  With the work you've done on it before and the new
 stuff in usb-next it should be working very well soon :-)

Dear me, no...  The changes I have made so far amount to perhaps 25% of
the work needed to make it truly reliable.  It has so many bugs, I
wouldn't even try to list them all.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ehci: remove old TT sched code

2013-10-15 Thread Dan Streetman
This removes the old EHCI transaction translator scheduling code,
leaving only the new (now over 7 years old) scheduling code.

---

I think it's been long enough to prove the new tt sched
code works, and we can drop the old stuff.  Alan, Greg,
and reason to keep the old tt sched code?

This patch is against usb-next branch.

(Sorry if anyone got multiples of this email, I had send-email
config problems that blocked delivery to the list)

 drivers/usb/host/Kconfig  |  20 
 drivers/usb/host/ehci-sched.c | 115 --
 2 files changed, 135 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 80e72fb..20cc435 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -64,26 +64,6 @@ config USB_EHCI_ROOT_HUB_TT
  This supports the EHCI implementation that's originally
  from ARC, and has since changed hands a few times.
 
-config USB_EHCI_TT_NEWSCHED
-   bool Improved Transaction Translator scheduling
-   depends on USB_EHCI_HCD
-   default y
-   ---help---
- This changes the periodic scheduling code to fill more of the low
- and full speed bandwidth available from the Transaction Translator
- (TT) in USB 2.0 hubs.  Without this, only one transfer will be
- issued in each microframe, significantly reducing the number of
- periodic low/fullspeed transfers possible.
-
- If you have multiple periodic low/fullspeed devices connected to a
- highspeed USB hub which is connected to a highspeed USB Host
- Controller, and some of those devices will not work correctly
- (possibly due to ENOSPC or -28 errors), say Y.  Conversely, if
- you have only one such device and it doesn't work, you could try
- saying N.
-
- If unsure, say Y.
-
 config USB_FSL_MPH_DR_OF
tristate
 
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 7ce5c2a..eaff175 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -318,8 +318,6 @@ static int __maybe_unused same_tt(struct usb_device *dev1,
return 1;
 }
 
-#ifdef CONFIG_USB_EHCI_TT_NEWSCHED
-
 /* Which uframe does the low/fullspeed transfer start in?
  *
  * The parameter is the mask of ssplits in H-frame terms
@@ -429,87 +427,6 @@ static int tt_available (
return 1;
 }
 
-#else
-
-/* return true iff the device's transaction translator is available
- * for a periodic transfer starting at the specified frame, using
- * all the uframes in the mask.
- */
-static int tt_no_collision (
-   struct ehci_hcd *ehci,
-   unsignedperiod,
-   struct usb_device   *dev,
-   unsignedframe,
-   u32 uf_mask
-)
-{
-   if (period == 0)/* error */
-   return 0;
-
-   /* note bandwidth wastage:  split never follows csplit
-* (different dev or endpoint) until the next uframe.
-* calling convention doesn't make that distinction.
-*/
-   for (; frame  ehci-periodic_size; frame += period) {
-   union ehci_shadow   here;
-   __hc32  type;
-   struct ehci_qh_hw   *hw;
-
-   here = ehci-pshadow [frame];
-   type = Q_NEXT_TYPE(ehci, ehci-periodic [frame]);
-   while (here.ptr) {
-   switch (hc32_to_cpu(ehci, type)) {
-   case Q_TYPE_ITD:
-   type = Q_NEXT_TYPE(ehci, here.itd-hw_next);
-   here = here.itd-itd_next;
-   continue;
-   case Q_TYPE_QH:
-   hw = here.qh-hw;
-   if (same_tt(dev, here.qh-ps.udev)) {
-   u32 mask;
-
-   mask = hc32_to_cpu(ehci,
-   hw-hw_info2);
-   /* knows no gap is needed */
-   mask |= mask  8;
-   if (mask  uf_mask)
-   break;
-   }
-   type = Q_NEXT_TYPE(ehci, hw-hw_next);
-   here = here.qh-qh_next;
-   continue;
-   case Q_TYPE_SITD:
-   if (same_tt (dev, here.sitd-urb-dev)) {
-   u16 mask;
-
-   mask = hc32_to_cpu(ehci, here.sitd
-   -hw_uframe);
-   /* FIXME assumes no gap for IN! */
-   mask |= mask  8;
-