Re: any chance of 2.6.0-test*?

[Paul Mackerras <paulus _at_ xxxxxxxxx>]

Alan Cox writes:

> The pcmcia stuff crashes erratically with the old IDE the bugs have not
> changed but the problem now shows up far more often. With a 2Gb type II
> PCMCIA IDE disk I get a crash about 1 in 3 ejects now.

I'm using the patch below, which makes sure that ide_release (in
ide-cs.c) runs in process context not interrupt context.  The specific
problem I saw was that ide_unregister calls various devfs routines
that don't like being called in interrupt context, but there may well
be other thing ide_unregister does that aren't safe in interrupt
context.

I am also hitting another problem on my powerbook that I would like
your advice on.  The thing is that my powerbook (a 500MHz G4 titanium
powerbook) has 3 IDE interfaces, but only the first two have anything
attached.  The pmac IDE driver goes through and initializes all three,
including setting hwif->selectproc for each one.  Then, when I plug in
a CF card in the pcmcia/cardbus slot, ide_register_hw goes and looks
for a free ide_hwifs[] entry.  Slot 2 appears to be available because
there are no devices attached, and so it uses that.  In the process of
setting up ide_hwifs[2], it sets hwif->io_ports but it doesn't reset
selectproc (or hwif->OUTB etc., AFAICS).  The result is that it calls
the pmac IDE selectproc, which crashes (since we munged hwif->io_ports
without telling it).

I have "fixed" the problem for now by adding a call to
init_hwif_data(index) in ide_register_hw just before the first
memcpy.  I'd be interested to know what the right fix is. :)

Regards,
Paul.

diff -urN linux-2.4/drivers/ide/legacy/ide-cs.c pmac/drivers/ide/legacy/ide-cs.c
--- linux-2.4/drivers/ide/legacy/ide-cs.c	2002-12-19 11:50:35.000000000 +1100
+++ pmac/drivers/ide/legacy/ide-cs.c	2003-01-13 14:02:31.000000000 +1100
@@ -92,10 +92,11 @@
     int		ndev;
     dev_node_t	node;
     int		hd;
+    struct tq_struct rel_task;
 } ide_info_t;
 
 static void ide_config(dev_link_t *link);
-static void ide_release(u_long arg);
+static void ide_release(void *arg);
 static int ide_event(event_t event, int priority,
 		     event_callback_args_t *args);
 
@@ -136,9 +137,8 @@
     if (!info) return NULL;
     memset(info, 0, sizeof(*info));
     link = &info->link; link->priv = info;
+    INIT_TQUEUE(&info->rel_task, ide_release, link);
 
-    link->release.function = &ide_release;
-    link->release.data = (u_long)link;
     link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
     link->io.Attributes2 = IO_DATA_PATH_WIDTH_8;
     link->io.IOAddrLines = 3;
@@ -187,6 +187,7 @@
 static void ide_detach(dev_link_t *link)
 {
     dev_link_t **linkp;
+    ide_info_t *info = link->priv;
     int ret;
 
     DEBUG(0, "ide_detach(0x%p)\n", link);
@@ -197,9 +198,10 @@
     if (*linkp == NULL)
 	return;
 
-    del_timer(&link->release);
-    if (link->state & DEV_CONFIG)
-	ide_release((u_long)link);
+    if (link->state & DEV_CONFIG) {
+	schedule_task(&info->rel_task);
+	flush_scheduled_tasks();
+    }
     
     if (link->handle) {
 	ret = CardServices(DeregisterClient, link->handle);
@@ -209,7 +211,7 @@
     
     /* Unlink, free device structure */
     *linkp = link->next;
-    kfree(link->priv);
+    kfree(info);
     
 } /* ide_detach */
 
@@ -381,7 +383,7 @@
 cs_failed:
     cs_error(link->handle, last_fn, last_ret);
 failed:
-    ide_release((u_long)link);
+    ide_release(link);
 
 } /* ide_config */
 
@@ -393,12 +395,15 @@
     
 ======================================================================*/
 
-void ide_release(u_long arg)
+static void ide_release(void *arg)
 {
-    dev_link_t *link = (dev_link_t *)arg;
+    dev_link_t *link = arg;
     ide_info_t *info = link->priv;
     
-    DEBUG(0, "ide_release(0x%p)\n", link);
+    if (!(link->state & DEV_CONFIG))
+	return;
+
+    DEBUG(0, "ide_do_release(0x%p)\n", link);
 
     if (info->ndev) {
         /* FIXME: if this fails we need to queue the cleanup somehow
@@ -435,6 +440,7 @@
 	      event_callback_args_t *args)
 {
     dev_link_t *link = args->client_data;
+    ide_info_t *info = link->priv;
 
     DEBUG(1, "ide_event(0x%06x)\n", event);
     
@@ -442,7 +448,7 @@
     case CS_EVENT_CARD_REMOVAL:
 	link->state &= ~DEV_PRESENT;
 	if (link->state & DEV_CONFIG)
-	    mod_timer(&link->release, jiffies + HZ/20);
+	    schedule_task(&info->rel_task);
 	break;
     case CS_EVENT_CARD_INSERTION:
 	link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo _at_ vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/