Skip to contentSkip to conversation

pcmcia: pcmcia_request_irq for !IRQ_HANDLE_PRESENT

PATCH In review Noah Misch 3 participants 4 messages Jul 17, 2005 – Jul 24, 2005 Pcmcia

lore
Share & tools
lore

Noah Misch

author
#0 lore raw

PATCH 1/1 pcmcia: pcmcia_request_irq for !IRQ_HANDLE_PRESENT

Between 2.6.10 and 2.6.11, the kernel began to print this message when I inserted my IBM Home and Away adapter (pcnet_cs) into an i82365 slot:

1.0: RequestIRQ: Unknown error code 0xffffffea

Initialization of the device then aborted; no `eth0' appeared. Another user recently reported the same problem:

http://lists.infradead.org/pipermail/linux-pcmcia/2005-July/002151.html

I believe this change to pcmcia_request_irq brought about the problem:

http://linus.bkbits.net:8080/linux-2.5/diffs/drivers/pcmcia/[email protected]?nav=index.html|src/|src/drivers|src/drivers/pcmcia|hist/drivers/pcmcia/cs.c

When a driver calls pcmcia_request_irq with IRQ_HANDLE_PRESENT unset, it looks for an open IRQ by request_irq()ing with a dummy handler and NULL dev_info. free_irq uses dev_info as a key for identifying the handler to free among those sharing an IRQ, so request_irq returns -EINVAL if dev_info is NULL and the IRQ may be shared. That unknown error code is the -EINVAL.

It looks like only pcnet_cs and axnet_cs are affected. Most other drivers let pcmcia_request_irq install their interrupt handlers. sym53c500_cs requests its IRQ manually, but it cannot share an IRQ.

The appended patch changes pcmcia_request_irq to pass an arbitrary, unique, non-NULL dev_info with the dummy handler.

drivers/pcmcia/pcmcia_resource.c +3 -3
@@ -800,7 +800,7 @@ int pcmcia_request_irq(struct pcmcia_dev
 	} else {
 		int try;
 		u32 mask = s->irq_mask;
-		void *data = NULL;
+		int data;
 
 		for (try = 0; try < 64; try++) {
 			irq = try % 32;
@@ -822,10 +822,10 @@ int pcmcia_request_irq(struct pcmcia_dev
 					   (s->functions > 1) ||
 					   (irq == s->pci_irq)) ? SA_SHIRQ : 0,
 					  p_dev->dev.bus_id,
-					  (req->Attributes & IRQ_HANDLE_PRESENT) ? req->Instance : data);
+					  (req->Attributes & IRQ_HANDLE_PRESENT) ? req->Instance : &data);
 			if (!ret) {
 				if (!(req->Attributes & IRQ_HANDLE_PRESENT))
-					free_irq(irq, data);
+					free_irq(irq, &data);
 				break;
 			}
 		}
#1 lore raw

Hi,

Quoted 6 lines

When a driver calls pcmcia_request_irq with IRQ_HANDLE_PRESENT unset, it looks for an open IRQ by request_irq()ing with a dummy handler and NULL dev_info. free_irq uses dev_info as a key for identifying the handler to free among those sharing an IRQ, so request_irq returns -EINVAL if dev_info is NULL and the IRQ may be shared. That unknown error code is the -EINVAL.

Quoted 6 lines

It looks like only pcnet_cs and axnet_cs are affected. Most other drivers let pcmcia_request_irq install their interrupt handlers. sym53c500_cs requests its IRQ manually, but it cannot share an IRQ.

The appended patch changes pcmcia_request_irq to pass an arbitrary, unique, non-NULL dev_info with the dummy handler.

Thanks for the excellent debugging. Your patch seems to work, however it might be better to do just this:

drivers/pcmcia/pcmcia_resource.c +1 -1
Index: 2.6.13-rc3-git2/drivers/pcmcia/pcmcia_resource.c
===================================================================
@@ -800,7 +800,7 @@ int pcmcia_request_irq(struct pcmcia_dev
 	} else {
 		int try;
 		u32 mask = s->irq_mask;
-		void *data = NULL;
+		void *data = test_action;
 
 		for (try = 0; try < 64; try++) {
 			irq = try % 32;

Thanks,
Dominik

#2 lore raw
Quoted 2 lines · Dominik Brodowski

Thanks for the excellent debugging. Your patch seems to work, however it might be better to do just this:

This can be racy if two drivers are simultaneously trying to request an IRQ. 'data' must be unique to different threads if they are to avoid interfering with each other.

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
#3 lore raw
Quoted 7 lines · Russell King
Quoted 2 lines · Dominik Brodowski

Thanks for the excellent debugging. Your patch seems to work, however it might be better to do just this:

This can be racy if two drivers are simultaneously trying to request an IRQ. 'data' must be unique to different threads if they are to avoid interfering with each other.

As it's enough to keep PCMCIA functions apart (there can't be two drivers registering with the same PCMCIA function at the same moment), I'll use that now.

	void *data = &p_dev->dev.driver; /* something unique to this device */

Thanks,
Dominik