Re: [PATCH v3 1/1] USB: EHCI: wait more than 3ms until the device enters full-speed idle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 11, 2014 at 10:48:39AM -0500, Alan Stern wrote:
> On Mon, 10 Feb 2014, Alan Stern wrote:
> 
> > On Fri, 7 Feb 2014, Peter Chen wrote:
> > 
> > > If the high-speed device does not enter full-speed idle after
> > > wakeup on disconnect logic has effected, there will be an
> > > unexpected disconnect wakeup interrupt due to the bus is still SE0.
> > > 
> > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> > > 
> > > Hi Alan,
> > > 
> > > If you can't met this issue, it may due to your hardware enables
> > > WKDN after detecting full-speed idle or the time between stop
> > > SoF and enable WKDN is more than 3ms (You need to know when
> > > the WKDN has really taken effect).
> > 
> > Yes.  However, I don't like the way your patch can increase the total
> > time if several ports have to be put into suspend.  Consider this patch 
> > instead.
> 
> I decided that patch was more complicated than it needed to be.  On 
> your system, a simple delay is all you need, right?  I assume this 
> means the wakeup signal isn't "sticky".  That is, once it has turned 
> on, it remains on only as long as the wakeup condition is present; it 
> doesn't stay on even after the wakeup condition goes away.

For my system, yes, only a 3-4ms delay to wait for full speed idle is ok.

> 
> In your case, the wakeup condition will be present while the bus uses
> high-speed terminations, but when the bus switches over to full-speed
> idle the wakeup condition will go away.  Thus, I assume the wakeup 
> signal will turn off after a few milliseconds, instead of remaining on 
> indefinitely.

At my case, the wakeup condition isn't existed before glue layer code
has run (put the phy enter low power mode).

wakeup condition = bus condition (SE0) + PHY condition (low power mode)

The wakeup condition should not be there, or the system can't enter
suspend, it is not we expect. If you don't think set 
suspendM and WKDN_E together will not met wakeup condition
at other systems, we can keep the patch like below, it can work
for my case.

Thanks.


> 
> Alan Stern
> 
> 
> 
> Index: usb-3.14/drivers/usb/host/ehci-hub.c
> ===================================================================
> --- usb-3.14.orig/drivers/usb/host/ehci-hub.c
> +++ usb-3.14/drivers/usb/host/ehci-hub.c
> @@ -238,6 +238,7 @@ static int ehci_bus_suspend (struct usb_
>  	int			port;
>  	int			mask;
>  	int			changed;
> +	bool			fs_idle_delay;
>  
>  	ehci_dbg(ehci, "suspend root hub\n");
>  
> @@ -272,6 +273,7 @@ static int ehci_bus_suspend (struct usb_
>  	ehci->bus_suspended = 0;
>  	ehci->owned_ports = 0;
>  	changed = 0;
> +	fs_idle_delay = false;
>  	port = HCS_N_PORTS(ehci->hcs_params);
>  	while (port--) {
>  		u32 __iomem	*reg = &ehci->regs->port_status [port];
> @@ -300,16 +302,32 @@ static int ehci_bus_suspend (struct usb_
>  		}
>  
>  		if (t1 != t2) {
> +			/*
> +			 * On some controllers, Wake-On-Disconnect will
> +			 * generate false wakeup signals until the bus
> +			 * switches over to full-speed idle.  For their
> +			 * sake, add a delay if we need one.
> +			 */
> +			if ((t2 & PORT_WKDISC_E) &&
> +					ehci_port_speed(ehci, t2) ==
> +						USB_PORT_STAT_HIGH_SPEED)
> +				fs_idle_delay = true;
>  			ehci_writel(ehci, t2, reg);
>  			changed = 1;
>  		}
>  	}
> +	spin_unlock_irq(&ehci->lock);
> +
> +	if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
> +		/*
> +		 * Wait for HCD to enter low-power mode or for the bus
> +		 * to switch to full-speed idle.
> +		 */
> +		usleep_range(5000, 5500);
> +	}
>  
>  	if (changed && ehci->has_tdi_phy_lpm) {
> -		spin_unlock_irq(&ehci->lock);
> -		msleep(5);	/* 5 ms for HCD to enter low-power mode */
>  		spin_lock_irq(&ehci->lock);
> -
>  		port = HCS_N_PORTS(ehci->hcs_params);
>  		while (port--) {
>  			u32 __iomem	*hostpc_reg = &ehci->regs->hostpc[port];
> @@ -322,8 +340,8 @@ static int ehci_bus_suspend (struct usb_
>  					port, (t3 & HOSTPC_PHCD) ?
>  					"succeeded" : "failed");
>  		}
> +		spin_unlock_irq(&ehci->lock);
>  	}
> -	spin_unlock_irq(&ehci->lock);
>  
>  	/* Apparently some devices need a >= 1-uframe delay here */
>  	if (ehci->bus_suspended)
> 
> 
> 

-- 

Best Regards,
Peter Chen

--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux