Re: [PATCH] provide functions for gpio configuration

[Phil Sutter <n0-1@freewrt.org>]

Hi,

On Mon, Nov 03, 2008 at 12:56:16AM +0300, Sergei Shtylyov wrote:
> >+	val &= ~( ~bitval << offset );   /* unset bit if bitval == 0 */
> >  
> 
>   If bitval == 0, ~bitval evaluates to all ones, and the final AND mask 
> ~(0xffffffff << offset) clears 32-offset MSBs. What you want is simply 
> ~(1 << offset).

Right, I messed up boolean and bitwise inverting. The correct line is:

| val &= ~(!bitval << offset);   /* unset bit if bitval == 0 */

That's the same as ~(1 << offset) when bitval is zero, but turns into a
no op when it's one. That's what I want here, successfully removing the
need for the if-else case.

> >@@ -176,117 +184,60 @@ static int rb532_gpio_direction_input(struct 
> >gpio_chip *chip, unsigned offset)
> > static int rb532_gpio_direction_output(struct gpio_chip *chip,
> > 					unsigned offset, int value)
> > {
> >-	unsigned long		flags;
> >-	u32			mask = 1 << offset;
> >-	u32			tmp;
> > 	struct rb532_gpio_chip	*gpch;
> >-	void __iomem		*gpdr;
> > 
> > 	gpch = container_of(chip, struct rb532_gpio_chip, chip);
> >-	writel(mask, gpch->regbase + GPIOD);
> >  
> 
>   Hm, the old code seems really borked here...

It's not as bad as this may look like. In fact I could just simplify
lots of the functions by having them call rb532_set_bit().

A patch fixing the above mentioned issue follows, thanks for your help!

Greetings, Phil