Re: [git patch] free_irq() fixes

[Jeff Garzik <jeff@garzik.org>]

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

Linus Torvalds wrote:
> Quite frankly, I'd actually prefer to just reinstate "dev" to the irq 
> registration instead.
> 
> Why? Because that field is how we are able to track multiple interrupt 
> registrations that share an IRQ. Now, the "request_irq()" logic has a 
> special rule that actually tests that NULL is a valid cookie if the 
> IRQF_SHARED bit isn't set, but isn't it a nice thing to double-check 
> regardless?
[...]
> This, in contrast, *really* sucks as a cookie. In fact, it's useless. If 
> there are multiple tp3780i instances on the same irq, they will always 
> have the same cookie.

AFAICS that will never happen for mwave hardware.

Nonetheless, it's a good point that NULL fails for disambiguation, so I 
created the attached, which should do what you want.  I'll push it after 
the compiler gives it a green light (unless it needs further revisions)

(note, for mwave I couldn't use pSettings, since that might fail the 
ambiguity test)

	Jeff




[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3646 bytes --]

diff --git a/arch/arm/mach-integrator/time.c b/arch/arm/mach-integrator/time.c
index 5235f64..8508a0d 100644
--- a/arch/arm/mach-integrator/time.c
+++ b/arch/arm/mach-integrator/time.c
@@ -124,8 +124,11 @@ static int rtc_probe(struct amba_device *dev, void *id)
 
 	xtime.tv_sec = __raw_readl(rtc_base + RTC_DR);
 
+	/* note that 'dev' is merely used for irq disambiguation;
+	 * it is not actually referenced in the irq handler
+	 */
 	ret = request_irq(dev->irq[0], arm_rtc_interrupt, IRQF_DISABLED,
-			  "rtc-pl030", NULL);
+			  "rtc-pl030", dev);
 	if (ret)
 		goto map_out;
 
diff --git a/drivers/char/mwave/tp3780i.c b/drivers/char/mwave/tp3780i.c
index 37fe80d..35e1723 100644
--- a/drivers/char/mwave/tp3780i.c
+++ b/drivers/char/mwave/tp3780i.c
@@ -97,16 +97,17 @@ static void EnableSRAM(THINKPAD_BD_DATA * pBDData)
 
 static irqreturn_t UartInterrupt(int irq, void *dev_id)
 {
-	int irqno = (int)(unsigned long) dev_id;
+	unsigned short *irqno = dev_id;
 
 	PRINTK_3(TRACE_TP3780I,
-		"tp3780i::UartInterrupt entry irq %x dev_id %p\n", irqno, dev_id);
+		"tp3780i::UartInterrupt entry irq %hx dev_id %p\n",
+		*irqno, dev_id);
 	return IRQ_HANDLED;
 }
 
 static irqreturn_t DspInterrupt(int irq, void *dev_id)
 {
-	int irqno = (int)(unsigned long) dev_id;
+	unsigned short *irqno = dev_id;
 
 	pMWAVE_DEVICE_DATA pDrvData = &mwave_s_mdd;
 	DSP_3780I_CONFIG_SETTINGS *pSettings = &pDrvData->rBDData.rDspSettings;
@@ -114,7 +115,8 @@ static irqreturn_t DspInterrupt(int irq, void *dev_id)
 	unsigned short usIPCSource = 0, usIsolationMask, usPCNum;
 
 	PRINTK_3(TRACE_TP3780I,
-		"tp3780i::DspInterrupt entry irq %x dev_id %p\n", irqno, dev_id);
+		"tp3780i::DspInterrupt entry irq %hx dev_id %p\n",
+		*irqno, dev_id);
 
 	if (dsp3780I_GetIPCSource(usDspBaseIO, &usIPCSource) == 0) {
 		PRINTK_2(TRACE_TP3780I,
@@ -274,7 +276,7 @@ int tp3780I_ReleaseResources(THINKPAD_BD_DATA * pBDData)
 	release_region(pSettings->usDspBaseIO & (~3), 16);
 
 	if (pSettings->bInterruptClaimed) {
-		free_irq(pSettings->usDspIrq, NULL);
+		free_irq(pSettings->usDspIrq, &pSettings->usDspIrq);
 		pSettings->bInterruptClaimed = FALSE;
 	}
 
@@ -366,15 +368,15 @@ int tp3780I_EnableDSP(THINKPAD_BD_DATA * pBDData)
 	pSettings->usChipletEnable = TP_CFG_ChipletEnable;
 
 	if (request_irq(pSettings->usUartIrq, &UartInterrupt, 0, "mwave_uart",
-			(void *)(unsigned long) pSettings->usUartIrq)) {
+			&pSettings->usUartIrq)) {
 		PRINTK_ERROR(KERN_ERR_MWAVE "tp3780i::tp3780I_EnableDSP: Error: Could not get UART IRQ %x\n", pSettings->usUartIrq);
 		goto exit_cleanup;
 	} else {		/* no conflict just release */
-		free_irq(pSettings->usUartIrq, NULL);
+		free_irq(pSettings->usUartIrq, &pSettings->usUartIrq);
 	}
 
 	if (request_irq(pSettings->usDspIrq, &DspInterrupt, 0, "mwave_3780i",
-			(void *)(unsigned long) pSettings->usDspIrq)) {
+			&pSettings->usDspIrq)) {
 		PRINTK_ERROR("tp3780i::tp3780I_EnableDSP: Error: Could not get 3780i IRQ %x\n", pSettings->usDspIrq);
 		goto exit_cleanup;
 	} else {
@@ -411,7 +413,7 @@ exit_cleanup:
 	if (bDSPPoweredUp)
 		smapi_set_DSP_power_state(FALSE);
 	if (bInterruptAllocated) {
-		free_irq(pSettings->usDspIrq, NULL);
+		free_irq(pSettings->usDspIrq, &pSettings->usDspIrq);
 		pSettings->bInterruptClaimed = FALSE;
 	}
 	return -EIO;
@@ -428,7 +430,7 @@ int tp3780I_DisableDSP(THINKPAD_BD_DATA * pBDData)
 	if (pBDData->bDSPEnabled) {
 		dsp3780I_DisableDSP(&pBDData->rDspSettings);
 		if (pSettings->bInterruptClaimed) {
-			free_irq(pSettings->usDspIrq, NULL);
+			free_irq(pSettings->usDspIrq, &pSettings->usDspIrq);
 			pSettings->bInterruptClaimed = FALSE;
 		}
 		smapi_set_DSP_power_state(FALSE);