Subject: Re: [PATCH] of/i2c: don't pass -1 to irq_dispose_mapping, otherwise kernel will oops

From: Anton Vorontsov <[email-masked]>
Date: Fri, 11 Jul 2008 23:19:30 +0400

On Fri, Jul 11, 2008 at 09:11:02PM +0200, Jochen Friedrich wrote:
> Hi Anton,
> 
> > diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
> > index b2ccdcb..95a24de 100644
> > --- a/drivers/of/of_i2c.c
> > +++ b/drivers/of/of_i2c.c
> > @@ -93,10 +93,8 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
> >  		if (info.irq == NO_IRQ)
> >  			info.irq = -1;
> >  
> > -		if (of_find_i2c_driver(node, &info) < 0) {
> > -			irq_dispose_mapping(info.irq);
> > -			continue;
> > -		}
> > +		if (of_find_i2c_driver(node, &info) < 0)
> > +			goto err;
> >  
> >  		info.addr = *addr;
> >  
> > @@ -107,9 +105,12 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
> >  			printk(KERN_ERR
> >  			       "of-i2c: Failed to load driver for %s\n",
> >  			       info.type);
> > -			irq_dispose_mapping(info.irq);
> > -			continue;
> > +			goto err;
> >  		}
> > +		continue;
> > +err:
> > +		if (info.irq > 0)
> > +			irq_dispose_mapping(info.irq);
> 
> Shouldn't this be:
> 		if (info.irq != NO_IRQ)

Maybe, maybe not. See

http://lkml.org/lkml/2005/11/21/211

I.e. maybe we should use "if (irq)". Or if we should use NO_IRQ, then we
need to fixup all the users of .irq.

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2