On Thu, 25 Apr 2013, ZhenHua wrote:
> >>> +#define UHCI_SUSPENDRH_RETRY_MAX 10
> >>> +#define UHCI_SUSPENDRH_RETRY_DELAY 100
> > Why is the delay set to 100 us? Isn't that excessively large? How
> > long does it take for this controller to go into suspend?
> This controller will take about 200~400 us, but I am not sure how long
> other devices will take.
> I set interval to 100 us, so it will save more time.
A 400-us delay is fairly long. It would be better to avoid it
entirely.
> > Why are these variables u16? Why not int?
> uhci_readw will return u16.
That's not a good reason, since u16 fits perfectly well inside an
int. But never mind...
> > Anyway, a better approach would be not to add a delay loop at all.
> > Instead, change this test:
> >
> > if (!auto_stop && !(uhci_readw(uhci, USBSTS) & USBSTS_HCH)) {
> > uhci->rh_state = UHCI_RH_SUSPENDING;
> > spin_unlock_irq(&uhci->lock);
> > msleep(1);
> > spin_lock_irq(&uhci->lock);
> > if (uhci->dead)
> > return;
> > }
> >
> > When the iLo controller is present, make the "if" statement always
> > succeed. Then you'll get a whole 1-ms delay.
> This will cause more operation and more time for other devices.
Actually what I wrote was wrong anyway. I forgot that when auto_stop
is set, the routine is not allowed to sleep.
A better way to solve your problem is to change uhci_hub_status_data().
In the UHCI_RH_RUNNING_NODEVS case, change the line that says
else if (time_after_eq(jiffies, uhci->auto_stop_time))
to
else if (time_after_eq(jiffies, uhci->auto_stop_time) &&
!uhci->no_auto_stops)
where uhci->no_auto_stops is a new bitflag that you set inside
uhci_pci_init() if you detect that the controller is an iLo virtual
UHCI controller.
This way there will always be a 1-ms delay, so the slow controller will
suspend successfully. And other types of host controllers won't be
affected, because the no_auto_stops flag won't get set for them.
Alan Stern
--
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
