Patchwork [1/1] ARM: i.MX50/53: debug-macro: fix UART_PADDR

login
register
mail settings
Submitter Troy Kisky
Date July 9, 2011, 10:51 p.m.
Message ID <1310251913-9877-1-git-send-email-troy.kisky@boundarydevices.com>
Download mbox | patch
Permalink /patch/104037/
State New
Headers show

Comments

Troy Kisky - July 9, 2011, 10:51 p.m.
The i.MX51 UART_PADDR value does not work for MX50/53.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Amit Kucheria - July 11, 2011, 9:06 a.m.
On 11 Jul 09, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Amit Kucheria <amit.kucheria@canonical.com>

but I think we should switch to runtime detection of the machine (machine
ID?) and then set the port address.

See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.

> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..3919ad4 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -37,13 +37,20 @@
>  #define UART_PADDR	MX3x_UART1_BASE_ADDR
>  #endif
>  
> -#ifdef CONFIG_ARCH_MX5
> +#ifdef CONFIG_ARCH_MX51
>  #ifdef UART_PADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
>  #endif
>  
> +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> +#ifdef UART_PADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#define UART_PADDR	MX53_UART1_BASE_ADDR
> +#endif
> +
>  #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
>  
>  		.macro	addruart, rp, rv
> -- 
> 1.7.0.4
>
Uwe Kleine-König - July 11, 2011, 9:30 a.m.
On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> On 11 Jul 09, Troy Kisky wrote:
> > The i.MX51 UART_PADDR value does not work for MX50/53.
> > 
> > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> 
> but I think we should switch to runtime detection of the machine (machine
> ID?) and then set the port address.
> 
> See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
mach/debug-macro.S and mach/uncompress.h are two different things. And
the uncompress stuff is dynamic for mxc AFAIK.

> > ---
> >  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > index 8e8d175..3919ad4 100644
> > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > @@ -37,13 +37,20 @@
> >  #define UART_PADDR	MX3x_UART1_BASE_ADDR
> >  #endif
> >  
> > -#ifdef CONFIG_ARCH_MX5
> > +#ifdef CONFIG_ARCH_MX51
> >  #ifdef UART_PADDR
> >  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> >  #endif
> >  #define UART_PADDR	MX51_UART1_BASE_ADDR
> >  #endif
> >  
> > +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> > +#ifdef UART_PADDR
> > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > +#endif
> > +#define UART_PADDR	MX53_UART1_BASE_ADDR
> > +#endif
> > +
Hm, even though it's obvious, maybe add a comment that
MX50_UART1_BASE_ADDR is identical to MX53_UART1_BASE_ADDR?

Best regards
Uwe
Amit Kucheria - July 11, 2011, 10:08 a.m.
On 11 Jul 11, Uwe Kleine-König wrote:
> On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> > On 11 Jul 09, Troy Kisky wrote:
> > > The i.MX51 UART_PADDR value does not work for MX50/53.
> > > 
> > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > 
> > Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> > 
> > but I think we should switch to runtime detection of the machine (machine
> > ID?) and then set the port address.
> > 
> > See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
> mach/debug-macro.S and mach/uncompress.h are two different things. And
> the uncompress stuff is dynamic for mxc AFAIK.
 
Indeed they're different. But it seems to me that
arch/arm/mach-omap2/include/mach/debug-macro.S and arch/arm/plat-omap/include/plat/uncompress.h
show clearly how we can deal with the serial port addresses (for DEBUG_LL)
dynamically. No?

> > > ---
> > >  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
> > >  1 files changed, 8 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > index 8e8d175..3919ad4 100644
> > > --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> > > @@ -37,13 +37,20 @@
> > >  #define UART_PADDR	MX3x_UART1_BASE_ADDR
> > >  #endif
> > >  
> > > -#ifdef CONFIG_ARCH_MX5
> > > +#ifdef CONFIG_ARCH_MX51
> > >  #ifdef UART_PADDR
> > >  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > >  #endif
> > >  #define UART_PADDR	MX51_UART1_BASE_ADDR
> > >  #endif
> > >  
> > > +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> > > +#ifdef UART_PADDR
> > > +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> > > +#endif
> > > +#define UART_PADDR	MX53_UART1_BASE_ADDR
> > > +#endif
> > > +
> Hm, even though it's obvious, maybe add a comment that
> MX50_UART1_BASE_ADDR is identical to MX53_UART1_BASE_ADDR?
>
Uwe Kleine-König - July 11, 2011, 1:58 p.m.
On Mon, Jul 11, 2011 at 01:08:38PM +0300, Amit Kucheria wrote:
> On 11 Jul 11, Uwe Kleine-König wrote:
> > On Mon, Jul 11, 2011 at 12:06:09PM +0300, Amit Kucheria wrote:
> > > On 11 Jul 09, Troy Kisky wrote:
> > > > The i.MX51 UART_PADDR value does not work for MX50/53.
> > > > 
> > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > > 
> > > Acked-by: Amit Kucheria <amit.kucheria@canonical.com>
> > > 
> > > but I think we should switch to runtime detection of the machine (machine
> > > ID?) and then set the port address.
> > > 
> > > See arch/arm/plat-omap/include/plat/uncompress.h for a possible scheme.
> > mach/debug-macro.S and mach/uncompress.h are two different things. And
> > the uncompress stuff is dynamic for mxc AFAIK.
>  
> Indeed they're different. But it seems to me that
> arch/arm/mach-omap2/include/mach/debug-macro.S and arch/arm/plat-omap/include/plat/uncompress.h
> show clearly how we can deal with the serial port addresses (for DEBUG_LL)
> dynamically. No?
Looking at arch/arm/mach-omap2/include/mach/debug-macro.S it doesn't
look easy. And considering that debug-macro.S is only needed for early
board bringup, it's totally OK to have hardcoded values for only a
single SOC at a time in it. And I really like to keep this simple as it
is the first working debug mechanism during boot.

Best regards
Uwe
Sascha Hauer - July 12, 2011, 1:32 p.m.
Troy,

On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
> The i.MX51 UART_PADDR value does not work for MX50/53.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
instead of CONFIG_ARCH_*.

Sascha

> ---
>  arch/arm/plat-mxc/include/mach/debug-macro.S |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index 8e8d175..3919ad4 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -37,13 +37,20 @@
>  #define UART_PADDR	MX3x_UART1_BASE_ADDR
>  #endif
>  
> -#ifdef CONFIG_ARCH_MX5
> +#ifdef CONFIG_ARCH_MX51
>  #ifdef UART_PADDR
>  #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
>  #endif
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
>  #endif
>  
> +#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
> +#ifdef UART_PADDR
> +#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> +#endif
> +#define UART_PADDR	MX53_UART1_BASE_ADDR
> +#endif
> +
>  #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
>  
>  		.macro	addruart, rp, rv
> -- 
> 1.7.0.4
> 
>
Troy Kisky - July 12, 2011, 5:04 p.m.
On 7/12/2011 6:32 AM, Sascha Hauer wrote:
> Troy,
> 
> On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
>> The i.MX51 UART_PADDR value does not work for MX50/53.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
> instead of CONFIG_ARCH_*.
> 
> Sascha
> 
>> ---

>From arch/arm/mach-mx5/Kconfig

# ARCH_MX5/50/53 are left to mark places where prevent multi-soc in single
# image. So for most time, SOC_IMX50/51/53 should be used.


So, it would seem to me that the ARCH variant would be correct.
Troy
Sascha Hauer - July 15, 2011, 7:56 a.m.
On Tue, Jul 12, 2011 at 10:04:39AM -0700, Troy Kisky wrote:
> On 7/12/2011 6:32 AM, Sascha Hauer wrote:
> > Troy,
> > 
> > On Sat, Jul 09, 2011 at 03:51:53PM -0700, Troy Kisky wrote:
> >> The i.MX51 UART_PADDR value does not work for MX50/53.
> >>
> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > 
> > Please rebase the patch on my for-next branch and use CONFIG_SOC_IMX*
> > instead of CONFIG_ARCH_*.
> > 
> > Sascha
> > 
> >> ---
> 
> From arch/arm/mach-mx5/Kconfig
> 
> # ARCH_MX5/50/53 are left to mark places where prevent multi-soc in single
> # image. So for most time, SOC_IMX50/51/53 should be used.
> 
> 
> So, it would seem to me that the ARCH variant would be correct.

No, we want to get rid of CONFIG_ARCH_MX*. The debug macro stuff is a
bit special as we intentionally disable multi soc kernels for the low
level debug case.

Sascha

Patch

diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
index 8e8d175..3919ad4 100644
--- a/arch/arm/plat-mxc/include/mach/debug-macro.S
+++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
@@ -37,13 +37,20 @@ 
 #define UART_PADDR	MX3x_UART1_BASE_ADDR
 #endif
 
-#ifdef CONFIG_ARCH_MX5
+#ifdef CONFIG_ARCH_MX51
 #ifdef UART_PADDR
 #error "CONFIG_DEBUG_LL is incompatible with multiple archs"
 #endif
 #define UART_PADDR	MX51_UART1_BASE_ADDR
 #endif
 
+#if defined(CONFIG_ARCH_MX50) || defined(CONFIG_ARCH_MX53)
+#ifdef UART_PADDR
+#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
+#endif
+#define UART_PADDR	MX53_UART1_BASE_ADDR
+#endif
+
 #define UART_VADDR	IMX_IO_ADDRESS(UART_PADDR)
 
 		.macro	addruart, rp, rv