[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
the debug UART address into the kernel, so that we can print before the
platform is initialised.

If the user inadvertently selects multiple platforms with DEBUG_LL
enabled, the UART address may not be correct and will likely cause the
kernel to hang in the very early stages of boot.

This patch, based on a skeleton from Russell, uses a Kconfig choice for
selecting the DEBUG_LL UART, therefore allowing the user to make a
choice about the supported platform when DEBUG_LL is enabled.

Cc: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig.debug |   41 ++++++++++++++++++++++++-----------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 81cbe40..11604c9 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -72,6 +72,30 @@ config DEBUG_LL
 	  in the kernel.  This is helpful if you are debugging code that
 	  executes before the console is initialized.
 
+choice
+	prompt "Kernel low-level debugging port"
+	depends on DEBUG_LL
+
+	config DEBUG_DC21285_PORT
+		bool "Kernel low-level debugging messages via footbridge serial port"
+		depends on FOOTBRIDGE
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the serial port in the DC21285 (Footbridge).
+		  Saying N will cause the debug messages to appear on the first
+		  16550 serial port.
+
+	config DEBUG_CLPS711X_UART2
+		bool "Kernel low-level debugging messages via UART2"
+		depends on ARCH_CLPS711X
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the second serial port on these devices.
+		  Saying N will cause the debug messages to appear on the first
+		  serial port.
+
+endchoice
+
 config EARLY_PRINTK
 	bool "Early printk"
 	depends on DEBUG_LL
@@ -100,23 +124,6 @@ config OC_ETM
 	  buffer driver that will allow you to collect traces of the
 	  kernel code.
 
-config DEBUG_DC21285_PORT
-	bool "Kernel low-level debugging messages via footbridge serial port"
-	depends on DEBUG_LL && FOOTBRIDGE
-	help
-	  Say Y here if you want the debug print routines to direct their
-	  output to the serial port in the DC21285 (Footbridge). Saying N
-	  will cause the debug messages to appear on the first 16550
-	  serial port.
-
-config DEBUG_CLPS711X_UART2
-	bool "Kernel low-level debugging messages via UART2"
-	depends on DEBUG_LL && ARCH_CLPS711X
-	help
-	  Say Y here if you want the debug print routines to direct their
-	  output to the second serial port on these devices.  Saying N will
-	  cause the debug messages to appear on the first serial port.
-
 config DEBUG_S3C_UART
 	depends on PLAT_SAMSUNG
 	int "S3C UART to use for low-level debug"
-- 
1.7.0.4

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Will Deacon]

Now that the DEBUG_LL UART can be selected by a Kconfig choice, convert
the Samsung UART selection to use a set of bools rather than an int.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig.debug        |   45 ++++++++++++++++++++++++++++++-----------
 arch/arm/plat-samsung/Kconfig |    7 ++++++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 11604c9..2f80564 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -94,6 +94,39 @@ choice
 		  Saying N will cause the debug messages to appear on the first
 		  serial port.
 
+	config DEBUG_S3C_UART0
+		depends on PLAT_SAMSUNG
+		bool "Use S3C UART 0 for low-level debug"
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART 0. The port must have been initialised
+		  by the boot-loader before use.
+
+		  The uncompressor code port configuration is now handled
+		  by CONFIG_S3C_LOWLEVEL_UART_PORT.
+
+	config DEBUG_S3C_UART1
+		depends on PLAT_SAMSUNG
+		bool "Use S3C UART 1 for low-level debug"
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART 1. The port must have been initialised
+		  by the boot-loader before use.
+
+		  The uncompressor code port configuration is now handled
+		  by CONFIG_S3C_LOWLEVEL_UART_PORT.
+
+	config DEBUG_S3C_UART2
+		depends on PLAT_SAMSUNG
+		bool "Use S3C UART 2 for low-level debug"
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART 2. The port must have been initialised
+		  by the boot-loader before use.
+
+		  The uncompressor code port configuration is now handled
+		  by CONFIG_S3C_LOWLEVEL_UART_PORT.
+
 endchoice
 
 config EARLY_PRINTK
@@ -124,16 +157,4 @@ config OC_ETM
 	  buffer driver that will allow you to collect traces of the
 	  kernel code.
 
-config DEBUG_S3C_UART
-	depends on PLAT_SAMSUNG
-	int "S3C UART to use for low-level debug"
-	default "0"
-	help
-	  Choice for UART for kernel low-level using S3C UARTS,
-	  should be between zero and two. The port must have been
-	  initialised by the boot-loader before use.
-
-	  The uncompressor code port configuration is now handled
-	  by CONFIG_S3C_LOWLEVEL_UART_PORT.
-
 endmenu
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index b3e1065..49b14b1 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -367,4 +367,11 @@ config SAMSUNG_PD
 	help
 	  Say Y here if you want to control Power Domain by Runtime PM.
 
+config DEBUG_S3C_UART
+	depends on PLAT_SAMSUNG
+	int
+	default "0" if DEBUG_S3C_UART0
+	default "1" if DEBUG_S3C_UART1
+	default "2" if DEBUG_S3C_UART2
+
 endif
-- 
1.7.0.4

[PATCH 3/3] ARM: realview: use Kconfig choice for debug UART selection

[Will Deacon]

Now that the DEBUG_LL UART can be selected by a Kconfig choice, simplify
the #ifdefery in debug-macro.S and add entries to the top-level
Kconfig.debug instead.

Cc: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig.debug                            |   16 ++++++++++++++++
 arch/arm/mach-realview/include/mach/debug-macro.S |   17 ++---------------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 2f80564..f23975a 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -127,6 +127,22 @@ choice
 		  The uncompressor code port configuration is now handled
 		  by CONFIG_S3C_LOWLEVEL_UART_PORT.
 
+	config DEBUG_REALVIEW_STD_PORT
+		bool "RealView Default UART"
+		depends on ARCH_REALVIEW
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the serial port on RealView EB, PB11MP, PBA8
+		  and PBX platforms.
+
+	config DEBUG_REALVIEW_PB1176_PORT
+		bool "RealView PB1176 UART"
+		depends on MACH_REALVIEW_PB1176
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the standard serial port on the RealView
+		  PB1176 platform.
+
 endchoice
 
 config EARLY_PRINTK
diff --git a/arch/arm/mach-realview/include/mach/debug-macro.S b/arch/arm/mach-realview/include/mach/debug-macro.S
index 90b687c..0f38722 100644
--- a/arch/arm/mach-realview/include/mach/debug-macro.S
+++ b/arch/arm/mach-realview/include/mach/debug-macro.S
@@ -10,23 +10,10 @@
  * published by the Free Software Foundation.
  */
 
-#if defined(CONFIG_MACH_REALVIEW_EB) || \
-    defined(CONFIG_MACH_REALVIEW_PB11MP) || \
-    defined(CONFIG_MACH_REALVIEW_PBA8) || \
-    defined(CONFIG_MACH_REALVIEW_PBX)
-#ifndef DEBUG_LL_UART_OFFSET
+#ifdef CONFIG_DEBUG_REALVIEW_STD_PORT
 #define DEBUG_LL_UART_OFFSET	0x00009000
-#elif DEBUG_LL_UART_OFFSET != 0x00009000
-#warning "DEBUG_LL_UART_OFFSET already defined to a different value"
-#endif
-#endif
-
-#ifdef CONFIG_MACH_REALVIEW_PB1176
-#ifndef DEBUG_LL_UART_OFFSET
+#elif defined(CONFIG_DEBUG_REALVIEW_PB1176_PORT)
 #define DEBUG_LL_UART_OFFSET	0x0010c000
-#elif DEBUG_LL_UART_OFFSET != 0x0010c000
-#warning "DEBUG_LL_UART_OFFSET already defined to a different value"
-#endif
 #endif
 
 #ifndef DEBUG_LL_UART_OFFSET
-- 
1.7.0.4

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Tue, 16 Aug 2011, Will Deacon wrote:

> Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
> the debug UART address into the kernel, so that we can print before the
> platform is initialised.
> 
> If the user inadvertently selects multiple platforms with DEBUG_LL
> enabled, the UART address may not be correct and will likely cause the
> kernel to hang in the very early stages of boot.
> 
> This patch, based on a skeleton from Russell, uses a Kconfig choice for
> selecting the DEBUG_LL UART, therefore allowing the user to make a
> choice about the supported platform when DEBUG_LL is enabled.
> 
> Cc: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

(ditto for the other 2 patches)

> ---
>  arch/arm/Kconfig.debug |   41 ++++++++++++++++++++++++-----------------
>  1 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 81cbe40..11604c9 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -72,6 +72,30 @@ config DEBUG_LL
>  	  in the kernel.  This is helpful if you are debugging code that
>  	  executes before the console is initialized.
>  
> +choice
> +	prompt "Kernel low-level debugging port"
> +	depends on DEBUG_LL
> +
> +	config DEBUG_DC21285_PORT
> +		bool "Kernel low-level debugging messages via footbridge serial port"
> +		depends on FOOTBRIDGE
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the serial port in the DC21285 (Footbridge).
> +		  Saying N will cause the debug messages to appear on the first
> +		  16550 serial port.
> +
> +	config DEBUG_CLPS711X_UART2
> +		bool "Kernel low-level debugging messages via UART2"
> +		depends on ARCH_CLPS711X
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the second serial port on these devices.
> +		  Saying N will cause the debug messages to appear on the first
> +		  serial port.
> +
> +endchoice
> +
>  config EARLY_PRINTK
>  	bool "Early printk"
>  	depends on DEBUG_LL
> @@ -100,23 +124,6 @@ config OC_ETM
>  	  buffer driver that will allow you to collect traces of the
>  	  kernel code.
>  
> -config DEBUG_DC21285_PORT
> -	bool "Kernel low-level debugging messages via footbridge serial port"
> -	depends on DEBUG_LL && FOOTBRIDGE
> -	help
> -	  Say Y here if you want the debug print routines to direct their
> -	  output to the serial port in the DC21285 (Footbridge). Saying N
> -	  will cause the debug messages to appear on the first 16550
> -	  serial port.
> -
> -config DEBUG_CLPS711X_UART2
> -	bool "Kernel low-level debugging messages via UART2"
> -	depends on DEBUG_LL && ARCH_CLPS711X
> -	help
> -	  Say Y here if you want the debug print routines to direct their
> -	  output to the second serial port on these devices.  Saying N will
> -	  cause the debug messages to appear on the first serial port.
> -
>  config DEBUG_S3C_UART
>  	depends on PLAT_SAMSUNG
>  	int "S3C UART to use for low-level debug"
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

On Thu, Aug 18, 2011 at 05:06:25AM +0100, Nicolas Pitre wrote:
> On Tue, 16 Aug 2011, Will Deacon wrote:
> 
> > Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
> > the debug UART address into the kernel, so that we can print before the
> > platform is initialised.
> > 
> > If the user inadvertently selects multiple platforms with DEBUG_LL
> > enabled, the UART address may not be correct and will likely cause the
> > kernel to hang in the very early stages of boot.
> > 
> > This patch, based on a skeleton from Russell, uses a Kconfig choice for
> > selecting the DEBUG_LL UART, therefore allowing the user to make a
> > choice about the supported platform when DEBUG_LL is enabled.
> > 
> > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> (ditto for the other 2 patches)

Thanks Nicolas. Hopefully some of the other platforms will start using this
once it's merged.

Will

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

Hi Shawn,

On Thu, Aug 18, 2011 at 05:11:06PM +0100, Shawn Guo wrote:
> On Tue, Aug 16, 2011 at 10:41:11PM +0100, Will Deacon wrote:
> > Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
> > the debug UART address into the kernel, so that we can print before the
> > platform is initialised.
> > 
> > If the user inadvertently selects multiple platforms with DEBUG_LL
> > enabled, the UART address may not be correct and will likely cause the
> > kernel to hang in the very early stages of boot.
> > 
> > This patch, based on a skeleton from Russell, uses a Kconfig choice for
> > selecting the DEBUG_LL UART, therefore allowing the user to make a
> > choice about the supported platform when DEBUG_LL is enabled.
> > 
> > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/Kconfig.debug |   41 ++++++++++++++++++++++++-----------------
> >  1 files changed, 24 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> > index 81cbe40..11604c9 100644
> > --- a/arch/arm/Kconfig.debug
> > +++ b/arch/arm/Kconfig.debug
> > @@ -72,6 +72,30 @@ config DEBUG_LL
> >  	  in the kernel.  This is helpful if you are debugging code that
> >  	  executes before the console is initialized.
> >  
> > +choice
> > +	prompt "Kernel low-level debugging port"
> > +	depends on DEBUG_LL
> > +
> > +	config DEBUG_DC21285_PORT
> > +		bool "Kernel low-level debugging messages via footbridge serial port"
> > +		depends on FOOTBRIDGE
> > +		help
> > +		  Say Y here if you want the debug print routines to direct
> > +		  their output to the serial port in the DC21285 (Footbridge).
> > +		  Saying N will cause the debug messages to appear on the first
> > +		  16550 serial port.
> > +
> > +	config DEBUG_CLPS711X_UART2
> 
> I would expect this will become a long list when other platforms start
> adding CONFIG symbol here.  It may be a good idea to sort this list
> in symbol from the beginning.

I'm not sure that sorting this list in alphabetical order is a good idea.
This is a Kconfig choice, so the default value is the first one in the list
that has its dependencies satisfied. Therefore, the ordering has a direct
impact on the default UART selection.

Will

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Shawn Guo]

On Tue, Aug 16, 2011 at 10:41:11PM +0100, Will Deacon wrote:
> Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
> the debug UART address into the kernel, so that we can print before the
> platform is initialised.
> 
> If the user inadvertently selects multiple platforms with DEBUG_LL
> enabled, the UART address may not be correct and will likely cause the
> kernel to hang in the very early stages of boot.
> 
> This patch, based on a skeleton from Russell, uses a Kconfig choice for
> selecting the DEBUG_LL UART, therefore allowing the user to make a
> choice about the supported platform when DEBUG_LL is enabled.
> 
> Cc: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/Kconfig.debug |   41 ++++++++++++++++++++++++-----------------
>  1 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 81cbe40..11604c9 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -72,6 +72,30 @@ config DEBUG_LL
>  	  in the kernel.  This is helpful if you are debugging code that
>  	  executes before the console is initialized.
>  
> +choice
> +	prompt "Kernel low-level debugging port"
> +	depends on DEBUG_LL
> +
> +	config DEBUG_DC21285_PORT
> +		bool "Kernel low-level debugging messages via footbridge serial port"
> +		depends on FOOTBRIDGE
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the serial port in the DC21285 (Footbridge).
> +		  Saying N will cause the debug messages to appear on the first
> +		  16550 serial port.
> +
> +	config DEBUG_CLPS711X_UART2

I would expect this will become a long list when other platforms start
adding CONFIG symbol here.  It may be a good idea to sort this list
in symbol from the beginning.

Regards,
Shawn

> +		bool "Kernel low-level debugging messages via UART2"
> +		depends on ARCH_CLPS711X
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the second serial port on these devices.
> +		  Saying N will cause the debug messages to appear on the first
> +		  serial port.
> +
> +endchoice
> +
>  config EARLY_PRINTK
>  	bool "Early printk"
>  	depends on DEBUG_LL
> @@ -100,23 +124,6 @@ config OC_ETM
>  	  buffer driver that will allow you to collect traces of the
>  	  kernel code.
>  
> -config DEBUG_DC21285_PORT
> -	bool "Kernel low-level debugging messages via footbridge serial port"
> -	depends on DEBUG_LL && FOOTBRIDGE
> -	help
> -	  Say Y here if you want the debug print routines to direct their
> -	  output to the serial port in the DC21285 (Footbridge). Saying N
> -	  will cause the debug messages to appear on the first 16550
> -	  serial port.
> -
> -config DEBUG_CLPS711X_UART2
> -	bool "Kernel low-level debugging messages via UART2"
> -	depends on DEBUG_LL && ARCH_CLPS711X
> -	help
> -	  Say Y here if you want the debug print routines to direct their
> -	  output to the second serial port on these devices.  Saying N will
> -	  cause the debug messages to appear on the first serial port.
> -
>  config DEBUG_S3C_UART
>  	depends on PLAT_SAMSUNG
>  	int "S3C UART to use for low-level debug"
> -- 
> 1.7.0.4

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Shawn Guo]

Now that the DEBUG_LL UART can be selected by a Kconfig choice,
simplify the #ifdefery in debug-macro.S and add entries to the
top-level Kconfig.debug instead.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/Kconfig.debug                       |   56 ++++++++++++++++++++++++++
 arch/arm/mach-mxs/include/mach/debug-macro.S |   12 +-----
 arch/arm/plat-mxc/include/mach/debug-macro.S |   38 +++---------------
 3 files changed, 64 insertions(+), 42 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index f23975a..947adef 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -94,6 +94,62 @@ choice
 		  Saying N will cause the debug messages to appear on the first
 		  serial port.
 
+	config DEBUG_IMX1_UART
+		bool "i.MX1 Debug UART"
+		depends on SOC_IMX1
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX1.
+
+	config DEBUG_IMX23_UART
+		bool "i.MX23 Debug UART"
+		depends on SOC_IMX23
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX23.
+
+	config DEBUG_IMX25_UART
+		bool "i.MX25 Debug UART"
+		depends on SOC_IMX25
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX25.
+
+	config DEBUG_IMX21_IMX27_UART
+		bool "i.MX21 and i.MX27 Debug UART"
+		depends on SOC_IMX21 || SOC_IMX27
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX21 or i.MX27.
+
+	config DEBUG_IMX28_UART
+		bool "i.MX28 Debug UART"
+		depends on SOC_IMX28
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX28.
+
+	config DEBUG_IMX31_IMX35_UART
+		bool "i.MX31 and i.MX35 Debug UART"
+		depends on SOC_IMX31 || SOC_IMX35
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX31 or i.MX35.
+
+	config DEBUG_IMX51_UART
+		bool "i.MX51 Debug UART"
+		depends on SOC_IMX51
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX51.
+
+	config DEBUG_IMX50_IMX53_UART
+		bool "i.MX50 and i.MX53 Debug UART"
+		depends on SOC_IMX50 || SOC_IMX53
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on i.MX50 or i.MX53.
+
 	config DEBUG_S3C_UART0
 		depends on PLAT_SAMSUNG
 		bool "Use S3C UART 0 for low-level debug"
diff --git a/arch/arm/mach-mxs/include/mach/debug-macro.S b/arch/arm/mach-mxs/include/mach/debug-macro.S
index 79650a1..6d98704 100644
--- a/arch/arm/mach-mxs/include/mach/debug-macro.S
+++ b/arch/arm/mach-mxs/include/mach/debug-macro.S
@@ -14,17 +14,9 @@
 #include <mach/mx23.h>
 #include <mach/mx28.h>
 
-#ifdef CONFIG_SOC_IMX23
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#ifdef CONFIG_DEBUG_IMX23_UART
 #define UART_PADDR	MX23_DUART_BASE_ADDR
-#endif
-
-#ifdef CONFIG_SOC_IMX28
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX28_UART)
 #define UART_PADDR	MX28_DUART_BASE_ADDR
 #endif
 
diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
index e4dde91..07cfdbe 100644
--- a/arch/arm/plat-mxc/include/mach/debug-macro.S
+++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
@@ -12,43 +12,17 @@
  */
 #include <mach/hardware.h>
 
-#ifdef CONFIG_SOC_IMX1
+#ifdef CONFIG_DEBUG_IMX1_UART
 #define UART_PADDR	MX1_UART1_BASE_ADDR
-#endif
-
-#ifdef CONFIG_SOC_IMX25
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX25_UART)
 #define UART_PADDR	MX25_UART1_BASE_ADDR
-#endif
-
-#if defined(CONFIG_SOC_IMX21) || defined (CONFIG_SOC_IMX27)
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX21_IMX27_UART)
 #define UART_PADDR	MX2x_UART1_BASE_ADDR
-#endif
-
-#if defined(CONFIG_SOC_IMX31) || defined(CONFIG_SOC_IMX35)
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX31_IMX35_UART)
 #define UART_PADDR	MX3x_UART1_BASE_ADDR
-#endif
-
-#ifdef CONFIG_SOC_IMX51
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX51_UART)
 #define UART_PADDR	MX51_UART1_BASE_ADDR
-#endif
-
-/* iMX50/53 have same addresses, but not iMX51 */
-#if defined(CONFIG_SOC_IMX50) || defined(CONFIG_SOC_IMX53)
-#ifdef UART_PADDR
-#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
-#endif
+#elif defined (CONFIG_DEBUG_IMX50_IMX53_UART)
 #define UART_PADDR	MX53_UART1_BASE_ADDR
 #endif
 
-- 
1.7.4.1

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Shawn Guo]

On Thu, Aug 18, 2011 at 05:07:12PM +0100, Will Deacon wrote:
> Hi Shawn,
> 
> On Thu, Aug 18, 2011 at 05:11:06PM +0100, Shawn Guo wrote:
> > On Tue, Aug 16, 2011 at 10:41:11PM +0100, Will Deacon wrote:
> > > Enabling CONFIG_DEBUG_LL (which is required for earlyprintk) hardwires
> > > the debug UART address into the kernel, so that we can print before the
> > > platform is initialised.
> > > 
> > > If the user inadvertently selects multiple platforms with DEBUG_LL
> > > enabled, the UART address may not be correct and will likely cause the
> > > kernel to hang in the very early stages of boot.
> > > 
> > > This patch, based on a skeleton from Russell, uses a Kconfig choice for
> > > selecting the DEBUG_LL UART, therefore allowing the user to make a
> > > choice about the supported platform when DEBUG_LL is enabled.
> > > 
> > > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm/Kconfig.debug |   41 ++++++++++++++++++++++++-----------------
> > >  1 files changed, 24 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> > > index 81cbe40..11604c9 100644
> > > --- a/arch/arm/Kconfig.debug
> > > +++ b/arch/arm/Kconfig.debug
> > > @@ -72,6 +72,30 @@ config DEBUG_LL
> > >  	  in the kernel.  This is helpful if you are debugging code that
> > >  	  executes before the console is initialized.
> > >  
> > > +choice
> > > +	prompt "Kernel low-level debugging port"
> > > +	depends on DEBUG_LL
> > > +
> > > +	config DEBUG_DC21285_PORT
> > > +		bool "Kernel low-level debugging messages via footbridge serial port"
> > > +		depends on FOOTBRIDGE
> > > +		help
> > > +		  Say Y here if you want the debug print routines to direct
> > > +		  their output to the serial port in the DC21285 (Footbridge).
> > > +		  Saying N will cause the debug messages to appear on the first
> > > +		  16550 serial port.
> > > +
> > > +	config DEBUG_CLPS711X_UART2
> > 
> > I would expect this will become a long list when other platforms start
> > adding CONFIG symbol here.  It may be a good idea to sort this list
> > in symbol from the beginning.
> 
> I'm not sure that sorting this list in alphabetical order is a good idea.
> This is a Kconfig choice, so the default value is the first one in the list
> that has its dependencies satisfied. Therefore, the ordering has a direct
> impact on the default UART selection.
> 
I'm unsure that the default UART selection makes much sense here.
When we build so many platforms into single image, it's hard to say
which one should be the default.  People anyway need to check if the
the UART selection matches the platform they are debugging on.

BTW, I just posted one patch for imx based on your series.  Are you
interested in folding it into yours?

-- 
Regards,
Shawn

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Sascha Hauer]

On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> simplify the #ifdefery in debug-macro.S and add entries to the
> top-level Kconfig.debug instead.

I'm unsure whether I like this. The ifdeffery does not look very good,
but the Kconfig snippet is not shorter, also it is in generic arm code
and not i.MX specific. The old way also makes sure that we do not
compile in incompatible lowlevel debug code.
At least this should be a choice in Kconfig to make clear that the
different low-level debug options are exclusive.

Sascha

> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/Kconfig.debug                       |   56 ++++++++++++++++++++++++++
>  arch/arm/mach-mxs/include/mach/debug-macro.S |   12 +-----
>  arch/arm/plat-mxc/include/mach/debug-macro.S |   38 +++---------------
>  3 files changed, 64 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index f23975a..947adef 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -94,6 +94,62 @@ choice
>  		  Saying N will cause the debug messages to appear on the first
>  		  serial port.
>  
> +	config DEBUG_IMX1_UART
> +		bool "i.MX1 Debug UART"
> +		depends on SOC_IMX1
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX1.
> +
> +	config DEBUG_IMX23_UART
> +		bool "i.MX23 Debug UART"
> +		depends on SOC_IMX23
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX23.
> +
> +	config DEBUG_IMX25_UART
> +		bool "i.MX25 Debug UART"
> +		depends on SOC_IMX25
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX25.
> +
> +	config DEBUG_IMX21_IMX27_UART
> +		bool "i.MX21 and i.MX27 Debug UART"
> +		depends on SOC_IMX21 || SOC_IMX27
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX21 or i.MX27.
> +
> +	config DEBUG_IMX28_UART
> +		bool "i.MX28 Debug UART"
> +		depends on SOC_IMX28
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX28.
> +
> +	config DEBUG_IMX31_IMX35_UART
> +		bool "i.MX31 and i.MX35 Debug UART"
> +		depends on SOC_IMX31 || SOC_IMX35
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX31 or i.MX35.
> +
> +	config DEBUG_IMX51_UART
> +		bool "i.MX51 Debug UART"
> +		depends on SOC_IMX51
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX51.
> +
> +	config DEBUG_IMX50_IMX53_UART
> +		bool "i.MX50 and i.MX53 Debug UART"
> +		depends on SOC_IMX50 || SOC_IMX53
> +		help
> +		  Say Y here if you want kernel low-level debugging support
> +		  on i.MX50 or i.MX53.
> +
>  	config DEBUG_S3C_UART0
>  		depends on PLAT_SAMSUNG
>  		bool "Use S3C UART 0 for low-level debug"
> diff --git a/arch/arm/mach-mxs/include/mach/debug-macro.S b/arch/arm/mach-mxs/include/mach/debug-macro.S
> index 79650a1..6d98704 100644
> --- a/arch/arm/mach-mxs/include/mach/debug-macro.S
> +++ b/arch/arm/mach-mxs/include/mach/debug-macro.S
> @@ -14,17 +14,9 @@
>  #include <mach/mx23.h>
>  #include <mach/mx28.h>
>  
> -#ifdef CONFIG_SOC_IMX23
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#ifdef CONFIG_DEBUG_IMX23_UART
>  #define UART_PADDR	MX23_DUART_BASE_ADDR
> -#endif
> -
> -#ifdef CONFIG_SOC_IMX28
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX28_UART)
>  #define UART_PADDR	MX28_DUART_BASE_ADDR
>  #endif
>  
> diff --git a/arch/arm/plat-mxc/include/mach/debug-macro.S b/arch/arm/plat-mxc/include/mach/debug-macro.S
> index e4dde91..07cfdbe 100644
> --- a/arch/arm/plat-mxc/include/mach/debug-macro.S
> +++ b/arch/arm/plat-mxc/include/mach/debug-macro.S
> @@ -12,43 +12,17 @@
>   */
>  #include <mach/hardware.h>
>  
> -#ifdef CONFIG_SOC_IMX1
> +#ifdef CONFIG_DEBUG_IMX1_UART
>  #define UART_PADDR	MX1_UART1_BASE_ADDR
> -#endif
> -
> -#ifdef CONFIG_SOC_IMX25
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX25_UART)
>  #define UART_PADDR	MX25_UART1_BASE_ADDR
> -#endif
> -
> -#if defined(CONFIG_SOC_IMX21) || defined (CONFIG_SOC_IMX27)
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX21_IMX27_UART)
>  #define UART_PADDR	MX2x_UART1_BASE_ADDR
> -#endif
> -
> -#if defined(CONFIG_SOC_IMX31) || defined(CONFIG_SOC_IMX35)
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX31_IMX35_UART)
>  #define UART_PADDR	MX3x_UART1_BASE_ADDR
> -#endif
> -
> -#ifdef CONFIG_SOC_IMX51
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX51_UART)
>  #define UART_PADDR	MX51_UART1_BASE_ADDR
> -#endif
> -
> -/* iMX50/53 have same addresses, but not iMX51 */
> -#if defined(CONFIG_SOC_IMX50) || defined(CONFIG_SOC_IMX53)
> -#ifdef UART_PADDR
> -#error "CONFIG_DEBUG_LL is incompatible with multiple archs"
> -#endif
> +#elif defined (CONFIG_DEBUG_IMX50_IMX53_UART)
>  #define UART_PADDR	MX53_UART1_BASE_ADDR
>  #endif
>  
> -- 
> 1.7.4.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Shawn Guo]

On Fri, Aug 19, 2011 at 08:35:33AM +0200, Sascha Hauer wrote:
> On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > simplify the #ifdefery in debug-macro.S and add entries to the
> > top-level Kconfig.debug instead.
> 
> I'm unsure whether I like this. The ifdeffery does not look very good,

It does not make the code any worse.  Instead, the ifdeffery becomes
shorter actually.

> but the Kconfig snippet is not shorter, also it is in generic arm code
> and not i.MX specific.

This comment is on the whole approach proposed by Will, I guess. But
I did not see any better alternatives until we can have device tree
into such a early boot stage, which is not possible for now. 

> The old way also makes sure that we do not
> compile in incompatible lowlevel debug code.
> At least this should be a choice in Kconfig to make clear that the
> different low-level debug options are exclusive.
> 
It is a choice.

-- 
Regards,
Shawn

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

On Fri, Aug 19, 2011 at 05:59:57AM +0100, Shawn Guo wrote:
> On Thu, Aug 18, 2011 at 05:07:12PM +0100, Will Deacon wrote:
> > I'm not sure that sorting this list in alphabetical order is a good idea.
> > This is a Kconfig choice, so the default value is the first one in the list
> > that has its dependencies satisfied. Therefore, the ordering has a direct
> > impact on the default UART selection.
> > 
> I'm unsure that the default UART selection makes much sense here.
> When we build so many platforms into single image, it's hard to say
> which one should be the default.  People anyway need to check if the
> the UART selection matches the platform they are debugging on.

Ok, how about having the default choice as a dummy option which doesn't
correspond to a UART?:

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index f23975a..455bc8c 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -65,8 +65,12 @@ config DEBUG_USER
 
 # These options are only for real kernel hackers who want to get their hands dirty.
 config DEBUG_LL
+       bool
+
+config DEBUG_LL_UART
        bool "Kernel low-level debugging functions"
        depends on DEBUG_KERNEL
+       select DEBUG_LL if !DEBUG_UART_NONE
        help
          Say Y here to include definitions of printascii, printch, printhex
          in the kernel.  This is helpful if you are debugging code that
@@ -74,7 +78,12 @@ config DEBUG_LL
 
 choice
        prompt "Kernel low-level debugging port"
-       depends on DEBUG_LL
+       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
+                                    PLAT_SAMSUNG || ARCH_REALVIEW)
+       default DEBUG_UART_NONE
+
+       config DEBUG_UART_NONE
+               bool "No UART selected (default)"
 
        config DEBUG_DC21285_PORT
                bool "Kernel low-level debugging messages via footbridge serial port"


That way, you really have to select the UART for DEBUG_LL to be enabled.

> BTW, I just posted one patch for imx based on your series.  Are you
> interested in folding it into yours?

Yep, I'll pick it up once Sascha is happy with it. If we go with the above,
I'm happy to make the necessary changes.

Cheers,

Will

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

Sascha,

On Fri, Aug 19, 2011 at 07:35:33AM +0100, Sascha Hauer wrote:
> On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > simplify the #ifdefery in debug-macro.S and add entries to the
> > top-level Kconfig.debug instead.
> 
> I'm unsure whether I like this. The ifdeffery does not look very good,
> but the Kconfig snippet is not shorter, also it is in generic arm code
> and not i.MX specific. The old way also makes sure that we do not
> compile in incompatible lowlevel debug code.

But it's an unfortunate hinderence to a single zImage kernel which we can
only solve sensibly in the generic ARM code.

> At least this should be a choice in Kconfig to make clear that the
> different low-level debug options are exclusive.

As Shawn pointed out, we are using a Kconfig choice so you can only select
one UART for low-level debug output.

Will

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Shawn Guo]

On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> On Fri, Aug 19, 2011 at 05:59:57AM +0100, Shawn Guo wrote:
> > On Thu, Aug 18, 2011 at 05:07:12PM +0100, Will Deacon wrote:
> > > I'm not sure that sorting this list in alphabetical order is a good idea.
> > > This is a Kconfig choice, so the default value is the first one in the list
> > > that has its dependencies satisfied. Therefore, the ordering has a direct
> > > impact on the default UART selection.
> > > 
> > I'm unsure that the default UART selection makes much sense here.
> > When we build so many platforms into single image, it's hard to say
> > which one should be the default.  People anyway need to check if the
> > the UART selection matches the platform they are debugging on.
> 
> Ok, how about having the default choice as a dummy option which doesn't
> correspond to a UART?:
> 
To me, it is a little bit overkilled.  Can we just sort them in
alphabetical order and let the first one be the winner?  We can take
this as a reward to the good naming :)

> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index f23975a..455bc8c 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -65,8 +65,12 @@ config DEBUG_USER
>  
>  # These options are only for real kernel hackers who want to get their hands dirty.
>  config DEBUG_LL
> +       bool
> +
> +config DEBUG_LL_UART
>         bool "Kernel low-level debugging functions"
>         depends on DEBUG_KERNEL
> +       select DEBUG_LL if !DEBUG_UART_NONE
>         help
>           Say Y here to include definitions of printascii, printch, printhex
>           in the kernel.  This is helpful if you are debugging code that
> @@ -74,7 +78,12 @@ config DEBUG_LL
>  
>  choice
>         prompt "Kernel low-level debugging port"
> -       depends on DEBUG_LL
> +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> +                                    PLAT_SAMSUNG || ARCH_REALVIEW)

We will have to list a lot of ARCH/PLAT symbols here.  This is what
I meant overkilled actually.

Regards,
Shawn

> +       default DEBUG_UART_NONE
> +
> +       config DEBUG_UART_NONE
> +               bool "No UART selected (default)"
>  
>         config DEBUG_DC21285_PORT
>                 bool "Kernel low-level debugging messages via footbridge serial port"
> 
> 
> That way, you really have to select the UART for DEBUG_LL to be enabled.
> 
> > BTW, I just posted one patch for imx based on your series.  Are you
> > interested in folding it into yours?
> 
> Yep, I'll pick it up once Sascha is happy with it. If we go with the above,
> I'm happy to make the necessary changes.
> 

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Sascha Hauer]

On Fri, Aug 19, 2011 at 12:09:41PM +0100, Will Deacon wrote:
> Sascha,
> 
> On Fri, Aug 19, 2011 at 07:35:33AM +0100, Sascha Hauer wrote:
> > On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > > simplify the #ifdefery in debug-macro.S and add entries to the
> > > top-level Kconfig.debug instead.
> > 
> > I'm unsure whether I like this. The ifdeffery does not look very good,
> > but the Kconfig snippet is not shorter, also it is in generic arm code
> > and not i.MX specific. The old way also makes sure that we do not
> > compile in incompatible lowlevel debug code.
> 
> But it's an unfortunate hinderence to a single zImage kernel which we can
> only solve sensibly in the generic ARM code.

My problem is that if this option is enabled the kernel will not run
on any other SoC except the one being selected here, at least when
earlyprintk is passed on the command line. One could argue
that this option is for people who exactly know what they do only.
Still there should pop up a big fat warning somewhere.
The old i.MX way ensured that by refusing to compile the kernel with
multi SoC support.

> 
> > At least this should be a choice in Kconfig to make clear that the
> > different low-level debug options are exclusive.
> 
> As Shawn pointed out, we are using a Kconfig choice so you can only select
> one UART for low-level debug output.

Ah, ok. I didn't see that Shawns mail was a reply to your series turning
this into a choice.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

On Fri, Aug 19, 2011 at 12:37:41PM +0100, Shawn Guo wrote:
> On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> > On Fri, Aug 19, 2011 at 05:59:57AM +0100, Shawn Guo wrote:
> > > On Thu, Aug 18, 2011 at 05:07:12PM +0100, Will Deacon wrote:
> > > > I'm not sure that sorting this list in alphabetical order is a good idea.
> > > > This is a Kconfig choice, so the default value is the first one in the list
> > > > that has its dependencies satisfied. Therefore, the ordering has a direct
> > > > impact on the default UART selection.
> > > > 
> > > I'm unsure that the default UART selection makes much sense here.
> > > When we build so many platforms into single image, it's hard to say
> > > which one should be the default.  People anyway need to check if the
> > > the UART selection matches the platform they are debugging on.
> > 
> > Ok, how about having the default choice as a dummy option which doesn't
> > correspond to a UART?:
> > 
> To me, it is a little bit overkilled.  Can we just sort them in
> alphabetical order and let the first one be the winner?  We can take
> this as a reward to the good naming :)

We could do this, I just thought it might be better to force the user to
select a UART rather than blindly using the first one in the list. Then
again, as Russell pointed out, DEBUG_LL should only be selected if you know
what you are doing.

> > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> > index f23975a..455bc8c 100644
> > --- a/arch/arm/Kconfig.debug
> > +++ b/arch/arm/Kconfig.debug
> > @@ -65,8 +65,12 @@ config DEBUG_USER
> >  
> >  # These options are only for real kernel hackers who want to get their hands dirty.
> >  config DEBUG_LL
> > +       bool
> > +
> > +config DEBUG_LL_UART
> >         bool "Kernel low-level debugging functions"
> >         depends on DEBUG_KERNEL
> > +       select DEBUG_LL if !DEBUG_UART_NONE
> >         help
> >           Say Y here to include definitions of printascii, printch, printhex
> >           in the kernel.  This is helpful if you are debugging code that
> > @@ -74,7 +78,12 @@ config DEBUG_LL
> >  
> >  choice
> >         prompt "Kernel low-level debugging port"
> > -       depends on DEBUG_LL
> > +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> > +                                    PLAT_SAMSUNG || ARCH_REALVIEW)
> 
> We will have to list a lot of ARCH/PLAT symbols here.  This is what
> I meant overkilled actually.

Ultimately, we will want to have all the platforms using this mechanism so
this list of symbols could eventually be removed. I take your point though;
so I'll leave the patch series as it is for now.

Will

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

On Fri, Aug 19, 2011 at 12:39:37PM +0100, Sascha Hauer wrote:
> On Fri, Aug 19, 2011 at 12:09:41PM +0100, Will Deacon wrote:
> > But it's an unfortunate hinderence to a single zImage kernel which we can
> > only solve sensibly in the generic ARM code.
> 
> My problem is that if this option is enabled the kernel will not run
> on any other SoC except the one being selected here, at least when
> earlyprintk is passed on the command line. One could argue
> that this option is for people who exactly know what they do only.
> Still there should pop up a big fat warning somewhere.

Could do - but where?

> The old i.MX way ensured that by refusing to compile the kernel with
> multi SoC support.

That's a worse solution, because now the kernel doesn't run *anywhere* if
you enable DEBUG_LL and multiple SoCs. A use case for the new method is that
you have a kernel .config file for a multi-SoC kernel which is known not to
fail early on a given board. You can enable DEBUG_LL for that board and solve
the problem. It should never be enabled for a kernel that is designed to be
portable!

> > 
> > > At least this should be a choice in Kconfig to make clear that the
> > > different low-level debug options are exclusive.
> > 
> > As Shawn pointed out, we are using a Kconfig choice so you can only select
> > one UART for low-level debug output.
> 
> Ah, ok. I didn't see that Shawns mail was a reply to your series turning
> this into a choice.

Yup - the idea is to force a single UART definition when DEBUG_LL is
enabled.

Will

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Fri, 19 Aug 2011, Shawn Guo wrote:

> On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> > On Fri, Aug 19, 2011 at 05:59:57AM +0100, Shawn Guo wrote:
> > > On Thu, Aug 18, 2011 at 05:07:12PM +0100, Will Deacon wrote:
> > > > I'm not sure that sorting this list in alphabetical order is a good idea.
> > > > This is a Kconfig choice, so the default value is the first one in the list
> > > > that has its dependencies satisfied. Therefore, the ordering has a direct
> > > > impact on the default UART selection.
> > > > 
> > > I'm unsure that the default UART selection makes much sense here.
> > > When we build so many platforms into single image, it's hard to say
> > > which one should be the default.  People anyway need to check if the
> > > the UART selection matches the platform they are debugging on.
> > 
> > Ok, how about having the default choice as a dummy option which doesn't
> > correspond to a UART?:
> > 
> To me, it is a little bit overkilled.  Can we just sort them in
> alphabetical order and let the first one be the winner?  We can take
> this as a reward to the good naming :)

Just use the alphabetical order, except for the first entry which could 
be a noop.

> >  choice
> >         prompt "Kernel low-level debugging port"
> > -       depends on DEBUG_LL
> > +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> > +                                    PLAT_SAMSUNG || ARCH_REALVIEW)
> 
> We will have to list a lot of ARCH/PLAT symbols here.  This is what
> I meant overkilled actually.

Agreed.  Please avoid the need for such localized long lists of 
dependencies which are going to create merge conflicts for sure, and 
which grow into a mess after a while.


Nicolas

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Fri, 19 Aug 2011, Will Deacon wrote:
> On Fri, Aug 19, 2011 at 12:37:41PM +0100, Shawn Guo wrote:
> > On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> > > @@ -74,7 +78,12 @@ config DEBUG_LL
> > >  
> > >  choice
> > >         prompt "Kernel low-level debugging port"
> > > -       depends on DEBUG_LL
> > > +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> > > +                                    PLAT_SAMSUNG || ARCH_REALVIEW)
> > 
> > We will have to list a lot of ARCH/PLAT symbols here.  This is what
> > I meant overkilled actually.
> 
> Ultimately, we will want to have all the platforms using this mechanism so
> this list of symbols could eventually be removed. I take your point though;
> so I'll leave the patch series as it is for now.

Well, ultimately I'd like to see something even better than that.  We 
should have the bootloader provide the required information to the 
kernel for it to be able to send bytes to a debug device.  It should be 
rather simple, especially for a serial UART.  All we need in that case 
is:

 - physical address of the transmit FIFO register

 - physical address of the register indicating "FIFO full" with a 
   corresponding bit mask

 - physical address of the register indicating "FIFO empty" with a
   corresponding bit mask

The bootloader should be able to pass that info into an ATAG or a DT 
node just fine.

But in the mean time, for legacy boards, the best we have is this 
Kconfig approach.


Nicolas

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Sascha Hauer]

On Fri, Aug 19, 2011 at 01:35:43PM +0100, Will Deacon wrote:
> On Fri, Aug 19, 2011 at 12:39:37PM +0100, Sascha Hauer wrote:
> > On Fri, Aug 19, 2011 at 12:09:41PM +0100, Will Deacon wrote:
> > > But it's an unfortunate hinderence to a single zImage kernel which we can
> > > only solve sensibly in the generic ARM code.
> > 
> > My problem is that if this option is enabled the kernel will not run
> > on any other SoC except the one being selected here, at least when
> > earlyprintk is passed on the command line. One could argue
> > that this option is for people who exactly know what they do only.
> > Still there should pop up a big fat warning somewhere.
> 
> Could do - but where?

Good question, I don't know.

> 
> > The old i.MX way ensured that by refusing to compile the kernel with
> > multi SoC support.
> 
> That's a worse solution, because now the kernel doesn't run *anywhere* if
> you enable DEBUG_LL and multiple SoCs.

Better a kernel that does not compile than a kernel that compiles and
does not run.

> A use case for the new method is that
> you have a kernel .config file for a multi-SoC kernel which is known not to
> fail early on a given board. You can enable DEBUG_LL for that board and solve
> the problem. It should never be enabled for a kernel that is designed to be
> portable!

Indeed.

How about the something like the following?

config DEBUG_LL
        bool "Kernel low-level debugging functions (read help!)"
        depends on DEBUG_KERNEL
        help
          Say Y here to include definitions of printascii, printch,printhex
          in the kernel.  This is helpful if you are debugging code that
          executes before the console is initialized.
	  The UART lowlevel functions will limit the kernel to exactly
	  the system specified below. Trying to use these on any other
	  system will lock up the kernel or even worse.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Fri, Aug 19, 2011 at 11:49:10AM -0400, Nicolas Pitre wrote:
> On Fri, 19 Aug 2011, Will Deacon wrote:
> > On Fri, Aug 19, 2011 at 12:37:41PM +0100, Shawn Guo wrote:
> > > On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> > > > @@ -74,7 +78,12 @@ config DEBUG_LL
> > > >  
> > > >  choice
> > > >         prompt "Kernel low-level debugging port"
> > > > -       depends on DEBUG_LL
> > > > +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> > > > +                                    PLAT_SAMSUNG || ARCH_REALVIEW)
> > > 
> > > We will have to list a lot of ARCH/PLAT symbols here.  This is what
> > > I meant overkilled actually.
> > 
> > Ultimately, we will want to have all the platforms using this mechanism so
> > this list of symbols could eventually be removed. I take your point though;
> > so I'll leave the patch series as it is for now.
> 
> Well, ultimately I'd like to see something even better than that.  We 
> should have the bootloader provide the required information to the 
> kernel for it to be able to send bytes to a debug device.  It should be 
> rather simple, especially for a serial UART.  All we need in that case 
> is:
> 
>  - physical address of the transmit FIFO register
> 
>  - physical address of the register indicating "FIFO full" with a 
>    corresponding bit mask
> 
>  - physical address of the register indicating "FIFO empty" with a
>    corresponding bit mask

You're missing something there.  It's not always about FIFO full and FIFO
empty.  On some platforms, we want _reliable_ debugging, so the debug
stuff waits while CTS is inactive.

Plus you need the virtual address too, because the LL debug stuff is
there to debug around places like the initial assembly, before C code
is setup.

Plus its not just about bitmasks, but also about size of access, and
polarity of bits.  So actually you'd need:

- virtual and physical address and size of transmit register
- virtual and physical address, mask and value of transmit status
- virtual and physical address, mask and value of CTS status

but then, we have debugging which doesn't have a physical address
associated with it (the jtag stuff) so this doesn't cover the full
story either.  Not only that of course, but boot loaders should not
be passing virtual addresses to the kernel.

So all in all, passing this information in from a boot loader is just
a plain and simple bad idea.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Russell King - ARM Linux]

On Fri, Aug 19, 2011 at 01:39:37PM +0200, Sascha Hauer wrote:
> On Fri, Aug 19, 2011 at 12:09:41PM +0100, Will Deacon wrote:
> > Sascha,
> > 
> > On Fri, Aug 19, 2011 at 07:35:33AM +0100, Sascha Hauer wrote:
> > > On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > > > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > > > simplify the #ifdefery in debug-macro.S and add entries to the
> > > > top-level Kconfig.debug instead.
> > > 
> > > I'm unsure whether I like this. The ifdeffery does not look very good,
> > > but the Kconfig snippet is not shorter, also it is in generic arm code
> > > and not i.MX specific. The old way also makes sure that we do not
> > > compile in incompatible lowlevel debug code.
> > 
> > But it's an unfortunate hinderence to a single zImage kernel which we can
> > only solve sensibly in the generic ARM code.
> 
> My problem is that if this option is enabled the kernel will not run
> on any other SoC except the one being selected here, at least when
> earlyprintk is passed on the command line.

I never liked the idea of coupling this into earlyprintk - and I think
I said so at the time.  I'll say it again:

The LL DEBUG stuff is there to be able to do low level "it won't boot"
debugging.  It's not there as a user option.  You are supposed to know
exactly what you are doing when using the option.

If we're going to start having earlyprintk be an argument against this,
I'll simply rip out the earlyprintk coupling to LL debug, and people
can go back to patching printk.c to make this work.

> One could argue
> that this option is for people who exactly know what they do only.

It _IS_ there for people who know what they're doing.  That's something
I keep on saying about the LL debug stuff.  It's there to allow people
to debug the early startup of the kernel.  It's not there for users.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

On Sun, Aug 21, 2011 at 10:18:51AM +0100, Russell King - ARM Linux wrote:
> On Fri, Aug 19, 2011 at 01:39:37PM +0200, Sascha Hauer wrote:
> > One could argue
> > that this option is for people who exactly know what they do only.
> 
> It _IS_ there for people who know what they're doing.  That's something
> I keep on saying about the LL debug stuff.  It's there to allow people
> to debug the early startup of the kernel.  It's not there for users.

Understood. I don't think it hurts to make this explicit in the Kconfig help
entry for DEBUG_LL (as Sascha suggested) so I'll do that for v2 of this
series.

Will

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Fri, Aug 19, 2011 at 11:49:10AM -0400, Nicolas Pitre wrote:
> > On Fri, 19 Aug 2011, Will Deacon wrote:
> > > On Fri, Aug 19, 2011 at 12:37:41PM +0100, Shawn Guo wrote:
> > > > On Fri, Aug 19, 2011 at 12:08:22PM +0100, Will Deacon wrote:
> > > > > @@ -74,7 +78,12 @@ config DEBUG_LL
> > > > >  
> > > > >  choice
> > > > >         prompt "Kernel low-level debugging port"
> > > > > -       depends on DEBUG_LL
> > > > > +       depends on DEBUG_LL_UART && (FOOTBRIDGE || ARCH_CLPS711X || \
> > > > > +                                    PLAT_SAMSUNG || ARCH_REALVIEW)
> > > > 
> > > > We will have to list a lot of ARCH/PLAT symbols here.  This is what
> > > > I meant overkilled actually.
> > > 
> > > Ultimately, we will want to have all the platforms using this mechanism so
> > > this list of symbols could eventually be removed. I take your point though;
> > > so I'll leave the patch series as it is for now.
> > 
> > Well, ultimately I'd like to see something even better than that.  We 
> > should have the bootloader provide the required information to the 
> > kernel for it to be able to send bytes to a debug device.  It should be 
> > rather simple, especially for a serial UART.  All we need in that case 
> > is:
> > 
> >  - physical address of the transmit FIFO register
> > 
> >  - physical address of the register indicating "FIFO full" with a 
> >    corresponding bit mask
> > 
> >  - physical address of the register indicating "FIFO empty" with a
> >    corresponding bit mask
> 
> You're missing something there.  It's not always about FIFO full and FIFO
> empty.  On some platforms, we want _reliable_ debugging, so the debug
> stuff waits while CTS is inactive.

OK, let's consider this too.  Another register address/bitmask pair.  
This could be combined into some "OK to transmit" list of such pairs.

> Plus you need the virtual address too, because the LL debug stuff is
> there to debug around places like the initial assembly, before C code
> is setup.

The kernel should determine or set up the virtual address by itself.  
This is obviously not something we want the bootloader to provide.  For 
the same reason, I've discarded the idea that the bootloader could 
simply have provided via a DT node a small segment of code (it's only a 
few assembly instructions after all) to drive the serial port because it 
would require a stable mapping to match that code's idea of the register 
locations.

> Plus its not just about bitmasks, but also about size of access, and
> polarity of bits.  So actually you'd need:
> 
> - virtual and physical address and size of transmit register
> - virtual and physical address, mask and value of transmit status
> - virtual and physical address, mask and value of CTS status
> 
> but then, we have debugging which doesn't have a physical address
> associated with it (the jtag stuff) so this doesn't cover the full
> story either. 

I'm not trying to cover the full story.  This is mainly for 98% of those 
cases where a plain serial port is used for both the DEBUG_LL _and_ the 
early output from the decompressor.  This is especially important if we 
want to boot a single zImage on multiple SOCs while preserving the early 
serial output capability.

The alternative is of course to wait for vendors to provide a BIOS-like 
interface, which is something I know they would be more than happy to do 
in the near future (they have ... big ideas beyond a generic serial 
output facility).  So I'd much prefer if we came up with a way to 
preserve as much independence from any such ROM firmware and keep 
control of the execution environment on Linux's side.

Those people with a JTAG debugger and the knowledge to use it really 
don't need any generic infrastructure to get some early debugging 
information out.  They can reconfigure their kernel and even hack the 
source to suit their needs.  But the people who are going to be the main 
consumers of a multi-SOC single-binary kernel won't be the ones 
recompiling their kernel just to provide us with debugging info.


Nicolas

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Fri, Aug 19, 2011 at 01:39:37PM +0200, Sascha Hauer wrote:
> > On Fri, Aug 19, 2011 at 12:09:41PM +0100, Will Deacon wrote:
> > > Sascha,
> > > 
> > > On Fri, Aug 19, 2011 at 07:35:33AM +0100, Sascha Hauer wrote:
> > > > On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > > > > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > > > > simplify the #ifdefery in debug-macro.S and add entries to the
> > > > > top-level Kconfig.debug instead.
> > > > 
> > > > I'm unsure whether I like this. The ifdeffery does not look very good,
> > > > but the Kconfig snippet is not shorter, also it is in generic arm code
> > > > and not i.MX specific. The old way also makes sure that we do not
> > > > compile in incompatible lowlevel debug code.
> > > 
> > > But it's an unfortunate hinderence to a single zImage kernel which we can
> > > only solve sensibly in the generic ARM code.
> > 
> > My problem is that if this option is enabled the kernel will not run
> > on any other SoC except the one being selected here, at least when
> > earlyprintk is passed on the command line.
> 
> I never liked the idea of coupling this into earlyprintk - and I think
> I said so at the time.  I'll say it again:
> 
> The LL DEBUG stuff is there to be able to do low level "it won't boot"
> debugging.  It's not there as a user option.  You are supposed to know
> exactly what you are doing when using the option.
> 
> If we're going to start having earlyprintk be an argument against this,
> I'll simply rip out the earlyprintk coupling to LL debug, and people
> can go back to patching printk.c to make this work.

But we need a functional earlyprintk.  It has to be usable by simple 
_users_ who are not developers.  ARM is not going to remain this obscur 
embedded architecture forever.

> > One could argue
> > that this option is for people who exactly know what they do only.
> 
> It _IS_ there for people who know what they're doing.  That's something
> I keep on saying about the LL debug stuff.  It's there to allow people
> to debug the early startup of the kernel.  It's not there for users.

That is not sufficient.


Nicolas

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 01:59:44PM -0400, Nicolas Pitre wrote:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > I never liked the idea of coupling this into earlyprintk - and I think
> > I said so at the time.  I'll say it again:
> > 
> > The LL DEBUG stuff is there to be able to do low level "it won't boot"
> > debugging.  It's not there as a user option.  You are supposed to know
> > exactly what you are doing when using the option.
> > 
> > If we're going to start having earlyprintk be an argument against this,
> > I'll simply rip out the earlyprintk coupling to LL debug, and people
> > can go back to patching printk.c to make this work.
> 
> But we need a functional earlyprintk.  It has to be usable by simple 
> _users_ who are not developers.  ARM is not going to remain this obscur 
> embedded architecture forever.

I repeat: that is not the point of DEBUG_LL.  DEBUG_LL is there for
*developers* to debug their initial board bringup.  Nothing else.

I repeat: I'm not going to have DEBUG_LL buggered up by do-gooders
and people wanting BIOS shite and then have to re-invent it so that
we can have decent debugging again.  That's NOT going to happen.  No
way at all.

> > > One could argue
> > > that this option is for people who exactly know what they do only.
> > 
> > It _IS_ there for people who know what they're doing.  That's something
> > I keep on saying about the LL debug stuff.  It's there to allow people
> > to debug the early startup of the kernel.  It's not there for users.
> 
> That is not sufficient.

Tough.  That's the way it is and it has to be lived with.  Early board
bring up has absolutely nothing to do with users.  If users are doing
early board bringup, they are kernel developers by definition.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 01:35:33PM -0400, Nicolas Pitre wrote:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > Plus you need the virtual address too, because the LL debug stuff is
> > there to debug around places like the initial assembly, before C code
> > is setup.
> 
> The kernel should determine or set up the virtual address by itself.  
> This is obviously not something we want the bootloader to provide.  For 
> the same reason, I've discarded the idea that the bootloader could 
> simply have provided via a DT node a small segment of code (it's only a 
> few assembly instructions after all) to drive the serial port because it 
> would require a stable mapping to match that code's idea of the register 
> locations.

Yes, and the virtual and physical base addresses are set via the
existing macros.

To take the virtual address out of that means that we then have to find
some way of storing that data - which can't be inside the normal kernel
bss or data sections.  BSS has not been zeroed at the point where we want
working DEBUG_LL stuff.

Defining a offset-fixed memory location from the kernel is fragile, and
will end up wasting the entire page - which would have to be permanently
reserved.

This is just getting _idiotic_.  There are times when "no" is the word
which has to be used, and this is one of them.

> I'm not trying to cover the full story.  This is mainly for 98% of those 
> cases where a plain serial port is used for both the DEBUG_LL _and_ the 
> early output from the decompressor.

So yet again we end up with another half baked "solution", which will
result in "end-users" being confused because it'll work on some stuff
but not on other stuff.

That is *no* solution what so ever.

> Those people with a JTAG debugger and the knowledge to use it really 
> don't need any generic infrastructure to get some early debugging 
> information out.  They can reconfigure their kernel and even hack the 
> source to suit their needs.  But the people who are going to be the main 
> consumers of a multi-SOC single-binary kernel won't be the ones 
> recompiling their kernel just to provide us with debugging info.

That is a stupid argument.  The people who need the early console are
those bringing up a new board.  By the act of being involved in bringing
up a new board, they are a developer.  They are not a user.  They will
be having to rebuild the kernel.

There is no technical problem here.  It's just entirely conceptual,
and one of trying to use stuff inappropriately.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Sun, Aug 21, 2011 at 01:59:44PM -0400, Nicolas Pitre wrote:
> > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > > I never liked the idea of coupling this into earlyprintk - and I think
> > > I said so at the time.  I'll say it again:
> > > 
> > > The LL DEBUG stuff is there to be able to do low level "it won't boot"
> > > debugging.  It's not there as a user option.  You are supposed to know
> > > exactly what you are doing when using the option.
> > > 
> > > If we're going to start having earlyprintk be an argument against this,
> > > I'll simply rip out the earlyprintk coupling to LL debug, and people
> > > can go back to patching printk.c to make this work.
> > 
> > But we need a functional earlyprintk.  It has to be usable by simple 
> > _users_ who are not developers.  ARM is not going to remain this obscur 
> > embedded architecture forever.
> 
> I repeat: that is not the point of DEBUG_LL.  DEBUG_LL is there for
> *developers* to debug their initial board bringup.  Nothing else.
> 
> I repeat: I'm not going to have DEBUG_LL buggered up by do-gooders
> and people wanting BIOS shite and then have to re-invent it so that
> we can have decent debugging again.  That's NOT going to happen.  No
> way at all.
> 
> > > > One could argue
> > > > that this option is for people who exactly know what they do only.
> > > 
> > > It _IS_ there for people who know what they're doing.  That's something
> > > I keep on saying about the LL debug stuff.  It's there to allow people
> > > to debug the early startup of the kernel.  It's not there for users.
> > 
> > That is not sufficient.
> 
> Tough.  That's the way it is and it has to be lived with.  Early board
> bring up has absolutely nothing to do with users.  If users are doing
> early board bringup, they are kernel developers by definition.

So be it if you insist.  We'll then have to invent a better 
infrastructure in parallel to be able to do early enough serial output 
for users, both for earlyprintk and the zImage decompressor.


Nicolas

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 02:28:46PM -0400, Nicolas Pitre wrote:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > Tough.  That's the way it is and it has to be lived with.  Early board
> > bring up has absolutely nothing to do with users.  If users are doing
> > early board bringup, they are kernel developers by definition.
> 
> So be it if you insist.  We'll then have to invent a better 
> infrastructure in parallel to be able to do early enough serial output 
> for users, both for earlyprintk and the zImage decompressor.

"Users" have nothing to do with it.

In any case, that will probably be _much_ better because it can be
independent of the debugging code to be used for sort out the *assembly*
part of the kernel boot.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Sun, Aug 21, 2011 at 01:35:33PM -0400, Nicolas Pitre wrote:
> > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > > Plus you need the virtual address too, because the LL debug stuff is
> > > there to debug around places like the initial assembly, before C code
> > > is setup.
> > 
> > The kernel should determine or set up the virtual address by itself.  
> > This is obviously not something we want the bootloader to provide.  For 
> > the same reason, I've discarded the idea that the bootloader could 
> > simply have provided via a DT node a small segment of code (it's only a 
> > few assembly instructions after all) to drive the serial port because it 
> > would require a stable mapping to match that code's idea of the register 
> > locations.
> 
> Yes, and the virtual and physical base addresses are set via the
> existing macros.
> 
> To take the virtual address out of that means that we then have to find
> some way of storing that data - which can't be inside the normal kernel
> bss or data sections.  BSS has not been zeroed at the point where we want
> working DEBUG_LL stuff.
> 
> Defining a offset-fixed memory location from the kernel is fragile, and
> will end up wasting the entire page - which would have to be permanently
> reserved.

Obviously the way to go is to use a .data variable.

And it can be referenced from assembly code in a position independent 
manner:

        .macro  get_ptr_pic, ptr, symbol
        b       199f
        .align
198:    .long   .
        .long   \symbol
199:    adr     \ptr, 198b              @ get relative address of 198b
        ldr     ip, [\ptr]              @ get absolute address of 198b
        sub     ip, ip, \ptr            @ offset between abs and rel
        ldr     \ptr, [\ptr, #4]        @ absolute address of \symbol
        sub     \ptr, \ptr, ip          @ make it relative
        .endm

> This is just getting _idiotic_.  There are times when "no" is the word
> which has to be used, and this is one of them.

You are saying "no" to the possibility of getting rid of the zImage 
decompressor serial output.

And now you're saying "no" to any possibility of keeping it alive in the 
context of a multi-SOC kernel.

Are you also saying "no" to the multi-SOC kernel?  If so you'll have to 
convince many more people than just me.

> > I'm not trying to cover the full story.  This is mainly for 98% of those 
> > cases where a plain serial port is used for both the DEBUG_LL _and_ the 
> > early output from the decompressor.
> 
> So yet again we end up with another half baked "solution", which will
> result in "end-users" being confused because it'll work on some stuff
> but not on other stuff.

The stuff this doesn't work on is not something "end-users" will ever 
deal with.  So this is a non argument.

> > Those people with a JTAG debugger and the knowledge to use it really 
> > don't need any generic infrastructure to get some early debugging 
> > information out.  They can reconfigure their kernel and even hack the 
> > source to suit their needs.  But the people who are going to be the main 
> > consumers of a multi-SOC single-binary kernel won't be the ones 
> > recompiling their kernel just to provide us with debugging info.
> 
> That is a stupid argument.  The people who need the early console are
> those bringing up a new board.  By the act of being involved in bringing
> up a new board, they are a developer.  They are not a user.  They will
> be having to rebuild the kernel.

Please stop thinking in terms of the legacy embedded environment we've 
been 
dealing with for the last 15 years.  People and even companies with lots 
of influence are pushing for ARM to become more ubiquitous, and for 
kernel _binaries_ to be bootable on yet-to-exist future boards 
unchanged, just like x86 has always done.  The near availability of an 
alternative operating system named after panes of glass is probably not 
unrelated to this.

But, just like on x86, booting a kernel binary (say, from a packaged 
distro) on a new machine might not exactly boot satisfactorily.  And 
this is likely to be the users who notice it first, in which case having 
functional early serial output might be pretty handy.

> There is no technical problem here.  It's just entirely conceptual,
> and one of trying to use stuff inappropriately.

I'm all ears to your suggestions about using stuff appropriately in the 
stated context.


Nicolas

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 03:02:25PM -0400, Nicolas Pitre wrote:
> Obviously the way to go is to use a .data variable.
> 
> And it can be referenced from assembly code in a position independent 
> manner:
> 
>         .macro  get_ptr_pic, ptr, symbol
>         b       199f
>         .align
> 198:    .long   .
>         .long   \symbol
> 199:    adr     \ptr, 198b              @ get relative address of 198b
>         ldr     ip, [\ptr]              @ get absolute address of 198b
>         sub     ip, ip, \ptr            @ offset between abs and rel
>         ldr     \ptr, [\ptr, #4]        @ absolute address of \symbol
>         sub     \ptr, \ptr, ip          @ make it relative
>         .endm

And have you checked that you can crap over 'ip' in the early assembly
code?  There are some (small) places where that register is used, which
means you accidentally corrupt that register.

> > This is just getting _idiotic_.  There are times when "no" is the word
> > which has to be used, and this is one of them.
> 
> You are saying "no" to the possibility of getting rid of the zImage 
> decompressor serial output.
>
> And now you're saying "no" to any possibility of keeping it alive in the 
> context of a multi-SOC kernel.

The decompressor has nothing to do with this.

> Are you also saying "no" to the multi-SOC kernel?  If so you'll have to 
> convince many more people than just me.

No, I am saying no to the low level debugging code which *developers*
rely upon working being fucked up and rendered useless - and so requiring
a new implementation to be written - in the name of multi-SoC support.
That is what I'm *strongly* objecting to.

The LL debug code is there *explicitly* so that it can be used to debug
the early kernel assembly and initial part of the kernel bring-up.  It
is *NOT* there for USERS.

The fact that it was tied into earlyprintk *against my better judgement*
and now its wanting to be fucked up so it can't be used for its primary
purpose just goes to prove that my original assertions at the time that
it was used for early printk have been *proven*.

So I'm now saying a very very firm NO to this further creap of it.  I am
not having its utility destroyed.

> But, just like on x86, booting a kernel binary (say, from a packaged 
> distro) on a new machine might not exactly boot satisfactorily.  And 
> this is likely to be the users who notice it first, in which case having 
> functional early serial output might be pretty handy.

Then provide a *proper* early printk implementation which doesn't involve
buggering up assembly whos purpose is to do low level assembly debugging.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 08:18:35PM +0100, Russell King - ARM Linux wrote:
> On Sun, Aug 21, 2011 at 03:02:25PM -0400, Nicolas Pitre wrote:
> > Obviously the way to go is to use a .data variable.
> > 
> > And it can be referenced from assembly code in a position independent 
> > manner:
> > 
> >         .macro  get_ptr_pic, ptr, symbol
> >         b       199f
> >         .align
> > 198:    .long   .
> >         .long   \symbol
> > 199:    adr     \ptr, 198b              @ get relative address of 198b
> >         ldr     ip, [\ptr]              @ get absolute address of 198b
> >         sub     ip, ip, \ptr            @ offset between abs and rel
> >         ldr     \ptr, [\ptr, #4]        @ absolute address of \symbol
> >         sub     \ptr, \ptr, ip          @ make it relative
> >         .endm
> 
> And have you checked that you can crap over 'ip' in the early assembly
> code?  There are some (small) places where that register is used, which
> means you accidentally corrupt that register.

And further to this, I'll point out that the debugging functions are
*explicitly* designed to avoid corrupting any more than just r0-r3
and lr.  That's not just the IO functions but also the hex and string
printing functions.

And the head*.S code is explicitly written to expect r0-r3 to be
corrupted - which basically means that no long-term values are held in
those registers.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Sun, Aug 21, 2011 at 03:02:25PM -0400, Nicolas Pitre wrote:
> > Obviously the way to go is to use a .data variable.
> > 
> > And it can be referenced from assembly code in a position independent 
> > manner:
> > 
> >         .macro  get_ptr_pic, ptr, symbol
> >         b       199f
> >         .align
> > 198:    .long   .
> >         .long   \symbol
> > 199:    adr     \ptr, 198b              @ get relative address of 198b
> >         ldr     ip, [\ptr]              @ get absolute address of 198b
> >         sub     ip, ip, \ptr            @ offset between abs and rel
> >         ldr     \ptr, [\ptr, #4]        @ absolute address of \symbol
> >         sub     \ptr, \ptr, ip          @ make it relative
> >         .endm
> 
> And have you checked that you can crap over 'ip' in the early assembly
> code?  There are some (small) places where that register is used, which
> means you accidentally corrupt that register.

You are down to details now.  I'm still trying to validate concepts.  
Implementation details such this can be sorted out trivially later.

> > > This is just getting _idiotic_.  There are times when "no" is the word
> > > which has to be used, and this is one of them.
> > 
> > You are saying "no" to the possibility of getting rid of the zImage 
> > decompressor serial output.
> >
> > And now you're saying "no" to any possibility of keeping it alive in the 
> > context of a multi-SOC kernel.
> 
> The decompressor has nothing to do with this.

Sure it does, since it relies on early serial output capabilities, just 
like earlyprintk, or possibly even the senduart code.  Solving this once 
for them all, *appropriately*, would certainly be nice.  I would be glad 
to see this need addressed without making further ugly abuses like the 
OMAP people did with their OMAP_UART_INFO and the 
arch/arm/plat-omap/include/plat/uncompress.h abomination.

> > Are you also saying "no" to the multi-SOC kernel?  If so you'll have to 
> > convince many more people than just me.
> 
> No, I am saying no to the low level debugging code which *developers*
> rely upon working being fucked up and rendered useless - and so requiring
> a new implementation to be written - in the name of multi-SoC support.
> That is what I'm *strongly* objecting to.

I really don't mind leaving that code well alone actually.

We can well work something out for earlyprintk and zImage needs first, 
see how it works, get some confidence in it, and then if that appears to 
be a worthwhile thing to do then _maybe_ a version of the DEBUG_LL code 
could be implemented in terms of it.

In the meantime, if CONFIG_DEBUG_LL is really really meant to be _only_ 
for early assembly debugging by seasoned developers then I don't think 
that expanding its configurability through a Kconfig menu is a good 
thing.  Instead, it should probably be _removed_ from Kconfig entirely.


Nicolas

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> And further to this, I'll point out that the debugging functions are
> *explicitly* designed to avoid corrupting any more than just r0-r3
> and lr.  That's not just the IO functions but also the hex and string
> printing functions.
> 
> And the head*.S code is explicitly written to expect r0-r3 to be
> corrupted - which basically means that no long-term values are held in
> those registers.

Well, not exactly.  I actually have a patch to that effect I made a 
while ago so all the early code could be unaffected by inserted function 
call, but held on to it because nothing yet justified its need.  Here it 
is for reference:

commit f2c97ae9f677c4abca8efe87539beab7e32e3e6c
Author: Nicolas Pitre <nico@fluxnic.net>
Date:   Thu Feb 24 23:02:20 2011 -0500

    ARM: make head.S register allocation more convenient
    
    The r1 (machine ID) and r2 (boot data pointer) values are getting
    in the way of standard procedure calls as those registers are normally
    clobbered by function calls.  Move those to r6 and r7 respectively,
    and adjust the code accordingly.
    
    Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index c84b57d..6421c90 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -26,32 +26,32 @@
  */
 	__HEAD
 
-/* Determine validity of the r2 atags pointer.  The heuristic requires
- * that the pointer be aligned, in the first 16k of physical RAM and
- * that the ATAG_CORE marker is first and present.  Future revisions
+/* Determine validity of the r2 (now in r7) atags pointer.  The heuristic
+ * requires that the pointer be aligned, in the first 16k of physical RAM
+ * and that the ATAG_CORE marker is first and present.  Future revisions
  * of this function may be more lenient with the physical address and
  * may also be able to move the ATAGS block if necessary.
  *
  * Returns:
- *  r2 either valid atags pointer, or zero
- *  r5, r6 corrupted
+ *  r7 either valid atags pointer, or zero
+ *  r1, r5 corrupted
  */
 __vet_atags:
-	tst	r2, #0x3			@ aligned?
+	tst	r7, #0x3			@ aligned?
 	bne	1f
 
-	ldr	r5, [r2, #0]			@ is first tag ATAG_CORE?
+	ldr	r5, [r7, #0]			@ is first tag ATAG_CORE?
 	cmp	r5, #ATAG_CORE_SIZE
 	cmpne	r5, #ATAG_CORE_SIZE_EMPTY
 	bne	1f
-	ldr	r5, [r2, #4]
-	ldr	r6, =ATAG_CORE
-	cmp	r5, r6
+	ldr	r5, [r7, #4]
+	ldr	r1, =ATAG_CORE
+	cmp	r5, r1
 	bne	1f
 
 	mov	pc, lr				@ atag pointer is ok
 
-1:	mov	r2, #0
+1:	mov	r7, #0
 	mov	pc, lr
 ENDPROC(__vet_atags)
 
@@ -60,48 +60,48 @@ ENDPROC(__vet_atags)
  * and uses absolute addresses; this is not position independent.
  *
  *  r0  = cp#15 control register
- *  r1  = machine ID
- *  r2  = atags pointer
+ *  r6  = machine ID
+ *  r7  = atags pointer
  *  r9  = processor ID
  */
 	__INIT
 __mmap_switched:
 	adr	r3, __mmap_switched_data
 
-	ldmia	r3!, {r4, r5, r6, r7}
-	cmp	r4, r5				@ Copy data segment if needed
-1:	cmpne	r5, r6
-	ldrne	fp, [r4], #4
-	strne	fp, [r5], #4
+	ldmia	r3!, {r1, r2, r4, r5}
+	cmp	r1, r2				@ Copy data segment if needed
+1:	cmpne	r2, r4
+	ldrne	fp, [r1], #4
+	strne	fp, [r2], #4
 	bne	1b
 
 	mov	fp, #0				@ Clear BSS (and zero fp)
-1:	cmp	r6, r7
-	strcc	fp, [r6],#4
+1:	cmp	r4, r5
+	strcc	fp, [r4], #4
 	bcc	1b
 
- ARM(	ldmia	r3, {r4, r5, r6, r7, sp})
- THUMB(	ldmia	r3, {r4, r5, r6, r7}	)
+ ARM(	ldmia	r3, {r1, r2, r4, r5, sp})
+ THUMB(	ldmia	r3, {r1, r2, r4, r5}	)
  THUMB(	ldr	sp, [r3, #16]		)
-	str	r9, [r4]			@ Save processor ID
-	str	r1, [r5]			@ Save machine type
-	str	r2, [r6]			@ Save atags pointer
-	bic	r4, r0, #CR_A			@ Clear 'A' bit
-	stmia	r7, {r0, r4}			@ Save control register values
+	str	r9, [r1]			@ Save processor ID
+	str	r6, [r2]			@ Save machine type
+	str	r7, [r4]			@ Save atags pointer
+	bic	r1, r0, #CR_A			@ Clear 'A' bit
+	stmia	r5, {r0, r1}			@ Save control register values
 	b	start_kernel
 ENDPROC(__mmap_switched)
 
 	.align	2
 	.type	__mmap_switched_data, %object
 __mmap_switched_data:
-	.long	__data_loc			@ r4
-	.long	_sdata				@ r5
-	.long	__bss_start			@ r6
-	.long	_end				@ r7
-	.long	processor_id			@ r4
-	.long	__machine_arch_type		@ r5
-	.long	__atags_pointer			@ r6
-	.long	cr_alignment			@ r7
+	.long	__data_loc			@ r1
+	.long	_sdata				@ r2
+	.long	__bss_start			@ r4
+	.long	_end				@ r5
+	.long	processor_id			@ r1
+	.long	__machine_arch_type		@ r2
+	.long	__atags_pointer			@ r4
+	.long	cr_alignment			@ r5
 	.long	init_thread_union + THREAD_START_SP @ sp
 	.size	__mmap_switched_data, . - __mmap_switched_data
 
@@ -109,11 +109,10 @@ __mmap_switched_data:
  * This provides a C-API version of __lookup_processor_type
  */
 ENTRY(lookup_processor_type)
-	stmfd	sp!, {r4 - r6, r9, lr}
+	stmfd	sp!, {r9, lr}
 	mov	r9, r0
 	bl	__lookup_processor_type
-	mov	r0, r5
-	ldmfd	sp!, {r4 - r6, r9, pc}
+	ldmfd	sp!, {r9, pc}
 ENDPROC(lookup_processor_type)
 
 /*
@@ -125,25 +124,25 @@ ENDPROC(lookup_processor_type)
  *
  *	r9 = cpuid
  * Returns:
- *	r3, r4, r6 corrupted
- *	r5 = proc_info pointer in physical address space
+ *	r1, r2, r3 corrupted
+ *	r0 = proc_info pointer in physical address space
  *	r9 = cpuid (preserved)
  */
 	__CPUINIT
 __lookup_processor_type:
 	adr	r3, __lookup_processor_type_data
-	ldmia	r3, {r4 - r6}
-	sub	r3, r3, r4			@ get offset between virt&phys
-	add	r5, r5, r3			@ convert virt addresses to
-	add	r6, r6, r3			@ physical address space
-1:	ldmia	r5, {r3, r4}			@ value, mask
-	and	r4, r4, r9			@ mask wanted bits
-	teq	r3, r4
+	ldmia	r3, {r0 - r2}
+	sub	r3, r3, r0			@ get offset between virt&phys
+	add	r0, r1, r3			@ convert virt addresses to
+	add	r1, r2, r3			@ physical address space
+1:	ldmia	r0, {r2, r3}			@ value, mask
+	and	r3, r3, r9			@ mask wanted bits
+	teq	r2, r3
 	beq	2f
-	add	r5, r5, #PROC_INFO_SZ		@ sizeof(proc_info_list)
-	cmp	r5, r6
+	add	r0, r0, #PROC_INFO_SZ		@ sizeof(proc_info_list)
+	cmp	r0, r1
 	blo	1b
-	mov	r5, #0				@ unknown processor
+	mov	r0, #0				@ unknown processor
 2:	mov	pc, lr
 ENDPROC(__lookup_processor_type)
 
diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index 6b1e0ad..c210e01 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -36,13 +36,15 @@
 ENTRY(stext)
 	setmode	PSR_F_BIT | PSR_I_BIT | SVC_MODE, r9 @ ensure svc mode
 						@ and irqs disabled
+	mov	r6, r1				@ preserve machine ID
+	mov	r7, r2				@ preserve boot data pointer
 #ifndef CONFIG_CPU_CP15
 	ldr	r9, =CONFIG_PROCESSOR_ID
 #else
 	mrc	p15, 0, r9, c0, c0		@ get processor id
 #endif
 	bl	__lookup_processor_type		@ r5=procinfo r9=cpuid
-	movs	r10, r5				@ invalid processor (r5=0)?
+	movs	r10, r0				@ invalid processor (r5=0)?
 	beq	__error_p				@ yes, error 'p'
 
 	adr	lr, BSYM(__after_proc_init)	@ return (PIC) address
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 8f96ca0..a52aa76 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -75,9 +75,11 @@
 ENTRY(stext)
 	setmode	PSR_F_BIT | PSR_I_BIT | SVC_MODE, r9 @ ensure svc mode
 						@ and irqs disabled
+	mov	r6, r1				@ preserve machine ID
+	mov	r7, r2				@ preserve boot data pointer
 	mrc	p15, 0, r9, c0, c0		@ get processor id
 	bl	__lookup_processor_type		@ r5=procinfo r9=cpuid
-	movs	r10, r5				@ invalid processor (r5=0)?
+	movs	r10, r0				@ invalid processor (r5=0)?
  THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_p			@ yes, error 'p'
 
@@ -91,7 +93,7 @@ ENTRY(stext)
 #endif
 
 	/*
-	 * r1 = machine no, r2 = atags,
+	 * r6 = machine no, r7 = atags,
 	 * r8 = phys_offset, r9 = cpuid, r10 = procinfo
 	 */
 	bl	__vet_atags
@@ -132,7 +134,7 @@ ENDPROC(stext)
  * r8 = phys_offset, r9 = cpuid, r10 = procinfo
  *
  * Returns:
- *  r0, r3, r5-r7 corrupted
+ *  r0, r1, r2, r3, r5 corrupted
  *  r4 = physical page table address
  */
 __create_page_tables:
@@ -143,49 +145,49 @@ __create_page_tables:
 	 */
 	mov	r0, r4
 	mov	r3, #0
-	add	r6, r0, #0x4000
+	add	r1, r0, #0x4000
 1:	str	r3, [r0], #4
 	str	r3, [r0], #4
 	str	r3, [r0], #4
 	str	r3, [r0], #4
-	teq	r0, r6
+	teq	r0, r1
 	bne	1b
 
-	ldr	r7, [r10, #PROCINFO_MM_MMUFLAGS] @ mm_mmuflags
+	ldr	r2, [r10, #PROCINFO_MM_MMUFLAGS] @ mm_mmuflags
 
 	/*
 	 * Create identity mapping to cater for __enable_mmu.
 	 * This identity mapping will be removed by paging_init().
 	 */
 	adr	r0, __enable_mmu_loc
-	ldmia	r0, {r3, r5, r6}
-	sub	r0, r0, r3			@ virt->phys offset
-	add	r5, r5, r0			@ phys __enable_mmu
-	add	r6, r6, r0			@ phys __enable_mmu_end
+	ldmia	r0, {r1, r3, r5}
+	sub	r0, r0, r1			@ virt->phys offset
+	add	r3, r3, r0			@ phys __enable_mmu
+	add	r5, r5, r0			@ phys __enable_mmu_end
+	mov	r3, r3, lsr #20
 	mov	r5, r5, lsr #20
-	mov	r6, r6, lsr #20
 
-1:	orr	r3, r7, r5, lsl #20		@ flags + kernel base
-	str	r3, [r4, r5, lsl #2]		@ identity mapping
-	teq	r5, r6
-	addne	r5, r5, #1			@ next section
+1:	orr	r1, r2, r3, lsl #20		@ flags + kernel base
+	str	r1, [r4, r3, lsl #2]		@ identity mapping
+	teq	r3, r5
+	addne	r3, r3, #1			@ next section
 	bne	1b
 
 	/*
 	 * Now setup the pagetables for our kernel direct
 	 * mapped region.
 	 */
-	mov	r3, pc
-	mov	r3, r3, lsr #20
-	orr	r3, r7, r3, lsl #20
+	mov	r5, pc
+	mov	r5, r5, lsr #20
+	orr	r5, r2, r5, lsl #20
 	add	r0, r4,  #(KERNEL_START & 0xff000000) >> 18
-	str	r3, [r0, #(KERNEL_START & 0x00f00000) >> 18]!
-	ldr	r6, =(KERNEL_END - 1)
+	str	r5, [r0, #(KERNEL_START & 0x00f00000) >> 18]!
+	ldr	r3, =(KERNEL_END - 1)
 	add	r0, r0, #4
-	add	r6, r4, r6, lsr #18
-1:	cmp	r0, r6
-	add	r3, r3, #1 << 20
-	strls	r3, [r0], #4
+	add	r3, r4, r3, lsr #18
+1:	cmp	r0, r3
+	add	r5, r5, #1 << 20
+	strls	r5, [r0], #4
 	bls	1b
 
 #ifdef CONFIG_XIP_KERNEL
@@ -193,30 +195,30 @@ __create_page_tables:
 	 * Map some ram to cover our .data and .bss areas.
 	 */
 	add	r3, r8, #TEXT_OFFSET
-	orr	r3, r3, r7
+	orr	r3, r3, r2
 	add	r0, r4,  #(KERNEL_RAM_VADDR & 0xff000000) >> 18
 	str	r3, [r0, #(KERNEL_RAM_VADDR & 0x00f00000) >> 18]!
-	ldr	r6, =(_end - 1)
+	ldr	r1, =(_end - 1)
 	add	r0, r0, #4
-	add	r6, r4, r6, lsr #18
-1:	cmp	r0, r6
+	add	r1, r4, r1, lsr #18
+1:	cmp	r0, r1
 	add	r3, r3, #1 << 20
 	strls	r3, [r0], #4
 	bls	1b
 #endif
 
 	/*
-	 * Then map boot params address in r2 or
+	 * Then map boot params address in r7 or
 	 * the first 1MB of ram if boot params address is not specified.
 	 */
-	mov	r0, r2, lsr #20
+	mov	r0, r7, lsr #20
 	movs	r0, r0, lsl #20
 	moveq	r0, r8
 	sub	r3, r0, r8
 	add	r3, r3, #PAGE_OFFSET
 	add	r3, r4, r3, lsr #18
-	orr	r6, r7, r0
-	str	r6, [r3]
+	orr	r1, r2, r0
+	str	r1, [r3]
 
 #ifdef CONFIG_DEBUG_LL
 #ifndef CONFIG_DEBUG_ICEDCC
@@ -225,7 +227,7 @@ __create_page_tables:
 	 * This allows debug messages to be output
 	 * via a serial console before paging_init.
 	 */
-	addruart r7, r3
+	addruart r2, r3
 
 	mov	r3, r3, lsr #20
 	mov	r3, r3, lsl #2
@@ -234,18 +236,18 @@ __create_page_tables:
 	rsb	r3, r3, #0x4000			@ PTRS_PER_PGD*sizeof(long)
 	cmp	r3, #0x0800			@ limit to 512MB
 	movhi	r3, #0x0800
-	add	r6, r0, r3
-	mov	r3, r7, lsr #20
-	ldr	r7, [r10, #PROCINFO_IO_MMUFLAGS] @ io_mmuflags
-	orr	r3, r7, r3, lsl #20
+	add	r1, r0, r3
+	mov	r3, r2, lsr #20
+	ldr	r2, [r10, #PROCINFO_IO_MMUFLAGS] @ io_mmuflags
+	orr	r3, r2, r3, lsl #20
 1:	str	r3, [r0], #4
 	add	r3, r3, #1 << 20
-	teq	r0, r6
+	teq	r0, r1
 	bne	1b
 
 #else /* CONFIG_DEBUG_ICEDCC */
 	/* we don't need any serial debugging mappings for ICEDCC */
-	ldr	r7, [r10, #PROCINFO_IO_MMUFLAGS] @ io_mmuflags
+	ldr	r2, [r10, #PROCINFO_IO_MMUFLAGS] @ io_mmuflags
 #endif /* !CONFIG_DEBUG_ICEDCC */
 
 #if defined(CONFIG_ARCH_NETWINDER) || defined(CONFIG_ARCH_CATS)
@@ -254,7 +256,7 @@ __create_page_tables:
 	 * in the 16550-type serial port for the debug messages
 	 */
 	add	r0, r4, #0xff000000 >> 18
-	orr	r3, r7, #0x7c000000
+	orr	r3, r2, #0x7c000000
 	str	r3, [r0]
 #endif
 #ifdef CONFIG_ARCH_RPC
@@ -264,7 +266,7 @@ __create_page_tables:
 	 * only for Acorn RiscPC architectures.
 	 */
 	add	r0, r4, #0x02000000 >> 18
-	orr	r3, r7, #0x02000000
+	orr	r3, r2, #0x02000000
 	str	r3, [r0]
 	add	r0, r4, #0xd8000000 >> 18
 	str	r3, [r0]
@@ -301,9 +303,9 @@ ENTRY(secondary_startup)
 	 * Use the page tables supplied from  __cpu_up.
 	 */
 	adr	r4, __secondary_data
-	ldmia	r4, {r5, r7, r12}		@ address to jump to after
-	sub	r4, r4, r5			@ mmu has been enabled
-	ldr	r4, [r7, r4]			@ get secondary_data.pgdir
+	ldmia	r4, {r2, r3, r12}		@ address to jump to after
+	sub	r4, r4, r2			@ mmu has been enabled
+	ldr	r4, [r3, r4]			@ get secondary_data.pgdir
 	adr	lr, BSYM(__enable_mmu)		@ return address
 	mov	r13, r12			@ __secondary_switched address
  ARM(	add	pc, r10, #PROCINFO_INITFUNC	) @ initialise processor
@@ -313,10 +315,10 @@ ENTRY(secondary_startup)
 ENDPROC(secondary_startup)
 
 	/*
-	 * r6  = &secondary_data
+	 * r1  = &secondary_data
 	 */
 ENTRY(__secondary_switched)
-	ldr	sp, [r7, #4]			@ get secondary_data.stack
+	ldr	sp, [r2, #4]			@ get secondary_data.stack
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
@@ -338,9 +340,9 @@ __secondary_data:
  * registers.
  *
  *  r0  = cp#15 control register
- *  r1  = machine ID
- *  r2  = atags pointer
  *  r4  = page table pointer
+ *  r6  = machine ID
+ *  r7  = atags pointer
  *  r9  = processor ID
  *  r13 = *virtual* address to jump to upon completion
  */
@@ -375,8 +377,8 @@ ENDPROC(__enable_mmu)
  * mailing list archives BEFORE sending another post to the list.
  *
  *  r0  = cp#15 control register
- *  r1  = machine ID
- *  r2  = atags pointer
+ *  r6  = machine ID
+ *  r7  = atags pointer
  *  r9  = processor ID
  *  r13 = *virtual* address to jump to upon completion
  *
@@ -440,25 +442,25 @@ smp_on_up:
 __do_fixup_smp_on_up:
 	cmp	r4, r5
 	movhs	pc, lr
-	ldmia	r4!, {r0, r6}
- ARM(	str	r6, [r0, r3]	)
+	ldmia	r4!, {r0, r1}
+ ARM(	str	r1, [r0, r3]	)
  THUMB(	add	r0, r0, r3	)
 #ifdef __ARMEB__
- THUMB(	mov	r6, r6, ror #16	)	@ Convert word order for big-endian.
+ THUMB(	mov	r1, r1, ror #16	)	@ Convert word order for big-endian.
 #endif
- THUMB(	strh	r6, [r0], #2	)	@ For Thumb-2, store as two halfwords
- THUMB(	mov	r6, r6, lsr #16	)	@ to be robust against misaligned r3.
- THUMB(	strh	r6, [r0]	)
+ THUMB(	strh	r1, [r0], #2	)	@ For Thumb-2, store as two halfwords
+ THUMB(	mov	r1, r1, lsr #16	)	@ to be robust against misaligned r3.
+ THUMB(	strh	r1, [r0]	)
 	b	__do_fixup_smp_on_up
 ENDPROC(__do_fixup_smp_on_up)
 
 ENTRY(fixup_smp)
-	stmfd	sp!, {r4 - r6, lr}
+	stmfd	sp!, {r4, r5, lr}
 	mov	r4, r0
 	add	r5, r0, r1
 	mov	r3, #0
 	bl	__do_fixup_smp_on_up
-	ldmfd	sp!, {r4 - r6, pc}
+	ldmfd	sp!, {r4, r5, pc}
 ENDPROC(fixup_smp)
 
 #ifdef CONFIG_ARM_PATCH_PHYS_VIRT
@@ -471,20 +473,20 @@ ENDPROC(fixup_smp)
 	__HEAD
 __fixup_pv_table:
 	adr	r0, 1f
-	ldmia	r0, {r3-r5, r7}
-	sub	r3, r0, r3	@ PHYS_OFFSET - PAGE_OFFSET
-	add	r4, r4, r3	@ adjust table start address
-	add	r5, r5, r3	@ adjust table end address
-	str	r8, [r7, r3]!	@ save computed PHYS_OFFSET to __pv_phys_offset
+	ldmia	r0, {r2 - r5}
+	sub	r2, r0, r2	@ PHYS_OFFSET - PAGE_OFFSET
+	add	r3, r3, r2	@ adjust table start address
+	add	r4, r4, r2	@ adjust table end address
+	str	r8, [r5, r2]!	@ save computed PHYS_OFFSET to __pv_phys_offset
 #ifndef CONFIG_ARM_PATCH_PHYS_VIRT_16BIT
-	mov	r6, r3, lsr #24	@ constant for add/sub instructions
-	teq	r3, r6, lsl #24 @ must be 16MiB aligned
+	mov	r1, r2, lsr #24	@ constant for add/sub instructions
+	teq	r2, r1, lsl #24 @ must be 16MiB aligned
 #else
-	mov	r6, r3, lsr #16	@ constant for add/sub instructions
-	teq	r3, r6, lsl #16	@ must be 64kiB aligned
+	mov	r1, r2, lsr #16	@ constant for add/sub instructions
+	teq	r2, r1, lsl #16	@ must be 64kiB aligned
 #endif
 	bne	__error
-	str	r6, [r7, #4]	@ save to __pv_offset
+	str	r1, [r5, #4]	@ save to __pv_offset
 	b	__fixup_a_pv_table
 ENDPROC(__fixup_pv_table)
 
@@ -497,33 +499,33 @@ ENDPROC(__fixup_pv_table)
 	.text
 __fixup_a_pv_table:
 #ifdef CONFIG_ARM_PATCH_PHYS_VIRT_16BIT
-	and	r0, r6, #255	@ offset bits 23-16
-	mov	r6, r6, lsr #8	@ offset bits 31-24
+	and	r0, r1, #255	@ offset bits 23-16
+	mov	r1, r1, lsr #8	@ offset bits 31-24
 #else
 	mov	r0, #0		@ just in case...
 #endif
 	b	3f
-2:	ldr	ip, [r7, r3]
+2:	ldr	ip, [r5, r2]
 	bic	ip, ip, #0x000000ff
 	tst	ip, #0x400	@ rotate shift tells us LS or MS byte
-	orrne	ip, ip, r6	@ mask in offset bits 31-24
+	orrne	ip, ip, r1	@ mask in offset bits 31-24
 	orreq	ip, ip, r0	@ mask in offset bits 23-16
-	str	ip, [r7, r3]
-3:	cmp	r4, r5
-	ldrcc	r7, [r4], #4	@ use branch for delay slot
+	str	ip, [r5, r2]
+3:	cmp	r3, r4
+	ldrcc	r5, [r3], #4	@ use branch for delay slot
 	bcc	2b
 	mov	pc, lr
 ENDPROC(__fixup_a_pv_table)
 
 ENTRY(fixup_pv_table)
-	stmfd	sp!, {r4 - r7, lr}
-	ldr	r2, 2f			@ get address of __pv_phys_offset
-	mov	r3, #0			@ no offset
-	mov	r4, r0			@ r0 = table start
-	add	r5, r0, r1		@ r1 = table size
-	ldr	r6, [r2, #4]		@ get __pv_offset
+	stmfd	sp!, {r4, r5, lr}
+	ldr	r5, 2f			@ get address of __pv_phys_offset
+	mov	r2, #0			@ no offset
+	mov	r3, r0			@ r0 = table start
+	add	r4, r0, r1		@ r1 = table size
+	ldr	r1, [r5, #4]		@ get __pv_offset
 	bl	__fixup_a_pv_table
-	ldmfd	sp!, {r4 - r7, pc}
+	ldmfd	sp!, {r4, r5, pc}
 ENDPROC(fixup_pv_table)
 
 	.align

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 04:07:37PM -0400, Nicolas Pitre wrote:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> 
> > And further to this, I'll point out that the debugging functions are
> > *explicitly* designed to avoid corrupting any more than just r0-r3
> > and lr.  That's not just the IO functions but also the hex and string
> > printing functions.
> > 
> > And the head*.S code is explicitly written to expect r0-r3 to be
> > corrupted - which basically means that no long-term values are held in
> > those registers.
> 
> Well, not exactly.  I actually have a patch to that effect I made a 
> while ago so all the early code could be unaffected by inserted function 
> call, but held on to it because nothing yet justified its need.  Here it 
> is for reference:

And so this buggers up the ability to insert calls to the debugging code
by placing values into r0-r3.  So that patch will get a nak too.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Sun, Aug 21, 2011 at 04:07:37PM -0400, Nicolas Pitre wrote:
> > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > 
> > > And further to this, I'll point out that the debugging functions are
> > > *explicitly* designed to avoid corrupting any more than just r0-r3
> > > and lr.  That's not just the IO functions but also the hex and string
> > > printing functions.
> > > 
> > > And the head*.S code is explicitly written to expect r0-r3 to be
> > > corrupted - which basically means that no long-term values are held in
> > > those registers.
> > 
> > Well, not exactly.  I actually have a patch to that effect I made a 
> > while ago so all the early code could be unaffected by inserted function 
> > call, but held on to it because nothing yet justified its need.  Here it 
> > is for reference:
> 
> And so this buggers up the ability to insert calls to the debugging code
> by placing values into r0-r3.  So that patch will get a nak too.

What?  Please look again at the patch and tell me what is wrong with it.
Because I can't make sense of your last sentence.


Nicolas

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Sun, Aug 21, 2011 at 05:00:46PM -0400, Nicolas Pitre wrote:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> 
> > On Sun, Aug 21, 2011 at 04:07:37PM -0400, Nicolas Pitre wrote:
> > > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > > 
> > > > And further to this, I'll point out that the debugging functions are
> > > > *explicitly* designed to avoid corrupting any more than just r0-r3
> > > > and lr.  That's not just the IO functions but also the hex and string
> > > > printing functions.
> > > > 
> > > > And the head*.S code is explicitly written to expect r0-r3 to be
> > > > corrupted - which basically means that no long-term values are held in
> > > > those registers.
> > > 
> > > Well, not exactly.  I actually have a patch to that effect I made a 
> > > while ago so all the early code could be unaffected by inserted function 
> > > call, but held on to it because nothing yet justified its need.  Here it 
> > > is for reference:
> > 
> > And so this buggers up the ability to insert calls to the debugging code
> > by placing values into r0-r3.  So that patch will get a nak too.
> 
> What?  Please look again at the patch and tell me what is wrong with it.
> Because I can't make sense of your last sentence.

Have you not been reading what I've been saying.

Point 1: the code explicitly _avoids_ using r0-r3 except in small short
code sequences.

Point 2: the debugging macros explicitly use r0-r3 because they know that
these registers aren't going to be used in the assembly code except in
small short code sequences.

So, changing all the assembly to use r0-r3 is going to bugger up the
ability to use the debugging macros.  Therefore this is a change with
a net reduction in facility.  Therefore I don't want it.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Nicolas Pitre]

On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:

> On Sun, Aug 21, 2011 at 05:00:46PM -0400, Nicolas Pitre wrote:
> > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > 
> > > On Sun, Aug 21, 2011 at 04:07:37PM -0400, Nicolas Pitre wrote:
> > > > On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > > > 
> > > > > And further to this, I'll point out that the debugging functions are
> > > > > *explicitly* designed to avoid corrupting any more than just r0-r3
> > > > > and lr.  That's not just the IO functions but also the hex and string
> > > > > printing functions.
> > > > > 
> > > > > And the head*.S code is explicitly written to expect r0-r3 to be
> > > > > corrupted - which basically means that no long-term values are held in
> > > > > those registers.
> > > > 
> > > > Well, not exactly.  I actually have a patch to that effect I made a 
> > > > while ago so all the early code could be unaffected by inserted function 
> > > > call, but held on to it because nothing yet justified its need.  Here it 
> > > > is for reference:
> > > 
> > > And so this buggers up the ability to insert calls to the debugging code
> > > by placing values into r0-r3.  So that patch will get a nak too.
> > 
> > What?  Please look again at the patch and tell me what is wrong with it.
> > Because I can't make sense of your last sentence.
> 
> Have you not been reading what I've been saying.
> 
> Point 1: the code explicitly _avoids_ using r0-r3 except in small short
> code sequences.
> 
> Point 2: the debugging macros explicitly use r0-r3 because they know that
> these registers aren't going to be used in the assembly code except in
> small short code sequences.
> 
I totally agree.

Now, Have you not been reading what my patch does, and what my patch log 
message says?

What you explained above is not in agreement with the current state of 
the code, and my patch was actually making the code conform to what you 
say.

> So, changing all the assembly to use r0-r3 is going to bugger up the
> ability to use the debugging macros.  Therefore this is a change with
> a net reduction in facility.  Therefore I don't want it.

Clearly you didn't read my patch.  So let me resume its purpose here: 
throughout the whole head*.S code, r1 and r2 are live with the machine 
ID and ATAG pointer, right up to before the call to start_kernel.  My 
patch move them away so r0-r3 are really avoided except for short code 
sequences.


Nicolas

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [110821 22:21]:
> On Sun, 21 Aug 2011, Russell King - ARM Linux wrote:
> > 
> > The decompressor has nothing to do with this.
> 
> Sure it does, since it relies on early serial output capabilities, just 
> like earlyprintk, or possibly even the senduart code.  Solving this once 
> for them all, *appropriately*, would certainly be nice.  I would be glad 
> to see this need addressed without making further ugly abuses like the 
> OMAP people did with their OMAP_UART_INFO and the 
> arch/arm/plat-omap/include/plat/uncompress.h abomination.

Well the way the debug_ll code is currenty set up just won't scale.
The basic problem is that the debug_ll code should be machine specific,
not arch specific..

Probably the best long term solution is to set up the debug uart based
on DT data and initialize it in the uncompress code. Anybody debugging
lower level stuff can certainly patch in the ucompress debug_ll code.

Regards,

Tony

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Tue, Sep 06, 2011 at 12:28:17PM +0300, Tony Lindgren wrote:
> Probably the best long term solution is to set up the debug uart based
> on DT data and initialize it in the uncompress code. Anybody debugging
> lower level stuff can certainly patch in the ucompress debug_ll code.

How do you do that before the MMU has been setup?  Are you going to
write a DT data parser in pure assembly to extract this information?

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [110906 02:04]:
> On Tue, Sep 06, 2011 at 12:28:17PM +0300, Tony Lindgren wrote:
> > Probably the best long term solution is to set up the debug uart based
> > on DT data and initialize it in the uncompress code. Anybody debugging
> > lower level stuff can certainly patch in the ucompress debug_ll code.
> 
> How do you do that before the MMU has been setup?  Are you going to
> write a DT data parser in pure assembly to extract this information?

Don't we already have that with ATAGs to DT support in the DT append
patches? At least it calls fdt_setprop so calling fdt_getprop should
also work. The patch I'm talking about is:

http://permalink.gmane.org/gmane.linux.drivers.devicetree/5938 

Tony

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Tue, Sep 06, 2011 at 03:27:03AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [110906 02:04]:
> > On Tue, Sep 06, 2011 at 12:28:17PM +0300, Tony Lindgren wrote:
> > > Probably the best long term solution is to set up the debug uart based
> > > on DT data and initialize it in the uncompress code. Anybody debugging
> > > lower level stuff can certainly patch in the ucompress debug_ll code.
> > 
> > How do you do that before the MMU has been setup?  Are you going to
> > write a DT data parser in pure assembly to extract this information?
> 
> Don't we already have that with ATAGs to DT support in the DT append
> patches? At least it calls fdt_setprop so calling fdt_getprop should
> also work. The patch I'm talking about is:
> 
> http://permalink.gmane.org/gmane.linux.drivers.devicetree/5938 

Let me reiterate.  How are you going to do that before the MMU has been
setup.

I'll give you a hint: in the kernel (not the boot loader) you can't
call *ANY* C code until you have the MMU enabled.  One of the points
of the *LOW LEVEL* debug code is so you can debug the low level
assembly in the head*.S files before the MMU has been enabled.

That means you can't call the C function "fdt_getprop" to get the
parameters.

So, should we have a chunk of assembly code to enable the MMU so that
we can run fdt_getprop() to get the parameters for the LL debug code,
turn the MMU off, and then run through the head.S code to enable the
MMU for the kernel proper?  If you think that's silly, you're starting
to get the picture I've had for years about all this bastardization of
the LL debug code to solve problems beyond what it was originally
intended for - which, again, was to debug the head.S code.

Personally, I think this whole thing is getting impractical for the
use which people are trying to stretch it too - it's trying to be used
to solve the "how do we get console output before the console is up"
problem as well as the "how do we debug the low level assembly code".

With the advent of multi-platform kernels, the two requirements have
been well proving to be mutually exclusive.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [110906 03:18]:
> On Tue, Sep 06, 2011 at 03:27:03AM -0700, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [110906 02:04]:
> > > On Tue, Sep 06, 2011 at 12:28:17PM +0300, Tony Lindgren wrote:
> > > > Probably the best long term solution is to set up the debug uart based
> > > > on DT data and initialize it in the uncompress code. Anybody debugging
> > > > lower level stuff can certainly patch in the ucompress debug_ll code.
> > > 
> > > How do you do that before the MMU has been setup?  Are you going to
> > > write a DT data parser in pure assembly to extract this information?
> > 
> > Don't we already have that with ATAGs to DT support in the DT append
> > patches? At least it calls fdt_setprop so calling fdt_getprop should
> > also work. The patch I'm talking about is:
> > 
> > http://permalink.gmane.org/gmane.linux.drivers.devicetree/5938 
> 
> Let me reiterate.  How are you going to do that before the MMU has been
> setup.
> 
> I'll give you a hint: in the kernel (not the boot loader) you can't
> call *ANY* C code until you have the MMU enabled.  One of the points
> of the *LOW LEVEL* debug code is so you can debug the low level
> assembly in the head*.S files before the MMU has been enabled.
> 
> That means you can't call the C function "fdt_getprop" to get the
> parameters.

I suggested we set it up in the uncompress code and pass the debug_ll
configuration to the kernel. This only leaves out the debug_ll code
to debug the uncompress code, which can be patched in as needed.
 
> So, should we have a chunk of assembly code to enable the MMU so that
> we can run fdt_getprop() to get the parameters for the LL debug code,
> turn the MMU off, and then run through the head.S code to enable the
> MMU for the kernel proper?  If you think that's silly, you're starting
> to get the picture I've had for years about all this bastardization of
> the LL debug code to solve problems beyond what it was originally
> intended for - which, again, was to debug the head.S code.

That's not needed if we rely on the uncompress code to set it up.
 
> Personally, I think this whole thing is getting impractical for the
> use which people are trying to stretch it too - it's trying to be used
> to solve the "how do we get console output before the console is up"
> problem as well as the "how do we debug the low level assembly code".
> 
> With the advent of multi-platform kernels, the two requirements have
> been well proving to be mutually exclusive.

Yes I agree. As an alternative, how about we drop all the debug_ll code
and allow people to patch it in as needed?

Most users could get by with early_printk based setup that sets up the
machine specific configuration fairly early instead of relying on the
debug_ll functions. And of course the init order needs to be sane so
not much can go wrong before that.

Regards,

Tony

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Russell King - ARM Linux]

On Tue, Sep 06, 2011 at 04:01:43AM -0700, Tony Lindgren wrote:
> Yes I agree. As an alternative, how about we drop all the debug_ll code
> and allow people to patch it in as needed?

Given all this argument over it, I'm mindful to rip out all the debug_ll
code and keep it permanently as an offline patch to prevent further abuse.

It means that people will have to individually write their own accessors
rather than having the utility of it being merged in the kernel, but
I'm sick of trying to stop the feature creap destroying its utility.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Uwe Kleine-König]

On Tue, Sep 06, 2011 at 12:07:26PM +0100, Russell King - ARM Linux wrote:
> On Tue, Sep 06, 2011 at 04:01:43AM -0700, Tony Lindgren wrote:
> > Yes I agree. As an alternative, how about we drop all the debug_ll code
> > and allow people to patch it in as needed?
> 
> Given all this argument over it, I'm mindful to rip out all the debug_ll
> code and keep it permanently as an offline patch to prevent further abuse.
Can you please just nack the changes and keep DEBUG_LL as is? You'll
have my and IIRC Sascha's support for it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Stephen Boyd]

On 08/16/11 14:41, Will Deacon wrote:
> +choice
> +	prompt "Kernel low-level debugging port"
> +	depends on DEBUG_LL
> +
> +	config DEBUG_DC21285_PORT
> +		bool "Kernel low-level debugging messages via footbridge serial port"
> +		depends on FOOTBRIDGE
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the serial port in the DC21285 (Footbridge).
> +		  Saying N will cause the debug messages to appear on the first
> +		  16550 serial port.
> +
> +	config DEBUG_CLPS711X_UART2
> +		bool "Kernel low-level debugging messages via UART2"
> +		depends on ARCH_CLPS711X
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the second serial port on these devices.
> +		  Saying N will cause the debug messages to appear on the first
> +		  serial port.

How would I unselect this UART2 config to get serial output on UART1? As
far as I can tell there isn't an option for NONE in a choice menu.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART

[Will Deacon]

Hi Stephen,

On Thu, Sep 15, 2011 at 06:34:27PM +0100, Stephen Boyd wrote:
> On 08/16/11 14:41, Will Deacon wrote:
> > +choice
> > +	prompt "Kernel low-level debugging port"
> > +	depends on DEBUG_LL
> > +
> > +	config DEBUG_DC21285_PORT
> > +		bool "Kernel low-level debugging messages via footbridge serial port"
> > +		depends on FOOTBRIDGE
> > +		help
> > +		  Say Y here if you want the debug print routines to direct
> > +		  their output to the serial port in the DC21285 (Footbridge).
> > +		  Saying N will cause the debug messages to appear on the first
> > +		  16550 serial port.
> > +
> > +	config DEBUG_CLPS711X_UART2
> > +		bool "Kernel low-level debugging messages via UART2"
> > +		depends on ARCH_CLPS711X
> > +		help
> > +		  Say Y here if you want the debug print routines to direct
> > +		  their output to the second serial port on these devices.
> > +		  Saying N will cause the debug messages to appear on the first
> > +		  serial port.
> 
> How would I unselect this UART2 config to get serial output on UART1? As
> far as I can tell there isn't an option for NONE in a choice menu.

Ah yes. This will need to be added as part of the platform updates to go via
Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
for example, and the platform code will fall back to the first UART.

Will

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

ARM patch 7072/1 (debug: use kconfig choice for selecting
DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
there is no NONE option in a choice menu. Add a UART1 choice so
that users can still choose UART1 explicitly.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 09/16/11 04:41, Will Deacon wrote:
> Ah yes. This will need to be added as part of the platform updates to go via
> Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
> for example, and the platform code will fall back to the first UART.

I couldn't find these patches applied in Arnd's tree so I based it off of
Russell's for-next branch.

 arch/arm/Kconfig.debug |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 60d1846..a37e646 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -90,14 +90,19 @@ choice
 		  Saying N will cause the debug messages to appear on the first
 		  16550 serial port.
 
+	config DEBUG_CLPS711X_UART1
+		bool "Kernel low-level debugging messages via UART1"
+		depends on ARCH_CLPS711X
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the first serial port on these devices.
+
 	config DEBUG_CLPS711X_UART2
 		bool "Kernel low-level debugging messages via UART2"
 		depends on ARCH_CLPS711X
 		help
 		  Say Y here if you want the debug print routines to direct
 		  their output to the second serial port on these devices.
-		  Saying N will cause the debug messages to appear on the first
-		  serial port.
 
 endchoice
 
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

ARM patch 7072/1 (debug: use kconfig choice for selecting
DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
there is no NONE option in a choice menu. Add a UART1 choice so
that users can still choose UART1 explicitly.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 09/16/11 04:41, Will Deacon wrote:
> Ah yes. This will need to be added as part of the platform updates to go via
> Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
> for example, and the platform code will fall back to the first UART.

I couldn't find these patches applied in Arnd's tree so I based it off of
Russell's for-next branch.

 arch/arm/Kconfig.debug |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 60d1846..a37e646 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -90,14 +90,19 @@ choice
 		  Saying N will cause the debug messages to appear on the first
 		  16550 serial port.
 
+	config DEBUG_CLPS711X_UART1
+		bool "Kernel low-level debugging messages via UART1"
+		depends on ARCH_CLPS711X
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the first serial port on these devices.
+
 	config DEBUG_CLPS711X_UART2
 		bool "Kernel low-level debugging messages via UART2"
 		depends on ARCH_CLPS711X
 		help
 		  Say Y here if you want the debug print routines to direct
 		  their output to the second serial port on these devices.
-		  Saying N will cause the debug messages to appear on the first
-		  serial port.
 
 endchoice
 
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/2] ARM: debug: Move DEBUG_ICEDCC into the DEBUG_LL choice

[Stephen Boyd]

DEBUG_ICEDCC support is just another DEBUG_LL choice and
selecting it along with other DEBUG_LL options doesn't make
much sense. Put it into the DEBUG_LL choice to avoid confusion.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/Kconfig.debug |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index a37e646..9661c51 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -81,6 +81,18 @@ choice
 	prompt "Kernel low-level debugging port"
 	depends on DEBUG_LL
 
+	config DEBUG_ICEDCC
+		bool "Kernel low-level debugging via EmbeddedICE DCC channel"
+		depends on DEBUG_LL
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the EmbeddedICE macrocell's DCC channel using
+		  co-processor 14. This is known to work on the ARM9 style ICE
+		  channel and on the XScale with the PEEDI.
+
+		  It does include a timeout to ensure that the system does not
+		  totally freeze when there is nothing connected to read.
+
 	config DEBUG_DC21285_PORT
 		bool "Kernel low-level debugging messages via footbridge serial port"
 		depends on FOOTBRIDGE
@@ -114,18 +126,6 @@ config EARLY_PRINTK
 	  kernel low-level debugging functions. Add earlyprintk to your
 	  kernel parameters to enable this console.
 
-config DEBUG_ICEDCC
-	bool "Kernel low-level debugging via EmbeddedICE DCC channel"
-	depends on DEBUG_LL
-	help
-	  Say Y here if you want the debug print routines to direct their
-	  output to the EmbeddedICE macrocell's DCC channel using
-	  co-processor 14. This is known to work on the ARM9 style ICE
-	  channel and on the XScale with the PEEDI.
-
-	  It does include a timeout to ensure that the system does not
-	  totally freeze when there is nothing connected to read.
-
 config OC_ETM
 	bool "On-chip ETM and ETB"
 	select ARM_AMBA
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/2] ARM: debug: Move DEBUG_ICEDCC into the DEBUG_LL choice

[Stephen Boyd]

DEBUG_ICEDCC support is just another DEBUG_LL choice and
selecting it along with other DEBUG_LL options doesn't make
much sense. Put it into the DEBUG_LL choice to avoid confusion.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/Kconfig.debug |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index a37e646..9661c51 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -81,6 +81,18 @@ choice
 	prompt "Kernel low-level debugging port"
 	depends on DEBUG_LL
 
+	config DEBUG_ICEDCC
+		bool "Kernel low-level debugging via EmbeddedICE DCC channel"
+		depends on DEBUG_LL
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the EmbeddedICE macrocell's DCC channel using
+		  co-processor 14. This is known to work on the ARM9 style ICE
+		  channel and on the XScale with the PEEDI.
+
+		  It does include a timeout to ensure that the system does not
+		  totally freeze when there is nothing connected to read.
+
 	config DEBUG_DC21285_PORT
 		bool "Kernel low-level debugging messages via footbridge serial port"
 		depends on FOOTBRIDGE
@@ -114,18 +126,6 @@ config EARLY_PRINTK
 	  kernel low-level debugging functions. Add earlyprintk to your
 	  kernel parameters to enable this console.
 
-config DEBUG_ICEDCC
-	bool "Kernel low-level debugging via EmbeddedICE DCC channel"
-	depends on DEBUG_LL
-	help
-	  Say Y here if you want the debug print routines to direct their
-	  output to the EmbeddedICE macrocell's DCC channel using
-	  co-processor 14. This is known to work on the ARM9 style ICE
-	  channel and on the XScale with the PEEDI.
-
-	  It does include a timeout to ensure that the system does not
-	  totally freeze when there is nothing connected to read.
-
 config OC_ETM
 	bool "On-chip ETM and ETB"
 	select ARM_AMBA
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Will Deacon]

Hi Stephen,

On Mon, Sep 19, 2011 at 07:01:39PM +0100, Stephen Boyd wrote:
> ARM patch 7072/1 (debug: use kconfig choice for selecting
> DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
> there is no NONE option in a choice menu. Add a UART1 choice so
> that users can still choose UART1 explicitly.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> On 09/16/11 04:41, Will Deacon wrote:
> > Ah yes. This will need to be added as part of the platform updates to go via
> > Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
> > for example, and the platform code will fall back to the first UART.
> 
> I couldn't find these patches applied in Arnd's tree so I based it off of
> Russell's for-next branch.

Thanks for cooking the patch, it was somewhere on my list of things to do
this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
has a similar `if not selected then use a different UART' behaviour. It can be
fixed in the same way as you have done in this patch.

Would you like me to take the three patches in with the ones I currently
have for other platforms (Realview, Samsung, imx) or would you prefer to
handle these separately? I was planning to send all of the platform bits to
Arnd once I've got my branches sorted out (been on holiday for the past two
weeks).

Cheers,

Will

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Will Deacon]

Hi Stephen,

On Mon, Sep 19, 2011 at 07:01:39PM +0100, Stephen Boyd wrote:
> ARM patch 7072/1 (debug: use kconfig choice for selecting
> DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
> there is no NONE option in a choice menu. Add a UART1 choice so
> that users can still choose UART1 explicitly.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> On 09/16/11 04:41, Will Deacon wrote:
> > Ah yes. This will need to be added as part of the platform updates to go via
> > Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
> > for example, and the platform code will fall back to the first UART.
> 
> I couldn't find these patches applied in Arnd's tree so I based it off of
> Russell's for-next branch.

Thanks for cooking the patch, it was somewhere on my list of things to do
this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
has a similar `if not selected then use a different UART' behaviour. It can be
fixed in the same way as you have done in this patch.

Would you like me to take the three patches in with the ones I currently
have for other platforms (Realview, Samsung, imx) or would you prefer to
handle these separately? I was planning to send all of the platform bits to
Arnd once I've got my branches sorted out (been on holiday for the past two
weeks).

Cheers,

Will

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

On 09/19/11 14:25, Will Deacon wrote:
> Hi Stephen,
>
> On Mon, Sep 19, 2011 at 07:01:39PM +0100, Stephen Boyd wrote:
>> ARM patch 7072/1 (debug: use kconfig choice for selecting
>> DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
>> there is no NONE option in a choice menu. Add a UART1 choice so
>> that users can still choose UART1 explicitly.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>
>> On 09/16/11 04:41, Will Deacon wrote:
>>> Ah yes. This will need to be added as part of the platform updates to go via
>>> Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
>>> for example, and the platform code will fall back to the first UART.
>> I couldn't find these patches applied in Arnd's tree so I based it off of
>> Russell's for-next branch.
> Thanks for cooking the patch, it was somewhere on my list of things to do
> this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
> has a similar `if not selected then use a different UART' behaviour. It can be
> fixed in the same way as you have done in this patch.'

Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
Feel free to squash, etc.


diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 9661c51..b976e04 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -93,14 +93,19 @@ choice
                  It does include a timeout to ensure that the system does not
                  totally freeze when there is nothing connected to read.
 
+       config DEBUG_DC21285_PORT1
+               bool "Kernel low-level debugging messages via footbridge serial port 1"
+               depends on FOOTBRIDGE
+               help
+                 Say Y here if you want the debug print routines to direct
+                 their output to the first serial port in the DC21285.
+
        config DEBUG_DC21285_PORT
-               bool "Kernel low-level debugging messages via footbridge serial port"
+               bool "Kernel low-level debugging messages via footbridge serial port 2"
                depends on FOOTBRIDGE
                help
                  Say Y here if you want the debug print routines to direct
-                 their output to the serial port in the DC21285 (Footbridge).
-                 Saying N will cause the debug messages to appear on the first
-                 16550 serial port.
+                 their output to the second serial port in the DC21285.
 
        config DEBUG_CLPS711X_UART1
                bool "Kernel low-level debugging messages via UART1"



>
> Would you like me to take the three patches in with the ones I currently
> have for other platforms (Realview, Samsung, imx) or would you prefer to
> handle these separately? I was planning to send all of the platform bits to
> Arnd once I've got my branches sorted out (been on holiday for the past two
> weeks).
>

If you want to handle them that sounds fine. I can't figure out who is
coordinating the DEBUG_LL changes. It seems that Russell has at least
applied the initial DEBUG_LL patches, so his tree will have some
breakage without this fixup patch.

I have some more patches on top of this one for MSM DEBUG_LL support
too. Hopefully I can send them out in a little bit.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

On 09/19/11 14:25, Will Deacon wrote:
> Hi Stephen,
>
> On Mon, Sep 19, 2011 at 07:01:39PM +0100, Stephen Boyd wrote:
>> ARM patch 7072/1 (debug: use kconfig choice for selecting
>> DEBUG_LL UART) only allowed CLSP711X_UART2 to be selected because
>> there is no NONE option in a choice menu. Add a UART1 choice so
>> that users can still choose UART1 explicitly.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>
>> On 09/16/11 04:41, Will Deacon wrote:
>>> Ah yes. This will need to be added as part of the platform updates to go via
>>> Arnd. It should be easy enough just to have a DEBUG_CLPS711X_UART1 option,
>>> for example, and the platform code will fall back to the first UART.
>> I couldn't find these patches applied in Arnd's tree so I based it off of
>> Russell's for-next branch.
> Thanks for cooking the patch, it was somewhere on my list of things to do
> this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
> has a similar `if not selected then use a different UART' behaviour. It can be
> fixed in the same way as you have done in this patch.'

Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
Feel free to squash, etc.


diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 9661c51..b976e04 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -93,14 +93,19 @@ choice
                  It does include a timeout to ensure that the system does not
                  totally freeze when there is nothing connected to read.
 
+       config DEBUG_DC21285_PORT1
+               bool "Kernel low-level debugging messages via footbridge serial port 1"
+               depends on FOOTBRIDGE
+               help
+                 Say Y here if you want the debug print routines to direct
+                 their output to the first serial port in the DC21285.
+
        config DEBUG_DC21285_PORT
-               bool "Kernel low-level debugging messages via footbridge serial port"
+               bool "Kernel low-level debugging messages via footbridge serial port 2"
                depends on FOOTBRIDGE
                help
                  Say Y here if you want the debug print routines to direct
-                 their output to the serial port in the DC21285 (Footbridge).
-                 Saying N will cause the debug messages to appear on the first
-                 16550 serial port.
+                 their output to the second serial port in the DC21285.
 
        config DEBUG_CLPS711X_UART1
                bool "Kernel low-level debugging messages via UART1"



>
> Would you like me to take the three patches in with the ones I currently
> have for other platforms (Realview, Samsung, imx) or would you prefer to
> handle these separately? I was planning to send all of the platform bits to
> Arnd once I've got my branches sorted out (been on holiday for the past two
> weeks).
>

If you want to handle them that sounds fine. I can't figure out who is
coordinating the DEBUG_LL changes. It seems that Russell has at least
applied the initial DEBUG_LL patches, so his tree will have some
breakage without this fixup patch.

I have some more patches on top of this one for MSM DEBUG_LL support
too. Hopefully I can send them out in a little bit.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 03:12:04PM -0700, Stephen Boyd wrote:
> On 09/19/11 14:25, Will Deacon wrote:
> > Thanks for cooking the patch, it was somewhere on my list of things to do
> > this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
> > has a similar `if not selected then use a different UART' behaviour. It can be
> > fixed in the same way as you have done in this patch.'
> 
> Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
> Feel free to squash, etc.

Err, the DC21285 only has one serial port.

Some footbridge platforms expose the DC21285 to the outside world, others
don't.  Some footbridge platforms have a separate 8250 UART at the standard
PCI COM1 address, others don't.

What DEBUG_DC21285_PORT is selecting between is whether to use the DC21285
port (when set) or the 8250 at PCI COM1 (when unset).

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 03:12:04PM -0700, Stephen Boyd wrote:
> On 09/19/11 14:25, Will Deacon wrote:
> > Thanks for cooking the patch, it was somewhere on my list of things to do
> > this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
> > has a similar `if not selected then use a different UART' behaviour. It can be
> > fixed in the same way as you have done in this patch.'
> 
> Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
> Feel free to squash, etc.

Err, the DC21285 only has one serial port.

Some footbridge platforms expose the DC21285 to the outside world, others
don't.  Some footbridge platforms have a separate 8250 UART at the standard
PCI COM1 address, others don't.

What DEBUG_DC21285_PORT is selecting between is whether to use the DC21285
port (when set) or the 8250 at PCI COM1 (when unset).

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

On 09/19/11 15:41, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 03:12:04PM -0700, Stephen Boyd wrote:
>> On 09/19/11 14:25, Will Deacon wrote:
>>> Thanks for cooking the patch, it was somewhere on my list of things to do
>>> this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
>>> has a similar `if not selected then use a different UART' behaviour. It can be
>>> fixed in the same way as you have done in this patch.'
>> Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
>> Feel free to squash, etc.
> Err, the DC21285 only has one serial port.
>
> Some footbridge platforms expose the DC21285 to the outside world, others
> don't.  Some footbridge platforms have a separate 8250 UART at the standard
> PCI COM1 address, others don't.
>
> What DEBUG_DC21285_PORT is selecting between is whether to use the DC21285
> port (when set) or the 8250 at PCI COM1 (when unset).

Ah ok. How about this instead?

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 9661c51..31896f4 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -93,14 +93,19 @@ choice
                  It does include a timeout to ensure that the system does not
                  totally freeze when there is nothing connected to read.
 
+       config DEBUG_FOOTBRIDGE_COM1
+               bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
+               depends on FOOTBRIDGE
+               help
+                 Say Y here if you want the debug print routines to direct
+                 their output to the 8250 at PCI COM1.
+
        config DEBUG_DC21285_PORT
                bool "Kernel low-level debugging messages via footbridge serial port"
                depends on FOOTBRIDGE
                help
                  Say Y here if you want the debug print routines to direct
                  their output to the serial port in the DC21285 (Footbridge).
-                 Saying N will cause the debug messages to appear on the first
-                 16550 serial port.
 
        config DEBUG_CLPS711X_UART1
                bool "Kernel low-level debugging messages via UART1"



-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Stephen Boyd]

On 09/19/11 15:41, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 03:12:04PM -0700, Stephen Boyd wrote:
>> On 09/19/11 14:25, Will Deacon wrote:
>>> Thanks for cooking the patch, it was somewhere on my list of things to do
>>> this week! I think we also need to fix the DEBUG_DC21285_PORT option as that
>>> has a similar `if not selected then use a different UART' behaviour. It can be
>>> fixed in the same way as you have done in this patch.'
>> Ah my eyes glossed over the DC21285 one. Here's one on top of patch 2.
>> Feel free to squash, etc.
> Err, the DC21285 only has one serial port.
>
> Some footbridge platforms expose the DC21285 to the outside world, others
> don't.  Some footbridge platforms have a separate 8250 UART at the standard
> PCI COM1 address, others don't.
>
> What DEBUG_DC21285_PORT is selecting between is whether to use the DC21285
> port (when set) or the 8250 at PCI COM1 (when unset).

Ah ok. How about this instead?

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 9661c51..31896f4 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -93,14 +93,19 @@ choice
                  It does include a timeout to ensure that the system does not
                  totally freeze when there is nothing connected to read.
 
+       config DEBUG_FOOTBRIDGE_COM1
+               bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
+               depends on FOOTBRIDGE
+               help
+                 Say Y here if you want the debug print routines to direct
+                 their output to the 8250 at PCI COM1.
+
        config DEBUG_DC21285_PORT
                bool "Kernel low-level debugging messages via footbridge serial port"
                depends on FOOTBRIDGE
                help
                  Say Y here if you want the debug print routines to direct
                  their output to the serial port in the DC21285 (Footbridge).
-                 Saying N will cause the debug messages to appear on the first
-                 16550 serial port.
 
        config DEBUG_CLPS711X_UART1
                bool "Kernel low-level debugging messages via UART1"



-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Will Deacon]

On Mon, Sep 19, 2011 at 11:12:04PM +0100, Stephen Boyd wrote:
> On 09/19/11 14:25, Will Deacon wrote:
> >
> > Would you like me to take the three patches in with the ones I currently
> > have for other platforms (Realview, Samsung, imx) or would you prefer to
> > handle these separately? I was planning to send all of the platform bits to
> > Arnd once I've got my branches sorted out (been on holiday for the past two
> > weeks).
> >
> 
> If you want to handle them that sounds fine. I can't figure out who is
> coordinating the DEBUG_LL changes. It seems that Russell has at least
> applied the initial DEBUG_LL patches, so his tree will have some
> breakage without this fixup patch.

I sent the main changes via Russell and I plan to send new platforms moving
to the Kconfig choice via Arnd. However, as you point out, there's some breakage
without your three patches applied, so actually it's probably best if you
send these via the patch system. In which case:

Acked-by: Will Deacon <will.deacon@arm.com>

(this is for the second version of the Footbridge patch too)

> I have some more patches on top of this one for MSM DEBUG_LL support
> too. Hopefully I can send them out in a little bit.

Ok, great. These are the ones that can go via Arnd with imx, Realview etc.

Cheers,

Will

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Will Deacon]

On Mon, Sep 19, 2011 at 11:12:04PM +0100, Stephen Boyd wrote:
> On 09/19/11 14:25, Will Deacon wrote:
> >
> > Would you like me to take the three patches in with the ones I currently
> > have for other platforms (Realview, Samsung, imx) or would you prefer to
> > handle these separately? I was planning to send all of the platform bits to
> > Arnd once I've got my branches sorted out (been on holiday for the past two
> > weeks).
> >
> 
> If you want to handle them that sounds fine. I can't figure out who is
> coordinating the DEBUG_LL changes. It seems that Russell has at least
> applied the initial DEBUG_LL patches, so his tree will have some
> breakage without this fixup patch.

I sent the main changes via Russell and I plan to send new platforms moving
to the Kconfig choice via Arnd. However, as you point out, there's some breakage
without your three patches applied, so actually it's probably best if you
send these via the patch system. In which case:

Acked-by: Will Deacon <will.deacon@arm.com>

(this is for the second version of the Footbridge patch too)

> I have some more patches on top of this one for MSM DEBUG_LL support
> too. Hopefully I can send them out in a little bit.

Ok, great. These are the ones that can go via Arnd with imx, Realview etc.

Cheers,

Will

Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

Hi Will,

On 17 August 2011 03:11, Will Deacon <will.deacon@arm.com> wrote:
> Now that the DEBUG_LL UART can be selected by a Kconfig choice, convert
> the Samsung UART selection to use a set of bools rather than an int.
>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/Kconfig.debug        |   45 ++++++++++++++++++++++++++++++-----------
>  arch/arm/plat-samsung/Kconfig |    7 ++++++
>  2 files changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 11604c9..2f80564 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -94,6 +94,39 @@ choice
>                  Saying N will cause the debug messages to appear on the first
>                  serial port.
>
> +       config DEBUG_S3C_UART0
> +               depends on PLAT_SAMSUNG
> +               bool "Use S3C UART 0 for low-level debug"
> +               help
> +                 Say Y here if you want the debug print routines to direct
> +                 their output to UART 0. The port must have been initialised
> +                 by the boot-loader before use.
> +
> +                 The uncompressor code port configuration is now handled
> +                 by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
> +       config DEBUG_S3C_UART1
> +               depends on PLAT_SAMSUNG
> +               bool "Use S3C UART 1 for low-level debug"
> +               help
> +                 Say Y here if you want the debug print routines to direct
> +                 their output to UART 1. The port must have been initialised
> +                 by the boot-loader before use.
> +
> +                 The uncompressor code port configuration is now handled
> +                 by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
> +       config DEBUG_S3C_UART2
> +               depends on PLAT_SAMSUNG
> +               bool "Use S3C UART 2 for low-level debug"
> +               help
> +                 Say Y here if you want the debug print routines to direct
> +                 their output to UART 2. The port must have been initialised
> +                 by the boot-loader before use.
> +
> +                 The uncompressor code port configuration is now handled
> +                 by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
>  endchoice
>
>  config EARLY_PRINTK
> @@ -124,16 +157,4 @@ config OC_ETM
>          buffer driver that will allow you to collect traces of the
>          kernel code.
>
> -config DEBUG_S3C_UART
> -       depends on PLAT_SAMSUNG
> -       int "S3C UART to use for low-level debug"
> -       default "0"
> -       help
> -         Choice for UART for kernel low-level using S3C UARTS,
> -         should be between zero and two. The port must have been
> -         initialised by the boot-loader before use.
> -
> -         The uncompressor code port configuration is now handled
> -         by CONFIG_S3C_LOWLEVEL_UART_PORT.
> -
>  endmenu
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index b3e1065..49b14b1 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -367,4 +367,11 @@ config SAMSUNG_PD
>        help
>          Say Y here if you want to control Power Domain by Runtime PM.
>
> +config DEBUG_S3C_UART
> +       depends on PLAT_SAMSUNG
> +       int
> +       default "0" if DEBUG_S3C_UART0
> +       default "1" if DEBUG_S3C_UART1
> +       default "2" if DEBUG_S3C_UART2
> +
>  endif
> --
> 1.7.0.4


What is your opinion about the following diff instead of the above one?


diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 65cf8c6..035f5cd 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -120,6 +120,15 @@ choice
 		  Say Y here if you want the debug print routines to direct
 		  their output to the second serial port on these devices.

+	config DEBUG_SAMSUNG_UART
+		bool "Kernel low-level debugging messages via samsung serial port"
+		depends on PLAT_SAMSUNG
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the serial port for Samsung platforms. Choose
+		  the uart port with the "S3C UART to use for low-level debug"
+		  config option.
+
 endchoice

 config EARLY_PRINTK
@@ -139,7 +148,7 @@ config OC_ETM
 	  kernel code.

 config DEBUG_S3C_UART
-	depends on PLAT_SAMSUNG
+	depends on DEBUG_SAMSUNG_UART
 	int "S3C UART to use for low-level debug"
 	default "0"
 	help


Thanks,
Thomas.

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

Hi Will,

On 17 August 2011 03:11, Will Deacon <will.deacon@arm.com> wrote:
> Now that the DEBUG_LL UART can be selected by a Kconfig choice, convert
> the Samsung UART selection to use a set of bools rather than an int.
>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> ?arch/arm/Kconfig.debug ? ? ? ?| ? 45 ++++++++++++++++++++++++++++++-----------
> ?arch/arm/plat-samsung/Kconfig | ? ?7 ++++++
> ?2 files changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 11604c9..2f80564 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -94,6 +94,39 @@ choice
> ? ? ? ? ? ? ? ? ?Saying N will cause the debug messages to appear on the first
> ? ? ? ? ? ? ? ? ?serial port.
>
> + ? ? ? config DEBUG_S3C_UART0
> + ? ? ? ? ? ? ? depends on PLAT_SAMSUNG
> + ? ? ? ? ? ? ? bool "Use S3C UART 0 for low-level debug"
> + ? ? ? ? ? ? ? help
> + ? ? ? ? ? ? ? ? Say Y here if you want the debug print routines to direct
> + ? ? ? ? ? ? ? ? their output to UART 0. The port must have been initialised
> + ? ? ? ? ? ? ? ? by the boot-loader before use.
> +
> + ? ? ? ? ? ? ? ? The uncompressor code port configuration is now handled
> + ? ? ? ? ? ? ? ? by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
> + ? ? ? config DEBUG_S3C_UART1
> + ? ? ? ? ? ? ? depends on PLAT_SAMSUNG
> + ? ? ? ? ? ? ? bool "Use S3C UART 1 for low-level debug"
> + ? ? ? ? ? ? ? help
> + ? ? ? ? ? ? ? ? Say Y here if you want the debug print routines to direct
> + ? ? ? ? ? ? ? ? their output to UART 1. The port must have been initialised
> + ? ? ? ? ? ? ? ? by the boot-loader before use.
> +
> + ? ? ? ? ? ? ? ? The uncompressor code port configuration is now handled
> + ? ? ? ? ? ? ? ? by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
> + ? ? ? config DEBUG_S3C_UART2
> + ? ? ? ? ? ? ? depends on PLAT_SAMSUNG
> + ? ? ? ? ? ? ? bool "Use S3C UART 2 for low-level debug"
> + ? ? ? ? ? ? ? help
> + ? ? ? ? ? ? ? ? Say Y here if you want the debug print routines to direct
> + ? ? ? ? ? ? ? ? their output to UART 2. The port must have been initialised
> + ? ? ? ? ? ? ? ? by the boot-loader before use.
> +
> + ? ? ? ? ? ? ? ? The uncompressor code port configuration is now handled
> + ? ? ? ? ? ? ? ? by CONFIG_S3C_LOWLEVEL_UART_PORT.
> +
> ?endchoice
>
> ?config EARLY_PRINTK
> @@ -124,16 +157,4 @@ config OC_ETM
> ? ? ? ? ?buffer driver that will allow you to collect traces of the
> ? ? ? ? ?kernel code.
>
> -config DEBUG_S3C_UART
> - ? ? ? depends on PLAT_SAMSUNG
> - ? ? ? int "S3C UART to use for low-level debug"
> - ? ? ? default "0"
> - ? ? ? help
> - ? ? ? ? Choice for UART for kernel low-level using S3C UARTS,
> - ? ? ? ? should be between zero and two. The port must have been
> - ? ? ? ? initialised by the boot-loader before use.
> -
> - ? ? ? ? The uncompressor code port configuration is now handled
> - ? ? ? ? by CONFIG_S3C_LOWLEVEL_UART_PORT.
> -
> ?endmenu
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index b3e1065..49b14b1 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -367,4 +367,11 @@ config SAMSUNG_PD
> ? ? ? ?help
> ? ? ? ? ?Say Y here if you want to control Power Domain by Runtime PM.
>
> +config DEBUG_S3C_UART
> + ? ? ? depends on PLAT_SAMSUNG
> + ? ? ? int
> + ? ? ? default "0" if DEBUG_S3C_UART0
> + ? ? ? default "1" if DEBUG_S3C_UART1
> + ? ? ? default "2" if DEBUG_S3C_UART2
> +
> ?endif
> --
> 1.7.0.4


What is your opinion about the following diff instead of the above one?


diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 65cf8c6..035f5cd 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -120,6 +120,15 @@ choice
 		  Say Y here if you want the debug print routines to direct
 		  their output to the second serial port on these devices.

+	config DEBUG_SAMSUNG_UART
+		bool "Kernel low-level debugging messages via samsung serial port"
+		depends on PLAT_SAMSUNG
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to the serial port for Samsung platforms. Choose
+		  the uart port with the "S3C UART to use for low-level debug"
+		  config option.
+
 endchoice

 config EARLY_PRINTK
@@ -139,7 +148,7 @@ config OC_ETM
 	  kernel code.

 config DEBUG_S3C_UART
-	depends on PLAT_SAMSUNG
+	depends on DEBUG_SAMSUNG_UART
 	int "S3C UART to use for low-level debug"
 	default "0"
 	help


Thanks,
Thomas.

Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Will Deacon]

On Mon, Oct 10, 2011 at 12:56:24PM +0100, Thomas Abraham wrote:
> Hi Will,

Hi Thomas,
> 
> What is your opinion about the following diff instead of the above one?
> 
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 65cf8c6..035f5cd 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -120,6 +120,15 @@ choice
>  		  Say Y here if you want the debug print routines to direct
>  		  their output to the second serial port on these devices.
> 
> +	config DEBUG_SAMSUNG_UART
> +		bool "Kernel low-level debugging messages via samsung serial port"
> +		depends on PLAT_SAMSUNG
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the serial port for Samsung platforms. Choose
> +		  the uart port with the "S3C UART to use for low-level debug"
> +		  config option.
> +
>  endchoice
> 
>  config EARLY_PRINTK
> @@ -139,7 +148,7 @@ config OC_ETM
>  	  kernel code.
> 
>  config DEBUG_S3C_UART
> -	depends on PLAT_SAMSUNG
> +	depends on DEBUG_SAMSUNG_UART
>  	int "S3C UART to use for low-level debug"
>  	default "0"
>  	help

Well, it's smaller so that's always a plus. However, I don't think the extra
level of indirection helps (that is, the "Kernel low-level debugging UART")
choice should be where the UART is specified, rather than pointing you at
another (platform-specific) option).

Did your suggestion come purely out of aesthetics or are you running into
difficulties with the original patch?

Will

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Will Deacon]

On Mon, Oct 10, 2011 at 12:56:24PM +0100, Thomas Abraham wrote:
> Hi Will,

Hi Thomas,
> 
> What is your opinion about the following diff instead of the above one?
> 
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 65cf8c6..035f5cd 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -120,6 +120,15 @@ choice
>  		  Say Y here if you want the debug print routines to direct
>  		  their output to the second serial port on these devices.
> 
> +	config DEBUG_SAMSUNG_UART
> +		bool "Kernel low-level debugging messages via samsung serial port"
> +		depends on PLAT_SAMSUNG
> +		help
> +		  Say Y here if you want the debug print routines to direct
> +		  their output to the serial port for Samsung platforms. Choose
> +		  the uart port with the "S3C UART to use for low-level debug"
> +		  config option.
> +
>  endchoice
> 
>  config EARLY_PRINTK
> @@ -139,7 +148,7 @@ config OC_ETM
>  	  kernel code.
> 
>  config DEBUG_S3C_UART
> -	depends on PLAT_SAMSUNG
> +	depends on DEBUG_SAMSUNG_UART
>  	int "S3C UART to use for low-level debug"
>  	default "0"
>  	help

Well, it's smaller so that's always a plus. However, I don't think the extra
level of indirection helps (that is, the "Kernel low-level debugging UART")
choice should be where the UART is specified, rather than pointing you at
another (platform-specific) option).

Did your suggestion come purely out of aesthetics or are you running into
difficulties with the original patch?

Will

Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

On 10 October 2011 17:53, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 10, 2011 at 12:56:24PM +0100, Thomas Abraham wrote:
>> Hi Will,
>
> Hi Thomas,
>>
>> What is your opinion about the following diff instead of the above one?
>>
>>
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index 65cf8c6..035f5cd 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -120,6 +120,15 @@ choice
>>                 Say Y here if you want the debug print routines to direct
>>                 their output to the second serial port on these devices.
>>
>> +     config DEBUG_SAMSUNG_UART
>> +             bool "Kernel low-level debugging messages via samsung serial port"
>> +             depends on PLAT_SAMSUNG
>> +             help
>> +               Say Y here if you want the debug print routines to direct
>> +               their output to the serial port for Samsung platforms. Choose
>> +               the uart port with the "S3C UART to use for low-level debug"
>> +               config option.
>> +
>>  endchoice
>>
>>  config EARLY_PRINTK
>> @@ -139,7 +148,7 @@ config OC_ETM
>>         kernel code.
>>
>>  config DEBUG_S3C_UART
>> -     depends on PLAT_SAMSUNG
>> +     depends on DEBUG_SAMSUNG_UART
>>       int "S3C UART to use for low-level debug"
>>       default "0"
>>       help
>
> Well, it's smaller so that's always a plus. However, I don't think the extra
> level of indirection helps (that is, the "Kernel low-level debugging UART")
> choice should be where the UART is specified, rather than pointing you at
> another (platform-specific) option).
>
> Did your suggestion come purely out of aesthetics or are you running into
> difficulties with the original patch?

There are no difficulties with the original patch. But that was not
how Samsung boards have been selecting the low level debug uart port
number. The proposed patch tried to maintain the old style.

Another point is that Samsung's Exynos4 (and few other SoC's) has a
fourth UART port as well. Not that it is used as a debug port
currently, but there is no technical limitation in using that as a
console port. So that might need 'DEBUG_S3C_UART3' as well.

Thanks,
Thomas.

>
> Will
>

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

On 10 October 2011 17:53, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 10, 2011 at 12:56:24PM +0100, Thomas Abraham wrote:
>> Hi Will,
>
> Hi Thomas,
>>
>> What is your opinion about the following diff instead of the above one?
>>
>>
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index 65cf8c6..035f5cd 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -120,6 +120,15 @@ choice
>> ? ? ? ? ? ? ? ? Say Y here if you want the debug print routines to direct
>> ? ? ? ? ? ? ? ? their output to the second serial port on these devices.
>>
>> + ? ? config DEBUG_SAMSUNG_UART
>> + ? ? ? ? ? ? bool "Kernel low-level debugging messages via samsung serial port"
>> + ? ? ? ? ? ? depends on PLAT_SAMSUNG
>> + ? ? ? ? ? ? help
>> + ? ? ? ? ? ? ? Say Y here if you want the debug print routines to direct
>> + ? ? ? ? ? ? ? their output to the serial port for Samsung platforms. Choose
>> + ? ? ? ? ? ? ? the uart port with the "S3C UART to use for low-level debug"
>> + ? ? ? ? ? ? ? config option.
>> +
>> ?endchoice
>>
>> ?config EARLY_PRINTK
>> @@ -139,7 +148,7 @@ config OC_ETM
>> ? ? ? ? kernel code.
>>
>> ?config DEBUG_S3C_UART
>> - ? ? depends on PLAT_SAMSUNG
>> + ? ? depends on DEBUG_SAMSUNG_UART
>> ? ? ? int "S3C UART to use for low-level debug"
>> ? ? ? default "0"
>> ? ? ? help
>
> Well, it's smaller so that's always a plus. However, I don't think the extra
> level of indirection helps (that is, the "Kernel low-level debugging UART")
> choice should be where the UART is specified, rather than pointing you at
> another (platform-specific) option).
>
> Did your suggestion come purely out of aesthetics or are you running into
> difficulties with the original patch?

There are no difficulties with the original patch. But that was not
how Samsung boards have been selecting the low level debug uart port
number. The proposed patch tried to maintain the old style.

Another point is that Samsung's Exynos4 (and few other SoC's) has a
fourth UART port as well. Not that it is used as a debug port
currently, but there is no technical limitation in using that as a
console port. So that might need 'DEBUG_S3C_UART3' as well.

Thanks,
Thomas.

>
> Will
>

Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Will Deacon]

On Mon, Oct 10, 2011 at 01:35:54PM +0100, Thomas Abraham wrote:
> There are no difficulties with the original patch. But that was not
> how Samsung boards have been selecting the low level debug uart port
> number. The proposed patch tried to maintain the old style.
> 
> Another point is that Samsung's Exynos4 (and few other SoC's) has a
> fourth UART port as well. Not that it is used as a debug port
> currently, but there is no technical limitation in using that as a
> console port. So that might need 'DEBUG_S3C_UART3' as well.

Tell you what then: let's stick with the original patch for now and then you
can refactor the Samsung bit in the future if/when you add support for the
fourth UART. Does that sound OK?

Will

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Will Deacon]

On Mon, Oct 10, 2011 at 01:35:54PM +0100, Thomas Abraham wrote:
> There are no difficulties with the original patch. But that was not
> how Samsung boards have been selecting the low level debug uart port
> number. The proposed patch tried to maintain the old style.
> 
> Another point is that Samsung's Exynos4 (and few other SoC's) has a
> fourth UART port as well. Not that it is used as a debug port
> currently, but there is no technical limitation in using that as a
> console port. So that might need 'DEBUG_S3C_UART3' as well.

Tell you what then: let's stick with the original patch for now and then you
can refactor the Samsung bit in the future if/when you add support for the
fourth UART. Does that sound OK?

Will

Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

On 10 October 2011 19:04, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 10, 2011 at 01:35:54PM +0100, Thomas Abraham wrote:
>> There are no difficulties with the original patch. But that was not
>> how Samsung boards have been selecting the low level debug uart port
>> number. The proposed patch tried to maintain the old style.
>>
>> Another point is that Samsung's Exynos4 (and few other SoC's) has a
>> fourth UART port as well. Not that it is used as a debug port
>> currently, but there is no technical limitation in using that as a
>> console port. So that might need 'DEBUG_S3C_UART3' as well.
>
> Tell you what then: let's stick with the original patch for now and then you
> can refactor the Samsung bit in the future if/when you add support for the
> fourth UART. Does that sound OK?

Yes. That sounds fine. Thanks for having a look at the proposed diff.

Regards,
Thomas.

>
> Will
>

[PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection

[Thomas Abraham]

On 10 October 2011 19:04, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 10, 2011 at 01:35:54PM +0100, Thomas Abraham wrote:
>> There are no difficulties with the original patch. But that was not
>> how Samsung boards have been selecting the low level debug uart port
>> number. The proposed patch tried to maintain the old style.
>>
>> Another point is that Samsung's Exynos4 (and few other SoC's) has a
>> fourth UART port as well. Not that it is used as a debug port
>> currently, but there is no technical limitation in using that as a
>> console port. So that might need 'DEBUG_S3C_UART3' as well.
>
> Tell you what then: let's stick with the original patch for now and then you
> can refactor the Samsung bit in the future if/when you add support for the
> fourth UART. Does that sound OK?

Yes. That sounds fine. Thanks for having a look at the proposed diff.

Regards,
Thomas.

>
> Will
>

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Uwe Kleine-König]

Hello Shawn,

On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> simplify the #ifdefery in debug-macro.S and add entries to the
> top-level Kconfig.debug instead.
now that this patch is in Linus' tree as
f350b86121c7a004a5f866333fa1d23fe30263a6, this breaks the build when
selecting DEBUG_LL_UART_NONE (as our autobuilder did somehow). Maybe
that is OK because DEBUG_LL is an expert option? If not, either the
autoselection has to return or DEBUG_LL_UART_NONE shouldn't be
selectable for imx.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

Hi Uwe,

On Tue, Nov 22, 2011 at 08:58:34AM +0000, Uwe Kleine-K?nig wrote:
> On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > simplify the #ifdefery in debug-macro.S and add entries to the
> > top-level Kconfig.debug instead.
> now that this patch is in Linus' tree as
> f350b86121c7a004a5f866333fa1d23fe30263a6, this breaks the build when
> selecting DEBUG_LL_UART_NONE (as our autobuilder did somehow). Maybe
> that is OK because DEBUG_LL is an expert option? If not, either the
> autoselection has to return or DEBUG_LL_UART_NONE shouldn't be
> selectable for imx.

In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
the platforms which have not yet added an entry to the choice.

Will

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Uwe Kleine-König]

On Tue, Nov 22, 2011 at 11:50:17AM +0000, Will Deacon wrote:
> Hi Uwe,
> 
> On Tue, Nov 22, 2011 at 08:58:34AM +0000, Uwe Kleine-K?nig wrote:
> > On Fri, Aug 19, 2011 at 12:56:59PM +0800, Shawn Guo wrote:
> > > Now that the DEBUG_LL UART can be selected by a Kconfig choice,
> > > simplify the #ifdefery in debug-macro.S and add entries to the
> > > top-level Kconfig.debug instead.
> > now that this patch is in Linus' tree as
> > f350b86121c7a004a5f866333fa1d23fe30263a6, this breaks the build when
> > selecting DEBUG_LL_UART_NONE (as our autobuilder did somehow). Maybe
> > that is OK because DEBUG_LL is an expert option? If not, either the
> > autoselection has to return or DEBUG_LL_UART_NONE shouldn't be
> > selectable for imx.
> 
> In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
> the platforms which have not yet added an entry to the choice.
Why not remove it already now? If I remove it together with the entry
for my machine and modify mach/debug-macro.S to do the right thing
for my machine even without DEBUG_IMX25_UART, my build just works.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

On Tue, Nov 22, 2011 at 01:02:33PM +0000, Uwe Kleine-K?nig wrote:
> On Tue, Nov 22, 2011 at 11:50:17AM +0000, Will Deacon wrote:
> > In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
> > the platforms which have not yet added an entry to the choice.
> Why not remove it already now? If I remove it together with the entry
> for my machine and modify mach/debug-macro.S to do the right thing
> for my machine even without DEBUG_IMX25_UART, my build just works.

I'm fairly sure this will break the build for everybody who doesn't have an
entry in Kconfig.debug choice if they try and select DEBUG_LL.

Will

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Uwe Kleine-König]

On Tue, Nov 22, 2011 at 01:20:55PM +0000, Will Deacon wrote:
> On Tue, Nov 22, 2011 at 01:02:33PM +0000, Uwe Kleine-K?nig wrote:
> > On Tue, Nov 22, 2011 at 11:50:17AM +0000, Will Deacon wrote:
> > > In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
> > > the platforms which have not yet added an entry to the choice.
> > Why not remove it already now? If I remove it together with the entry
> > for my machine and modify mach/debug-macro.S to do the right thing
> > for my machine even without DEBUG_IMX25_UART, my build just works.
> 
> I'm fairly sure this will break the build for everybody who doesn't have an
> entry in Kconfig.debug choice if they try and select DEBUG_LL.
Ah, I see the problem. It's more subtile than just failure to build: it
autoselects DEBUG_ICEDCC then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Russell King - ARM Linux]

On Tue, Nov 22, 2011 at 02:30:40PM +0100, Uwe Kleine-K?nig wrote:
> On Tue, Nov 22, 2011 at 01:20:55PM +0000, Will Deacon wrote:
> > On Tue, Nov 22, 2011 at 01:02:33PM +0000, Uwe Kleine-K?nig wrote:
> > > On Tue, Nov 22, 2011 at 11:50:17AM +0000, Will Deacon wrote:
> > > > In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
> > > > the platforms which have not yet added an entry to the choice.
> > > Why not remove it already now? If I remove it together with the entry
> > > for my machine and modify mach/debug-macro.S to do the right thing
> > > for my machine even without DEBUG_IMX25_UART, my build just works.
> > 
> > I'm fairly sure this will break the build for everybody who doesn't have an
> > entry in Kconfig.debug choice if they try and select DEBUG_LL.
> Ah, I see the problem. It's more subtile than just failure to build: it
> autoselects DEBUG_ICEDCC then.

Which results in a kernel which builds fine, but when they attempt to
boot it appears to lock fairly solidly.  So we had to add this option
to allow the possibility for unconverted platforms to continue working.

At this point, I think the right answer is to remove the option in
linux-next, and tell anyone who complains that they need to convert
their platform properly.  (We're going to need them converted in this
way anyway for the single zImage project.)

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Uwe Kleine-König]

On Tue, Nov 22, 2011 at 03:48:38PM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 22, 2011 at 02:30:40PM +0100, Uwe Kleine-K?nig wrote:
> > On Tue, Nov 22, 2011 at 01:20:55PM +0000, Will Deacon wrote:
> > > On Tue, Nov 22, 2011 at 01:02:33PM +0000, Uwe Kleine-K?nig wrote:
> > > > On Tue, Nov 22, 2011 at 11:50:17AM +0000, Will Deacon wrote:
> > > > > In the end I'd like to remove DEBUG_LL_UART_NONE. It's only there to service
> > > > > the platforms which have not yet added an entry to the choice.
> > > > Why not remove it already now? If I remove it together with the entry
> > > > for my machine and modify mach/debug-macro.S to do the right thing
> > > > for my machine even without DEBUG_IMX25_UART, my build just works.
> > > 
> > > I'm fairly sure this will break the build for everybody who doesn't have an
> > > entry in Kconfig.debug choice if they try and select DEBUG_LL.
> > Ah, I see the problem. It's more subtile than just failure to build: it
> > autoselects DEBUG_ICEDCC then.
> 
> Which results in a kernel which builds fine, but when they attempt to
> boot it appears to lock fairly solidly.  So we had to add this option
> to allow the possibility for unconverted platforms to continue working.
yes.

> At this point, I think the right answer is to remove the option in
> linux-next, and tell anyone who complains that they need to convert
> their platform properly.  (We're going to need them converted in this
> way anyway for the single zImage project.)
I agree.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Mark Brown]

On Tue, Nov 22, 2011 at 05:38:12PM +0100, Uwe Kleine-K?nig wrote:
> On Tue, Nov 22, 2011 at 03:48:38PM +0000, Russell King - ARM Linux wrote:

> > > Ah, I see the problem. It's more subtile than just failure to build: it
> > > autoselects DEBUG_ICEDCC then.

> > At this point, I think the right answer is to remove the option in
> > linux-next, and tell anyone who complains that they need to convert
> > their platform properly.  (We're going to need them converted in this
> > way anyway for the single zImage project.)

> I agree.

It'd be nice if things could be arranged so that the build breaks rather
than selecting ICEDCC on unconverted platforms - when you run into the
problem it's not that easy to diagnose.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Uwe Kleine-König]

On Tue, Nov 22, 2011 at 04:47:07PM +0000, Mark Brown wrote:
> On Tue, Nov 22, 2011 at 05:38:12PM +0100, Uwe Kleine-K?nig wrote:
> > On Tue, Nov 22, 2011 at 03:48:38PM +0000, Russell King - ARM Linux wrote:
> 
> > > > Ah, I see the problem. It's more subtile than just failure to build: it
> > > > autoselects DEBUG_ICEDCC then.
> 
> > > At this point, I think the right answer is to remove the option in
> > > linux-next, and tell anyone who complains that they need to convert
> > > their platform properly.  (We're going to need them converted in this
> > > way anyway for the single zImage project.)
> 
> > I agree.
> 
> It'd be nice if things could be arranged so that the build breaks rather
> than selecting ICEDCC on unconverted platforms - when you run into the
> problem it's not that easy to diagnose.
What about the patch below? It's a small step back as DEBUG_ICEDCC is
moved out of the choice, but seems to work in my tests.

Best regards
Uwe

--------->8-------------------
From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Subject: [PATCH RFC] ARM: remove DEBUG_LL_UART_NONE from choice of debug UARTs

Moreover move DEBUG_ICEDCC out of the choice.
This way platforms that don't have an item to pick in the choice
automatically fall back to the traditional behaviour instead of
choosing DEBUG_ICEDCC when only removing DEBUG_LL_UART_NONE.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/Kconfig.debug |   32 ++++++++++++--------------------
 1 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index c5213e7..4cca427 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -77,28 +77,20 @@ config DEBUG_LL
 	  image on a different platform *will not work*, so this option should
 	  not be enabled for kernels that are intended to be portable.
 
-choice
-	prompt "Kernel low-level debugging port"
-	depends on DEBUG_LL
-
-	config DEBUG_LL_UART_NONE
-		bool "No low-level debugging UART"
-		help
-		  Say Y here if your platform doesn't provide a UART option
-		  below. This relies on your platform choosing the right UART
-		  definition internally in order for low-level debugging to
-		  work.
+config DEBUG_ICEDCC
+	bool "Kernel low-level debugging via EmbeddedICE DCC channel"
+	help
+	  Say Y here if you want the debug print routines to direct
+	  their output to the EmbeddedICE macrocell's DCC channel using
+	  co-processor 14. This is known to work on the ARM9 style ICE
+	  channel and on the XScale with the PEEDI.
 
-	config DEBUG_ICEDCC
-		bool "Kernel low-level debugging via EmbeddedICE DCC channel"
-		help
-		  Say Y here if you want the debug print routines to direct
-		  their output to the EmbeddedICE macrocell's DCC channel using
-		  co-processor 14. This is known to work on the ARM9 style ICE
-		  channel and on the XScale with the PEEDI.
+	  Note that the system will appear to hang during boot if there
+	  is nothing connected to read from the DCC.
 
-		  Note that the system will appear to hang during boot if there
-		  is nothing connected to read from the DCC.
+choice
+	prompt "Kernel low-level debugging port"
+	depends on DEBUG_LL && !DEBUG_ICEDCC
 
 	config DEBUG_FOOTBRIDGE_COM1
 		bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
-- 
1.7.7.2

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Russell King - ARM Linux]

On Tue, Nov 22, 2011 at 04:47:07PM +0000, Mark Brown wrote:
> On Tue, Nov 22, 2011 at 05:38:12PM +0100, Uwe Kleine-K?nig wrote:
> > On Tue, Nov 22, 2011 at 03:48:38PM +0000, Russell King - ARM Linux wrote:
> 
> > > > Ah, I see the problem. It's more subtile than just failure to build: it
> > > > autoselects DEBUG_ICEDCC then.
> 
> > > At this point, I think the right answer is to remove the option in
> > > linux-next, and tell anyone who complains that they need to convert
> > > their platform properly.  (We're going to need them converted in this
> > > way anyway for the single zImage project.)
> 
> > I agree.
> 
> It'd be nice if things could be arranged so that the build breaks rather
> than selecting ICEDCC on unconverted platforms - when you run into the
> problem it's not that easy to diagnose.

Well, we could leave the choice as is, and make the NONE option cause a
#error.

Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 03:55:10PM -0700, Stephen Boyd wrote:
> Ah ok. How about this instead?

Better...

> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 9661c51..31896f4 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -93,14 +93,19 @@ choice
>                   It does include a timeout to ensure that the system does not
>                   totally freeze when there is nothing connected to read.
>  
> +       config DEBUG_FOOTBRIDGE_COM1
> +               bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"

I don't think PC-isms help here - especially when these aren't labelled
using this terminology on devices.  Netwinder called it just a simple
"Serial" on the rear panel.

		bool "Kernel low-level debugging messages via 8250 serial port at 0x3f8"

> +               depends on FOOTBRIDGE
> +               help
> +                 Say Y here if you want the debug print routines to direct
> +                 their output to the 8250 at PCI COM1.

Same kind of fixup here.  Other than that, it seems fine.

[PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice

[Russell King - ARM Linux]

On Mon, Sep 19, 2011 at 03:55:10PM -0700, Stephen Boyd wrote:
> Ah ok. How about this instead?

Better...

> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 9661c51..31896f4 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -93,14 +93,19 @@ choice
>                   It does include a timeout to ensure that the system does not
>                   totally freeze when there is nothing connected to read.
>  
> +       config DEBUG_FOOTBRIDGE_COM1
> +               bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"

I don't think PC-isms help here - especially when these aren't labelled
using this terminology on devices.  Netwinder called it just a simple
"Serial" on the rear panel.

		bool "Kernel low-level debugging messages via 8250 serial port at 0x3f8"

> +               depends on FOOTBRIDGE
> +               help
> +                 Say Y here if you want the debug print routines to direct
> +                 their output to the 8250 at PCI COM1.

Same kind of fixup here.  Other than that, it seems fine.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Arnd Bergmann]

On Tuesday 22 November 2011 20:24:25 Russell King - ARM Linux wrote:
> On Tue, Nov 22, 2011 at 04:47:07PM +0000, Mark Brown wrote:
> > On Tue, Nov 22, 2011 at 05:38:12PM +0100, Uwe Kleine-K?nig wrote:
> > > On Tue, Nov 22, 2011 at 03:48:38PM +0000, Russell King - ARM Linux wrote:
> > 
> > > > > Ah, I see the problem. It's more subtile than just failure to build: it
> > > > > autoselects DEBUG_ICEDCC then.
> > 
> > > > At this point, I think the right answer is to remove the option in
> > > > linux-next, and tell anyone who complains that they need to convert
> > > > their platform properly.  (We're going to need them converted in this
> > > > way anyway for the single zImage project.)
> > 
> > > I agree.
> > 
> > It'd be nice if things could be arranged so that the build breaks rather
> > than selecting ICEDCC on unconverted platforms - when you run into the
> > problem it's not that easy to diagnose.
> 
> Well, we could leave the choice as is, and make the NONE option cause a
> #error.

In my randconfig branch, I have a temporary patch doing

--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -83,6 +83,8 @@ choice
        depends on DEBUG_LL
 
        config DEBUG_LL_UART_NONE
+               depends on !FOOTBRIDGE && !ARCH_CLPS711X && !ARCH_MXC && \
+                          !PLAT_SAMSUNG && !ARCH_REALVIEW && !ARCH_HIGHBANK
                bool "No low-level debugging UART"
                help
                  Say Y here if your platform doesn't provide a UART option

We can also move ICEDCC to the bottom of the list to ensure that the default
choice is a platform specific one.

In order to make the conversion nicer (avoiding a conflicting patch
every time someone adds "depends on !MY_PLATFORM", how about expressing
it as this:

	config DEBUG_LL_UART_NONE
		depends on !DEBUG_LL_LEGACY

and then selecting DEBUG_LL_LEGACY from all platforms that do their own
thing. That would avoid the possible randconfig errors.

	Arnd

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Mark Brown]

On Tue, Nov 22, 2011 at 10:19:53PM +0100, Arnd Bergmann wrote:

> and then selecting DEBUG_LL_LEGACY from all platforms that do their own
> thing. That would avoid the possible randconfig errors.

Your fix seems good to me, and avoids causing hassle for randconfig
folks, though Russell's will be more likely to encourage people to
convert I guess.

[PATCH] arm/imx: use Kconfig choice for low-level debug UART selection

[Will Deacon]

On Tue, Nov 22, 2011 at 11:00:31PM +0000, Mark Brown wrote:
> On Tue, Nov 22, 2011 at 10:19:53PM +0100, Arnd Bergmann wrote:
> 
> > and then selecting DEBUG_LL_LEGACY from all platforms that do their own
> > thing. That would avoid the possible randconfig errors.
> 
> Your fix seems good to me, and avoids causing hassle for randconfig
> folks, though Russell's will be more likely to encourage people to
> convert I guess.

Although Russell's approach is a build breaker for those platforms that have
not yet made the switch, that does provide an incentive for the platform
maintainers to update their code. If we settle for a legacy solution it will
probably sit around forever.

Given that we're now past -rc2, it might be best to wait until the next
merge window to introduce the #error logic, so the LEGACY solution could be
a stopgap for 3.2.

Will