[U-Boot] [PATCH] M28: Added pin name support in GPIO driver

Marek Vasut
2011-11-22T18:28:10Z

> This patch adds adds pin name support in the GPIO driver. With this patch
> applied, the gpio command supports pins to be addressed with friendly
> names.
>
> The user's manual refers to pins by the bank number they're in and their
> number within that bank. With this patch the friendly naming convention to
> address pin number 5 in bank 3 is b3p5. But names like B00000003p005 are
> interpreted correctly too.
>
> Signed-off-by: Robert Deli?n <robert@delien.nl>
> diff --git a/arch/arm/include/asm/arch-mx28/gpio.h
> b/arch/arm/include/asm/arch-mx28/gpio.h index be1c944..5ae66e6 100644
> --- a/arch/arm/include/asm/arch-mx28/gpio.h
> +++ b/arch/arm/include/asm/arch-mx28/gpio.h

No, move this to mxs_gpio.c and simply make the function not static.

> @@ -23,8 +23,44 @@
> #ifndef __MX28_GPIO_H__
> #define __MX28_GPIO_H__
>
> +#include <linux/ctype.h>
> +#include <asm/errno.h>
> +#include <asm/arch/iomux.h>
> +
> #ifdef CONFIG_MXS_GPIO
> void mxs_gpio_init(void);
> +
> +static inline int name_to_gpio(const char *name)
> +{
> + int pin_nr;
> +
> + if (name[0] >= '0' && name[0] <= '9') {

So if I pass name == NULL, we're done for here ;-)

> + pin_nr = simple_strtoul(name, (char **)&name, 10);
> + if (name[0])
> + return -EINVAL;
> + else
> + return pin_nr;
> + }
> +
> + if (tolower(name[0]) == 'b') {
> + name++;
> + pin_nr = (simple_strtoul(name, (char **)&name, 10) <<
> MXS_PAD_BANK_SHIFT) & MXS_PAD_BANK_MASK; + } else
> + return -EINVAL;

Braces missing around this return statement. Also, why not do something like:

if (tolower(name[0]) != 'b')
return -EINVAL;

name++;
pinnr = ...

if (tolower(name[0]) != 'p')
return -EINVAL;

name++;
...

It seems more explicit to me that way.

> +
> + if (tolower(name[0]) == 'p') {
> + name++;
> + pin_nr |= (simple_strtoul(name, (char **)&name, 10) <<
> MXS_PAD_PIN_SHIFT) & MXS_PAD_PIN_MASK; + } else
> + return -EINVAL;
> +
> + if (name[0])
> + return -EINVAL;
> +
> + return pin_nr;
> +}
> +#define name_to_gpio(n) name_to_gpio(n)

What's this define for ?

> +
> #else
> inline void mxs_gpio_init(void) {}
> #endif

Do you even need this if CONFIG_CMD_GPIO is undefined? Move the function to
mxs_gpio.c and make it not static should work for you.

M