On Fri, 15 Dec 2000, Linus Torvalds wrote:
> I suspect that the suspend/resume will do something bad to the BAT
> registers, which control the BIOS area mapping behaviour, and it just
> kills the forwarding of the legacy region to the PCI bus, or something.
FYI: I've identified a single byte in the hostbridges config space which
is altered after resume. Blindly restoring it makes the 0xe6000 pci bus
address mapping accessible again. But I think that's not the Right way to
fix it.
> I wonder if the PCI cardbus init code should just notice this, and force
> all cardbus windows to be re-initialized. That legacy area address really
> doesn't look right.
That's what I've done now. So I'm sending the modification I made in case
you would still like them for 2.4.0. I've separated it in 2 (almost
independent) patches meant to be applied in series against 2.4.0-t12 (final)
This is part 1. It provides:
- pm_state, pm_lock for cardbus socket to prevent multiple suspend/resume,
especially the reentrant case due to some driver sleeping in resume.
- cardbus_change_pm_state() to handle pm state transition. Suspend is
handled synchronously but resume schedules an asynchronous completion
handler since pcmcia_resume_socket() may sleep.
- placing the pci_set_power_state() calls at the right place
Mainly touching pci_socket.* it fixes the suspend/resume multiple entrance
issues due to some driver (notably cardbus itself) sleeping in resume.
Regards
Martin
-----
diff -Nur v2.4.0-test12/drivers/pcmcia/pci_socket.c v2.4.0-t12-yenta1/driver/pcmcia/pci_socket.c
--- v2.4.0-test12/drivers/pcmcia/pci_socket.c Wed Nov 29 21:47:10 2000
+++ v2.4.0-t12-yenta1/driver/pcmcia/pci_socket.c Sun Dec 17 19:59:27 2000
@@ -178,6 +178,8 @@
socket->op = ops;
dev->driver_data = socket;
spin_lock_init(&socket->event_lock);
+ socket->pm_state = 0;
+ spin_lock_init(&socket->pm_lock);
return socket->op->open(socket);
}
@@ -212,16 +214,83 @@
dev->driver_data = 0;
}
+/* Delayed handler scheduled to complete the D3->D0 transition in the
+ * upper layers. We may sleep in pcmcia_resume_socket() with pm_lock
+ * hold - so we are save from resume re-entry due to other drivers
+ * sleeping in pci_pm resume handling.
+ */
+
+static void cardbus_resume_bh(void *data)
+{
+ pci_socket_t *socket = (pci_socket_t *)data;
+
+ pcmcia_resume_socket(socket->pcmcia_socket);
+ socket->pm_state = 0;
+ spin_unlock(&socket->pm_lock);
+ MOD_DEC_USE_COUNT;
+}
+
+/* We are forced to implement asynch resume semantics because the
+ * pcmcia_resume path sleeps and we might get screwed by a second
+ * pci_pm_resume_device() hitting us in the middle of the first one.
+ * Which might happen anyway, if other drivers do not cooperate!
+ * So it's good to know we are protected by our socket->pm_lock.
+ */
+
+static void cardbus_change_pm_state(pci_socket_t *socket, int newstate)
+{
+ switch (newstate) {
+ case 3:
+ pcmcia_suspend_socket(socket->pcmcia_socket);
+ break;
+
+ case 0:
+ socket->tq_resume.routine = cardbus_resume_bh;
+ socket->tq_resume.data = socket;
+ MOD_INC_USE_COUNT;
+ schedule_task(&socket->tq_resume);
+ break;
+ default:
+ printk("cardbus: undefined power state\n");
+ break;
+ }
+}
+
+
static void cardbus_suspend (struct pci_dev *dev)
{
pci_socket_t *socket = (pci_socket_t *) dev->driver_data;
- pcmcia_suspend_socket (socket->pcmcia_socket);
+
+ spin_lock(&socket->pm_lock);
+ if (socket->pm_state != 0) {
+ spin_unlock(&socket->pm_lock);
+ printk("cardbus: suspend of already suspended socket blocked\n");
+ return;
+ }
+ cardbus_change_pm_state(socket,3);
+ pci_set_power_state(dev,3);
+ socket->pm_state = 3;
+ spin_unlock(&socket->pm_lock);
}
static void cardbus_resume (struct pci_dev *dev)
{
pci_socket_t *socket = (pci_socket_t *) dev->driver_data;
- pcmcia_resume_socket (socket->pcmcia_socket);
+
+ spin_lock(&socket->pm_lock);
+ if (socket->pm_state != 3) {
+ spin_unlock(&socket->pm_lock);
+ printk("cardbus: resume of non-suspended socket blocked\n");
+ return;
+ }
+ pci_set_power_state(dev,0);
+ cardbus_change_pm_state(socket, 0);
+
+ /* we intentionally leave with socket->pm_state not updated
+ * and socket->pm_lock still acquired!
+ * Will be released by the pending cardbus_resume_bh()
+ * Needed to protect against resume re-entry.
+ */
}
diff -Nur v2.4.0-test12/drivers/pcmcia/pci_socket.h v2.4.0-t12-yenta1/driver/pcmcia/pci_socket.h
--- v2.4.0-test12/drivers/pcmcia/pci_socket.h Wed Nov 29 21:47:10 2000
+++ v2.4.0-t12-yenta1/driver/pcmcia/pci_socket.h Sun Dec 17 20:04:32 2000
@@ -26,6 +26,14 @@
/* A few words of private data for the low-level driver.. */
unsigned int private[8];
+
+ /* used for delayed resume completion handler */
+ struct tq_struct tq_resume;
+
+ /* to protect our pm state management */
+ unsigned int pm_state;
+ spinlock_t pm_lock;
+
} pci_socket_t;
struct pci_socket_ops {
diff -Nur v2.4.0-test12/drivers/pcmcia/yenta.c v2.4.0-t12-yenta1/driver/pcmcia/yenta.c
--- v2.4.0-test12/drivers/pcmcia/yenta.c Wed Nov 29 21:47:10 2000
+++ v2.4.0-t12-yenta1/driver/pcmcia/yenta.c Sun Dec 17 20:00:17 2000
@@ -629,8 +629,6 @@
u16 bridge;
struct pci_dev *dev = socket->dev;
- pci_set_power_state(socket->dev, 0);
-
config_writel(socket, CB_LEGACY_MODE_BASE, 0);
config_writel(socket, PCI_BASE_ADDRESS_0, dev->resource[0].start);
config_writew(socket, PCI_COMMAND,
@@ -687,8 +685,6 @@
* the IO and MEM bridging region data.. That is
* something that pci_set_power_state() should
* probably know about bridges anyway.
- *
- pci_set_power_state(socket->dev, 3);
*/
return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Dec 23 2000 - 21:00:21 EST