Skip to contentSkip to conversation

tegra2: Use new GPIO APIs in gpio_config_uart()

PATCH New Stephen Warren 3 participants 5 messages Oct 6, 2011 – Dec 1, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tegra2: Use new GPIO APIs in gpio_config_uart()

... rather than open-coding the register accesses.

However, gpio_request() typically stores the "label" parameter in a global data structure. This causes problems when called from gpio_config_uart(), since the code is running before relocation. To solve this, pass a NULL string to gpio_request(), and modify gpio_request() not to touch the string if it's NULL.

---
I tested this on Seaboard. With the code in the patch below, the UART works. If I comment out the body of gpio_config_uart_seaboard(), the UART does not work.

This patch should work with ToT U-Boot, but the context relies on the rename of gpio_config_uart() to gpio_config_uart_seaboard() contained in:
http://patchwork.ozlabs.org/patch/118144/

 board/nvidia/seaboard/seaboard.c |   18 ++----------------
 drivers/gpio/tegra2_gpio.c       |    6 ++++--
 2 files changed, 6 insertions(+), 18 deletions(-)
board/nvidia/seaboard/seaboard.c +2 -16
index 260a56d..4492c28 100644
@@ -36,23 +36,9 @@
  */
 void gpio_config_uart_seaboard(void)
 {
-	int gp = GPIO_PI3;
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
-	u32 val;
-
 	/* Enable UART via GPIO_PI3 (port 8, bit 3) so serial console works */
-	val = readl(&bank->gpio_config[GPIO_PORT(gp)]);
-	val |= 1 << GPIO_BIT(gp);
-	writel(val, &bank->gpio_config[GPIO_PORT(gp)]);
-
-	val = readl(&bank->gpio_out[GPIO_PORT(gp)]);
-	val &= ~(1 << GPIO_BIT(gp));
-	writel(val, &bank->gpio_out[GPIO_PORT(gp)]);
-
-	val = readl(&bank->gpio_dir_out[GPIO_PORT(gp)]);
-	val |= 1 << GPIO_BIT(gp);
-	writel(val, &bank->gpio_dir_out[GPIO_PORT(gp)]);
+	gpio_request(GPIO_PI3, NULL);
+	gpio_direction_output(GPIO_PI3, 0);
 }
 
 void gpio_config_uart(void)
drivers/gpio/tegra2_gpio.c +4 -2
index f686e80..22669b6 100644
@@ -146,8 +146,10 @@ int gpio_request(int gp, const char *label)
 	if (gp >= MAX_NUM_GPIOS)
 		return -1;
 
-	strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
-	gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+	if (label != NULL) {
+		strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
+		gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+	}
 
 	/* Configure as a GPIO */
 	set_config(gp, 1);
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/1 tegra2: Use new GPIO APIs in gpio_config_uart()

Hi Stephen,

Quoted 9 lines · Stephen Warren

... rather than open-coding the register accesses.

However, gpio_request() typically stores the "label" parameter in a global data structure. This causes problems when called from gpio_config_uart(), since the code is running before relocation. To solve this, pass a NULL string to gpio_request(), and modify gpio_request() not to touch the string if it's NULL.

Quoted 64 lines

---
I tested this on Seaboard. With the code in the patch below, the UART works. If I comment out the body of gpio_config_uart_seaboard(), the UART does not work.

This patch should work with ToT U-Boot, but the context relies on the rename of gpio_config_uart() to gpio_config_uart_seaboard() contained in:
http://patchwork.ozlabs.org/patch/118144/

?board/nvidia/seaboard/seaboard.c | ? 18 ++---------------- ?drivers/gpio/tegra2_gpio.c ? ? ? | ? ?6 ++++--
?2 files changed, 6 insertions(+), 18 deletions(-)

board/nvidia/seaboard/seaboard.c +0 -0
index 260a56d..4492c28 100644
@@ -36,23 +36,9 @@

?*/
?void gpio_config_uart_seaboard(void)
?{
- ? ? ? int gp = GPIO_PI3;
- ? ? ? struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
- ? ? ? struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
- ? ? ? u32 val;
-
? ? ? ?/* Enable UART via GPIO_PI3 (port 8, bit 3) so serial console works */
- ? ? ? val = readl(&bank->gpio_config[GPIO_PORT(gp)]);
- ? ? ? val |= 1 << GPIO_BIT(gp);
- ? ? ? writel(val, &bank->gpio_config[GPIO_PORT(gp)]);

board/nvidia/seaboard/seaboard.c +2 -8
-
- ? ? ? val = readl(&bank->gpio_out[GPIO_PORT(gp)]);
- ? ? ? val &= ~(1 << GPIO_BIT(gp));
- ? ? ? writel(val, &bank->gpio_out[GPIO_PORT(gp)]);
-
- ? ? ? val = readl(&bank->gpio_dir_out[GPIO_PORT(gp)]);
- ? ? ? val |= 1 << GPIO_BIT(gp);
- ? ? ? writel(val, &bank->gpio_dir_out[GPIO_PORT(gp)]);
+ ? ? ? gpio_request(GPIO_PI3, NULL);
+ ? ? ? gpio_direction_output(GPIO_PI3, 0);

?}

?void gpio_config_uart(void)

drivers/gpio/tegra2_gpio.c +4 -2
index f686e80..22669b6 100644
@@ -146,8 +146,10 @@ int gpio_request(int gp, const char *label)
 ? ? ? ?if (gp >= MAX_NUM_GPIOS)
 ? ? ? ? ? ? ? ?return -1;
 
- ? ? ? strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
- ? ? ? gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+ ? ? ? if (label != NULL) {
+ ? ? ? ? ? ? ? strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
+ ? ? ? ? ? ? ? gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+ ? ? ? }
 

? ? ? ?/* Configure as a GPIO */
? ? ? ?set_config(gp, 1);
--
1.7.0.4

#2 lore raw

PATCH 1/1 tegra2: Use new GPIO APIs in gpio_config_uart()

Quoted 11 lines · Simon Glass

Hi Stephen,

Quoted 8 lines · Stephen Warren

... rather than open-coding the register accesses.

However, gpio_request() typically stores the "label" parameter in a global data structure. This causes problems when called from gpio_config_uart(), since the code is running before relocation. To solve this, pass a NULL string to gpio_request(), and modify gpio_request() not to touch the string if it's NULL.

Quoted 4 lines
Quoted 1 line

Albert, sorry I forgot to CC you on my original patch submission. Can you please also consider this patch. Thanks.

Quoted 62 lines
Quoted 62 lines

---
I tested this on Seaboard. With the code in the patch below, the UART works. If I comment out the body of gpio_config_uart_seaboard(), the UART does not work.

This patch should work with ToT U-Boot, but the context relies on the rename of gpio_config_uart() to gpio_config_uart_seaboard() contained in:
http://patchwork.ozlabs.org/patch/118144/

 board/nvidia/seaboard/seaboard.c |   18 ++----------------
 drivers/gpio/tegra2_gpio.c       |    6 ++++--
 2 files changed, 6 insertions(+), 18 deletions(-)
board/nvidia/seaboard/seaboard.c +2 -16
index 260a56d..4492c28 100644
@@ -36,23 +36,9 @@
 */
 void gpio_config_uart_seaboard(void)
 {
-       int gp = GPIO_PI3;
-       struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-       struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
-       u32 val;
-
       /* Enable UART via GPIO_PI3 (port 8, bit 3) so serial console works */
-       val = readl(&bank->gpio_config[GPIO_PORT(gp)]);
-       val |= 1 << GPIO_BIT(gp);
-       writel(val, &bank->gpio_config[GPIO_PORT(gp)]);
-
-       val = readl(&bank->gpio_out[GPIO_PORT(gp)]);
-       val &= ~(1 << GPIO_BIT(gp));
-       writel(val, &bank->gpio_out[GPIO_PORT(gp)]);
-
-       val = readl(&bank->gpio_dir_out[GPIO_PORT(gp)]);
-       val |= 1 << GPIO_BIT(gp);
-       writel(val, &bank->gpio_dir_out[GPIO_PORT(gp)]);
+       gpio_request(GPIO_PI3, NULL);
+       gpio_direction_output(GPIO_PI3, 0);
 }
 
 void gpio_config_uart(void)
drivers/gpio/tegra2_gpio.c +4 -3
index f686e80..22669b6 100644
@@ -146,8 +146,10 @@ int gpio_request(int gp, const char *label)
       if (gp >= MAX_NUM_GPIOS)
               return -1;
 
-       strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
-       gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+       if (label != NULL) {
+               strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
+               gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+       }
 
       /* Configure as a GPIO */
       set_config(gp, 1);
--

1.7.0.4

Signature · 1 line
nvpublic
#3 lore raw

PATCH 1/1 tegra2: Use new GPIO APIs in gpio_config_uart()

In reply to Stephen Warren

Hi Stephen,

Quoted 14 lines · On Thu, 6 Oct 2011 16:52:22 -0600 Stephen Warren <[email protected]> wrote:

... rather than open-coding the register accesses.

However, gpio_request() typically stores the "label" parameter in a global data structure. This causes problems when called from gpio_config_uart(), since the code is running before relocation. To solve this, pass a NULL string to gpio_request(), and modify gpio_request() not to touch the string if it's NULL.

---
I tested this on Seaboard. With the code in the patch below, the UART works. If I comment out the body of gpio_config_uart_seaboard(), the UART does not work.

Quoted 7 lines

This patch should work with ToT U-Boot, but the context relies on the rename of gpio_config_uart() to gpio_config_uart_seaboard() contained in:
http://patchwork.ozlabs.org/patch/118144/

 board/nvidia/seaboard/seaboard.c |   18 ++----------------
 drivers/gpio/tegra2_gpio.c       |    6 ++++--
 2 files changed, 6 insertions(+), 18 deletions(-)

This patch doesn't apply cleanly. There were a lot of tegra2 patches, so I'm not sure if it is not superseded by some newer patches. Can you please check and if the patch is needed, then re-base and resubmit. Thanks!

Anatolij

#4 lore raw

PATCH 1/1 tegra2: Use new GPIO APIs in gpio_config_uart()

Anatolij Gustschin wrote at Thursday, December 01, 2011 4:09 PM:

Quoted 10 lines
Quoted 7 lines · On Thu, 6 Oct 2011 16:52:22 -0600 Stephen Warren <[email protected]> wrote:

... rather than open-coding the register accesses.

However, gpio_request() typically stores the "label" parameter in a global data structure. This causes problems when called from gpio_config_uart(), since the code is running before relocation. To solve this, pass a NULL string to gpio_request(), and modify gpio_request() not to touch the string

...

Quoted 4 lines

This patch doesn't apply cleanly. There were a lot of tegra2 patches, so I'm not sure if it is not superseded by some newer patches. Can you please check and if the patch is needed, then re-base and resubmit. Thanks!

Yes, there are a lot of Tegra patches outstanding, and they need to be applied in a specific order. Tom Warren just got set up to create a u-boot-tegra.git and handle all the merging, so I think you can ignore this patch for now.

Thanks for taking a look though!

Signature · 1 line
nvpublic