* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
@ 2011-08-16 21:41 Will Deacon
2011-08-16 21:41 ` [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection Will Deacon
` (5 more replies)
0 siblings, 6 replies; 86+ messages in thread
From: Will Deacon @ 2011-08-16 21:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
@ 2011-08-16 21:41 ` Will Deacon
2011-10-10 11:56 ` Thomas Abraham
2011-08-16 21:41 ` [PATCH 3/3] ARM: realview: " Will Deacon
` (4 subsequent siblings)
5 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-16 21:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 3/3] ARM: realview: use Kconfig choice for debug UART selection
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
2011-08-16 21:41 ` [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection Will Deacon
@ 2011-08-16 21:41 ` Will Deacon
2011-08-18 4:06 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Nicolas Pitre
` (3 subsequent siblings)
5 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-08-16 21:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
2011-08-16 21:41 ` [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection Will Deacon
2011-08-16 21:41 ` [PATCH 3/3] ARM: realview: " Will Deacon
@ 2011-08-18 4:06 ` Nicolas Pitre
2011-08-18 9:33 ` Will Deacon
2011-08-18 16:11 ` Shawn Guo
` (2 subsequent siblings)
5 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-18 4:06 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-18 4:06 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Nicolas Pitre
@ 2011-08-18 9:33 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-08-18 9:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-18 16:11 ` Shawn Guo
@ 2011-08-18 16:07 ` Will Deacon
2011-08-19 4:59 ` Shawn Guo
0 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-18 16:07 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
` (2 preceding siblings ...)
2011-08-18 4:06 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Nicolas Pitre
@ 2011-08-18 16:11 ` Shawn Guo
2011-08-18 16:07 ` Will Deacon
2011-08-19 4:56 ` [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection Shawn Guo
2011-09-15 17:34 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Stephen Boyd
5 siblings, 1 reply; 86+ messages in thread
From: Shawn Guo @ 2011-08-18 16:11 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
` (3 preceding siblings ...)
2011-08-18 16:11 ` Shawn Guo
@ 2011-08-19 4:56 ` Shawn Guo
2011-08-19 6:35 ` Sascha Hauer
2011-11-22 8:58 ` Uwe Kleine-König
2011-09-15 17:34 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Stephen Boyd
5 siblings, 2 replies; 86+ messages in thread
From: Shawn Guo @ 2011-08-19 4:56 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-18 16:07 ` Will Deacon
@ 2011-08-19 4:59 ` Shawn Guo
2011-08-19 11:08 ` Will Deacon
0 siblings, 1 reply; 86+ messages in thread
From: Shawn Guo @ 2011-08-19 4:59 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 4:56 ` [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection Shawn Guo
@ 2011-08-19 6:35 ` Sascha Hauer
2011-08-19 7:00 ` Shawn Guo
2011-08-19 11:09 ` Will Deacon
2011-11-22 8:58 ` Uwe Kleine-König
1 sibling, 2 replies; 86+ messages in thread
From: Sascha Hauer @ 2011-08-19 6:35 UTC (permalink / raw)
To: linux-arm-kernel
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 |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 6:35 ` Sascha Hauer
@ 2011-08-19 7:00 ` Shawn Guo
2011-08-19 11:09 ` Will Deacon
1 sibling, 0 replies; 86+ messages in thread
From: Shawn Guo @ 2011-08-19 7:00 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 4:59 ` Shawn Guo
@ 2011-08-19 11:08 ` Will Deacon
2011-08-19 11:37 ` Shawn Guo
0 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-19 11:08 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 6:35 ` Sascha Hauer
2011-08-19 7:00 ` Shawn Guo
@ 2011-08-19 11:09 ` Will Deacon
2011-08-19 11:39 ` Sascha Hauer
1 sibling, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-19 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 11:08 ` Will Deacon
@ 2011-08-19 11:37 ` Shawn Guo
2011-08-19 12:32 ` Will Deacon
2011-08-19 14:54 ` Nicolas Pitre
0 siblings, 2 replies; 86+ messages in thread
From: Shawn Guo @ 2011-08-19 11:37 UTC (permalink / raw)
To: linux-arm-kernel
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.
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 11:09 ` Will Deacon
@ 2011-08-19 11:39 ` Sascha Hauer
2011-08-19 12:35 ` Will Deacon
2011-08-21 9:18 ` Russell King - ARM Linux
0 siblings, 2 replies; 86+ messages in thread
From: Sascha Hauer @ 2011-08-19 11:39 UTC (permalink / raw)
To: linux-arm-kernel
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 |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 11:37 ` Shawn Guo
@ 2011-08-19 12:32 ` Will Deacon
2011-08-19 15:49 ` Nicolas Pitre
2011-08-19 14:54 ` Nicolas Pitre
1 sibling, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-19 12:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 11:39 ` Sascha Hauer
@ 2011-08-19 12:35 ` Will Deacon
2011-08-19 17:15 ` Sascha Hauer
2011-08-21 9:18 ` Russell King - ARM Linux
1 sibling, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-08-19 12:35 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 11:37 ` Shawn Guo
2011-08-19 12:32 ` Will Deacon
@ 2011-08-19 14:54 ` Nicolas Pitre
1 sibling, 0 replies; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-19 14:54 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 12:32 ` Will Deacon
@ 2011-08-19 15:49 ` Nicolas Pitre
2011-08-21 9:14 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-19 15:49 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 12:35 ` Will Deacon
@ 2011-08-19 17:15 ` Sascha Hauer
0 siblings, 0 replies; 86+ messages in thread
From: Sascha Hauer @ 2011-08-19 17:15 UTC (permalink / raw)
To: linux-arm-kernel
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 |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-19 15:49 ` Nicolas Pitre
@ 2011-08-21 9:14 ` Russell King - ARM Linux
2011-08-21 17:35 ` Nicolas Pitre
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 9:14 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 11:39 ` Sascha Hauer
2011-08-19 12:35 ` Will Deacon
@ 2011-08-21 9:18 ` Russell King - ARM Linux
2011-08-21 11:25 ` Will Deacon
2011-08-21 17:59 ` Nicolas Pitre
1 sibling, 2 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 9:18 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-21 9:18 ` Russell King - ARM Linux
@ 2011-08-21 11:25 ` Will Deacon
2011-08-21 17:59 ` Nicolas Pitre
1 sibling, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-08-21 11:25 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 9:14 ` Russell King - ARM Linux
@ 2011-08-21 17:35 ` Nicolas Pitre
2011-08-21 18:26 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 17:35 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-21 9:18 ` Russell King - ARM Linux
2011-08-21 11:25 ` Will Deacon
@ 2011-08-21 17:59 ` Nicolas Pitre
2011-08-21 18:17 ` Russell King - ARM Linux
1 sibling, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 17:59 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-21 17:59 ` Nicolas Pitre
@ 2011-08-21 18:17 ` Russell King - ARM Linux
2011-08-21 18:28 ` Nicolas Pitre
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 18:17 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 17:35 ` Nicolas Pitre
@ 2011-08-21 18:26 ` Russell King - ARM Linux
2011-08-21 19:02 ` Nicolas Pitre
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 18:26 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-21 18:17 ` Russell King - ARM Linux
@ 2011-08-21 18:28 ` Nicolas Pitre
2011-08-21 18:33 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 18:28 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-21 18:28 ` Nicolas Pitre
@ 2011-08-21 18:33 ` Russell King - ARM Linux
0 siblings, 0 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 18:33 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 18:26 ` Russell King - ARM Linux
@ 2011-08-21 19:02 ` Nicolas Pitre
2011-08-21 19:18 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 19:02 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 19:02 ` Nicolas Pitre
@ 2011-08-21 19:18 ` Russell King - ARM Linux
2011-08-21 19:22 ` Russell King - ARM Linux
2011-08-21 19:53 ` Nicolas Pitre
0 siblings, 2 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 19:18 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 19:18 ` Russell King - ARM Linux
@ 2011-08-21 19:22 ` Russell King - ARM Linux
2011-08-21 20:07 ` Nicolas Pitre
2011-08-21 19:53 ` Nicolas Pitre
1 sibling, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 19:22 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 19:18 ` Russell King - ARM Linux
2011-08-21 19:22 ` Russell King - ARM Linux
@ 2011-08-21 19:53 ` Nicolas Pitre
2011-09-06 9:28 ` Tony Lindgren
1 sibling, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 19:53 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 19:22 ` Russell King - ARM Linux
@ 2011-08-21 20:07 ` Nicolas Pitre
2011-08-21 20:54 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 20:07 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 20:07 ` Nicolas Pitre
@ 2011-08-21 20:54 ` Russell King - ARM Linux
2011-08-21 21:00 ` Nicolas Pitre
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 20:54 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 20:54 ` Russell King - ARM Linux
@ 2011-08-21 21:00 ` Nicolas Pitre
2011-08-21 21:29 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 21:00 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 21:00 ` Nicolas Pitre
@ 2011-08-21 21:29 ` Russell King - ARM Linux
2011-08-21 22:00 ` Nicolas Pitre
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-08-21 21:29 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 21:29 ` Russell King - ARM Linux
@ 2011-08-21 22:00 ` Nicolas Pitre
0 siblings, 0 replies; 86+ messages in thread
From: Nicolas Pitre @ 2011-08-21 22:00 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-21 19:53 ` Nicolas Pitre
@ 2011-09-06 9:28 ` Tony Lindgren
2011-09-06 9:37 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Tony Lindgren @ 2011-09-06 9:28 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 9:28 ` Tony Lindgren
@ 2011-09-06 9:37 ` Russell King - ARM Linux
2011-09-06 10:27 ` Tony Lindgren
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-09-06 9:37 UTC (permalink / raw)
To: linux-arm-kernel
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?
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 9:37 ` Russell King - ARM Linux
@ 2011-09-06 10:27 ` Tony Lindgren
2011-09-06 10:52 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Tony Lindgren @ 2011-09-06 10:27 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 10:27 ` Tony Lindgren
@ 2011-09-06 10:52 ` Russell King - ARM Linux
2011-09-06 11:01 ` Tony Lindgren
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-09-06 10:52 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 10:52 ` Russell King - ARM Linux
@ 2011-09-06 11:01 ` Tony Lindgren
2011-09-06 11:07 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Tony Lindgren @ 2011-09-06 11:01 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 11:01 ` Tony Lindgren
@ 2011-09-06 11:07 ` Russell King - ARM Linux
2011-09-06 19:45 ` Uwe Kleine-König
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-09-06 11:07 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-06 11:07 ` Russell King - ARM Linux
@ 2011-09-06 19:45 ` Uwe Kleine-König
0 siblings, 0 replies; 86+ messages in thread
From: Uwe Kleine-König @ 2011-09-06 19:45 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
` (4 preceding siblings ...)
2011-08-19 4:56 ` [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection Shawn Guo
@ 2011-09-15 17:34 ` Stephen Boyd
2011-09-16 11:41 ` Will Deacon
5 siblings, 1 reply; 86+ messages in thread
From: Stephen Boyd @ 2011-09-15 17:34 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART
2011-09-15 17:34 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Stephen Boyd
@ 2011-09-16 11:41 ` Will Deacon
2011-09-19 18:01 ` Stephen Boyd
0 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-09-16 11:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-16 11:41 ` Will Deacon
@ 2011-09-19 18:01 ` Stephen Boyd
0 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 18:01 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-kernel, Will Deacon, Arnd Bergmann
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 18:01 ` Stephen Boyd
0 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 18:01 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 2/2] ARM: debug: Move DEBUG_ICEDCC into the DEBUG_LL choice
2011-09-19 18:01 ` Stephen Boyd
@ 2011-09-19 18:01 ` Stephen Boyd
-1 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 18:01 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-kernel, Will Deacon, Arnd Bergmann
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 2/2] ARM: debug: Move DEBUG_ICEDCC into the DEBUG_LL choice
@ 2011-09-19 18:01 ` Stephen Boyd
0 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 18:01 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 18:01 ` Stephen Boyd
@ 2011-09-19 21:25 ` Will Deacon
-1 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-09-19 21:25 UTC (permalink / raw)
To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 21:25 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-09-19 21:25 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 21:25 ` Will Deacon
@ 2011-09-19 22:12 ` Stephen Boyd
-1 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 22:12 UTC (permalink / raw)
To: Will Deacon; +Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 22:12 ` Stephen Boyd
0 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 22:12 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 22:12 ` Stephen Boyd
@ 2011-09-19 22:41 ` Russell King - ARM Linux
-1 siblings, 0 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-09-19 22:41 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Will Deacon, Arnd Bergmann, linux-kernel, linux-arm-kernel
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).
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 22:41 ` Russell King - ARM Linux
0 siblings, 0 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-09-19 22:41 UTC (permalink / raw)
To: linux-arm-kernel
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).
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 22:41 ` Russell King - ARM Linux
@ 2011-09-19 22:55 ` Stephen Boyd
-1 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 22:55 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Will Deacon, Arnd Bergmann, linux-kernel, linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 22:55 ` Stephen Boyd
0 siblings, 0 replies; 86+ messages in thread
From: Stephen Boyd @ 2011-09-19 22:55 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 22:12 ` Stephen Boyd
@ 2011-09-19 23:14 ` Will Deacon
-1 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-09-19 23:14 UTC (permalink / raw)
To: Stephen Boyd; +Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-09-19 23:14 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-09-19 23:14 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-08-16 21:41 ` [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection Will Deacon
@ 2011-10-10 11:56 ` Thomas Abraham
0 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 11:56 UTC (permalink / raw)
To: Will Deacon; +Cc: linux-arm-kernel, linux-samsung-soc
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
@ 2011-10-10 11:56 ` Thomas Abraham
0 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 11:56 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply related [flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-10-10 11:56 ` Thomas Abraham
@ 2011-10-10 12:23 ` Will Deacon
-1 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-10-10 12:23 UTC (permalink / raw)
To: Thomas Abraham; +Cc: linux-samsung-soc, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
@ 2011-10-10 12:23 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-10-10 12:23 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-10-10 12:23 ` Will Deacon
@ 2011-10-10 12:35 ` Thomas Abraham
-1 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 12:35 UTC (permalink / raw)
To: Will Deacon; +Cc: linux-arm-kernel, linux-samsung-soc
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
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
@ 2011-10-10 12:35 ` Thomas Abraham
0 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 12:35 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-10-10 12:35 ` Thomas Abraham
@ 2011-10-10 13:34 ` Will Deacon
-1 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-10-10 13:34 UTC (permalink / raw)
To: Thomas Abraham; +Cc: linux-arm-kernel, linux-samsung-soc
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
@ 2011-10-10 13:34 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-10-10 13:34 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
2011-10-10 13:34 ` Will Deacon
@ 2011-10-10 13:38 ` Thomas Abraham
-1 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 13:38 UTC (permalink / raw)
To: Will Deacon; +Cc: linux-arm-kernel, linux-samsung-soc
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
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection
@ 2011-10-10 13:38 ` Thomas Abraham
0 siblings, 0 replies; 86+ messages in thread
From: Thomas Abraham @ 2011-10-10 13:38 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-08-19 4:56 ` [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection Shawn Guo
2011-08-19 6:35 ` Sascha Hauer
@ 2011-11-22 8:58 ` Uwe Kleine-König
2011-11-22 11:50 ` Will Deacon
1 sibling, 1 reply; 86+ messages in thread
From: Uwe Kleine-König @ 2011-11-22 8:58 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 8:58 ` Uwe Kleine-König
@ 2011-11-22 11:50 ` Will Deacon
2011-11-22 13:02 ` Uwe Kleine-König
0 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-11-22 11:50 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 11:50 ` Will Deacon
@ 2011-11-22 13:02 ` Uwe Kleine-König
2011-11-22 13:20 ` Will Deacon
0 siblings, 1 reply; 86+ messages in thread
From: Uwe Kleine-König @ 2011-11-22 13:02 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 13:02 ` Uwe Kleine-König
@ 2011-11-22 13:20 ` Will Deacon
2011-11-22 13:30 ` Uwe Kleine-König
0 siblings, 1 reply; 86+ messages in thread
From: Will Deacon @ 2011-11-22 13:20 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 13:20 ` Will Deacon
@ 2011-11-22 13:30 ` Uwe Kleine-König
2011-11-22 15:48 ` Russell King - ARM Linux
0 siblings, 1 reply; 86+ messages in thread
From: Uwe Kleine-König @ 2011-11-22 13:30 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 13:30 ` Uwe Kleine-König
@ 2011-11-22 15:48 ` Russell King - ARM Linux
2011-11-22 16:38 ` Uwe Kleine-König
0 siblings, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-11-22 15:48 UTC (permalink / raw)
To: linux-arm-kernel
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.)
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 15:48 ` Russell King - ARM Linux
@ 2011-11-22 16:38 ` Uwe Kleine-König
2011-11-22 16:47 ` Mark Brown
0 siblings, 1 reply; 86+ messages in thread
From: Uwe Kleine-König @ 2011-11-22 16:38 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 16:38 ` Uwe Kleine-König
@ 2011-11-22 16:47 ` Mark Brown
2011-11-22 17:13 ` Uwe Kleine-König
2011-11-22 20:24 ` Russell King - ARM Linux
0 siblings, 2 replies; 86+ messages in thread
From: Mark Brown @ 2011-11-22 16:47 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 16:47 ` Mark Brown
@ 2011-11-22 17:13 ` Uwe Kleine-König
2011-11-22 20:24 ` Russell King - ARM Linux
1 sibling, 0 replies; 86+ messages in thread
From: Uwe Kleine-König @ 2011-11-22 17:13 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply related [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 16:47 ` Mark Brown
2011-11-22 17:13 ` Uwe Kleine-König
@ 2011-11-22 20:24 ` Russell King - ARM Linux
2011-11-22 21:19 ` Arnd Bergmann
1 sibling, 1 reply; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-11-22 20:24 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* Re: [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
2011-09-19 22:55 ` Stephen Boyd
@ 2011-11-22 20:42 ` Russell King - ARM Linux
-1 siblings, 0 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-11-22 20:42 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Will Deacon, Arnd Bergmann, linux-kernel, linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice
@ 2011-11-22 20:42 ` Russell King - ARM Linux
0 siblings, 0 replies; 86+ messages in thread
From: Russell King - ARM Linux @ 2011-11-22 20:42 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 20:24 ` Russell King - ARM Linux
@ 2011-11-22 21:19 ` Arnd Bergmann
2011-11-22 23:00 ` Mark Brown
0 siblings, 1 reply; 86+ messages in thread
From: Arnd Bergmann @ 2011-11-22 21:19 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 21:19 ` Arnd Bergmann
@ 2011-11-22 23:00 ` Mark Brown
2011-11-23 10:57 ` Will Deacon
0 siblings, 1 reply; 86+ messages in thread
From: Mark Brown @ 2011-11-22 23:00 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 86+ messages in thread
* [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection
2011-11-22 23:00 ` Mark Brown
@ 2011-11-23 10:57 ` Will Deacon
0 siblings, 0 replies; 86+ messages in thread
From: Will Deacon @ 2011-11-23 10:57 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 86+ messages in thread
end of thread, other threads:[~2011-11-23 10:57 UTC | newest]
Thread overview: 86+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-16 21:41 [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Will Deacon
2011-08-16 21:41 ` [PATCH 2/3] ARM: plat-samsung: use Kconfig choice for debug UART selection Will Deacon
2011-10-10 11:56 ` Thomas Abraham
2011-10-10 11:56 ` Thomas Abraham
2011-10-10 12:23 ` Will Deacon
2011-10-10 12:23 ` Will Deacon
2011-10-10 12:35 ` Thomas Abraham
2011-10-10 12:35 ` Thomas Abraham
2011-10-10 13:34 ` Will Deacon
2011-10-10 13:34 ` Will Deacon
2011-10-10 13:38 ` Thomas Abraham
2011-10-10 13:38 ` Thomas Abraham
2011-08-16 21:41 ` [PATCH 3/3] ARM: realview: " Will Deacon
2011-08-18 4:06 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Nicolas Pitre
2011-08-18 9:33 ` Will Deacon
2011-08-18 16:11 ` Shawn Guo
2011-08-18 16:07 ` Will Deacon
2011-08-19 4:59 ` Shawn Guo
2011-08-19 11:08 ` Will Deacon
2011-08-19 11:37 ` Shawn Guo
2011-08-19 12:32 ` Will Deacon
2011-08-19 15:49 ` Nicolas Pitre
2011-08-21 9:14 ` Russell King - ARM Linux
2011-08-21 17:35 ` Nicolas Pitre
2011-08-21 18:26 ` Russell King - ARM Linux
2011-08-21 19:02 ` Nicolas Pitre
2011-08-21 19:18 ` Russell King - ARM Linux
2011-08-21 19:22 ` Russell King - ARM Linux
2011-08-21 20:07 ` Nicolas Pitre
2011-08-21 20:54 ` Russell King - ARM Linux
2011-08-21 21:00 ` Nicolas Pitre
2011-08-21 21:29 ` Russell King - ARM Linux
2011-08-21 22:00 ` Nicolas Pitre
2011-08-21 19:53 ` Nicolas Pitre
2011-09-06 9:28 ` Tony Lindgren
2011-09-06 9:37 ` Russell King - ARM Linux
2011-09-06 10:27 ` Tony Lindgren
2011-09-06 10:52 ` Russell King - ARM Linux
2011-09-06 11:01 ` Tony Lindgren
2011-09-06 11:07 ` Russell King - ARM Linux
2011-09-06 19:45 ` Uwe Kleine-König
2011-08-19 14:54 ` Nicolas Pitre
2011-08-19 4:56 ` [PATCH] arm/imx: use Kconfig choice for low-level debug UART selection Shawn Guo
2011-08-19 6:35 ` Sascha Hauer
2011-08-19 7:00 ` Shawn Guo
2011-08-19 11:09 ` Will Deacon
2011-08-19 11:39 ` Sascha Hauer
2011-08-19 12:35 ` Will Deacon
2011-08-19 17:15 ` Sascha Hauer
2011-08-21 9:18 ` Russell King - ARM Linux
2011-08-21 11:25 ` Will Deacon
2011-08-21 17:59 ` Nicolas Pitre
2011-08-21 18:17 ` Russell King - ARM Linux
2011-08-21 18:28 ` Nicolas Pitre
2011-08-21 18:33 ` Russell King - ARM Linux
2011-11-22 8:58 ` Uwe Kleine-König
2011-11-22 11:50 ` Will Deacon
2011-11-22 13:02 ` Uwe Kleine-König
2011-11-22 13:20 ` Will Deacon
2011-11-22 13:30 ` Uwe Kleine-König
2011-11-22 15:48 ` Russell King - ARM Linux
2011-11-22 16:38 ` Uwe Kleine-König
2011-11-22 16:47 ` Mark Brown
2011-11-22 17:13 ` Uwe Kleine-König
2011-11-22 20:24 ` Russell King - ARM Linux
2011-11-22 21:19 ` Arnd Bergmann
2011-11-22 23:00 ` Mark Brown
2011-11-23 10:57 ` Will Deacon
2011-09-15 17:34 ` [PATCH 1/3] ARM: debug: use kconfig choice for selecting DEBUG_LL UART Stephen Boyd
2011-09-16 11:41 ` Will Deacon
2011-09-19 18:01 ` [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice Stephen Boyd
2011-09-19 18:01 ` Stephen Boyd
2011-09-19 18:01 ` [PATCH 2/2] ARM: debug: Move DEBUG_ICEDCC into the DEBUG_LL choice Stephen Boyd
2011-09-19 18:01 ` Stephen Boyd
2011-09-19 21:25 ` [PATCH 1/2] ARM: debug: Add CLSP711X_UART1 config choice Will Deacon
2011-09-19 21:25 ` Will Deacon
2011-09-19 22:12 ` Stephen Boyd
2011-09-19 22:12 ` Stephen Boyd
2011-09-19 22:41 ` Russell King - ARM Linux
2011-09-19 22:41 ` Russell King - ARM Linux
2011-09-19 22:55 ` Stephen Boyd
2011-09-19 22:55 ` Stephen Boyd
2011-11-22 20:42 ` Russell King - ARM Linux
2011-11-22 20:42 ` Russell King - ARM Linux
2011-09-19 23:14 ` Will Deacon
2011-09-19 23:14 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.