3c509: use proper suspend/resume API

RFT/PATCH Pekka Enberg 3 participants 8 messages Feb 14, 2006 – Feb 15, 2006

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Pekka Enberg

author

2006-02-14 16:39 UTC #0 lore raw

Hi,

I am looking for someone with 3c509 netword card that can do suspend/resume to test this patch.

Pekka

Subject: 3c509: use proper suspend/resume API
From: Pekka Enberg <[email protected]>

This patch converts 3c509 driver to use proper suspend/resume API instead of the deprecated pm_register/pm_unregister.

Signed-off-by Pekka Enberg <[email protected]>

---

drivers/net/3c509.c +19 ~~-47~~

Index: 2.6/drivers/net/3c509.c
===================================================================
@@ -174,9 +174,6 @@ struct el3_private {
 	/* skb send-queue */
 	int head, size;
 	struct sk_buff *queue[SKB_QUEUE_SIZE];
-#ifdef CONFIG_PM_LEGACY
-	struct pm_dev *pmdev;
-#endif
 	enum {
 		EL3_MCA,
 		EL3_PNP,

@@ -201,11 +198,15 @@ static void el3_tx_timeout (struct net_d
 static void el3_down(struct net_device *dev);
 static void el3_up(struct net_device *dev);
 static struct ethtool_ops ethtool_ops;
-#ifdef CONFIG_PM_LEGACY
-static int el3_suspend(struct pm_dev *pdev);
-static int el3_resume(struct pm_dev *pdev);
-static int el3_pm_callback(struct pm_dev *pdev, pm_request_t rqst, void *data);
+#ifdef CONFIG_PM
+static int el3_suspend(struct device *, pm_message_t);
+static int el3_resume(struct device *);
+#else
+#define el3_suspend NULL
+#define el3_resume NULL
 #endif
+
+
 /* generic device remove for all device types */
 #if defined(CONFIG_EISA) || defined(CONFIG_MCA)
 static int el3_device_remove (struct device *device);

@@ -229,7 +230,9 @@ static struct eisa_driver el3_eisa_drive
 		.driver   = {
 				.name    = "3c509",
 				.probe   = el3_eisa_probe,
-				.remove  = __devexit_p (el3_device_remove)
+				.remove  = __devexit_p (el3_device_remove),
+				.suspend = el3_suspend,
+				.resume  = el3_resume,
 		}
 };
 #endif

@@ -262,6 +265,8 @@ static struct mca_driver el3_mca_driver 
 				.bus = &mca_bus_type,
 				.probe = el3_mca_probe,
 				.remove = __devexit_p(el3_device_remove),
+				.suspend = el3_suspend,
+				.resume  = el3_resume,
 		},
 };
 #endif /* CONFIG_MCA */

@@ -362,10 +367,6 @@ static void el3_common_remove (struct ne
 	struct el3_private *lp = netdev_priv(dev);
 
 	(void) lp;				/* Keep gcc quiet... */
-#ifdef CONFIG_PM_LEGACY
-	if (lp->pmdev)
-		pm_unregister(lp->pmdev);
-#endif
 #if defined(__ISAPNP__)
 	if (lp->type == EL3_PNP)
 		pnp_device_detach(to_pnp_dev(lp->dev));

@@ -572,16 +573,6 @@ no_pnp:
 	if (err)
 		goto out1;
 
-#ifdef CONFIG_PM_LEGACY
-	/* register power management */
-	lp->pmdev = pm_register(PM_ISA_DEV, card_idx, el3_pm_callback);
-	if (lp->pmdev) {
-		struct pm_dev *p;
-		p = lp->pmdev;
-		p->data = (struct net_device *)dev;
-	}
-#endif
-
 	el3_cards++;
 	lp->next_dev = el3_root_dev;
 	el3_root_dev = dev;

@@ -1480,20 +1471,17 @@ el3_up(struct net_device *dev)
 }
 
 /* Power Management support functions */
-#ifdef CONFIG_PM_LEGACY
+#ifdef CONFIG_PM
 
 static int
-el3_suspend(struct pm_dev *pdev)
+el3_suspend(struct device *pdev, pm_message_t state)
 {
 	unsigned long flags;
 	struct net_device *dev;
 	struct el3_private *lp;
 	int ioaddr;
 	
-	if (!pdev && !pdev->data)
-		return -EINVAL;
-
-	dev = (struct net_device *)pdev->data;
+	dev = pdev->driver_data;
 	lp = netdev_priv(dev);
 	ioaddr = dev->base_addr;

@@ -1510,17 +1498,14 @@ el3_suspend(struct pm_dev *pdev)
 }
 
 static int
-el3_resume(struct pm_dev *pdev)
+el3_resume(struct device *pdev)
 {
 	unsigned long flags;
 	struct net_device *dev;
 	struct el3_private *lp;
 	int ioaddr;
 	
-	if (!pdev && !pdev->data)
-		return -EINVAL;
-
-	dev = (struct net_device *)pdev->data;
+	dev = pdev->driver_data;
 	lp = netdev_priv(dev);
 	ioaddr = dev->base_addr;

@@ -1536,20 +1521,7 @@ el3_resume(struct pm_dev *pdev)
 	return 0;
 }
 
-static int
-el3_pm_callback(struct pm_dev *pdev, pm_request_t rqst, void *data)
-{
-	switch (rqst) {
-		case PM_SUSPEND:
-			return el3_suspend(pdev);
-
-		case PM_RESUME:
-			return el3_resume(pdev);
-	}
-	return 0;
-}
-
-#endif /* CONFIG_PM_LEGACY */
+#endif /* CONFIG_PM */
 
 /* Parameters that may be passed into the module. */
 static int debug = -1;

Andrew Morton

2006-02-15 10:25 UTC #1 lore raw

Quoted 3 lines · Pekka Enberg

I am looking for someone with 3c509 netword card that can do
suspend/resume to test this patch.

I have a 3c509, and I'm not afraid to use it!

Problem is, it doesn't resume correctly either with or without the patch:
it needs rmmod+modprobe to get it going again. (Which is better than the aic7xxx driver, which has a coronary and panics the kernel on post-resume reboot).

But at least nothing got worse, and it makes that darn warning go away.

Pekka J Enberg

author

2006-02-15 11:00 UTC #2 lore raw

Quoted 6 lines · Andrew Morton

I have a 3c509, and I'm not afraid to use it!
Problem is, it doesn't resume correctly either with or without the patch:
it needs rmmod+modprobe to get it going again. (Which is better than the aic7xxx driver, which has a coronary and panics the kernel on post-resume reboot).

Is there anything in the logs to give us a clue what's going on? I can't see anything obvious looking at the code, but then again I don't have datasheets for 3c509 either.

Pekka

Andrew Morton

2006-02-15 11:52 UTC #3 lore raw

Quoted 4 lines · Pekka J Enberg

Quoted 2 lines · Andrew Morton
I have a 3c509, and I'm not afraid to use it!

Quoted 9 lines

Quoted 4 lines
Problem is, it doesn't resume correctly either with or without the patch:
it needs rmmod+modprobe to get it going again. (Which is better than the aic7xxx driver, which has a coronary and panics the kernel on post-resume reboot).
Is there anything in the logs to give us a clue what's going on? I can't see anything obvious looking at the code, but then again I don't have datasheets for 3c509 either.

eeprom reads 0xffff, that's all I noticed.

Pekka J Enberg

author

2006-02-15 11:23 UTC #4 lore raw

In reply to Andrew Morton

Hi Andrew,

Quoted 4 lines · Andrew Morton

Problem is, it doesn't resume correctly either with or without the patch:
it needs rmmod+modprobe to get it going again. (Which is better than the aic7xxx driver, which has a coronary and panics the kernel on post-resume reboot).

Hmm. Either I am totally confused or we don't even attempt suspend/resume for eisa and mca bus devices. Care to try this patch?

Pekka

drivers/eisa/eisa-bus.c +22 -0

Index: 2.6/drivers/eisa/eisa-bus.c
===================================================================
@@ -128,9 +128,31 @@ static int eisa_bus_match (struct device
 	return 0;
 }
 
+static int eisa_bus_suspend(struct device * dev, pm_message_t state)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->suspend)
+		ret = dev->driver->suspend(dev, state);
+
+	return ret;
+}
+
+static int eisa_bus_resume(struct device * dev)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->resume)
+		ret = dev->driver->resume(dev);
+
+	return ret;
+}
+
 struct bus_type eisa_bus_type = {
 	.name  = "eisa",
 	.match = eisa_bus_match,
+	.suspend = eisa_bus_suspend,
+	.resume = eisa_bus_resume,
 };
 
 int eisa_driver_register (struct eisa_driver *edrv)

drivers/mca/mca-bus.c +23 -0

Index: 2.6/drivers/mca/mca-bus.c
===================================================================
@@ -63,9 +63,32 @@ static int mca_bus_match (struct device 
 	return 0;
 }
 
+static int mca_bus_suspend(struct device * dev, pm_message_t state)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->suspend)
+		ret = dev->driver->suspend(dev, state);
+
+	return ret;
+}
+
+static int mca_bus_resume(struct device * dev)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->resume)
+		ret = dev->driver->resume(dev);
+
+	return ret;
+}
+
+
 struct bus_type mca_bus_type = {
 	.name  = "MCA",
 	.match = mca_bus_match,
+	.suspend = mca_bus_suspend,
+	.resume = mca_bus_resume,
 };
 EXPORT_SYMBOL (mca_bus_type);

Russell King

2006-02-15 12:40 UTC #5 lore raw

Quoted 2 lines · Pekka J Enberg

Hmm. Either I am totally confused or we don't even attempt suspend/resume for eisa and mca bus devices. Care to try this patch?

Please don't use struct device_driver suspend/resume methods.

Signature · 3 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Pekka Enberg

author

2006-02-15 17:32 UTC #6 lore raw

Quoted 2 lines · Pekka J Enberg

Quoted 2 lines
Hmm. Either I am totally confused or we don't even attempt suspend/resume for eisa and mca bus devices. Care to try this patch?

Quoted 1 line · Russell King

Please don't use struct device_driver suspend/resume methods.

So what would be more appropriate? Move suspend/resume/probe and friends to eisa_driver and mca_driver?

Pekka

Andrew Morton

2006-02-15 12:45 UTC #7 lore raw

In reply to Pekka J Enberg

Quoted 11 lines · Pekka J Enberg

Hi Andrew,
Quoted 4 lines · Andrew Morton
Problem is, it doesn't resume correctly either with or without the patch:
it needs rmmod+modprobe to get it going again. (Which is better than the aic7xxx driver, which has a coronary and panics the kernel on post-resume reboot).
Hmm. Either I am totally confused or we don't even attempt suspend/resume for eisa and mca bus devices. Care to try this patch?

No, el3_suspend() and el3_resume() don't seem to be called. And they're still not called with this patch applied..

(I got one resume in which the 3c509 still worked. Odd.)