Commit c4f34764 authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman
Browse files

USB: EHCI: fix up locking 



This patch (as1588) adjusts the locking in ehci-hcd's various halt,
shutdown, and suspend/resume pathways.  We want to hold the spinlock
while writing device registers and accessing shared variables, but not
while polling in a loop.

In addition, there's no need to call ehci_work() at times when no URBs
can be active, i.e., in ehci_stop() and ehci_bus_suspend().

Finally, ehci_adjust_port_wakeup_flags() is called only in situations
where interrupts are enabled; therefore it can use spin_lock_irq
rather than spin_lock_irqsave.

Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent f4289078

drivers/usb/host/ehci-hcd.c

+33 −15
Original line number Diff line number Diff line
@@ -167,21 +167,24 @@ static int tdi_in_host_mode (struct ehci_hcd *ehci) 
	return (tmp & 3) == USBMODE_CM_HC;

}



/* force HC to halt state from unknown (EHCI spec section 2.3) */

/*

 * Force HC to halt state from unknown (EHCI spec section 2.3).

 * Must be called with interrupts enabled and the lock not held.

 */

static int ehci_halt (struct ehci_hcd *ehci)

{

	u32	temp = ehci_readl(ehci, &ehci->regs->status);

	u32	temp;



	spin_lock_irq(&ehci->lock);



	/* disable any irqs left enabled by previous code */

	ehci_writel(ehci, 0, &ehci->regs->intr_enable);



	if (ehci_is_TDI(ehci) && tdi_in_host_mode(ehci) == 0) {

	if (ehci_is_TDI(ehci) && !tdi_in_host_mode(ehci)) {

		spin_unlock_irq(&ehci->lock);

		return 0;

	}



	if ((temp & STS_HALT) != 0)

		return 0;



	/*

	 * This routine gets called during probe before ehci->command

	 * has been initialized, so we can't rely on its value.
@@ -190,6 +193,10 @@ static int ehci_halt (struct ehci_hcd *ehci) 
	temp = ehci_readl(ehci, &ehci->regs->command);

	temp &= ~(CMD_RUN | CMD_IAAD);

	ehci_writel(ehci, temp, &ehci->regs->command);



	spin_unlock_irq(&ehci->lock);

	synchronize_irq(ehci_to_hcd(ehci)->irq);



	return handshake(ehci, &ehci->regs->status,

			  STS_HALT, STS_HALT, 16 * 125);

}
@@ -210,7 +217,10 @@ static void tdi_reset (struct ehci_hcd *ehci) 
	ehci_writel(ehci, tmp, &ehci->regs->usbmode);

}



/* reset a non-running (STS_HALT == 1) controller */

/*

 * Reset a non-running (STS_HALT == 1) controller.

 * Must be called with interrupts enabled and the lock not held.

 */

static int ehci_reset (struct ehci_hcd *ehci)

{

	int	retval;
@@ -248,7 +258,10 @@ static int ehci_reset (struct ehci_hcd *ehci) 
	return retval;

}



/* idle the controller (from running) */

/*

 * Idle the controller (turn off the schedules).

 * Must be called with interrupts enabled and the lock not held.

 */

static void ehci_quiesce (struct ehci_hcd *ehci)

{

	u32	temp;
@@ -261,8 +274,10 @@ static void ehci_quiesce (struct ehci_hcd *ehci) 
	handshake(ehci, &ehci->regs->status, STS_ASS | STS_PSS, temp, 16 * 125);



	/* then disable anything that's still active */

	spin_lock_irq(&ehci->lock);

	ehci->command &= ~(CMD_ASE | CMD_PSE);

	ehci_writel(ehci, ehci->command, &ehci->regs->command);

	spin_unlock_irq(&ehci->lock);



	/* hardware can take 16 microframes to turn off ... */

	handshake(ehci, &ehci->regs->status, STS_ASS | STS_PSS, 0, 16 * 125);
@@ -301,11 +316,14 @@ static void ehci_turn_off_all_ports(struct ehci_hcd *ehci) 


/*

 * Halt HC, turn off all ports, and let the BIOS use the companion controllers.

 * Should be called with ehci->lock held.

 * Must be called with interrupts enabled and the lock not held.

 */

static void ehci_silence_controller(struct ehci_hcd *ehci)

{

	ehci_halt(ehci);



	spin_lock_irq(&ehci->lock);

	ehci->rh_state = EHCI_RH_HALTED;

	ehci_turn_off_all_ports(ehci);



	/* make BIOS/etc use companion controller during reboot */
@@ -313,6 +331,7 @@ static void ehci_silence_controller(struct ehci_hcd *ehci) 


	/* unblock posted writes */

	ehci_readl(ehci, &ehci->regs->configured_flag);

	spin_unlock_irq(&ehci->lock);

}



/* ehci_shutdown kick in for silicon on any bus (not just pci, etc).
@@ -325,10 +344,11 @@ static void ehci_shutdown(struct usb_hcd *hcd) 


	spin_lock_irq(&ehci->lock);

	ehci->rh_state = EHCI_RH_STOPPING;

	ehci_silence_controller(ehci);

	ehci->enabled_hrtimer_events = 0;

	spin_unlock_irq(&ehci->lock);



	ehci_silence_controller(ehci);



	hrtimer_cancel(&ehci->hrtimer);

}
@@ -400,11 +420,11 @@ static void ehci_stop (struct usb_hcd *hcd) 


	spin_lock_irq(&ehci->lock);

	ehci->enabled_hrtimer_events = 0;

	ehci_quiesce(ehci);

	spin_unlock_irq(&ehci->lock);



	ehci_quiesce(ehci);

	ehci_silence_controller(ehci);

	ehci_reset (ehci);

	spin_unlock_irq(&ehci->lock);



	hrtimer_cancel(&ehci->hrtimer);

	remove_sysfs_files(ehci);
@@ -412,8 +432,6 @@ static void ehci_stop (struct usb_hcd *hcd) 


	/* root hub is shut down separately (first, when possible) */

	spin_lock_irq (&ehci->lock);

	if (ehci->async)

		ehci_work (ehci);

	end_free_itds(ehci);

	spin_unlock_irq (&ehci->lock);

	ehci_mem_cleanup (ehci);

drivers/usb/host/ehci-hub.c

+29 −12
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@ static void ehci_handover_companion_ports(struct ehci_hcd *ehci) 
	/* Give the connections some time to appear */

	msleep(20);



	spin_lock_irq(&ehci->lock);

	port = HCS_N_PORTS(ehci->hcs_params);

	while (port--) {

		if (test_bit(port, &ehci->owned_ports)) {
@@ -70,23 +71,30 @@ static void ehci_handover_companion_ports(struct ehci_hcd *ehci) 
				clear_bit(port, &ehci->owned_ports);

			else if (test_bit(port, &ehci->companion_ports))

				ehci_writel(ehci, status & ~PORT_PE, reg);

			else

			else {

				spin_unlock_irq(&ehci->lock);

				ehci_hub_control(hcd, SetPortFeature,

						USB_PORT_FEAT_RESET, port + 1,

						NULL, 0);

				spin_lock_irq(&ehci->lock);

			}

		}

	}

	spin_unlock_irq(&ehci->lock);



	if (!ehci->owned_ports)

		return;

	msleep(90);		/* Wait for resets to complete */



	spin_lock_irq(&ehci->lock);

	port = HCS_N_PORTS(ehci->hcs_params);

	while (port--) {

		if (test_bit(port, &ehci->owned_ports)) {

			spin_unlock_irq(&ehci->lock);

			ehci_hub_control(hcd, GetPortStatus,

					0, port + 1,

					(char *) &buf, sizeof(buf));

			spin_lock_irq(&ehci->lock);



			/* The companion should now own the port,

			 * but if something went wrong the port must not
@@ -105,6 +113,7 @@ static void ehci_handover_companion_ports(struct ehci_hcd *ehci) 
	}



	ehci->owned_ports = 0;

	spin_unlock_irq(&ehci->lock);

}



static int ehci_port_change(struct ehci_hcd *ehci)
@@ -133,7 +142,6 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, 
{

	int		port;

	u32		temp;

	unsigned long	flags;



	/* If remote wakeup is enabled for the root hub but disabled

	 * for the controller, we must adjust all the port wakeup flags
@@ -143,7 +151,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, 
	if (!ehci_to_hcd(ehci)->self.root_hub->do_remote_wakeup || do_wakeup)

		return;



	spin_lock_irqsave(&ehci->lock, flags);

	spin_lock_irq(&ehci->lock);



	/* clear phy low-power mode before changing wakeup flags */

	if (ehci->has_hostpc) {
@@ -154,9 +162,9 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, 
			temp = ehci_readl(ehci, hostpc_reg);

			ehci_writel(ehci, temp & ~HOSTPC_PHCD, hostpc_reg);

		}

		spin_unlock_irqrestore(&ehci->lock, flags);

		spin_unlock_irq(&ehci->lock);

		msleep(5);

		spin_lock_irqsave(&ehci->lock, flags);

		spin_lock_irq(&ehci->lock);

	}



	port = HCS_N_PORTS(ehci->hcs_params);
@@ -194,7 +202,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, 
	if (!suspending && ehci_port_change(ehci))

		usb_hcd_resume_root_hub(ehci_to_hcd(ehci));



	spin_unlock_irqrestore(&ehci->lock, flags);

	spin_unlock_irq(&ehci->lock);

}



static int ehci_bus_suspend (struct usb_hcd *hcd)
@@ -209,6 +217,9 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) 
	if (time_before (jiffies, ehci->next_statechange))

		msleep(5);



	/* stop the schedules */

	ehci_quiesce(ehci);



	spin_lock_irq (&ehci->lock);



	/* Once the controller is stopped, port resumes that are already
@@ -224,10 +235,6 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) 
		}

	}



	/* stop schedules, clean any completed work */

	ehci_quiesce(ehci);

	ehci_work(ehci);



	/* Unlike other USB host controller types, EHCI doesn't have

	 * any notion of "global" or bus-wide suspend.  The driver has

	 * to manually suspend all the active unsuspended ports, and
@@ -289,6 +296,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) 
					"succeeded" : "failed");

		}

	}

	spin_unlock_irq(&ehci->lock);



	/* Apparently some devices need a >= 1-uframe delay here */

	if (ehci->bus_suspended)
@@ -296,6 +304,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) 


	/* turn off now-idle HC */

	ehci_halt (ehci);



	spin_lock_irq(&ehci->lock);

	ehci->rh_state = EHCI_RH_SUSPENDED;



	end_unlink_async(ehci);
@@ -424,13 +434,14 @@ static int ehci_bus_resume (struct usb_hcd *hcd) 
	}



	ehci->next_statechange = jiffies + msecs_to_jiffies(5);

	spin_unlock_irq(&ehci->lock);



	ehci_handover_companion_ports(ehci);



	/* Now we can safely re-enable irqs */

	ehci_writel(ehci, INTR_MASK, &ehci->regs->intr_enable);

	(void) ehci_readl(ehci, &ehci->regs->intr_enable);



	spin_unlock_irq (&ehci->lock);

	ehci_handover_companion_ports(ehci);

	return 0;

}
@@ -1018,7 +1029,9 @@ static int ehci_hub_control ( 
		case USB_PORT_FEAT_TEST:

			if (!selector || selector > 5)

				goto error;

			spin_unlock_irqrestore(&ehci->lock, flags);

			ehci_quiesce(ehci);

			spin_lock_irqsave(&ehci->lock, flags);



			/* Put all enabled ports into suspend */

			while (ports--) {
@@ -1030,7 +1043,11 @@ static int ehci_hub_control ( 
					ehci_writel(ehci, temp | PORT_SUSPEND,

							sreg);

			}



			spin_unlock_irqrestore(&ehci->lock, flags);

			ehci_halt(ehci);

			spin_lock_irqsave(&ehci->lock, flags);



			temp = ehci_readl(ehci, status_reg);

			temp |= selector << 16;

			ehci_writel(ehci, temp, status_reg);

drivers/usb/host/ehci-tegra.c

+2 −3
Original line number Diff line number Diff line
@@ -445,12 +445,11 @@ static int controller_suspend(struct device *dev) 
	if (time_before(jiffies, ehci->next_statechange))

		msleep(10);



	spin_lock_irqsave(&ehci->lock, flags);

	ehci_halt(ehci);



	spin_lock_irqsave(&ehci->lock, flags);

	tegra->port_speed = (readl(&hw->port_status[0]) >> 26) & 0x3;

	ehci_halt(ehci);

	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);



	spin_unlock_irqrestore(&ehci->lock, flags);



	tegra_ehci_power_down(hcd);

CRA Git | Maintained and supported by SUSTech CRA and CCSE