linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel 2.4.22-pre6
@ 2003-07-15 21:56 Bhavesh P. Davda
  2003-07-16 13:16 ` Alan Cox
  0 siblings, 1 reply; 5+ messages in thread
From: Bhavesh P. Davda @ 2003-07-15 21:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: fischer, dahinds, Marcelo Tosatti

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

Juergen, David; please review this patch, and recommend that Marcelo apply
this patch if you think it is okay... Thanks!

Attached is a patch against linux-2.4.22-pre6, to fix a hang problem when an
Adaptec SlimSCSI (PCMCIA) adapter is ejected from a PCMCIA card reader (e.g.
yenta/TI1225 based).

The fix involves:

1. A change to the common aha152x driver to ignore an interrupt in the
top-half handler if it cannot read valid data from the I/O ports (possibly
due to a bad host-adapter chip or an ejected PCMCIA card). This way, a
shared interrupt handler (e.g. yenta) can pick up the interrupt if the IRQ
is really meant for it. This is where the original hang was taking place;
the aha152x bottom half was getting into an infinite loop, though the
SlimSCSI card had been ejected, and the actual IRQ was meant for yenta.

2. A change to the aha152x_cs stub driver to not use the SCSI error-handling
thread code. The aha152x_cs driver calls scsi_unregister_module() as a
queued timer task when it gets a CS_EVENT_CARD_REMOVAL event, which causes
scsi_unregister_host() to do a down() on a semaphore, calling schedule(),
when executing the timer_bh for the timer.

Thanks!

- Bhavesh
--
Bhavesh P. Davda     E-mail    : bhavesh@avaya.com
Avaya Inc.           Phone/Fax : (303) 538-4438
Room B3-B03, 1300 West 120th Avenue
Westminster, CO 80234

[-- Attachment #2: linux-2.4.22-aha152x.patch --]
[-- Type: application/octet-stream, Size: 1814 bytes --]

diff -Naur linux-2.4.22-pre6/drivers/scsi/aha152x.c linux-2.4.22-bpd/drivers/scsi/aha152x.c
--- linux-2.4.22-pre6/drivers/scsi/aha152x.c	2003-06-13 08:51:36.000000000 -0600
+++ linux-2.4.22-bpd/drivers/scsi/aha152x.c	2003-07-15 14:38:27.000000000 -0600
@@ -1936,6 +1936,23 @@
 		return;
 	}
 
+	/*
+	 * Read a couple of registers that are known to not be all 1's. If
+	 * we read all 1's (-1), that means that either:
+	 * a. The host adapter chip has gone bad, and we cannot control it,
+	 * OR
+	 * b. The host adapter is a PCMCIA card that has been ejected
+	 * In either case, we cannot do anything with the host adapter at
+	 * this point in time. So just ignore the interrupt and return.
+	 * In the latter case, the interrupt might actually be meant for
+	 * someone else sharing this IRQ, and that driver will handle it
+	 */
+	rev = GETPORT(REV);
+	dmacntrl0 = GETPORT(DMACNTRL0);
+	if ((rev == 0xFF) && (dmacntrl0 == 0xFF)) {
+		return;
+	}
+
 	/* no more interrupts from the controller, while we're busy.
 	   INTEN is restored by the BH handler */
 	CLRBITS(DMACNTRL0, INTEN);
diff -Naur linux-2.4.22-pre6/drivers/scsi/pcmcia/aha152x_stub.c linux-2.4.22-bpd/drivers/scsi/pcmcia/aha152x_stub.c
--- linux-2.4.22-pre6/drivers/scsi/pcmcia/aha152x_stub.c	2001-12-21 10:41:55.000000000 -0700
+++ linux-2.4.22-bpd/drivers/scsi/pcmcia/aha152x_stub.c	2003-07-15 14:41:20.000000000 -0600
@@ -244,6 +244,11 @@
 
     /* Configure card */
     driver_template.module = &__this_module;
+    /* 
+     * Don't use the SCSI error handling thread. This causes schedule() to
+     * be called from the timer_bh when the AHA152x card is ejected
+     */
+    driver_template.use_new_eh_code = 0;
     link->state |= DEV_CONFIG;
 
     tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel 2.4.22-pre6
  2003-07-15 21:56 [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel 2.4.22-pre6 Bhavesh P. Davda
@ 2003-07-16 13:16 ` Alan Cox
  2003-07-17 14:15   ` [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6 Bhavesh P. Davda
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Cox @ 2003-07-16 13:16 UTC (permalink / raw)
  To: Bhavesh P. Davda
  Cc: Linux Kernel Mailing List, fischer, dahinds, Marcelo Tosatti

On Maw, 2003-07-15 at 22:56, Bhavesh P. Davda wrote:
> 2. A change to the aha152x_cs stub driver to not use the SCSI error-handling
> thread code. The aha152x_cs driver calls scsi_unregister_module() as a
> queued timer task when it gets a CS_EVENT_CARD_REMOVAL event, which causes
> scsi_unregister_host() to do a down() on a semaphore, calling schedule(),
> when executing the timer_bh for the timer.

Right - scsi_unregister should not be called on a timer event, instead
it needs to kick off a task queue


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6
  2003-07-16 13:16 ` Alan Cox
@ 2003-07-17 14:15   ` Bhavesh P. Davda
  2003-07-17 17:55     ` David Hinds
  0 siblings, 1 reply; 5+ messages in thread
From: Bhavesh P. Davda @ 2003-07-17 14:15 UTC (permalink / raw)
  To: Alan Cox; +Cc: Linux Kernel Mailing List, fischer, dahinds, Marcelo Tosatti

> -----Original Message-----
> From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
> Sent: Wednesday, July 16, 2003 7:16 AM
> To: Bhavesh P. Davda
> Cc: Linux Kernel Mailing List; fischer@norbit.de;
> dahinds@users.sourceforge.net; Marcelo Tosatti
> Subject: Re: [PATCH] AHA152x driver hangs on PCMCIA card eject,
> kernel2.4.22-pre6
>
>
> On Maw, 2003-07-15 at 22:56, Bhavesh P. Davda wrote:
> > 2. A change to the aha152x_cs stub driver to not use the SCSI
> error-handling
> > thread code. The aha152x_cs driver calls scsi_unregister_module() as a
> > queued timer task when it gets a CS_EVENT_CARD_REMOVAL event,
> which causes
> > scsi_unregister_host() to do a down() on a semaphore, calling
> schedule(),
> > when executing the timer_bh for the timer.
>
> Right - scsi_unregister should not be called on a timer event, instead
> it needs to kick off a task queue


Thanks. I will give this a shot for the aha152x_cs stub driver, and post
another patch (unless David Hinds or Juergen Fischer get around to it
first).

However, the same problem exists for all the PCMCIA SCSI stub drivers, who
have all chosen to NOT use the scsi error handling thread, by setting
use_new_eh_code to 0 in the Scsi_Host_Template. I don't feel comfortable
posting patches to those stub drivers (fdomain, nsp, qlogic) to do the same,
since I don't have the hardware to test with.

Also, I wanted to warn you of a couple of leaps of faith I had to make to
answer questions I had for this patch:

1. Can the AIC-6360 host adapter ever have all 1's in the REV and DMACNTRL0
registers? I am guessing not, but waiting on specs from Adaptec to answer
this question.

2. What happens if there is no physical device hanging off an I/O port
address? I am guessing, that on an i386 host, the inb returns 0xFF, but am
not sure what happens on other architectures. I have a question outstanding
to Intel for this.

Thanks

- Bhavesh
--
Bhavesh P. Davda     E-mail    : bhavesh@avaya.com
Avaya Inc.           Phone/Fax : (303) 538-4438
Room B3-B03, 1300 West 120th Avenue
Westminster, CO 80234


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6
  2003-07-17 14:15   ` [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6 Bhavesh P. Davda
@ 2003-07-17 17:55     ` David Hinds
  2003-07-17 20:29       ` Bhavesh P. Davda
  0 siblings, 1 reply; 5+ messages in thread
From: David Hinds @ 2003-07-17 17:55 UTC (permalink / raw)
  To: Bhavesh P. Davda
  Cc: Alan Cox, Linux Kernel Mailing List, fischer, dahinds, Marcelo Tosatti

On Thu, Jul 17, 2003 at 08:15:39AM -0600, Bhavesh P. Davda wrote:

> > Right - scsi_unregister should not be called on a timer event, instead
> > it needs to kick off a task queue

The removal timers need to be taken out from most *_cs drivers; they
are a holdover from when card removal events were delivered in
interrupt context, and when that was changed to an event handler
thread, drivers were not changed accordingly.  The removal routine
should now just be called in-line instead of firing up a timer.

> 2. What happens if there is no physical device hanging off an I/O port
> address? I am guessing, that on an i386 host, the inb returns 0xFF, but am
> not sure what happens on other architectures. I have a question outstanding
> to Intel for this.

On most but not all x86 systems floating ports return 0xff.  Checking
for that or other "impossible" register values should be at least
harmless on other architectures.

-- Dave

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6
  2003-07-17 17:55     ` David Hinds
@ 2003-07-17 20:29       ` Bhavesh P. Davda
  0 siblings, 0 replies; 5+ messages in thread
From: Bhavesh P. Davda @ 2003-07-17 20:29 UTC (permalink / raw)
  To: David Hinds
  Cc: Alan Cox, Linux Kernel Mailing List, fischer, dahinds, Marcelo Tosatti

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

> -----Original Message-----
> From: David Hinds [mailto:dhinds@sonic.net]
> Sent: Thursday, July 17, 2003 11:56 AM
> To: Bhavesh P. Davda
> Cc: Alan Cox; Linux Kernel Mailing List; fischer@norbit.de;
> dahinds@users.sourceforge.net; Marcelo Tosatti
> Subject: Re: [PATCH] AHA152x driver hangs on PCMCIA card eject,
> kernel2.4.22-pre6
>
>
> On Thu, Jul 17, 2003 at 08:15:39AM -0600, Bhavesh P. Davda wrote:
>
> > > Right - scsi_unregister should not be called on a timer event, instead
> > > it needs to kick off a task queue
>
> The removal timers need to be taken out from most *_cs drivers; they
> are a holdover from when card removal events were delivered in
> interrupt context, and when that was changed to an event handler
> thread, drivers were not changed accordingly.  The removal routine
> should now just be called in-line instead of firing up a timer.
>
> > 2. What happens if there is no physical device hanging off an I/O port
> > address? I am guessing, that on an i386 host, the inb returns
> 0xFF, but am
> > not sure what happens on other architectures. I have a question
> outstanding
> > to Intel for this.
>
> On most but not all x86 systems floating ports return 0xff.  Checking
> for that or other "impossible" register values should be at least
> harmless on other architectures.
>
> -- Dave

Thank you, Dave!

Attached is a patch that takes into account comments I have received in
response to the original patch I posted. Please peruse it, and if it looks
okay, please recommend that Marcelo pick it up for 2.4.22.

FYI, I have tested this patch on a PIII/440BX with a TI1225 based PCMCIA
card reader and Adaptec SlimSCSI 1460D SCSI adapter card connected to a
Fujitsu SCSI MO drive.

Thanks!

- Bhavesh
--
Bhavesh P. Davda     E-mail    : bhavesh@avaya.com
Avaya Inc.           Phone/Fax : (303) 538-4438
Room B3-B03, 1300 West 120th Avenue
Westminster, CO 80234


[-- Attachment #2: linux-2.4.22-aha152x.patch --]
[-- Type: application/octet-stream, Size: 6645 bytes --]

diff -Naur linux-2.4.22-pre6/drivers/scsi/aha152x.c linux-2.4.22-bpd/drivers/scsi/aha152x.c
--- linux-2.4.22-pre6/drivers/scsi/aha152x.c	2003-06-13 08:51:36.000000000 -0600
+++ linux-2.4.22-bpd/drivers/scsi/aha152x.c	2003-07-15 16:05:09.000000000 -0600
@@ -1930,12 +1930,30 @@
 static void intr(int irqno, void *dev_id, struct pt_regs *regs)
 {
 	struct Scsi_Host *shpnt = lookup_irq(irqno);
+	unsigned char rev, dmacntrl0;
 
 	if (!shpnt) {
 		printk(KERN_ERR "aha152x: catched interrupt %d for unknown controller.\n", irqno);
 		return;
 	}
 
+	/*
+	 * Read a couple of registers that are known to not be all 1's. If
+	 * we read all 1's (-1), that means that either:
+	 * a. The host adapter chip has gone bad, and we cannot control it,
+	 * OR
+	 * b. The host adapter is a PCMCIA card that has been ejected
+	 * In either case, we cannot do anything with the host adapter at
+	 * this point in time. So just ignore the interrupt and return.
+	 * In the latter case, the interrupt might actually be meant for
+	 * someone else sharing this IRQ, and that driver will handle it
+	 */
+	rev = GETPORT(REV);
+	dmacntrl0 = GETPORT(DMACNTRL0);
+	if ((rev == 0xFF) && (dmacntrl0 == 0xFF)) {
+		return;
+	}
+
 	/* no more interrupts from the controller, while we're busy.
 	   INTEN is restored by the BH handler */
 	CLRBITS(DMACNTRL0, INTEN);
diff -Naur linux-2.4.22-pre6/drivers/scsi/pcmcia/aha152x_stub.c linux-2.4.22-bpd/drivers/scsi/pcmcia/aha152x_stub.c
--- linux-2.4.22-pre6/drivers/scsi/pcmcia/aha152x_stub.c	2001-12-21 10:41:55.000000000 -0700
+++ linux-2.4.22-bpd/drivers/scsi/pcmcia/aha152x_stub.c	2003-07-17 14:03:03.000000000 -0600
@@ -40,7 +40,6 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/timer.h>
 #include <linux/ioport.h>
 #include <scsi/scsi.h>
 #include <linux/major.h>
@@ -139,8 +138,6 @@
     if (!info) return NULL;
     memset(info, 0, sizeof(*info));
     link = &info->link; link->priv = info;
-    link->release.function = &aha152x_release_cs;
-    link->release.data = (u_long)link;
 
     link->io.NumPorts1 = 0x20;
     link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
@@ -193,7 +190,6 @@
     if (*linkp == NULL)
 	return;
 
-    del_timer(&link->release);
     if (link->state & DEV_CONFIG) {
 	aha152x_release_cs((u_long)link);
 	if (link->state & DEV_STALE_CONFIG) {
@@ -383,7 +379,7 @@
     case CS_EVENT_CARD_REMOVAL:
 	link->state &= ~DEV_PRESENT;
 	if (link->state & DEV_CONFIG)
-	    mod_timer(&link->release, jiffies + HZ/20);
+	    aha152x_release_cs((u_long)link);
 	break;
     case CS_EVENT_CARD_INSERTION:
 	link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
diff -Naur linux-2.4.22-pre6/drivers/scsi/pcmcia/fdomain_stub.c linux-2.4.22-bpd/drivers/scsi/pcmcia/fdomain_stub.c
--- linux-2.4.22-pre6/drivers/scsi/pcmcia/fdomain_stub.c	2001-12-21 10:41:55.000000000 -0700
+++ linux-2.4.22-bpd/drivers/scsi/pcmcia/fdomain_stub.c	2003-07-17 14:04:36.000000000 -0600
@@ -37,7 +37,6 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/timer.h>
 #include <linux/ioport.h>
 #include <scsi/scsi.h>
 #include <linux/major.h>
@@ -125,8 +124,6 @@
     if (!info) return NULL;
     memset(info, 0, sizeof(*info));
     link = &info->link; link->priv = info;
-    link->release.function = &fdomain_release;
-    link->release.data = (u_long)link;
 
     link->io.NumPorts1 = 0x10;
     link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
@@ -179,7 +176,6 @@
     if (*linkp == NULL)
 	return;
 
-    del_timer(&link->release);
     if (link->state & DEV_CONFIG) {
 	fdomain_release((u_long)link);
 	if (link->state & DEV_STALE_CONFIG) {
@@ -350,7 +346,7 @@
     case CS_EVENT_CARD_REMOVAL:
 	link->state &= ~DEV_PRESENT;
 	if (link->state & DEV_CONFIG)
-	    mod_timer(&link->release, jiffies + HZ/20);
+	    fdomain_release((u_long)link);
 	break;
     case CS_EVENT_CARD_INSERTION:
 	link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
diff -Naur linux-2.4.22-pre6/drivers/scsi/pcmcia/nsp_cs.c linux-2.4.22-bpd/drivers/scsi/pcmcia/nsp_cs.c
--- linux-2.4.22-pre6/drivers/scsi/pcmcia/nsp_cs.c	2003-06-13 08:51:36.000000000 -0600
+++ linux-2.4.22-bpd/drivers/scsi/pcmcia/nsp_cs.c	2003-07-17 14:07:43.000000000 -0600
@@ -36,7 +36,6 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/timer.h>
 #include <linux/ioport.h>
 #include <linux/delay.h>
 #include <linux/tqueue.h>
@@ -1341,10 +1340,6 @@
 	link = &info->link;
 	link->priv = info;
 
-	/* Initialize the dev_link_t structure */
-	link->release.function	 = &nsp_cs_release;
-	link->release.data	 = (u_long)link;
-
 	/* The io structure describes IO port mapping */
 	link->io.NumPorts1	 = 0x10;
 	link->io.Attributes1	 = IO_DATA_PATH_WIDTH_AUTO;
@@ -1415,7 +1410,6 @@
 		return;
 	}
 
-	del_timer(&link->release);
 	if (link->state & DEV_CONFIG) {
 		nsp_cs_release((u_long)link);
 		if (link->state & DEV_STALE_CONFIG) {
@@ -1660,7 +1654,7 @@
 		link->state &= ~DEV_PRESENT;
 		if (link->state & DEV_CONFIG) {
 			((scsi_info_t *)link->priv)->stop = 1;
-			mod_timer(&link->release, jiffies + HZ/20);
+			nsp_cs_release((u_long)link);
 		}
 		break;
 
diff -Naur linux-2.4.22-pre6/drivers/scsi/pcmcia/qlogic_stub.c linux-2.4.22-bpd/drivers/scsi/pcmcia/qlogic_stub.c
--- linux-2.4.22-pre6/drivers/scsi/pcmcia/qlogic_stub.c	2001-12-21 10:41:55.000000000 -0700
+++ linux-2.4.22-bpd/drivers/scsi/pcmcia/qlogic_stub.c	2003-07-17 14:05:36.000000000 -0600
@@ -37,7 +37,6 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/timer.h>
 #include <linux/ioport.h>
 #include <asm/io.h>
 #include <asm/byteorder.h>
@@ -132,8 +131,6 @@
     if (!info) return NULL;
     memset(info, 0, sizeof(*info));
     link = &info->link; link->priv = info;
-    link->release.function = &qlogic_release;
-    link->release.data = (u_long)link;
 
     link->io.NumPorts1 = 16;
     link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
@@ -186,7 +183,6 @@
     if (*linkp == NULL)
 	return;
 
-    del_timer(&link->release);
     if (link->state & DEV_CONFIG) {
 	qlogic_release((u_long)link);
 	if (link->state & DEV_STALE_CONFIG) {
@@ -371,7 +367,7 @@
     case CS_EVENT_CARD_REMOVAL:
 	link->state &= ~DEV_PRESENT;
 	if (link->state & DEV_CONFIG)
-	    mod_timer(&link->release, jiffies + HZ/20);
+	    qlogic_release((u_long)link);
 	break;
     case CS_EVENT_CARD_INSERTION:
 	link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2003-07-17 20:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-15 21:56 [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel 2.4.22-pre6 Bhavesh P. Davda
2003-07-16 13:16 ` Alan Cox
2003-07-17 14:15   ` [PATCH] AHA152x driver hangs on PCMCIA card eject, kernel2.4.22-pre6 Bhavesh P. Davda
2003-07-17 17:55     ` David Hinds
2003-07-17 20:29       ` Bhavesh P. Davda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).