Re: fix-u32-vs-pm_message_t-in-usb

Re: fix-u32-vs-pm_message_t-in-usb

[Pavel Machek]

Hi!

> This patch has been spitting warnings:
> 
> drivers/usb/host/uhci-hcd.c:838: warning: initialization from incompatible pointer type
> drivers/usb/host/ohci-pci.c:191: warning: initialization from incompatible pointer type
> 
> Because hc_driver.suspend() takes a u32 as its second arg.  Changing that
> to pci_power_t causes build failures and including pci.h in usb.h seems
> wrong.
> 
> Wanna fix it sometime?

Yep. Here it is.

This fixes remaining confusion. Part of my old patch was merged, I
later decided passing pci_power_t down to drivers is bad idea; this
passes them pm_message_t which contains more info (and actually works
:-). Please apply,

Signed-off-by: Pavel Machek <pavel@suse.cz>

								Pavel

--- clean-mm/drivers/usb/core/hcd-pci.c	2005-03-11 11:25:37.000000000 +0100
+++ linux-mm/drivers/usb/core/hcd-pci.c	2005-03-11 13:46:41.000000000 +0100
@@ -205,11 +205,12 @@
  *
  * 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 pmsg)
 {
 	struct usb_hcd		*hcd;
 	int			retval = 0;
 	int			has_pci_pm;
+	pci_power_t		state;
 
 	hcd = pci_get_drvdata(dev);
 
@@ -218,8 +219,10 @@
 	 * PM-sensitive HCDs may already have done this.
 	 */
 	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (state > 4)
-		state = 4;
+
+	state = pci_choose_state(dev, pmsg);
+	if (state > PCI_D3cold)
+		state = PCI_D3cold;
 
 	switch (hcd->state) {
 
@@ -228,7 +231,7 @@
 	 */
 	case USB_STATE_RUNNING:
 		hcd->state = USB_STATE_QUIESCING;
-		retval = hcd->driver->suspend (hcd, state);
+		retval = hcd->driver->suspend (hcd, pmsg);
 		if (retval) {
 			dev_dbg (hcd->self.controller, 
 					"suspend fail, retval %d\n",
@@ -291,9 +294,6 @@
 		break;
 	}
 
-	/* update power_state **ONLY** to make sysfs happier */
-	if (retval == 0)
-		dev->dev.power.power_state = state;
 	return retval;
 }
 EXPORT_SYMBOL (usb_hcd_pci_suspend);
--- clean-mm/drivers/usb/core/hcd.h	2005-03-11 11:25:37.000000000 +0100
+++ linux-mm/drivers/usb/core/hcd.h	2005-03-11 13:44:18.000000000 +0100
@@ -175,7 +175,7 @@
 	 * a whole, not just the root hub; they're for bus glue.
 	 */
 	/* called after all devices were suspended */
-	int	(*suspend) (struct usb_hcd *hcd, u32 state);
+	int	(*suspend) (struct usb_hcd *hcd, pm_message_t state);
 
 	/* called before any devices get resumed */
 	int	(*resume) (struct usb_hcd *hcd);
--- clean-mm/drivers/usb/host/ehci-hcd.c	2005-03-11 11:25:37.000000000 +0100
+++ linux-mm/drivers/usb/host/ehci-hcd.c	2005-03-11 13:44:18.000000000 +0100
@@ -721,7 +721,7 @@
  * the right sort of wakeup.  
  */
 
-static int ehci_suspend (struct usb_hcd *hcd, pci_power_t state)
+static int ehci_suspend (struct usb_hcd *hcd, pm_message_t state)
 {
 	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
 
--- clean-mm/drivers/usb/host/ohci-pci.c	2005-03-11 11:25:37.000000000 +0100
+++ linux-mm/drivers/usb/host/ohci-pci.c	2005-03-11 13:44:18.000000000 +0100
@@ -102,7 +102,7 @@
 
 #ifdef	CONFIG_PM
 
-static int ohci_pci_suspend (struct usb_hcd *hcd, pci_power_t state)
+static int ohci_pci_suspend (struct usb_hcd *hcd, pm_message_t state)
 {
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
 
--- clean-mm/drivers/usb/host/uhci-hcd.c	2005-03-11 11:25:37.000000000 +0100
+++ linux-mm/drivers/usb/host/uhci-hcd.c	2005-03-11 13:44:18.000000000 +0100
@@ -758,7 +758,7 @@
 }
 
 #ifdef CONFIG_PM
-static int uhci_suspend(struct usb_hcd *hcd, pci_power_t state)
+static int uhci_suspend(struct usb_hcd *hcd, pm_message_t state)
 {
 	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 
--- clean-mm/drivers/video/aty/radeon_pm.c	2005-03-11 11:25:38.000000000 +0100
+++ linux-mm/drivers/video/aty/radeon_pm.c	2005-03-11 13:56:52.000000000 +0100
@@ -2521,7 +2521,7 @@
 
 static/*extern*/ int susdisking = 0;
 
-int radeonfb_pci_suspend(struct pci_dev *pdev, u32 state)
+int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t state)
 {
         struct fb_info *info = pci_get_drvdata(pdev);
         struct radeonfb_info *rinfo = info->par;
--- clean-mm/include/linux/usb.h	2005-03-11 11:25:39.000000000 +0100
+++ linux-mm/include/linux/usb.h	2005-03-11 13:44:18.000000000 +0100
@@ -558,7 +558,7 @@
 
 	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 state);
 	int (*resume) (struct usb_interface *intf);
 
 	const struct usb_device_id *id_table;
@@ -977,7 +977,7 @@
 	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 state);
 extern int usb_resume_device(struct usb_device *dev);
 
 

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: fix-u32-vs-pm_message_t-in-usb

[Greg KH]

On Fri, Mar 11, 2005 at 02:08:31PM +0100, Pavel Machek wrote:
> Hi!
> 
> > This patch has been spitting warnings:
> > 
> > drivers/usb/host/uhci-hcd.c:838: warning: initialization from incompatible pointer type
> > drivers/usb/host/ohci-pci.c:191: warning: initialization from incompatible pointer type
> > 
> > Because hc_driver.suspend() takes a u32 as its second arg.  Changing that
> > to pci_power_t causes build failures and including pci.h in usb.h seems
> > wrong.
> > 
> > Wanna fix it sometime?
> 
> Yep. Here it is.
> 
> This fixes remaining confusion. Part of my old patch was merged, I
> later decided passing pci_power_t down to drivers is bad idea; this
> passes them pm_message_t which contains more info (and actually works
> :-). Please apply,

Argh, this one is already partially in, and another one you just sent me
has half of it in the tree too...

Care to just rediff off of 2.6.12-rc1?  Then we can hopefully get these
changes in :)

thanks,

greg k-h

Re: fix-u32-vs-pm_message_t-in-usb

[Pavel Machek]

On Pá 18-03-05 13:43:36, Greg KH wrote:
> On Fri, Mar 11, 2005 at 02:08:31PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > This patch has been spitting warnings:
> > > 
> > > drivers/usb/host/uhci-hcd.c:838: warning: initialization from incompatible pointer type
> > > drivers/usb/host/ohci-pci.c:191: warning: initialization from incompatible pointer type
> > > 
> > > Because hc_driver.suspend() takes a u32 as its second arg.  Changing that
> > > to pci_power_t causes build failures and including pci.h in usb.h seems
> > > wrong.
> > > 
> > > Wanna fix it sometime?
> > 
> > Yep. Here it is.
> > 
> > This fixes remaining confusion. Part of my old patch was merged, I
> > later decided passing pci_power_t down to drivers is bad idea; this
> > passes them pm_message_t which contains more info (and actually works
> > :-). Please apply,
> 
> Argh, this one is already partially in, and another one you just sent me
> has half of it in the tree too...
> 
> Care to just rediff off of 2.6.12-rc1?  Then we can hopefully get these
> changes in :)

I can do the rediff tommorow. I just hope there are not some other
changis waiting in -mm to spoil this ;-).
							Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: fix-u32-vs-pm_message_t-in-usb

[Andrew Morton]

Pavel Machek <pavel@suse.cz> wrote:
>
> > Care to just rediff off of 2.6.12-rc1?  Then we can hopefully get these
>  > changes in :)
> 
>  I can do the rediff tommorow. I just hope there are not some other
>  changis waiting in -mm to spoil this ;-).

I have a boatload of these darn pm_message_t patches floating about.  I
don't know if they depend on Greg's stuff or not.

Should I just hose them at him?

fix-pm_message_t-in-generic-code.patch
fix-u32-vs-pm_message_t-in-usb.patch
fix-u32-vs-pm_message_t-in-usb-fix.patch
more-pm_message_t-fixes.patch
fix-u32-vs-pm_message_t-confusion-in-oss.patch
fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch
fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch
fix-u32-vs-pm_message_t-confusion-in-mmc.patch
fix-u32-vs-pm_message_t-confusion-in-serials.patch
fix-u32-vs-pm_message_t-in-macintosh.patch
fix-u32-vs-pm_message_t-confusion-in-agp.patch

Re: fix-u32-vs-pm_message_t-in-usb

[Pavel Machek]

Hi!

> > > Care to just rediff off of 2.6.12-rc1?  Then we can hopefully get these
> >  > changes in :)
> > 
> >  I can do the rediff tommorow. I just hope there are not some other
> >  changis waiting in -mm to spoil this ;-).
> 
> I have a boatload of these darn pm_message_t patches floating about.  I
> don't know if they depend on Greg's stuff or not.
> 
> Should I just hose them at him?

I don't know what "hose" means in this context... Greg, can you take
those patches from Andrew? I think we managed to fix it right in his
tree.

								Pavel

> fix-pm_message_t-in-generic-code.patch
> fix-u32-vs-pm_message_t-in-usb.patch
> fix-u32-vs-pm_message_t-in-usb-fix.patch
> more-pm_message_t-fixes.patch
> fix-u32-vs-pm_message_t-confusion-in-oss.patch
> fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch
> fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch
> fix-u32-vs-pm_message_t-confusion-in-mmc.patch
> fix-u32-vs-pm_message_t-confusion-in-serials.patch
> fix-u32-vs-pm_message_t-in-macintosh.patch
> fix-u32-vs-pm_message_t-confusion-in-agp.patch
> 

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: fix-u32-vs-pm_message_t-in-usb

[Pavel Machek]

On Pá 18-03-05 15:53:13, Andrew Morton wrote:
> Pavel Machek <pavel@suse.cz> wrote:
> >
> > > Care to just rediff off of 2.6.12-rc1?  Then we can hopefully get these
> >  > changes in :)
> > 
> >  I can do the rediff tommorow. I just hope there are not some other
> >  changis waiting in -mm to spoil this ;-).
> 
> I have a boatload of these darn pm_message_t patches floating about.  I
> don't know if they depend on Greg's stuff or not.
> 
> Should I just hose them at him?

I see patches on their way, thanks!
								Pavel

> fix-pm_message_t-in-generic-code.patch
> fix-u32-vs-pm_message_t-in-usb.patch
> fix-u32-vs-pm_message_t-in-usb-fix.patch
> more-pm_message_t-fixes.patch
> fix-u32-vs-pm_message_t-confusion-in-oss.patch
> fix-u32-vs-pm_message_t-confusion-in-pcmcia.patch
> fix-u32-vs-pm_message_t-confusion-in-framebuffers.patch
> fix-u32-vs-pm_message_t-confusion-in-mmc.patch
> fix-u32-vs-pm_message_t-confusion-in-serials.patch
> fix-u32-vs-pm_message_t-in-macintosh.patch
> fix-u32-vs-pm_message_t-confusion-in-agp.patch
> 

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: fix-u32-vs-pm_message_t-in-usb

[Pavel Machek]

Hi!

> This patch has been spitting warnings:
> 
> drivers/usb/host/uhci-hcd.c:838: warning: initialization from incompatible pointer type
> drivers/usb/host/ohci-pci.c:191: warning: initialization from incompatible pointer type
> 
> Because hc_driver.suspend() takes a u32 as its second arg.  Changing that
> to pci_power_t causes build failures and including pci.h in usb.h seems
> wrong.
> 
> Wanna fix it sometime?

While fixing usb, I stomped on a few more fixes. Print error when some
device fails to power down, and 2 is no longer valid state to pass in
pm_message_t. Please apply,

									Pavel

--- clean-mm/drivers/base/power/suspend.c	2005-01-12 11:07:39.000000000 +0100
+++ linux-mm/drivers/base/power/suspend.c	2005-03-11 13:46:41.000000000 +0100
@@ -134,6 +134,8 @@
  Done:
 	return error;
  Error:
+	printk(KERN_ERR "Could not power down device %s: "
+		"error %d\n", kobject_name(&dev->kobj), error);
 	dpm_power_up();
 	goto Done;
 }
Only in linux-mm/drivers/char: consolemap_deftbl.c
--- clean-mm/drivers/ide/ide-disk.c	2005-03-11 11:25:34.000000000 +0100
+++ linux-mm/drivers/ide/ide-disk.c	2005-03-11 13:44:18.000000000 +0100
@@ -1096,7 +1096,7 @@
 	}
 
 	printk("Shutdown: %s\n", drive->name);
-	dev->bus->suspend(dev, PM_SUSPEND_STANDBY);
+	dev->bus->suspend(dev, PMSG_SUSPEND);
 }
 
 /*
--- clean-mm/drivers/pci/pci.c	2005-03-11 11:25:36.000000000 +0100
+++ linux-mm/drivers/pci/pci.c	2005-03-11 13:55:50.000000000 +0100
@@ -312,22 +312,24 @@
 /**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
- * @state: target sleep state for the whole system
+ * @state: target sleep state for the whole system. This is the value
+ *	that is passed to suspend() function.
  *
  * Returns PCI power state suitable for given device and given system
  * message.
  */
 
-pci_power_t pci_choose_state(struct pci_dev *dev, u32 state)
+pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
 {
 	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
 		return PCI_D0;
 
 	switch (state) {
-	case 0:	return PCI_D0;
-	case 2: return PCI_D2;
+	case 0: return PCI_D0;
 	case 3: return PCI_D3hot;
-	default: BUG();
+	default: 
+		printk("They asked me for state %d\n", state);
+		BUG();
 	}
 	return PCI_D0;
 }

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!