Hi Simon,
Thank you for comments.
On Sun, Jan 27, 2013 at 1:29 AM, Simon Glass sjg@chromium.org wrote:
Hi Rajeshwari,
On Wed, Jan 23, 2013 at 2:48 AM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
This patch enables GPIO Command for EXYNOS5. Function has been added to asm/gpio.h to decode the input gpio name to gpio number. example: gpio set gpa00 GPIO_INPUT in cmd_gpio.c has been modified to GPIO_DIRECTION_INPUT as GPIO_INPUT is alraedy defined in exynos5 and leading to a error.
Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
Great to see this - some comments below.
Chnages in V2: - New patch arch/arm/include/asm/arch-exynos/gpio.h | 88 +++++++++++++++++++++++++++++++ common/cmd_gpio.c | 6 +- include/configs/exynos5250-dt.h | 1 + 3 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/gpio.h b/arch/arm/include/asm/arch-exynos/gpio.h index af882dd..66e4a8e 100644 --- a/arch/arm/include/asm/arch-exynos/gpio.h +++ b/arch/arm/include/asm/arch-exynos/gpio.h @@ -657,6 +657,94 @@ static inline unsigned int s5p_gpio_part_max(int nr) void gpio_cfg_pin(int gpio, int cfg); void gpio_set_pull(int gpio, int mode); void gpio_set_drv(int gpio, int mode);
+#include <common.h>
+static inline int name_to_gpio(const char *name) +{
int num;name++;if (*name == 'p') {++name;switch (*name) {case 'a':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_A00 + num;break;case 'b':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_B00 + num;break;case 'c':name++;num = simple_strtoul(name, NULL, 10);if (num >= 40)num = GPIO_C40 + (num - 40);else {num = simple_strtoul(name, NULL, 8);num = GPIO_C00 + num;}break;case 'd':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_D00 + num;break;case 'y':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_Y00 + num;break;case 'x':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_X00 + num;break;case 'e':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_E00 + num;break;case 'f':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_F00 + num;break;case 'g':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_G00 + num;break;case 'h':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_H00 + num;break;case 'v':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_V00 + num;break;case 'z':name++;num = simple_strtoul(name, NULL, 8);num = GPIO_Z0 + num;break;default:It seems like you need a table (C struct) containing the GPIO letter and the associated GPIO_... value. Then this code could become a for() loop to search for the match. This would reduce code duplication. 'c' would presumable still be a special case.
-Okay
return -1;}} elsereturn -1;return num;+}
+#define name_to_gpio(n) name_to_gpio(n) #endif
/* Pin configurations */ diff --git a/common/cmd_gpio.c b/common/cmd_gpio.c index 47eee89..450e6d1 100644 --- a/common/cmd_gpio.c +++ b/common/cmd_gpio.c @@ -16,7 +16,7 @@ #endif
enum gpio_cmd {
GPIO_INPUT,
GPIO_DIRECTION_INPUT,Actually I think it is exynos that should change - perhaps put an EXYNOS prefix on that one.
Ya but since we use a generic driver for S5P only renaming with EXYNOS prefix is not correct. Hence I had renamed in cmd_gpio.c file as it would not effect anything else.
GPIO_SET, GPIO_CLEAR, GPIO_TOGGLE,@@ -44,7 +44,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
/* parse the behavior */ switch (*str_cmd) {
case 'i': sub_cmd = GPIO_INPUT; break;
case 'i': sub_cmd = GPIO_DIRECTION_INPUT; break; case 's': sub_cmd = GPIO_SET; break; case 'c': sub_cmd = GPIO_CLEAR; break; case 't': sub_cmd = GPIO_TOGGLE; break;@@ -63,7 +63,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
/* finally, let's do it: set direction and exec command */
if (sub_cmd == GPIO_INPUT) {
if (sub_cmd == GPIO_DIRECTION_INPUT) { gpio_direction_input(gpio); value = gpio_get_value(gpio); } else {diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 7b9c393..b20b8f2 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -113,6 +113,7 @@ #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT #define CONFIG_CMD_NET +#define CONFIG_CMD_GPIO
#define CONFIG_BOOTDELAY 3
#define CONFIG_ZERO_BOOTDELAY_CHECK
1.7.4.4
Can you also please implement gpio_info(), so it will list out the GPIOs by name, and their state?
Okay would take it up as separate patch. What details would we exactly print in state info?
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot