[PATCHv2 1/2] ARM: debug: Add UART1 config choices

[PATCHv2 1/2] ARM: debug: Add UART1 config choices

[Stephen Boyd]

ARM patch 7072/1 (debug: use kconfig choice for selecting
DEBUG_LL UART) didn't notice that the Kconfigs relied on being
unselected to configure a different serial port. Since there is
no NONE option in a choice menu, explicitly add the other option
so that both serial ports can be selected.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Will Deacon <will.deacon@arm.com>
---

I plan to put these in the patch tracker if there are no further
comments.

 arch/arm/Kconfig.debug |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 60d1846..6dbcb00 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -81,14 +81,27 @@ choice
 	prompt "Kernel low-level debugging port"
 	depends on DEBUG_LL
 
+
+	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"
+		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"
@@ -96,8 +109,6 @@ choice
 		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.

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

[Stephen Boyd]

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

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig.debug |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 6dbcb00..31896f4 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -81,6 +81,17 @@ 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_FOOTBRIDGE_COM1
 		bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
@@ -120,18 +131,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.

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

[Stephen Boyd]

On 9/20/2011 6:23 PM, Stephen Boyd wrote:
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 6dbcb00..31896f4 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -81,6 +81,17 @@ 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

Oops. This depends isn't necessary. Will fix before submitting to patch
tracker.

> +		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_FOOTBRIDGE_COM1
>  		bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
>

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

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

[Stephen Boyd]

On 09/20/11 18:23, Stephen Boyd wrote:
> 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>
> Acked-by: Will Deacon <will.deacon@arm.com>
> ---
>

Chatting with David Brown made me realize that this patch now forces
ICEDCC for anybody who selects DEBUG_LL and hasn't moved to adding a
Kconfig in the choice menu. I'm not sure if that actually matters since
it may be incentive to put your machine of choice into this menu.

Otherwise, we may want to drop this until a later point, or just leave
it out of the choice menu altogether.

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

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

[Will Deacon]

On Thu, Sep 29, 2011 at 03:05:16AM +0100, Stephen Boyd wrote:
> On 09/20/11 18:23, Stephen Boyd wrote:
> > 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>
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > ---
> >
> 
> Chatting with David Brown made me realize that this patch now forces
> ICEDCC for anybody who selects DEBUG_LL and hasn't moved to adding a
> Kconfig in the choice menu. I'm not sure if that actually matters since
> it may be incentive to put your machine of choice into this menu.
> 
> Otherwise, we may want to drop this until a later point, or just leave
> it out of the choice menu altogether.

Given that the ICEDCC stuff shouldn't cause any other problems by having it
enabled (that is, your kernel should still boot) then we should be alright
leaving this as-is while platforms move over to the new scheme.

However, if we get attacked by the United Army of Enraged Platform Maintainers
wanting to use earlyprintk, then I'm hiding behind you :)

Will

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

[Will Deacon]

On Thu, Sep 29, 2011 at 10:21:02AM +0100, Will Deacon wrote:
> On Thu, Sep 29, 2011 at 03:05:16AM +0100, Stephen Boyd wrote:
> > Chatting with David Brown made me realize that this patch now forces
> > ICEDCC for anybody who selects DEBUG_LL and hasn't moved to adding a
> > Kconfig in the choice menu. I'm not sure if that actually matters since
> > it may be incentive to put your machine of choice into this menu.
> > 
> > Otherwise, we may want to drop this until a later point, or just leave
> > it out of the choice menu altogether.
> 
> Given that the ICEDCC stuff shouldn't cause any other problems by having it
> enabled (that is, your kernel should still boot) then we should be alright
> leaving this as-is while platforms move over to the new scheme.

Bah, just took this for a spin on my Realview-PBX (Cortex-A9) platform and
the board won't boot unless I have a hardware debugger enabled that can
service the comms channel. This contradicts the Kconfig text which says:

	  It does include a timeout to ensure that the system does not
	  totally freeze when there is nothing connected to read.

so we certainly need to fix something!

Will

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

[Russell King - ARM Linux]

On Thu, Sep 29, 2011 at 10:42:43AM +0100, Will Deacon wrote:
> Bah, just took this for a spin on my Realview-PBX (Cortex-A9) platform and
> the board won't boot unless I have a hardware debugger enabled that can
> service the comms channel. This contradicts the Kconfig text which says:
> 
> 	  It does include a timeout to ensure that the system does not
> 	  totally freeze when there is nothing connected to read.
> 
> so we certainly need to fix something!

One of the problems you'll encounter is that there's places where we
don't have any timer infrastructure (eg, decompressor) and software
timing loops won't work (they'll be too slow for the lower-end CPUs
and too fast for the upper-end CPUs.)

The best fix is probably to leave that as-is, but introduce a 'default'
output entry instead - so platforms which don't have any listed will
get that instead of the ICEDDC stuff.

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

[Will Deacon]

Hi Russell,

On Thu, Sep 29, 2011 at 10:46:02AM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 29, 2011 at 10:42:43AM +0100, Will Deacon wrote:
> > Bah, just took this for a spin on my Realview-PBX (Cortex-A9) platform and
> > the board won't boot unless I have a hardware debugger enabled that can
> > service the comms channel. This contradicts the Kconfig text which says:
> > 
> > 	  It does include a timeout to ensure that the system does not
> > 	  totally freeze when there is nothing connected to read.
> > 
> > so we certainly need to fix something!
> 
> One of the problems you'll encounter is that there's places where we
> don't have any timer infrastructure (eg, decompressor) and software
> timing loops won't work (they'll be too slow for the lower-end CPUs
> and too fast for the upper-end CPUs.)

Agreed, software timing loops are the only option and it's impossible to get
them right everywhere. I think the Kconfig help text should be updated so
that the timeout paragraph is removed.

> The best fix is probably to leave that as-is, but introduce a 'default'
> output entry instead - so platforms which don't have any listed will
> get that instead of the ICEDDC stuff.

Ok. Rather than 'default' (which people will probably select in preference
to choosing the correct UART) how about something like below? I can split
this into two patches if you prefer.

Will


----8<----

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 32b1be4..bfdf86b 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -81,6 +81,14 @@ 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
@@ -89,8 +97,8 @@ choice
                  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.
+                 Note that this may cause the system to hang during boot if
+                 there is nothing to read from the DCC.
 
        config DEBUG_FOOTBRIDGE_COM1
                bool "Kernel low-level debugging messages via footbridge 8250@PCI COM1"

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

[Russell King - ARM Linux]

On Thu, Sep 29, 2011 at 11:39:39AM +0100, Will Deacon wrote:
> Ok. Rather than 'default' (which people will probably select in preference
> to choosing the correct UART) how about something like below? I can split
> this into two patches if you prefer.

Yup, that's what I meant.

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

[Stephen Boyd]

On 09/29/11 03:39, Will Deacon wrote:
> Hi Russell,
>
> On Thu, Sep 29, 2011 at 10:46:02AM +0100, Russell King - ARM Linux wrote:
>> On Thu, Sep 29, 2011 at 10:42:43AM +0100, Will Deacon wrote:
>>> Bah, just took this for a spin on my Realview-PBX (Cortex-A9) platform and
>>> the board won't boot unless I have a hardware debugger enabled that can
>>> service the comms channel. This contradicts the Kconfig text which says:
>>>
>>> 	  It does include a timeout to ensure that the system does not
>>> 	  totally freeze when there is nothing connected to read.
>>>
>>> so we certainly need to fix something!
>> One of the problems you'll encounter is that there's places where we
>> don't have any timer infrastructure (eg, decompressor) and software
>> timing loops won't work (they'll be too slow for the lower-end CPUs
>> and too fast for the upper-end CPUs.)
> Agreed, software timing loops are the only option and it's impossible to get
> them right everywhere. I think the Kconfig help text should be updated so
> that the timeout paragraph is removed.

Yes if you wait a long time the timeout for each character will expire
and the execution will continue.

>
>> The best fix is probably to leave that as-is, but introduce a 'default'
>> output entry instead - so platforms which don't have any listed will
>> get that instead of the ICEDDC stuff.
[...]
> ----8<----
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 32b1be4..bfdf86b 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -81,6 +81,14 @@ choice
>         prompt "Kernel low-level debugging port"
>         depends on DEBUG_LL
>  
> +       config DEBUG_LL_UART_NONE
> +               bool "No low-level debugging UART"

Should this grow a long list of platforms that are in this choice menu?
For example:

    depends on !FOOTBRIDGE && !ARCH_CLPS711X && ...

> +               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
> @@ -89,8 +97,8 @@ choice
>                   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.
> +                 Note that this may cause the system to hang during boot if
> +                 there is nothing to read from the DCC.
>  
>         config DEBUG_FOOTBRIDGE_COM1
>                 bool "Kernel low-level debugging messages via footbridge 8250 at PCI COM1"
>

Otherwise

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

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

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

[Will Deacon]

On Thu, Sep 29, 2011 at 06:22:42PM +0100, Stephen Boyd wrote:
> On 09/29/11 03:39, Will Deacon wrote:
> > Agreed, software timing loops are the only option and it's impossible to get
> > them right everywhere. I think the Kconfig help text should be updated so
> > that the timeout paragraph is removed.
> 
> Yes if you wait a long time the timeout for each character will expire
> and the execution will continue.

Well I got impatient, so I figured it was best to update the help text!

> > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> > index 32b1be4..bfdf86b 100644
> > --- a/arch/arm/Kconfig.debug
> > +++ b/arch/arm/Kconfig.debug
> > @@ -81,6 +81,14 @@ choice
> >         prompt "Kernel low-level debugging port"
> >         depends on DEBUG_LL
> >  
> > +       config DEBUG_LL_UART_NONE
> > +               bool "No low-level debugging UART"
> 
> Should this grow a long list of platforms that are in this choice menu?
> For example:
> 
>     depends on !FOOTBRIDGE && !ARCH_CLPS711X && ...

It might be nice to avoid that if we can. Once all the platforms have moved
over to the choice, we can delete the dummy option anyway.

> Otherwise
> 
> Acked-by: Stephen Boyd <sboyd@codeaurora.org>

Thanks,

Will

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

[David Brown]

On Thu, Sep 29, 2011 at 10:22:42AM -0700, Stephen Boyd wrote:
> >> One of the problems you'll encounter is that there's places where we
> >> don't have any timer infrastructure (eg, decompressor) and software
> >> timing loops won't work (they'll be too slow for the lower-end CPUs
> >> and too fast for the upper-end CPUs.)
> > Agreed, software timing loops are the only option and it's impossible to get
> > them right everywhere. I think the Kconfig help text should be updated so
> > that the timeout paragraph is removed.
> 
> Yes if you wait a long time the timeout for each character will expire
> and the execution will continue.

Wouldn't it be better to detect the first timeout and then stop trying
to write to the DCC?

But, do we really want fancy auto-detection stuff in DEBUG_LL?  A
timeout might make sense for a DCC console, but not here.

David

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