Subject: Re: [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero

From: Jean Delvare <[email-masked]>
Date: Fri, 2 May 2008 16:46:10 +0200

Hi Jon,

On Fri, 2 May 2008 10:23:01 -0400, Jon Smirl wrote:
> On 2/19/08, Jean Delvare <[email-masked]> wrote:
> >  >       i2c->irq = platform_get_irq(pdev, 0);
> >  > -     if (i2c->irq < 0) {
> >  > +     if (i2c->irq < NO_IRQ) {
> >
> >
> > I am skeptical about this one. Can platform_get_irq() really return
> >  NO_IRQ? I thought that the IRQ resource would be plain missing if the
> >  device has no IRQ, so I would expect:
> >
> >
> >         i2c->irq = platform_get_irq(pdev, 0);
> >         if (i2c->irq < 0)
> >
> >                 i2c->irq = NO_IRQ; /* Use polling */
> >
> >  Testing against NO_IRQ suggests that devices with no IRQ would still
> >  have an IRQ resource defined and explicitly set to NO_IRQ. Sounds weird
> >  to me. Can you please clarify this point?
> 
> Your fix is correct. I'm not sure polling worked in the original driver.

OK, can you send an updated patch then?

Thanks.

> >  For what it's worth, no other kernel driver checks for irq < NO_IRQ.
> >  They all check for irq < 0 after calling platform_get_irq().
> >
> >
> >  >               result = -ENXIO;
> >  >               goto fail_get_irq;
> >  >       }

-- 
Jean Delvare