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
