M28: Added pin name support in GPIO driver

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.

Please note:
Function gpio_name_to_pin checks for set bits getting lost in translation and returns -EINVAL if this occurs, but a correctly translated name doesn't automatically produce a valid pin. It only means the bank/pin numbers fit the designated bitfields. After successful translation, validity needs to be checked separately.

Signed-off-by Robert Deli?n <[email protected]>

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-mx28/gpio.h index 36f3be8..1eb54ee 100644

arch/arm/include/asm/arch-mx28/gpio.h +0 -0

 extern int gpio_invalid(int gp);
 #define gpio_invalid(gpio)	gpio_invalid(gpio)
 

(quoted diff) +0 -0

b/arch/blackfin/include/asm/gpio.h index 224688f..8128df1 100644

arch/blackfin/include/asm/gpio.h +20 -3

@@ -1,7 +1,24 @@
 /*
- * Copyright 2006-2009 Analog Devices Inc.
+ * (C) Copyright 2006-2009
+ * Analog Devices Inc.
 *
- * Licensed under the GPL-2 or later.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
 */
 
 #ifndef __ARCH_BLACKFIN_GPIO_H__

@@ -199,7 +216,7 @@ static inline int gpio_is_valid(int number)
 
 #include <linux/ctype.h>
 
-static inline int name_to_gpio(const char *name)
+static inline int gpio_name_to_pin(const char *name)
 {
 	int port_base;
 

arch/blackfin/include/asm/gpio.h +0 -0

 #define gpio_status() bfin_gpio_labels()
 

common/cmd_gpio.c +20 -5

index f81c7d8..d214f7a 100644
@@ -1,9 +1,24 @@
 /*
- * Control GPIO pins on the fly
+ * (C) Copyright 2008-2011
+ * Analog Devices Inc.
 *
- * Copyright (c) 2008-2011 Analog Devices Inc.
+ * See file CREDITS for list of people who contributed to this
+ * project.
 *
- * Licensed under the GPL-2 or later.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
 */
 
 #include <common.h>

@@ -11,8 +26,8 @@
 
 #include <asm/gpio.h>
 
-#ifndef name_to_gpio
-#define name_to_gpio(name) simple_strtoul(name, NULL, 10)
+#ifndef gpio_name_to_pin
+#define gpio_name_to_pin(name)	simple_strtoul(name, NULL, 10)
 #endif
 
 #ifndef gpio_invalid

@@ -36,13 +51,13 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int

argc, char * const argv[]) #ifdef gpio_status

	if (argc == 2 && !strcmp(argv[1], "status")) {
		gpio_status();

common/cmd_gpio.c +1 -1

-		return 0;
+		return (0);

	}

#endif

	if (argc != 3)

- show_usage:

common/cmd_gpio.c +2 -1

-		return cmd_usage(cmdtp);
+		goto show_usage;
+

	str_cmd = argv[1];
	str_gpio = argv[2];

common/cmd_gpio.c +5 -3

@@ -56,9 +71,11 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc,
 char * const argv[]) }
 
 	/* turn the gpio name into a gpio number */
-	gpio = name_to_gpio(str_gpio);
-	if (gpio < 0)
-		goto show_usage;
+	gpio = gpio_name_to_pin(str_gpio);
+	if (gpio < 0) {
+		printf("gpio: unknown pin %s\n", str_gpio);
+		return (-1);
+	}
 
 	/* check bank and pin number for validity */
 	if (gpio_invalid(gpio)) {

@@ -69,7 +86,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc,

char * const argv[]) /* grab the pin before we tweak it */

	if (gpio_request(gpio, "cmd_gpio")) {
		printf("gpio: requesting pin %u failed\n", gpio);

common/cmd_gpio.c +1 -1

-		return -1;
+		return (-1);

	}

	/* finally, let's do it: set direction and exec command */

common/cmd_gpio.c +2 -1

@@ -90,7 +107,9 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc,
 char * const argv[])
 
 	gpio_free(gpio);
 
-	return value;
+	return (value);
+show_usage:
 }
 

U_BOOT_CMD(gpio, 3, 0, do_gpio,

drivers/gpio/mxs_gpio.c +25 -0

index d2e9bac..50a0e6a 100644
@@ -23,6 +23,7 @@
 * MA 02111-1307 USA
 */
 
+#include <linux/ctype.h>
 #include <common.h>
 #include <netdev.h>
 #include <asm/errno.h>

@@ -81,6 +82,45 @@ void mxs_gpio_init(void)
 	}
 }
 
+int gpio_name_to_pin(const char *name)
+{
+	int bank;
+	int pin;
+
+	if (!name)
+		return (-EINVAL);
+
+	if (name[0] >= '0' && name[0] <= '9') {
+		pin = simple_strtoul(name, (char **)&name, 10);
+		if (name[0])
+			return (-EINVAL);
+		if (pin & ~(MXS_PAD_BANK_MASK | MXS_PAD_PIN_MASK))
+			return (-EINVAL);
+
+		return (pin);
+	}
+
+	if (tolower(name[0]) != 'b')
+		return (-EINVAL);
+	name++;
+	bank = simple_strtoul(name, (char **)&name, 10);
+	if (bank & ~(MXS_PAD_BANK_MASK >> MXS_PAD_BANK_SHIFT))
+		return (-EINVAL);

Review

Quoted 3 lines

(quoted diff) +3 -0

+
+	if (name[0])
+		return (-EINVAL);

Why the parenthesis ?

Quoted 8 lines

(quoted diff) +5 -0

+
+	return ( ((bank << MXS_PAD_BANK_SHIFT) & MXS_PAD_BANK_MASK) |
+		 ((pin  << MXS_PAD_PIN_SHIFT ) & MXS_PAD_PIN_MASK ) );
+}
+

int gpio_invalid(int gp)

{
	if ( (gp & ~(MXS_PAD_BANK_MASK | MXS_PAD_PIN_MASK)) ||

This was there before or are we missing some previous commit here ?

Quoted 5 lines

Review

Quoted 3 lines

(quoted diff) +3 -0

+
+	if (name[0])
+		return (-EINVAL);

Why the parenthesis ?

Because you asked for them? But OK, I'll remove them again...

Quoted 9 lines

Quoted 9 lines

(quoted diff) +5 -0

+
+	return ( ((bank << MXS_PAD_BANK_SHIFT) & MXS_PAD_BANK_MASK) |
+		 ((pin  << MXS_PAD_PIN_SHIFT ) & MXS_PAD_PIN_MASK ) );
+}
+

int gpio_invalid(int gp)
{

Quoted 3 lines

Quoted 1 line

	if ( (gp & ~(MXS_PAD_BANK_MASK | MXS_PAD_PIN_MASK)) ||

This was there before or are we missing some previous commit here ?

To apply cleanly, this patch needs the pin validity patch applied first. However, it will apply without it, yet not cleanly.

I asked you to add braces around the else part of the branch! if { ... } else { ... } ... this stuff. Sorry for the confusion!

I think I didn't see the pin validity patch then.