[U-Boot] [PATCH 1/4] gpio: Adds GPIO driver support for Armada100

[Mike Frysinger]

On Mon, Jul 18, 2011 at 05:41, Ajay Bhargav wrote:
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada100/gpio.h
>
> +#ifndef __ASSEMBLY__
> +#include <asm/types.h>
> +#endif
> ...
> +struct armdgpio_registers {
> +       u32 gplr0;      /* Pin Level Register - 0x0000 */
> ...

considering you dont protect actual C code in this file with
__ASSEMBLY__, it doesnt make much sense to protect the include

> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
>
>  COBJS-$(CONFIG_AT91_GPIO)      += at91_gpio.o
> +COBJS-$(CONFIG_ARMADA100_GPIO) += armada100_gpio.o

"R" comes before "T", so your new entry should be above the at91 one

> --- /dev/null
> +++ b/drivers/gpio/armada100_gpio.c
>
> +int gpio_request(int gp, const char *label)
> +{
> +       /*
> +        * Assumes corresponding MFP is configured peoperly
> +        * for use as GPIO
> +        */
> +       return 0;
> +}

i think you mean "properly" in the comment, but this assumption is
bogus.  error checking for valid pins has to be in the gpio_request()
so that all the other funcs can assume the pin is valid.  not the
other way around.

> +void gpio_free(int gp)
> +{
> +       /* default direction for GPIO is input */
> +       gpio_direction_input(gp);
> +}

usually people dont do this.  usually the free step leaves the pin
alone.  i know in the linux kernel, they assume (and thus require)
that behavior.
-mike