Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO

From: Wolfgang Grandegger
Date: Mon Dec 17 2012 - 06:37:25 EST


Hi Roland,

On 12/15/2012 12:49 AM, Roland Stigge wrote:
> Hi Wolfgang,
>
> thank you for the patch!
>
> On 14/12/12 18:58, Wolfgang Grandegger wrote:
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> +
>> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> +}
>> +
>
> Without having an AT91 available right now, I guess the hardware
> interface of this GPIO chip is different from the GPIO block API. While
> the hardware has clear and set registers, the val parameter of
> at91_gpiolib_set_block() should be interpreted as the actual output
> values. See lpc32xx_gpo_set_block() for an example for handling set and
> clear registers like this: First, set_bits and clear_bits words are
> calculated from mask and val parameters, and finally written to the
> respective hardware registers.
>
> Note that one .set_block() can result in writing both the set and clear
> registers of the hardware when val contains both 0s and 1s in
> respectively masked positions.

Oops, I obviously did not test GPIO block write. The patch below does
work now. Feel free to add it to the next version of your series.

I tested with a block having both, inputs and outputs. The handling
of such a mixed block is clumsy because both, read and write do depend
on "block->cur_mask". It needs to be re-set when switching from read to
write or vice versa. Defining two blocks, one for input and the other
for output seems to be the better solution.

Hope this block gpio support will show up in mainline soon.

Thanks.

Wolfgang.