On Thu, 4 Aug 2005, Alan Stern wrote:
> On Thu, 4 Aug 2005, Olav Kongas wrote:
>
> > Greg,
> >
> > This patch changes the isp116x-hcd to use interrupt instead
> > of root hub polling.
> >
> > Please apply, if Alan won't complain.
> > Olav
>
> There's one little problem with this snatch of code:
>
> > + /* Report no status change now, if we are scheduled to be
> > + called later */
> > + if (time_before(jiffies, hcd->rh_timer.expires))
> > + return 0;
>
> You shouldn't be checking against the expiration time; you should call
> timer_pending() instead. After all, the expiration time is meaningless if
> the timer isn't active.
Right, thanks. The attached patch uses timer_pending(),
superceding the original patch. Greg, please apply.
Olav
Switch isp116x-hcd over from root hub polling to interrupt.
This change closes also a race that was present with the old
polling scheme: status polling could happen in a
time window, where root hub status bits were not stable.
Signed-off-by: Olav Kongas <[EMAIL PROTECTED]>
--- linux-2.6.13-rc4-tmp3/drivers/usb/host/isp116x-hcd.c.5 2005-08-04
17:14:13.000000000 +0300
+++ linux-2.6.13-rc4-tmp3/drivers/usb/host/isp116x-hcd.c 2005-08-05
13:58:32.031805578 +0300
@@ -83,7 +83,7 @@
#include "../core/hcd.h"
#include "isp116x.h"
-#define DRIVER_VERSION "08 Apr 2005"
+#define DRIVER_VERSION "05 Aug 2005"
#define DRIVER_DESC "ISP116x USB Host Controller Driver"
MODULE_DESCRIPTION(DRIVER_DESC);
@@ -629,14 +629,12 @@ static irqreturn_t isp116x_irq(struct us
ERR("Unrecoverable error\n");
/* What should we do here? Reset? */
}
- if (intstat & HCINT_RHSC) {
- isp116x->rhstatus =
- isp116x_read_reg32(isp116x, HCRHSTATUS);
- isp116x->rhport[0] =
- isp116x_read_reg32(isp116x, HCRHPORT1);
- isp116x->rhport[1] =
- isp116x_read_reg32(isp116x, HCRHPORT2);
- }
+ if (intstat & HCINT_RHSC)
+ /* When root hub or any of its ports is going
+ to come out of suspend, it may take more
+ than 10ms for status bits to stabilize. */
+ mod_timer(&hcd->rh_timer, jiffies
+ + msecs_to_jiffies(20) + 1);
if (intstat & HCINT_RD) {
DBG("---- remote wakeup\n");
schedule_work(&isp116x->rh_resume);
@@ -925,20 +923,27 @@ static int isp116x_hub_status_data(struc
{
struct isp116x *isp116x = hcd_to_isp116x(hcd);
int ports, i, changed = 0;
+ unsigned long flags;
if (!HC_IS_RUNNING(hcd->state))
return -ESHUTDOWN;
- ports = isp116x->rhdesca & RH_A_NDP;
+ /* Report no status change now, if we are scheduled to be
+ called later */
+ if (timer_pending(&hcd->rh_timer))
+ return 0;
- /* init status */
+ ports = isp116x->rhdesca & RH_A_NDP;
+ spin_lock_irqsave(&isp116x->lock, flags);
+ isp116x->rhstatus = isp116x_read_reg32(isp116x, HCRHSTATUS);
if (isp116x->rhstatus & (RH_HS_LPSC | RH_HS_OCIC))
buf[0] = changed = 1;
else
buf[0] = 0;
for (i = 0; i < ports; i++) {
- u32 status = isp116x->rhport[i];
+ u32 status = isp116x->rhport[i] =
+ isp116x_read_reg32(isp116x, i ? HCRHPORT2 : HCRHPORT1);
if (status & (RH_PS_CSC | RH_PS_PESC | RH_PS_PSSC
| RH_PS_OCIC | RH_PS_PRSC)) {
@@ -947,6 +952,7 @@ static int isp116x_hub_status_data(struc
continue;
}
}
+ spin_unlock_irqrestore(&isp116x->lock, flags);
return changed;
}
@@ -1536,6 +1542,9 @@ static int isp116x_start(struct usb_hcd
return -ENODEV;
}
+ /* To be removed in future */
+ hcd->uses_new_polling = 1;
+
isp116x_write_reg16(isp116x, HCITLBUFLEN, ISP116x_ITL_BUFSIZE);
isp116x_write_reg16(isp116x, HCATLBUFLEN, ISP116x_ATL_BUFSIZE);
@@ -1639,7 +1648,7 @@ static int __init_or_module isp116x_remo
struct platform_device *pdev;
struct resource *res;
- if(!hcd)
+ if (!hcd)
return 0;
isp116x = hcd_to_isp116x(hcd);
pdev = container_of(dev, struct platform_device, dev);
