Re: [PATCH] bcm47xx: Fix GPIO API return codes

[Michael Buesch <mb@bu3sch.de>]

On Wednesday 18 February 2009 01:29:18 Andrew Morton wrote:
> On Sat, 14 Feb 2009 21:27:19 +0100
> Michael Buesch <mb@bu3sch.de> wrote:
> 
> > The GPIO API is supposed to return 0 or a negative error code,
> > but the SSB GPIO functions return the bitmask of the GPIO register.
> > Fix this by ignoring the bitmask and always returning 0. The SSB GPIO functions can't fail.
> > 
> > Signed-off-by: Michael Buesch <mb@bu3sch.de>
> > 
> > ---
> > 
> > Index: linux-2.6/arch/mips/include/asm/mach-bcm47xx/gpio.h
> > ===================================================================
> > --- linux-2.6.orig/arch/mips/include/asm/mach-bcm47xx/gpio.h	2009-01-01 19:27:06.000000000 +0100
> > +++ linux-2.6/arch/mips/include/asm/mach-bcm47xx/gpio.h	2009-02-14 21:26:14.000000000 +0100
> > @@ -31,24 +31,28 @@ static inline void gpio_set_value(unsign
> >  
> >  static inline int gpio_direction_input(unsigned gpio)
> >  {
> > -	return ssb_gpio_outen(&ssb_bcm47xx, 1 << gpio, 0);
> > +	ssb_gpio_outen(&ssb_bcm47xx, 1 << gpio, 0);
> > +	return 0;
> >  }
> >  
> >  static inline int gpio_direction_output(unsigned gpio, int value)
> >  {
> > -	return ssb_gpio_outen(&ssb_bcm47xx, 1 << gpio, 1 << gpio);
> > +	ssb_gpio_outen(&ssb_bcm47xx, 1 << gpio, 1 << gpio);
> > +	return 0;
> >  }
> >  
> > -static int gpio_intmask(unsigned gpio, int value)
> > +static inline int gpio_intmask(unsigned gpio, int value)
> >  {
> > -	return ssb_gpio_intmask(&ssb_bcm47xx, 1 << gpio,
> > -				value ? 1 << gpio : 0);
> > +	ssb_gpio_intmask(&ssb_bcm47xx, 1 << gpio,
> > +			 value ? 1 << gpio : 0);
> > +	return 0;
> >  }
> >  
> > -static int gpio_polarity(unsigned gpio, int value)
> > +static inline int gpio_polarity(unsigned gpio, int value)
> >  {
> > -	return ssb_gpio_polarity(&ssb_bcm47xx, 1 << gpio,
> > -				 value ? 1 << gpio : 0);
> > +	ssb_gpio_polarity(&ssb_bcm47xx, 1 << gpio,
> > +			  value ? 1 << gpio : 0);
> > +	return 0;
> >  }
> 
> What are the consequences of the bug which you fixed?  User-visible
> runtime failures?  Something else?
> 
> Please always include this information in the changelogs - without it I
> cannot make which-kernel-needs-this decisions.

Well, it fixes all users of the API that don't ignore the return value.
"Fixes" means "turns them from completely not working at all into - working properly".
This is the case for gpio-spi, for example. gpio-spi doesn't work on bcm47xx without
this fix. gpio-spi is in 2.6.29.
I didn't search for other API-users that might be broken.

So I'd like to see this going to Linus as soon as possible.

-- 
Greetings, Michael.