[PATCH 0/2] PM / PCMCIA: Fix CardBus suspend/resume regression

[PATCH 0/2] PM / PCMCIA: Fix CardBus suspend/resume regression

[Rafael J. Wysocki]

Hi,

The following two patches fix a regression causing CardBus resume to fail,
introduced during the 2.6.29 development cycle.

[1/2] Drops the second argument of pcmcia_socket_dev_suspend(), which is not
      used.

[2/2] Fixes the regression by moving the yenta suspend/resume to the late phase
      of suspend and early phase of resume, respectively.

If there are no objections, I'll push these patches through the suspend-2.6
tree early next week.

Thanks,
Rafael

[PATCH 1/2] PM / PCMCIA: Drop second argument of pcmcia_socket_dev_suspend()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

pcmcia_socket_dev_suspend() doesn't use its second argument, so it
may be dropped safely.

This change is necessary for the subsequent yenta suspend/resume fix.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pcmcia/at91_cf.c        |    2 +-
 drivers/pcmcia/au1000_generic.c |    2 +-
 drivers/pcmcia/bfin_cf_pcmcia.c |    2 +-
 drivers/pcmcia/cs.c             |    2 +-
 drivers/pcmcia/i82092.c         |    2 +-
 drivers/pcmcia/i82365.c         |    2 +-
 drivers/pcmcia/m32r_cfc.c       |    2 +-
 drivers/pcmcia/m32r_pcc.c       |    2 +-
 drivers/pcmcia/m8xx_pcmcia.c    |    2 +-
 drivers/pcmcia/omap_cf.c        |    2 +-
 drivers/pcmcia/pd6729.c         |    2 +-
 drivers/pcmcia/pxa2xx_base.c    |    2 +-
 drivers/pcmcia/sa1100_generic.c |    2 +-
 drivers/pcmcia/sa1111_generic.c |    2 +-
 drivers/pcmcia/tcic.c           |    2 +-
 drivers/pcmcia/vrc4171_card.c   |    2 +-
 include/pcmcia/ss.h             |    2 +-
 17 files changed, 17 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/pcmcia/at91_cf.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/at91_cf.c
+++ linux-2.6/drivers/pcmcia/at91_cf.c
@@ -363,7 +363,7 @@ static int at91_cf_suspend(struct platfo
 	struct at91_cf_socket	*cf = platform_get_drvdata(pdev);
 	struct at91_cf_data	*board = cf->board;
 
-	pcmcia_socket_dev_suspend(&pdev->dev, mesg);
+	pcmcia_socket_dev_suspend(&pdev->dev);
 	if (device_may_wakeup(&pdev->dev)) {
 		enable_irq_wake(board->det_pin);
 		if (board->irq_pin)
Index: linux-2.6/drivers/pcmcia/au1000_generic.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/au1000_generic.c
+++ linux-2.6/drivers/pcmcia/au1000_generic.c
@@ -515,7 +515,7 @@ static int au1x00_drv_pcmcia_probe(struc
 static int au1x00_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int au1x00_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/bfin_cf_pcmcia.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/bfin_cf_pcmcia.c
+++ linux-2.6/drivers/pcmcia/bfin_cf_pcmcia.c
@@ -302,7 +302,7 @@ static int __devexit bfin_cf_remove(stru
 
 static int bfin_cf_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
-	return pcmcia_socket_dev_suspend(&pdev->dev, mesg);
+	return pcmcia_socket_dev_suspend(&pdev->dev);
 }
 
 static int bfin_cf_resume(struct platform_device *pdev)
Index: linux-2.6/drivers/pcmcia/cs.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/cs.c
+++ linux-2.6/drivers/pcmcia/cs.c
@@ -101,7 +101,7 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
 static int socket_resume(struct pcmcia_socket *skt);
 static int socket_suspend(struct pcmcia_socket *skt);
 
-int pcmcia_socket_dev_suspend(struct device *dev, pm_message_t state)
+int pcmcia_socket_dev_suspend(struct device *dev)
 {
 	struct pcmcia_socket *socket;
 
Index: linux-2.6/drivers/pcmcia/i82092.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/i82092.c
+++ linux-2.6/drivers/pcmcia/i82092.c
@@ -42,7 +42,7 @@ MODULE_DEVICE_TABLE(pci, i82092aa_pci_id
 #ifdef CONFIG_PM
 static int i82092aa_socket_suspend (struct pci_dev *dev, pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int i82092aa_socket_resume (struct pci_dev *dev)
Index: linux-2.6/include/pcmcia/ss.h
===================================================================
--- linux-2.6.orig/include/pcmcia/ss.h
+++ linux-2.6/include/pcmcia/ss.h
@@ -279,7 +279,7 @@ extern struct pccard_resource_ops pccard
 extern struct pccard_resource_ops pccard_nonstatic_ops;
 
 /* socket drivers are expected to use these callbacks in their .drv struct */
-extern int pcmcia_socket_dev_suspend(struct device *dev, pm_message_t state);
+extern int pcmcia_socket_dev_suspend(struct device *dev);
 extern int pcmcia_socket_dev_resume(struct device *dev);
 
 /* socket drivers use this callback in their IRQ handler */
Index: linux-2.6/drivers/pcmcia/i82365.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/i82365.c
+++ linux-2.6/drivers/pcmcia/i82365.c
@@ -1241,7 +1241,7 @@ static int pcic_init(struct pcmcia_socke
 static int i82365_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int i82365_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/m32r_cfc.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/m32r_cfc.c
+++ linux-2.6/drivers/pcmcia/m32r_cfc.c
@@ -699,7 +699,7 @@ static struct pccard_operations pcc_oper
 static int cfc_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int cfc_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/m32r_pcc.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/m32r_pcc.c
+++ linux-2.6/drivers/pcmcia/m32r_pcc.c
@@ -675,7 +675,7 @@ static struct pccard_operations pcc_oper
 static int pcc_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int pcc_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/m8xx_pcmcia.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/m8xx_pcmcia.c
+++ linux-2.6/drivers/pcmcia/m8xx_pcmcia.c
@@ -1296,7 +1296,7 @@ static int m8xx_remove(struct of_device 
 #ifdef CONFIG_PM
 static int m8xx_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&pdev->dev, state);
+	return pcmcia_socket_dev_suspend(&pdev->dev);
 }
 
 static int m8xx_resume(struct platform_device *pdev)
Index: linux-2.6/drivers/pcmcia/omap_cf.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/omap_cf.c
+++ linux-2.6/drivers/pcmcia/omap_cf.c
@@ -334,7 +334,7 @@ static int __exit omap_cf_remove(struct 
 
 static int omap_cf_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
-	return pcmcia_socket_dev_suspend(&pdev->dev, mesg);
+	return pcmcia_socket_dev_suspend(&pdev->dev);
 }
 
 static int omap_cf_resume(struct platform_device *pdev)
Index: linux-2.6/drivers/pcmcia/pd6729.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/pd6729.c
+++ linux-2.6/drivers/pcmcia/pd6729.c
@@ -758,7 +758,7 @@ static void __devexit pd6729_pci_remove(
 #ifdef CONFIG_PM
 static int pd6729_socket_suspend(struct pci_dev *dev, pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int pd6729_socket_resume(struct pci_dev *dev)
Index: linux-2.6/drivers/pcmcia/pxa2xx_base.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/pxa2xx_base.c
+++ linux-2.6/drivers/pcmcia/pxa2xx_base.c
@@ -302,7 +302,7 @@ static int pxa2xx_drv_pcmcia_remove(stru
 
 static int pxa2xx_drv_pcmcia_suspend(struct device *dev)
 {
-	return pcmcia_socket_dev_suspend(dev, PMSG_SUSPEND);
+	return pcmcia_socket_dev_suspend(dev);
 }
 
 static int pxa2xx_drv_pcmcia_resume(struct device *dev)
Index: linux-2.6/drivers/pcmcia/sa1100_generic.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/sa1100_generic.c
+++ linux-2.6/drivers/pcmcia/sa1100_generic.c
@@ -89,7 +89,7 @@ static int sa11x0_drv_pcmcia_remove(stru
 static int sa11x0_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int sa11x0_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/sa1111_generic.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/sa1111_generic.c
+++ linux-2.6/drivers/pcmcia/sa1111_generic.c
@@ -159,7 +159,7 @@ static int __devexit pcmcia_remove(struc
 
 static int pcmcia_suspend(struct sa1111_dev *dev, pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int pcmcia_resume(struct sa1111_dev *dev)
Index: linux-2.6/drivers/pcmcia/tcic.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/tcic.c
+++ linux-2.6/drivers/pcmcia/tcic.c
@@ -366,7 +366,7 @@ static int __init get_tcic_id(void)
 static int tcic_drv_pcmcia_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int tcic_drv_pcmcia_resume(struct platform_device *dev)
Index: linux-2.6/drivers/pcmcia/vrc4171_card.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/vrc4171_card.c
+++ linux-2.6/drivers/pcmcia/vrc4171_card.c
@@ -707,7 +707,7 @@ __setup("vrc4171_card=", vrc4171_card_se
 static int vrc4171_card_suspend(struct platform_device *dev,
 				     pm_message_t state)
 {
-	return pcmcia_socket_dev_suspend(&dev->dev, state);
+	return pcmcia_socket_dev_suspend(&dev->dev);
 }
 
 static int vrc4171_card_resume(struct platform_device *dev)

[PATCH 2/2] PM / yenta: Fix cardbus suspend/resume regression

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since 2.6.29 the PCI PM core have been restoring the standard
configuration registers of PCI devices in the early phase of
resume.  In particular, PCI devices without drivers have been handled
this way since commit 355a72d75b3b4f4877db4c9070c798238028ecb5
(PCI: Rework default handling of suspend and resume).  Unfortunately,
this leads to post-resume problems with CardBus devices which cannot
be accessed in the early phase of resume, because the sockets they
are on have not been woken up yet at that point.

To solve this problem, move the yenta socket resume to the early
phase of resume and, analogously, move the suspend of it to the late
phase of suspend.  Additionally, remove some unnecessary PCI code
from the yenta socket's resume routine.

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13092, which is a
post-2.6.28 regression.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reported-by: Florian <fs-kernelbugzilla@spline.de>
---
 drivers/pcmcia/yenta_socket.c |   92 ++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 42 deletions(-)

Index: linux-2.6-stable/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6-stable.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6-stable/drivers/pcmcia/yenta_socket.c
@@ -1225,60 +1225,71 @@ static int __devinit yenta_probe (struct
 }
 
 #ifdef CONFIG_PM
-static int yenta_dev_suspend (struct pci_dev *dev, pm_message_t state)
+static int yenta_dev_suspend_noirq(struct device *dev)
 {
-	struct yenta_socket *socket = pci_get_drvdata(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct yenta_socket *socket = pci_get_drvdata(pdev);
 	int ret;
 
-	ret = pcmcia_socket_dev_suspend(&dev->dev, state);
+	ret = pcmcia_socket_dev_suspend(dev);
 
-	if (socket) {
-		if (socket->type && socket->type->save_state)
-			socket->type->save_state(socket);
-
-		/* FIXME: pci_save_state needs to have a better interface */
-		pci_save_state(dev);
-		pci_read_config_dword(dev, 16*4, &socket->saved_state[0]);
-		pci_read_config_dword(dev, 17*4, &socket->saved_state[1]);
-		pci_disable_device(dev);
-
-		/*
-		 * Some laptops (IBM T22) do not like us putting the Cardbus
-		 * bridge into D3.  At a guess, some other laptop will
-		 * probably require this, so leave it commented out for now.
-		 */
-		/* pci_set_power_state(dev, 3); */
-	}
+	if (!socket)
+		return ret;
+
+	if (socket->type && socket->type->save_state)
+		socket->type->save_state(socket);
+
+	pci_save_state(pdev);
+	pci_read_config_dword(pdev, 16*4, &socket->saved_state[0]);
+	pci_read_config_dword(pdev, 17*4, &socket->saved_state[1]);
+	pci_disable_device(pdev);
+
+	/*
+	 * Some laptops (IBM T22) do not like us putting the Cardbus
+	 * bridge into D3.  At a guess, some other laptop will
+	 * probably require this, so leave it commented out for now.
+	 */
+	/* pci_set_power_state(dev, 3); */
 
 	return ret;
 }
 
-
-static int yenta_dev_resume (struct pci_dev *dev)
+static int yenta_dev_resume_noirq(struct device *dev)
 {
-	struct yenta_socket *socket = pci_get_drvdata(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct yenta_socket *socket = pci_get_drvdata(pdev);
+	int ret;
 
-	if (socket) {
-		int rc;
+	if (!socket)
+		return 0;
 
-		pci_set_power_state(dev, 0);
-		/* FIXME: pci_restore_state needs to have a better interface */
-		pci_restore_state(dev);
-		pci_write_config_dword(dev, 16*4, socket->saved_state[0]);
-		pci_write_config_dword(dev, 17*4, socket->saved_state[1]);
+	pci_write_config_dword(pdev, 16*4, socket->saved_state[0]);
+	pci_write_config_dword(pdev, 17*4, socket->saved_state[1]);
 
-		rc = pci_enable_device(dev);
-		if (rc)
-			return rc;
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
 
-		pci_set_master(dev);
+	pci_set_master(pdev);
 
-		if (socket->type && socket->type->restore_state)
-			socket->type->restore_state(socket);
-	}
+	if (socket->type && socket->type->restore_state)
+		socket->type->restore_state(socket);
 
-	return pcmcia_socket_dev_resume(&dev->dev);
+	return pcmcia_socket_dev_resume(dev);
 }
+
+static struct dev_pm_ops yenta_pm_ops = {
+	.suspend_noirq = yenta_dev_suspend_noirq,
+	.resume_noirq = yenta_dev_resume_noirq,
+	.freeze_noirq = yenta_dev_suspend_noirq,
+	.thaw_noirq = yenta_dev_resume_noirq,
+	.poweroff_noirq = yenta_dev_suspend_noirq,
+	.restore_noirq = yenta_dev_resume_noirq,
+};
+
+#define YENTA_PM_OPS	(&yenta_pm_ops)
+#else
+#define YENTA_PM_OPS	NULL
 #endif
 
 #define CB_ID(vend,dev,type)				\
@@ -1376,10 +1387,7 @@ static struct pci_driver yenta_cardbus_d
 	.id_table	= yenta_table,
 	.probe		= yenta_probe,
 	.remove		= __devexit_p(yenta_close),
-#ifdef CONFIG_PM
-	.suspend	= yenta_dev_suspend,
-	.resume		= yenta_dev_resume,
-#endif
+	.driver.pm	= YENTA_PM_OPS,
 };
 
 

Re: [PATCH 1/2] PM / PCMCIA: Drop second argument of pcmcia_socket_dev_suspend()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On Sat, Sep 26, 2009 at 01:25:11AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> pcmcia_socket_dev_suspend() doesn't use its second argument, so it
> may be dropped safely.
> 
> This change is necessary for the subsequent yenta suspend/resume fix.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/pcmcia/at91_cf.c        |    2 +-
>  drivers/pcmcia/au1000_generic.c |    2 +-
>  drivers/pcmcia/bfin_cf_pcmcia.c |    2 +-
>  drivers/pcmcia/cs.c             |    2 +-
>  drivers/pcmcia/i82092.c         |    2 +-
>  drivers/pcmcia/i82365.c         |    2 +-
>  drivers/pcmcia/m32r_cfc.c       |    2 +-
>  drivers/pcmcia/m32r_pcc.c       |    2 +-
>  drivers/pcmcia/m8xx_pcmcia.c    |    2 +-
>  drivers/pcmcia/omap_cf.c        |    2 +-
>  drivers/pcmcia/pd6729.c         |    2 +-
>  drivers/pcmcia/pxa2xx_base.c    |    2 +-
>  drivers/pcmcia/sa1100_generic.c |    2 +-
>  drivers/pcmcia/sa1111_generic.c |    2 +-
>  drivers/pcmcia/tcic.c           |    2 +-
>  drivers/pcmcia/vrc4171_card.c   |    2 +-
>  include/pcmcia/ss.h             |    2 +-

Hmm, you are not converting yenta_socket here. Won't that break building the
kernel (git bisect)?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/2] PM / PCMCIA: Drop second argument of pcmcia_socket_dev_suspend()

[Rafael J. Wysocki]

On Monday 28 September 2009, Wolfram Sang wrote:
> On Sat, Sep 26, 2009 at 01:25:11AM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > pcmcia_socket_dev_suspend() doesn't use its second argument, so it
> > may be dropped safely.
> > 
> > This change is necessary for the subsequent yenta suspend/resume fix.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/pcmcia/at91_cf.c        |    2 +-
> >  drivers/pcmcia/au1000_generic.c |    2 +-
> >  drivers/pcmcia/bfin_cf_pcmcia.c |    2 +-
> >  drivers/pcmcia/cs.c             |    2 +-
> >  drivers/pcmcia/i82092.c         |    2 +-
> >  drivers/pcmcia/i82365.c         |    2 +-
> >  drivers/pcmcia/m32r_cfc.c       |    2 +-
> >  drivers/pcmcia/m32r_pcc.c       |    2 +-
> >  drivers/pcmcia/m8xx_pcmcia.c    |    2 +-
> >  drivers/pcmcia/omap_cf.c        |    2 +-
> >  drivers/pcmcia/pd6729.c         |    2 +-
> >  drivers/pcmcia/pxa2xx_base.c    |    2 +-
> >  drivers/pcmcia/sa1100_generic.c |    2 +-
> >  drivers/pcmcia/sa1111_generic.c |    2 +-
> >  drivers/pcmcia/tcic.c           |    2 +-
> >  drivers/pcmcia/vrc4171_card.c   |    2 +-
> >  include/pcmcia/ss.h             |    2 +-
> 
> Hmm, you are not converting yenta_socket here. Won't that break building the
> kernel (git bisect)?

You're right, I should have modified yenta_socket.c here as well.  Will fix.

Thanks,
Rafael