Messages in this thread Patch in this message | | | Date | Tue, 22 Apr 2008 18:59:49 -0400 | From | Jeff Garzik <> | Subject | Re: [git patch] free_irq() fixes |
| |
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
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); | |