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

Re: sleep works in 2.6.12-rc2 (was Re: applying patch)



On Sun, 2005-04-10 at 17:11 +1000, Cedric Pradalier wrote:
> >
> >It is not a good idea. If you unload the OHCI or EHCI module during
> >sleep, the controller will not be put to sleep _at_all_
> >
> >The USB standard specifies a specific "suspended" state that is using
> >during machine sleep. In this state, only a little bit of power is
> >provided to the bus, enough to let some USB devices to trigger a wakeup.
> >
> >The kernel drivers do suspend the root hubs.
> >
> 
> My experience, back with 2.6.4, is that with my usb mouse plugged and the usb modules
> loaded, I run out of power in a night (in sleep mode), without it I can stand a week.
> 
> I've not done any new experience with new kernel though

Interesting ... I can barely do 2 days without any USB device on mine :)

I suggest you try 2.6.12-rc2 with the patches posted recently by David
Brownell and Colin Leroy (attached). With those, I can also get the
latest laptop models to sleep/wakeup (provided they have an ATI chip of
course, that is not the 12" model)


This has a variety of updates to the shared suspend/resume code for
PCI based USB host controllers.

    - Cope with pm_message_t replacing the target system state.
      This is actually a loss of functionality; PCI D1 and D2
      states will no longer be used, and it's no longer knowable
      that D3cold is on the way so power will be lost.

    - Most importantly, some of the resume paths are reworked and
      cleaned up.  They're now an exact mirror of suspend paths,
      and more care is taken to ensure the hardware is reactivated
      before the hardware re-enables interrupts.

Plus comment and diagnostic cleanups; there are some nasty cases here 
especially combined with swsusp, now they're somewhat commented.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>

 25-akpm/drivers/usb/core/hcd-pci.c |  151 ++++++++++++++++++++++---------------
 1 files changed, 91 insertions(+), 60 deletions(-)

--- 25/drivers/usb/core/hcd-pci.c~usb-resume-fixes	Wed Mar 30 13:26:29 2005
+++ 25-akpm/drivers/usb/core/hcd-pci.c	Wed Mar 30 13:26:29 2005
@@ -33,7 +33,7 @@
 #include "hcd.h"
 
 
-/* PCI-based HCs are normal, but custom bus glue should be ok */
+/* PCI-based HCs are common, but plenty of non-PCI HCs are used too */
 
 
 /*-------------------------------------------------------------------------*/
@@ -67,8 +67,8 @@ int usb_hcd_pci_probe (struct pci_dev *d
 
 	if (pci_enable_device (dev) < 0)
 		return -ENODEV;
-	dev->current_state = 0;
-	dev->dev.power.power_state = 0;
+	dev->current_state = PCI_D0;
+	dev->dev.power.power_state = PMSG_ON;
 	
         if (!dev->irq) {
         	dev_err (&dev->dev,
@@ -186,26 +186,14 @@ EXPORT_SYMBOL (usb_hcd_pci_remove);
 
 #ifdef	CONFIG_PM
 
-static char __attribute_used__ *pci_state(u32 state)
-{
-	switch (state) {
-	case 0:		return "D0";
-	case 1:		return "D1";
-	case 2:		return "D2";
-	case 3:		return "D3hot";
-	case 4:		return "D3cold";
-	}
-	return NULL;
-}
-
 /**
  * usb_hcd_pci_suspend - power management suspend of a PCI-based HCD
  * @dev: USB Host Controller being suspended
- * @state: state that the controller is going into
+ * @message: semantics in flux
  *
  * Store this function in the HCD's struct pci_driver as suspend().
  */
-int usb_hcd_pci_suspend (struct pci_dev *dev, u32 state)
+int usb_hcd_pci_suspend (struct pci_dev *dev, pm_message_t message)
 {
 	struct usb_hcd		*hcd;
 	int			retval = 0;
@@ -213,13 +201,23 @@ int usb_hcd_pci_suspend (struct pci_dev 
 
 	hcd = pci_get_drvdata(dev);
 
+	/* FIXME until the generic PM interfaces change a lot more, this
+	 * can't use PCI D1 and D2 states.  For example, the confusion
+	 * between messages and states will need to vanish, and messages
+	 * will need to provide a target system state again.
+	 *
+	 * It'll be important to learn characteristics of the target state,
+	 * especially on embedded hardware where the HCD will often be in
+	 * charge of an external VBUS power supply and one or more clocks.
+	 * Some target system states will leave them active; others won't.
+	 * (With PCI, that's often handled by platform BIOS code.)
+	 */
+
 	/* even when the PCI layer rejects some of the PCI calls
 	 * below, HCs can try global suspend and reduce DMA traffic.
 	 * PM-sensitive HCDs may already have done this.
 	 */
 	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (state > 4)
-		state = 4;
 
 	switch (hcd->state) {
 
@@ -228,7 +226,7 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	 */
 	case HC_STATE_RUNNING:
 		hcd->state = HC_STATE_QUIESCING;
-		retval = hcd->driver->suspend (hcd, state);
+		retval = hcd->driver->suspend (hcd, message);
 		if (retval) {
 			dev_dbg (hcd->self.controller, 
 					"suspend fail, retval %d\n",
@@ -246,14 +244,11 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	 * have been called, otherwise root hub timers still run ...
 	 */
 	case HC_STATE_SUSPENDED:
-		if (state <= dev->current_state)
-			break;
-
-		/* no DMA or IRQs except in D0 */
-		if (!dev->current_state) {
+		/* no DMA or IRQs except when HC is active */
+		if (dev->current_state == PCI_D0) {
+			free_irq (hcd->irq, hcd);
 			pci_save_state (dev);
 			pci_disable_device (dev);
-			free_irq (hcd->irq, hcd);
 		}
 
 		if (!has_pci_pm) {
@@ -261,25 +256,19 @@ int usb_hcd_pci_suspend (struct pci_dev 
 			break;
 		}
 
-		/* POLICY: ignore D1/D2/D3hot differences;
-		 * we know D3hot will always work.
+		/* NOTE:  dev->current_state becomes nonzero only here, and
+		 * only for devices that support PCI PM.  Also, exiting
+		 * PCI_D3 (but not PCI_D1 or PCI_D2) is allowed to reset
+		 * some device state (e.g. as part of clock reinit).
 		 */
-		retval = pci_set_power_state (dev, state);
-		if (retval < 0 && state < 3) {
-			retval = pci_set_power_state (dev, 3);
-			if (retval == 0)
-				state = 3;
-		}
+		retval = pci_set_power_state (dev, PCI_D3hot);
 		if (retval == 0) {
-			dev_dbg (hcd->self.controller, "--> PCI %s\n",
-					pci_state(dev->current_state));
-#ifdef	CONFIG_USB_SUSPEND
-			pci_enable_wake (dev, state, hcd->remote_wakeup);
-			pci_enable_wake (dev, 4, hcd->remote_wakeup);
-#endif
+			dev_dbg (hcd->self.controller, "--> PCI D3\n");
+			pci_enable_wake (dev, PCI_D3hot, hcd->remote_wakeup);
+			pci_enable_wake (dev, PCI_D3cold, hcd->remote_wakeup);
 		} else if (retval < 0) {
-			dev_dbg (&dev->dev, "PCI %s suspend fail, %d\n",
-					pci_state(state), retval);
+			dev_dbg (&dev->dev, "PCI D3 suspend fail, %d\n",
+					retval);
 			(void) usb_hcd_pci_resume (dev);
 			break;
 		}
@@ -287,13 +276,14 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	default:
 		dev_dbg (hcd->self.controller, "hcd state %d; not suspended\n",
 			hcd->state);
+		WARN_ON(1);
 		retval = -EINVAL;
 		break;
 	}
 
 	/* update power_state **ONLY** to make sysfs happier */
 	if (retval == 0)
-		dev->dev.power.power_state = state;
+		dev->dev.power.power_state = message;
 	return retval;
 }
 EXPORT_SYMBOL (usb_hcd_pci_suspend);
@@ -308,7 +298,6 @@ int usb_hcd_pci_resume (struct pci_dev *
 {
 	struct usb_hcd		*hcd;
 	int			retval;
-	int			has_pci_pm;
 
 	hcd = pci_get_drvdata(dev);
 	if (hcd->state != HC_STATE_SUSPENDED) {
@@ -316,31 +305,73 @@ int usb_hcd_pci_resume (struct pci_dev *
 				"can't resume, not suspended!\n");
 		return 0;
 	}
-	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 
-	/* D3cold resume isn't usually reported this way... */
-	dev_dbg(hcd->self.controller, "resume from PCI %s%s\n",
-			pci_state(dev->current_state),
-			has_pci_pm ? "" : " (legacy)");
+	/* NOTE:  chip docs cover clean "real suspend" cases (what Linux
+	 * calls "standby", "suspend to RAM", and so on).  There are also
+	 * dirty cases when swsusp fakes a suspend in "shutdown" mode.
+	 */
+	if (dev->current_state != PCI_D0) {
+#ifdef	DEBUG
+		int	pci_pm;
+		u16	pmcr;
+
+		pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+		pci_read_config_word(dev, pci_pm + PCI_PM_CTRL, &pmcr);
+		pmcr &= PCI_PM_CTRL_STATE_MASK;
+		if (pmcr) {
+			/* Clean case:  power to USB and to HC registers was
+			 * maintained; remote wakeup is easy.
+			 */
+			dev_dbg(hcd->self.controller, "resume from PCI D%d\n",
+					pmcr);
+		} else {
+			/* Clean:  HC lost Vcc power, D0 uninitialized
+			 *   + Vaux may have preserved port and transceiver
+			 *     state ... for remote wakeup from D3cold
+			 *   + or not; HCD must reinit + re-enumerate
+			 *
+			 * Dirty: D0 semi-initialized cases with swsusp
+			 *   + after BIOS init
+			 *   + after Linux init (HCD statically linked)
+			 */
+			dev_dbg(hcd->self.controller,
+				"PCI D0, from previous PCI D%d\n",
+				dev->current_state);
+		}
+#endif
+		pci_enable_wake (dev, dev->current_state, 0);
+		pci_enable_wake (dev, PCI_D3cold, 0);
+	} else {
+		/* Same basic cases: clean (powered/not), dirty */
+		dev_dbg(hcd->self.controller, "PCI legacy resume\n");
+	}
 
-	hcd->state = HC_STATE_RESUMING;
+	/* NOTE:  the PCI API itself is asymmetric here.  We don't need to
+	 * pci_set_power_state(PCI_D0) since that's part of re-enabling;
+	 * but that won't re-enable bus mastering.  Yet pci_disable_device()
+	 * explicitly disables bus mastering...
+	 */
+	retval = pci_enable_device (dev);
+	if (retval < 0) {
+		dev_err (hcd->self.controller,
+			"can't re-enable after resume, %d!\n", retval);
+		return retval;
+	}
+	pci_set_master (dev);
+	pci_restore_state (dev);
 
-	if (has_pci_pm)
-		pci_set_power_state (dev, 0);
-	dev->dev.power.power_state = 0;
+	dev->dev.power.power_state = PMSG_ON;
+
+	hcd->state = HC_STATE_RESUMING;
+	hcd->saw_irq = 0;
 	retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
-				hcd->driver->description, hcd);
+				hcd->irq_descr, hcd);
 	if (retval < 0) {
 		dev_err (hcd->self.controller,
 			"can't restore IRQ after resume!\n");
+		usb_hc_died (hcd);
 		return retval;
 	}
-	hcd->saw_irq = 0;
-	pci_restore_state (dev);
-#ifdef	CONFIG_USB_SUSPEND
-	pci_enable_wake (dev, dev->current_state, 0);
-	pci_enable_wake (dev, 4, 0);
-#endif
 
 	retval = hcd->driver->resume (hcd);
 	if (!HC_IS_RUNNING (hcd->state)) {
_

This is the first of a few installments of PM API updates to match the
recent switch to "pm_message_t".  This installment primarily affects
USB device drivers (for USB interfaces), and it changes the handful of
drivers which currently implement suspend methods:

    - <linux/usb.h> and usbcore, signature change

    - Some drivers only changed the signature, net effect this just
      shuts up "sparse -Wbitwise":
	* hid-core
	* stir4200

    - Two network drivers did that, and also grew slightly more
      featureful suspend code ... they now properly shut down
      their activities.  (As should stir4200...)
	* pegasus
	* usbnet

Note that the Wake-On-Lan (WOL) support in pegasus doesn't yet work; looks
to me like it's missing a request to turn it on, vs just configuring it.
The ASIX code in usbnet also has WOL hooks that are ready to use; untested.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>

 25-akpm/drivers/net/irda/stir4200.c  |    4 ++--
 25-akpm/drivers/usb/core/hub.c       |    4 ++--
 25-akpm/drivers/usb/core/usb.c       |    6 +++---
 25-akpm/drivers/usb/input/hid-core.c |    6 +++---
 25-akpm/drivers/usb/net/pegasus.c    |   22 +++++++++++++++++++++-
 25-akpm/drivers/usb/net/usbnet.c     |   10 +++++++++-
 25-akpm/include/linux/usb.h          |    4 ++--
 7 files changed, 42 insertions(+), 14 deletions(-)

--- 25/drivers/net/irda/stir4200.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/net/irda/stir4200.c	Wed Mar 30 13:26:31 2005
@@ -1128,8 +1128,8 @@ static void stir_disconnect(struct usb_i
 }
 
 #ifdef CONFIG_PM
-/* Power management suspend, so power off the transmitter/receiver */
-static int stir_suspend(struct usb_interface *intf, u32 state)
+/* USB suspend, so power off the transmitter/receiver */
+static int stir_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct stir_cb *stir = usb_get_intfdata(intf);
 
--- 25/drivers/usb/core/hub.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/usb/core/hub.c	Wed Mar 30 13:26:31 2005
@@ -1731,7 +1731,7 @@ static int finish_port_resume(struct usb
 			struct usb_driver	*driver;
 
 			intf = udev->actconfig->interface[i];
-			if (intf->dev.power.power_state == PM_SUSPEND_ON)
+			if (intf->dev.power.power_state == PMSG_SUSPEND)
 				continue;
 			if (!intf->dev.driver) {
 				/* FIXME maybe force to alt 0 */
@@ -1745,7 +1745,7 @@ static int finish_port_resume(struct usb
 
 			/* can we do better than just logging errors? */
 			status = driver->resume(intf);
-			if (intf->dev.power.power_state != PM_SUSPEND_ON
+			if (intf->dev.power.power_state != PMSG_ON
 					|| status)
 				dev_dbg(&intf->dev,
 					"resume fail, state %d code %d\n",
--- 25/drivers/usb/core/usb.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/usb/core/usb.c	Wed Mar 30 13:26:31 2005
@@ -1387,13 +1387,13 @@ void usb_buffer_unmap_sg (struct usb_dev
 			usb_pipein (pipe) ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
 }
 
-static int usb_generic_suspend(struct device *dev, u32 state)
+static int usb_generic_suspend(struct device *dev, pm_message_t message)
 {
 	struct usb_interface *intf;
 	struct usb_driver *driver;
 
 	if (dev->driver == &usb_generic_driver)
-		return usb_suspend_device (to_usb_device(dev), state);
+		return usb_suspend_device (to_usb_device(dev), message);
 
 	if ((dev->driver == NULL) ||
 	    (dev->driver_data == &usb_generic_driver_data))
@@ -1407,7 +1407,7 @@ static int usb_generic_suspend(struct de
 		return 0;
 
 	if (driver->suspend)
-		return driver->suspend(intf, state);
+		return driver->suspend(intf, message);
 	return 0;
 }
 
--- 25/drivers/usb/input/hid-core.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/usb/input/hid-core.c	Wed Mar 30 13:26:31 2005
@@ -1801,12 +1801,12 @@ static int hid_probe(struct usb_interfac
 	return 0;
 }
 
-static int hid_suspend(struct usb_interface *intf, u32 state)
+static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct hid_device *hid = usb_get_intfdata (intf);
 
 	usb_kill_urb(hid->urbin);
-	intf->dev.power.power_state = state;
+	intf->dev.power.power_state = PMSG_SUSPEND;
 	dev_dbg(&intf->dev, "suspend\n");
 	return 0;
 }
@@ -1816,7 +1816,7 @@ static int hid_resume(struct usb_interfa
 	struct hid_device *hid = usb_get_intfdata (intf);
 	int status;
 
-	intf->dev.power.power_state = PM_SUSPEND_ON;
+	intf->dev.power.power_state = PMSG_ON;
 	if (hid->open)
 		status = usb_submit_urb(hid->urbin, GFP_NOIO);
 	else
--- 25/drivers/usb/net/pegasus.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/usb/net/pegasus.c	Wed Mar 30 13:26:31 2005
@@ -1364,11 +1364,18 @@ static void pegasus_disconnect(struct us
 	free_netdev(pegasus->net);
 }
 
-static int pegasus_suspend (struct usb_interface *intf, pm_message_t state)
+static int pegasus_suspend (struct usb_interface *intf, pm_message_t message)
 {
 	struct pegasus *pegasus = usb_get_intfdata(intf);
 	
 	netif_device_detach (pegasus->net);
+	if (netif_running(pegasus->net)) {
+		cancel_delayed_work(&pegasus->carrier_check);
+
+		usb_kill_urb(pegasus->rx_urb);
+		usb_kill_urb(pegasus->intr_urb);
+	}
+	intf->dev.power.power_state = PMSG_SUSPEND;
 	return 0;
 }
 
@@ -1376,7 +1383,20 @@ static int pegasus_resume (struct usb_in
 {
 	struct pegasus *pegasus = usb_get_intfdata(intf);
 
+	intf->dev.power.power_state = PMSG_ON;
 	netif_device_attach (pegasus->net);
+	if (netif_running(pegasus->net)) {
+		pegasus->rx_urb->status = 0;
+		pegasus->rx_urb->actual_length = 0;
+		read_bulk_callback(pegasus->rx_urb, 0);
+
+		pegasus->intr_urb->status = 0;
+		pegasus->intr_urb->actual_length = 0;
+		intr_callback(pegasus->intr_urb, 0);
+
+		queue_delayed_work(pegasus_workqueue, &pegasus->carrier_check,
+					CARRIER_CHECK_DELAY);
+	}
 	return 0;
 }
 
--- 25/drivers/usb/net/usbnet.c~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/drivers/usb/net/usbnet.c	Wed Mar 30 13:26:31 2005
@@ -3732,11 +3732,17 @@ out:
 
 #ifdef	CONFIG_PM
 
-static int usbnet_suspend (struct usb_interface *intf, u32 state)
+static int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet		*dev = usb_get_intfdata(intf);
 	
+	/* accelerate emptying of the rx and queues, to avoid
+	 * having everything error out.
+	 */
 	netif_device_detach (dev->net);
+	(void) unlink_urbs (dev, &dev->rxq);
+	(void) unlink_urbs (dev, &dev->txq);
+	intf->dev.power.power_state = PMSG_SUSPEND;
 	return 0;
 }
 
@@ -3744,7 +3750,9 @@ static int usbnet_resume (struct usb_int
 {
 	struct usbnet		*dev = usb_get_intfdata(intf);
 
+	intf->dev.power.power_state = PMSG_ON;
 	netif_device_attach (dev->net);
+	tasklet_schedule (&dev->bh);
 	return 0;
 }
 
--- 25/include/linux/usb.h~usb-suspend-updates-interface-suspend	Wed Mar 30 13:26:31 2005
+++ 25-akpm/include/linux/usb.h	Wed Mar 30 13:26:31 2005
@@ -557,7 +557,7 @@ struct usb_driver {
 
 	int (*ioctl) (struct usb_interface *intf, unsigned int code, void *buf);
 
-	int (*suspend) (struct usb_interface *intf, u32 state);
+	int (*suspend) (struct usb_interface *intf, pm_message_t message);
 	int (*resume) (struct usb_interface *intf);
 
 	const struct usb_device_id *id_table;
@@ -976,7 +976,7 @@ extern int usb_bulk_msg(struct usb_devic
 	int timeout);
 
 /* selective suspend/resume */
-extern int usb_suspend_device(struct usb_device *dev, u32 state);
+extern int usb_suspend_device(struct usb_device *dev, pm_message_t message);
 extern int usb_resume_device(struct usb_device *dev);
 
 
Miscellaneous updates for EHCI.

 - Mostly updates the power switching on EHCI controllers.  One routine
   centralizes the "power on/off all ports" logic, and the capability to
   do that is reported more correctly.

 - Courtesy Colin Leroy, a patch to always power up ports after resumes
   which didn't keep a USB device suspended.  The reset-everything logic
   powers down those ports (on some hardware) so something needs to turn
   them back on.

 - Minor tweaks/bugfixes for the debug port support.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

--- a/drivers/usb/host/ehci-hcd.c	2005-04-05 15:04:13 -07:00
+++ b/drivers/usb/host/ehci-hcd.c	2005-04-05 15:04:13 -07:00
@@ -346,6 +347,22 @@
 	return 0;
 }
 
+static void ehci_port_power (struct ehci_hcd *ehci, int is_on)
+{
+	unsigned port;
+
+	if (!HCS_PPC (ehci->hcs_params))
+		return;
+
+	ehci_dbg (ehci, "...power%s ports...\n", is_on ? "up" : "down");
+	for (port = HCS_N_PORTS (ehci->hcs_params); port > 0; )
+		(void) ehci_hub_control(ehci_to_hcd(ehci),
+				is_on ? SetPortFeature : ClearPortFeature,
+				USB_PORT_FEAT_POWER,
+				port--, NULL, 0);
+	msleep(20);
+}
+
 
 /* called by khubd or root hub init threads */
 
@@ -362,8 +379,10 @@
 	dbg_hcs_params (ehci, "reset");
 	dbg_hcc_params (ehci, "reset");
 
+	/* cache this readonly data; minimize chip reads */
+	ehci->hcs_params = readl (&ehci->caps->hcs_params);
+
 #ifdef	CONFIG_PCI
-	/* EHCI 0.96 and later may have "extended capabilities" */
 	if (hcd->self.controller->bus == &pci_bus_type) {
 		struct pci_dev	*pdev = to_pci_dev(hcd->self.controller);
 
@@ -383,9 +402,30 @@
 			break;
 		}
 
+		/* optional debug port, normally in the first BAR */
+		temp = pci_find_capability (pdev, 0x0a);
+		if (temp) {
+			pci_read_config_dword(pdev, temp, &temp);
+			temp >>= 16;
+			if ((temp & (3 << 13)) == (1 << 13)) {
+				temp &= 0x1fff;
+				ehci->debug = hcd->regs + temp;
+				temp = readl (&ehci->debug->control);
+				ehci_info (ehci, "debug port %d%s\n",
+					HCS_DEBUG_PORT(ehci->hcs_params),
+					(temp & DBGP_ENABLED)
+						? " IN USE"
+						: "");
+				if (!(temp & DBGP_ENABLED))
+					ehci->debug = NULL;
+			}
+		}
+
 		temp = HCC_EXT_CAPS (readl (&ehci->caps->hcc_params));
 	} else
 		temp = 0;
+
+	/* EHCI 0.96 and later may have "extended capabilities" */
 	while (temp && count--) {
 		u32		cap;
 
@@ -414,8 +454,7 @@
 		ehci_reset (ehci);
 #endif
 
-	/* cache this readonly data; minimize PCI reads */
-	ehci->hcs_params = readl (&ehci->caps->hcs_params);
+	ehci_port_power (ehci, 0);
 
 	/* at least the Genesys GL880S needs fixup here */
 	temp = HCS_N_CC(ehci->hcs_params) * HCS_N_PCC(ehci->hcs_params);
@@ -657,16 +696,11 @@
 static void ehci_stop (struct usb_hcd *hcd)
 {
 	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
-	u8			rh_ports, port;
 
 	ehci_dbg (ehci, "stop\n");
 
 	/* Turn off port power on all root hub ports. */
-	rh_ports = HCS_N_PORTS (ehci->hcs_params);
-	for (port = 1; port <= rh_ports; port++)
-		(void) ehci_hub_control(hcd,
-			ClearPortFeature, USB_PORT_FEAT_POWER,
-			port, NULL, 0);
+	ehci_port_power (ehci, 0);
 
 	/* no more interrupts ... */
 	del_timer_sync (&ehci->watchdog);
@@ -748,7 +782,6 @@
 	unsigned		port;
 	struct usb_device	*root = hcd->self.root_hub;
 	int			retval = -EINVAL;
-	int			powerup = 0;
 
 	// maybe restore (PCI) FLADJ
 
@@ -766,8 +799,6 @@
 			up (&hcd->self.root_hub->serialize);
 			break;
 		}
-		if ((status & PORT_POWER) == 0)
-			powerup = 1;
 		if (!root->children [port])
 			continue;
 		dbg_port (ehci, __FUNCTION__, port + 1, status);
@@ -794,16 +825,9 @@
 		retval = ehci_start (hcd);
 
 		/* here we "know" root ports should always stay powered;
-		 * but some controllers may lost all power.
+		 * but some controllers may lose all power.
 		 */
-		if (powerup) {
-			ehci_dbg (ehci, "...powerup ports...\n");
-			for (port = HCS_N_PORTS (ehci->hcs_params); port > 0; )
-				(void) ehci_hub_control(hcd,
-					SetPortFeature, USB_PORT_FEAT_POWER,
-						port--, NULL, 0);
-			msleep(20);
-		}
+		ehci_port_power (ehci, 1);
 	}
 
 	return retval;
--- a/drivers/usb/host/ehci-hub.c	2005-04-05 15:04:13 -07:00
+++ b/drivers/usb/host/ehci-hub.c	2005-04-05 15:04:13 -07:00
@@ -281,6 +281,8 @@
 	temp = 0x0008;			/* per-port overcurrent reporting */
 	if (HCS_PPC (ehci->hcs_params))
 		temp |= 0x0001;		/* per-port power control */
+	else
+		temp |= 0x0002;		/* no power switching */
 #if 0
 // re-enable when we support USB_PORT_FEAT_INDICATOR below.
 	if (HCS_INDICATOR (ehci->hcs_params))
--- a/drivers/usb/host/ehci.h	2005-04-05 15:04:13 -07:00
+++ b/drivers/usb/host/ehci.h	2005-04-05 15:04:13 -07:00
@@ -47,6 +47,12 @@
 #define	EHCI_MAX_ROOT_PORTS	15		/* see HCS_N_PORTS */
 
 struct ehci_hcd {			/* one per controller */
+	/* glue to PCI and HCD framework */
+	struct ehci_caps __iomem *caps;
+	struct ehci_regs __iomem *regs;
+	struct ehci_dbg_port __iomem *debug;
+
+	__u32			hcs_params;	/* cached register copy */
 	spinlock_t		lock;
 
 	/* async schedule support */
@@ -84,11 +90,6 @@
 
 	unsigned		is_tdi_rh_tt:1;	/* TDI roothub with TT */
 
-	/* glue to PCI and HCD framework */
-	struct ehci_caps __iomem *caps;
-	struct ehci_regs __iomem *regs;
-	__u32			hcs_params;	/* cached register copy */
-
 	/* irq statistics */
 #ifdef EHCI_STATS
 	struct ehci_stats	stats;
@@ -165,7 +166,7 @@
 	/* these fields are specified as 8 and 16 bit registers,
 	 * but some hosts can't perform 8 or 16 bit PCI accesses.
 	 */
-	u32	hc_capbase;
+	u32		hc_capbase;
 #define HC_LENGTH(p)		(((p)>>00)&0x00ff)	/* bits 7:0 */
 #define HC_VERSION(p)		(((p)>>16)&0xffff)	/* bits 31:16 */
 	u32		hcs_params;     /* HCSPARAMS - offset 0x4 */
@@ -273,7 +274,7 @@
 #define DBGP_ENABLED	(1<<28)
 #define DBGP_DONE	(1<<16)
 #define DBGP_INUSE	(1<<10)
-#define DBGP_ERRCODE(x)	(((x)>>7)&0x0f)
+#define DBGP_ERRCODE(x)	(((x)>>7)&0x07)
 #	define DBGP_ERR_BAD	1
 #	define DBGP_ERR_SIGNAL	2
 #define DBGP_ERROR	(1<<6)
@@ -282,11 +283,11 @@
 #define DBGP_LEN(x)	(((x)>>0)&0x0f)
 	u32	pids;
 #define DBGP_PID_GET(x)		(((x)>>16)&0xff)
-#define DBGP_PID_SET(data,tok)	(((data)<<8)|(tok));
+#define DBGP_PID_SET(data,tok)	(((data)<<8)|(tok))
 	u32	data03;
 	u32	data47;
 	u32	address;
-#define DBGP_EPADDR(dev,ep)	(((dev)<<8)|(ep));
+#define DBGP_EPADDR(dev,ep)	(((dev)<<8)|(ep))
 } __attribute__ ((packed));
 
 /*-------------------------------------------------------------------------*/

Reply to: