Patchwork [U-Boot,1/2] tegra: add alternate UART1 funcmux entry

login
register
mail settings
Submitter Stephen Warren
Date May 14, 2012, 11:13 p.m.
Message ID <1337037226-15589-1-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/159214/
State Accepted
Commit b9607e70614823893d1a47a377232d2a12e4464f
Headers show

Comments

Stephen Warren - May 14, 2012, 11:13 p.m.
From: Stephen Warren <swarren@nvidia.com>

(In at least some configurations) Whistler uses UART1 on pingroups
UAA, UAB.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/cpu/armv7/tegra2/board.c          |   14 +++++++++++++-
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   13 ++++++++++++-
 arch/arm/include/asm/arch-tegra2/funcmux.h |    1 +
 3 files changed, 26 insertions(+), 2 deletions(-)
Lucas Stach - May 15, 2012, 3:07 p.m.
Hello Stephen,

this patch looks reasonable. I also need to change the pingroup config
for UARTA on the Colibri T20 board and came up with a similar approach,
but I will rebase my work on top of this.

As I'm not in the position to give anything more official, I just wanted
to give this patch a public +1 to show that I'm in favour of this
change.

-- Lucas
Am Montag, den 14.05.2012, 17:13 -0600 schrieb Stephen Warren:
> From: Stephen Warren <swarren@nvidia.com>
> 
> (In at least some configurations) Whistler uses UART1 on pingroups
> UAA, UAB.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/cpu/armv7/tegra2/board.c          |   14 +++++++++++++-
>  arch/arm/cpu/armv7/tegra2/funcmux.c        |   13 ++++++++++++-
>  arch/arm/include/asm/arch-tegra2/funcmux.h |    1 +
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
> index a50b1b9..629ad5d 100644
> --- a/arch/arm/cpu/armv7/tegra2/board.c
> +++ b/arch/arm/cpu/armv7/tegra2/board.c
> @@ -101,6 +101,18 @@ int arch_cpu_init(void)
>  }
>  #endif
>  
> +static int uart_configs[] = {
> +#ifdef CONFIG_TEGRA2_UARTA_UAA_UAB
> +	FUNCMUX_UART1_UAA_UAB,
> +#else
> +	FUNCMUX_UART1_IRRX_IRTX,
> +#endif
> +	FUNCMUX_UART2_IRDA,
> +	-1,
> +	FUNCMUX_UART4_GMC,
> +	-1,
> +};
> +
>  /**
>   * Set up the specified uarts
>   *
> @@ -120,7 +132,7 @@ static void setup_uarts(int uart_ids)
>  		if (uart_ids & (1 << i)) {
>  			enum periph_id id = id_for_uart[i];
>  
> -			funcmux_select(id, FUNCMUX_DEFAULT);
> +			funcmux_select(id, uart_configs[i]);
>  			clock_ll_start_uart(id);
>  		}
>  	}
> diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
> index 0ef7753..e2d1273 100644
> --- a/arch/arm/cpu/armv7/tegra2/funcmux.c
> +++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
> @@ -31,11 +31,22 @@ int funcmux_select(enum periph_id id, int config)
>  
>  	switch (id) {
>  	case PERIPH_ID_UART1:
> -		if (config == FUNCMUX_UART1_IRRX_IRTX) {
> +		switch (config) {
> +		case FUNCMUX_UART1_IRRX_IRTX:
>  			pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
>  			pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
>  			pinmux_tristate_disable(PINGRP_IRRX);
>  			pinmux_tristate_disable(PINGRP_IRTX);
> +			break;
> +		case FUNCMUX_UART1_UAA_UAB:
> +			pinmux_set_func(PINGRP_UAA, PMUX_FUNC_UARTA);
> +			pinmux_set_func(PINGRP_UAB, PMUX_FUNC_UARTA);
> +			pinmux_tristate_disable(PINGRP_UAA);
> +			pinmux_tristate_disable(PINGRP_UAB);
> +			bad_config = 0;
> +			break;
> +		}
> +		if (!bad_config) {
>  			/*
>  			 * Tegra appears to boot with function UARTA pre-
>  			 * selected on mux group SDB. If two mux groups are
> diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
> index ae73c72..b455122 100644
> --- a/arch/arm/include/asm/arch-tegra2/funcmux.h
> +++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
> @@ -30,6 +30,7 @@ enum {
>  
>  	/* UART configs */
>  	FUNCMUX_UART1_IRRX_IRTX = 0,
> +	FUNCMUX_UART1_UAA_UAB,
>  	FUNCMUX_UART2_IRDA = 0,
>  	FUNCMUX_UART4_GMC = 0,
>
Simon Glass - May 22, 2012, 12:47 a.m.
Hi Stephen,

On Mon, May 14, 2012 at 4:13 PM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> From: Stephen Warren <swarren@nvidia.com>
>
> (In at least some configurations) Whistler uses UART1 on pingroups
> UAA, UAB.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/cpu/armv7/tegra2/board.c          |   14 +++++++++++++-
>  arch/arm/cpu/armv7/tegra2/funcmux.c        |   13 ++++++++++++-
>  arch/arm/include/asm/arch-tegra2/funcmux.h |    1 +
>  3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/tegra2/board.c
> b/arch/arm/cpu/armv7/tegra2/board.c
> index a50b1b9..629ad5d 100644
> --- a/arch/arm/cpu/armv7/tegra2/board.c
> +++ b/arch/arm/cpu/armv7/tegra2/board.c
> @@ -101,6 +101,18 @@ int arch_cpu_init(void)
>  }
>  #endif
>
> +static int uart_configs[] = {
> +#ifdef CONFIG_TEGRA2_UARTA_UAA_UAB
> +       FUNCMUX_UART1_UAA_UAB,
> +#else
> +       FUNCMUX_UART1_IRRX_IRTX,
> +#endif
>

We really shouldn't be using configs to deal with this. It should be a
run-time check.

I recall you saying that we could use the ODM bits for this?


> +       FUNCMUX_UART2_IRDA,
> +       -1,
> +       FUNCMUX_UART4_GMC,
> +       -1,
> +};
> +
>  /**
>  * Set up the specified uarts
>  *
> @@ -120,7 +132,7 @@ static void setup_uarts(int uart_ids)
>                if (uart_ids & (1 << i)) {
>                        enum periph_id id = id_for_uart[i];
>
> -                       funcmux_select(id, FUNCMUX_DEFAULT);
> +                       funcmux_select(id, uart_configs[i]);
>                        clock_ll_start_uart(id);
>                }
>        }
> diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c
> b/arch/arm/cpu/armv7/tegra2/funcmux.c
> index 0ef7753..e2d1273 100644
> --- a/arch/arm/cpu/armv7/tegra2/funcmux.c
> +++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
> @@ -31,11 +31,22 @@ int funcmux_select(enum periph_id id, int config)
>
>        switch (id) {
>        case PERIPH_ID_UART1:
> -               if (config == FUNCMUX_UART1_IRRX_IRTX) {
> +               switch (config) {
> +               case FUNCMUX_UART1_IRRX_IRTX:
>                        pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
>                        pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
>                        pinmux_tristate_disable(PINGRP_IRRX);
>                        pinmux_tristate_disable(PINGRP_IRTX);
> +                       break;
> +               case FUNCMUX_UART1_UAA_UAB:
> +                       pinmux_set_func(PINGRP_UAA, PMUX_FUNC_UARTA);
> +                       pinmux_set_func(PINGRP_UAB, PMUX_FUNC_UARTA);
> +                       pinmux_tristate_disable(PINGRP_UAA);
> +                       pinmux_tristate_disable(PINGRP_UAB);
> +                       bad_config = 0;
> +                       break;
> +               }
> +               if (!bad_config) {
>                        /*
>                         * Tegra appears to boot with function UARTA pre-
>                         * selected on mux group SDB. If two mux groups are
> diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h
> b/arch/arm/include/asm/arch-tegra2/funcmux.h
> index ae73c72..b455122 100644
> --- a/arch/arm/include/asm/arch-tegra2/funcmux.h
> +++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
> @@ -30,6 +30,7 @@ enum {
>
>        /* UART configs */
>        FUNCMUX_UART1_IRRX_IRTX = 0,
> +       FUNCMUX_UART1_UAA_UAB,
>        FUNCMUX_UART2_IRDA = 0,
>        FUNCMUX_UART4_GMC = 0,
>
> --
> 1.7.0.4
>
>
Regards,
Simon
Stephen Warren - May 22, 2012, 2:49 a.m.
On 05/21/2012 06:47 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, May 14, 2012 at 4:13 PM, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     From: Stephen Warren <swarren@nvidia.com <mailto:swarren@nvidia.com>>
> 
>     (In at least some configurations) Whistler uses UART1 on pingroups
>     UAA, UAB.

>     diff --git a/arch/arm/cpu/armv7/tegra2/board.c

>     +static int uart_configs[] = {
>     +#ifdef CONFIG_TEGRA2_UARTA_UAA_UAB
>     +       FUNCMUX_UART1_UAA_UAB,
>     +#else
>     +       FUNCMUX_UART1_IRRX_IRTX,
>     +#endif
> 
> 
> We really shouldn't be using configs to deal with this. It should be a
> run-time check.
> 
> I recall you saying that we could use the ODM bits for this?

The ODM bits can indicate which UART is to be used to the debug serial
port, but not the pinmux options. We have no alternative but to make
this a compile-time constant.

> 
>     +       FUNCMUX_UART2_IRDA,
>     +       -1,
>     +       FUNCMUX_UART4_GMC,
>     +       -1,
>     +};
Simon Glass - May 31, 2012, 10:27 p.m.
Hi Stephen,

On Mon, May 21, 2012 at 7:49 PM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 05/21/2012 06:47 PM, Simon Glass wrote:
> > Hi Stephen,
> >
> > On Mon, May 14, 2012 at 4:13 PM, Stephen Warren <swarren@wwwdotorg.org
> > <mailto:swarren@wwwdotorg.org>> wrote:
> >
> >     From: Stephen Warren <swarren@nvidia.com <mailto:swarren@nvidia.com
> >>
> >
> >     (In at least some configurations) Whistler uses UART1 on pingroups
> >     UAA, UAB.
>
> >     diff --git a/arch/arm/cpu/armv7/tegra2/board.c
>
> >     +static int uart_configs[] = {
> >     +#ifdef CONFIG_TEGRA2_UARTA_UAA_UAB
> >     +       FUNCMUX_UART1_UAA_UAB,
> >     +#else
> >     +       FUNCMUX_UART1_IRRX_IRTX,
> >     +#endif
> >
> >
> > We really shouldn't be using configs to deal with this. It should be a
> > run-time check.
> >
> > I recall you saying that we could use the ODM bits for this?
>
> The ODM bits can indicate which UART is to be used to the debug serial
> port, but not the pinmux options. We have no alternative but to make
> this a compile-time constant.
>

To solve this properly we will need a run-time way of selecting the pinmux
for UARTs I suppose. We haven't cracked that problem yet. Let's clean this
up later.


>
> >
> >     +       FUNCMUX_UART2_IRDA,
> >     +       -1,
> >     +       FUNCMUX_UART4_GMC,
> >     +       -1,
> >     +};
>
> Regards,
Simon

Patch

diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index a50b1b9..629ad5d 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -101,6 +101,18 @@  int arch_cpu_init(void)
 }
 #endif
 
+static int uart_configs[] = {
+#ifdef CONFIG_TEGRA2_UARTA_UAA_UAB
+	FUNCMUX_UART1_UAA_UAB,
+#else
+	FUNCMUX_UART1_IRRX_IRTX,
+#endif
+	FUNCMUX_UART2_IRDA,
+	-1,
+	FUNCMUX_UART4_GMC,
+	-1,
+};
+
 /**
  * Set up the specified uarts
  *
@@ -120,7 +132,7 @@  static void setup_uarts(int uart_ids)
 		if (uart_ids & (1 << i)) {
 			enum periph_id id = id_for_uart[i];
 
-			funcmux_select(id, FUNCMUX_DEFAULT);
+			funcmux_select(id, uart_configs[i]);
 			clock_ll_start_uart(id);
 		}
 	}
diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index 0ef7753..e2d1273 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -31,11 +31,22 @@  int funcmux_select(enum periph_id id, int config)
 
 	switch (id) {
 	case PERIPH_ID_UART1:
-		if (config == FUNCMUX_UART1_IRRX_IRTX) {
+		switch (config) {
+		case FUNCMUX_UART1_IRRX_IRTX:
 			pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
 			pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
 			pinmux_tristate_disable(PINGRP_IRRX);
 			pinmux_tristate_disable(PINGRP_IRTX);
+			break;
+		case FUNCMUX_UART1_UAA_UAB:
+			pinmux_set_func(PINGRP_UAA, PMUX_FUNC_UARTA);
+			pinmux_set_func(PINGRP_UAB, PMUX_FUNC_UARTA);
+			pinmux_tristate_disable(PINGRP_UAA);
+			pinmux_tristate_disable(PINGRP_UAB);
+			bad_config = 0;
+			break;
+		}
+		if (!bad_config) {
 			/*
 			 * Tegra appears to boot with function UARTA pre-
 			 * selected on mux group SDB. If two mux groups are
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index ae73c72..b455122 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -30,6 +30,7 @@  enum {
 
 	/* UART configs */
 	FUNCMUX_UART1_IRRX_IRTX = 0,
+	FUNCMUX_UART1_UAA_UAB,
 	FUNCMUX_UART2_IRDA = 0,
 	FUNCMUX_UART4_GMC = 0,