lkml.org 
[lkml]   [2008]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [git patch] free_irq() fixes


On Tue, 22 Apr 2008, Jeff Garzik wrote:
>
> diff --git a/arch/arm/mach-integrator/time.c b/arch/arm/mach-integrator/time.c
> index 5235f64..28726ee 100644
> --- a/arch/arm/mach-integrator/time.c
> +++ b/arch/arm/mach-integrator/time.c
> @@ -137,7 +137,7 @@ static int rtc_probe(struct amba_device *dev, void *id)
> return 0;
>
> irq_out:
> - free_irq(dev->irq[0], dev);
> + free_irq(dev->irq[0], NULL);
> map_out:
> iounmap(rtc_base);
> rtc_base = NULL;
> @@ -153,7 +153,7 @@ static int rtc_remove(struct amba_device *dev)
>
> writel(0, rtc_base + RTC_CR);
>
> - free_irq(dev->irq[0], dev);
> + free_irq(dev->irq[0], NULL);
> unregister_rtc(&rtc_ops);
>
> iounmap(rtc_base);

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?



> index 37fe80d..5681c01 100644
> --- a/drivers/char/mwave/tp3780i.c
> +++ b/drivers/char/mwave/tp3780i.c
> @@ -274,7 +274,8 @@ 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,
> + (void *)(unsigned long) pSettings->usDspIrq);

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.

That's why we pass in a "device" pointer or similar - exactly to make sure
that the cookie is unique for that irq resource!

So it would be much nicer to fix the cookie to be a real device pointer
(why not "pSettings" itself?) that is stable across the whole series of
request_irq/free_irq. And that also would tend to get rid of the
butt-ugly casts.

Willing to fix those up?

Linus


\
 
 \ /
  Last update: 2008-04-23 00:29    [W:0.073 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site