[U-Boot] [RFC PATCH 1/3] at91: add new gpio pin macros

[Andreas Bießmann]

Hi Bo,

+Jens, he did the rewrite of the at91 gpio driver.

On 10/30/2013 10:56 AM, Bo Shen wrote:
> On 10/29/2013 20:53, Andreas Bießmann wrote:
>> Signed-off-by: Andreas Bießmann <andreas.devel at googlemail.com>
>> ---
>>   arch/arm/include/asm/arch-at91/gpio.h |   34
>> ++++++++++++++++++++++++++++-----
>>   1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-at91/gpio.h
>> b/arch/arm/include/asm/arch-at91/gpio.h
>> index 0700427..f9ca11e 100644
>> --- a/arch/arm/include/asm/arch-at91/gpio.h
>> +++ b/arch/arm/include/asm/arch-at91/gpio.h
>> @@ -227,8 +227,32 @@ static inline unsigned pin_to_mask(unsigned pin)
>>       at91_set_pio_value((x - PIN_BASE) / 32,(x % 32), y)
>>   #define at91_get_gpio_value(x) \
>>       at91_get_pio_value((x - PIN_BASE) / 32,(x % 32))
>> -#else
>> -#define at91_set_gpio_value(x, y)    at91_set_pio_value(x, y)
>> -#define at91_get_gpio_value(x)        at91_get_pio_value(x)
>> -#endif
>> -#endif
>> +#else /* CONFIG_ATMEL_LEGACY */
>> +
>> +#define GPIO_PIOA_BASE  (0)
>> +#define GPIO_PIOB_BASE  (GPIO_PIOA_BASE + 32)
>> +#define GPIO_PIOC_BASE  (GPIO_PIOB_BASE + 32)
>> +#define GPIO_PIOD_BASE  (GPIO_PIOC_BASE + 32)
>> +#define GPIO_PIOE_BASE  (GPIO_PIOD_BASE + 32)
>> +#define GPIO_PIN_PA(x)  (GPIO_PIOA_BASE + (x))
>> +#define GPIO_PIN_PB(x)  (GPIO_PIOB_BASE + (x))
>> +#define GPIO_PIN_PC(x)  (GPIO_PIOC_BASE + (x))
>> +#define GPIO_PIN_PD(x)  (GPIO_PIOD_BASE + (x))
>> +#define GPIO_PIN_PE(x)  (GPIO_PIOE_BASE + (x))
>> +
>> +#define at91_gpio_to_port(gpio) (gpio / 32)
>> +#define at91_gpio_to_pin(gpio)  (gpio % 32)
> 
> This is an exception for at91sam9x5 and at91sam9n12 SoCs, which PIOB and
> PIOD are not 32 pins. PIOB only has 19 pins, while PIOD only has 22 pins.

I think this is a job for gpio_is_valid() then. How is this case handled
in kernel?

Here are a few things to discuss. First of all I'd like to get some
insights why the at91 gpio API was changed back in 2010. It was a plain
number before and was changed to PORT + PIN as two separate parameters.
Jens, could you please shed some light on this?

The second thing is this specific RFC implementation. It contains again
different points.

 a) it defines another way to get a plain number for a GPIO
    (GPIO_PIN_Px())

I know that the kernel uses AT91_PIN_Pxy notation for defining a GPIO
number. The above approach is copied from AVR32 which I think is a bit
better cause of smaller include file. This change however should be
discussed here.

 b) the fact that we shift back to a plain number for defining a GPIO

I'd like to have a plain number defining a GPIO (which doesn't need to
correlate to a number in SoC spec's). It is just a plain number for
feeding generic gpio API. Which I'd like to use in the drivers to be
able to share with AVR32 or other generic stuff like status led API.
Currently some drivers do not define the CONFIG_ATMEL_LEGACY (to hide
the old defines) but use the PORT + PIN pair for example in pm9g45:

#define	CONFIG_RED_LED		AT91_PIO_PORTD, 31

and this is ugly!

Just to mention, I'd like to hide the current API of PORT + PIN as
separate parameters but not delete it. It is Ok in at91 specific
surrounding but where we share code with others we need to use that
single number that represents a GPIO. Especially is it misleading
approach in board headers to define the PORT + PIN tuple as an single
parameter to be placed in by preprocessor.

Any comments?

Best regards

Andreas Bießmann