[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[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.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
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(-)

diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
index 260a56d..4492c28 100644
--- a/board/nvidia/seaboard/seaboard.c
+++ b/board/nvidia/seaboard/seaboard.c
@@ -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)
diff --git a/drivers/gpio/tegra2_gpio.c b/drivers/gpio/tegra2_gpio.c
index f686e80..22669b6 100644
--- a/drivers/gpio/tegra2_gpio.c
+++ b/drivers/gpio/tegra2_gpio.c
@@ -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

[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[Simon Glass]

Hi Stephen,

On Thu, Oct 6, 2011 at 3:52 PM, Stephen Warren <swarren@nvidia.com> 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.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Acked-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>

> ---
> 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(-)
>
> diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
> index 260a56d..4492c28 100644
> --- a/board/nvidia/seaboard/seaboard.c
> +++ b/board/nvidia/seaboard/seaboard.c
> @@ -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)
> diff --git a/drivers/gpio/tegra2_gpio.c b/drivers/gpio/tegra2_gpio.c
> index f686e80..22669b6 100644
> --- a/drivers/gpio/tegra2_gpio.c
> +++ b/drivers/gpio/tegra2_gpio.c
> @@ -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
>
>

[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[Stephen Warren]

On 10/11/2011 06:07 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Thu, Oct 6, 2011 at 3:52 PM, Stephen Warren <swarren@nvidia.com> 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.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Acked-by: Simon Glass <sjg@chromium.org>
> Tested-by: Simon Glass <sjg@chromium.org>

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

>> ---
>> 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(-)
>>
>> diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
>> index 260a56d..4492c28 100644
>> --- a/board/nvidia/seaboard/seaboard.c
>> +++ b/board/nvidia/seaboard/seaboard.c
>> @@ -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)
>> diff --git a/drivers/gpio/tegra2_gpio.c b/drivers/gpio/tegra2_gpio.c
>> index f686e80..22669b6 100644
>> --- a/drivers/gpio/tegra2_gpio.c
>> +++ b/drivers/gpio/tegra2_gpio.c
>> @@ -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

-- 
nvpublic

[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[Anatolij Gustschin]

Hi Stephen,

On Thu,  6 Oct 2011 16:52:22 -0600
Stephen Warren <swarren@nvidia.com> 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.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> 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(-)

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

[U-Boot] [PATCH] tegra2: Use new GPIO APIs in gpio_config_uart()

[Stephen Warren]

Anatolij Gustschin wrote at Thursday, December 01, 2011 4:09 PM:
> On Thu,  6 Oct 2011 16:52:22 -0600
> Stephen Warren <swarren@nvidia.com> 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
...
> 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!

-- 
nvpublic