All of lore.kernel.org
 help / color / mirror / Atom feed
From: srinivas.kandagatla@linaro.org (Srinivas Kandagatla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: debug: fix broken DEBUG_LL_INCLUDE for multi platform
Date: Tue, 10 Jun 2014 11:47:00 +0100	[thread overview]
Message-ID: <5396E224.1070806@linaro.org> (raw)
In-Reply-To: <20140610101021.GX23430@n2100.arm.linux.org.uk>



On 10/06/14 11:10, Russell King - ARM Linux wrote:
> On Tue, Jun 10, 2014 at 10:57:16AM +0100, Srinivas Kandagatla wrote:
>> On multi_v7_defconfig using def_bool in Kconfig can override the selection
>> made as part of DEBUG_LL. This is because def_bool will set the config to true
>> if the expression evaluates to true, which is what was happening in
>> multi_v7_defconfig. ARCH_SPEAR13XX selects DEBUG_UART_PL01X overiding any
>> previous DEBUG_LL selections.
>>
>> Making the def_bool to bool and depends made sense in this case, and
>> fixes the issue too.
>
> NAK.
>
> 1. you haven't tested this - if you select DEBUG_BCM2835 in the choice,
>     it will select DEBUG_UART_PL01X, which, with your patch, will not
>     have its new dependencies satisfied.

I see your point.
>
> 2. there is nothing to select this debug option on the platforms which
>     you make it depend upon.

>
> The real solution here is to convert these (and the other) remaining
> platforms to the choice mechanism.  Same for the 8250 one.
>
Yes, I can see the mess here,

Choice menu already provides options for selecting pl01x and 8250 via 
DEBUG_LL_UART_PL01X and DEBUG_LL_UART_8250 options. so I don't see point 
of having dependencies or def_bool options for PL01X and 8250. Other 
then getting them selected automatically and overriding DEBUG_LL selections.

This new patch removes those dependencies or default selections and 
forces the platforms to go via choice mechanism.

What do you think?

Subject: [PATCH v2] ARM: debug: fix broken DEBUG_LL_INCLUDE for multi 
platform

On multi_v7_defconfig using def_bool in Kconfig can override the selection
made as part of DEBUG_LL. This is because def_bool will set the config 
to true
if the expression evaluates to true, which is what was happening in
multi_v7_defconfig. ARCH_SPEAR13XX selects DEBUG_UART_PL01X overiding any
previous DEBUG_LL selections.

Making the def_bool to bool made sense in this case, and fixes the issue 
too.
This means that the platforms should go via DEBUG_LL choice mechanism to
select PL01X or 8250 debug uart.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
  arch/arm/Kconfig.debug | 17 +++--------------
  1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 40ee328..ce676bb 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -962,22 +962,11 @@ config DEBUG_LL_INCLUDE
  	default "debug/zynq.S" if DEBUG_ZYNQ_UART0 || DEBUG_ZYNQ_UART1
  	default "mach/debug-macro.S"

-# Compatibility options for PL01x
  config DEBUG_UART_PL01X
-	def_bool ARCH_EP93XX || \
-		ARCH_INTEGRATOR || \
-		ARCH_SPEAR3XX || \
-		ARCH_SPEAR6XX || \
-		ARCH_SPEAR13XX || \
-		ARCH_VERSATILE
-
-# Compatibility options for 8250
+	bool
+
  config DEBUG_UART_8250
-	def_bool ARCH_DOVE || ARCH_EBSA110 || \
-		(FOOTBRIDGE && !DEBUG_DC21285_PORT) || \
-		ARCH_GEMINI || ARCH_IOP13XX || ARCH_IOP32X || \
-		ARCH_IOP33X || ARCH_IXP4XX || ARCH_KIRKWOOD || \
-		ARCH_LPC32XX || ARCH_MV78XX0 || ARCH_ORION5X || ARCH_RPC
+	bool

  config DEBUG_UART_PHYS
  	hex "Physical base address of debug UART"
-- 

--srini

WARNING: multiple messages have this Message-ID (diff)
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: debug: fix broken DEBUG_LL_INCLUDE for multi platform
Date: Tue, 10 Jun 2014 11:47:00 +0100	[thread overview]
Message-ID: <5396E224.1070806@linaro.org> (raw)
In-Reply-To: <20140610101021.GX23430@n2100.arm.linux.org.uk>



On 10/06/14 11:10, Russell King - ARM Linux wrote:
> On Tue, Jun 10, 2014 at 10:57:16AM +0100, Srinivas Kandagatla wrote:
>> On multi_v7_defconfig using def_bool in Kconfig can override the selection
>> made as part of DEBUG_LL. This is because def_bool will set the config to true
>> if the expression evaluates to true, which is what was happening in
>> multi_v7_defconfig. ARCH_SPEAR13XX selects DEBUG_UART_PL01X overiding any
>> previous DEBUG_LL selections.
>>
>> Making the def_bool to bool and depends made sense in this case, and
>> fixes the issue too.
>
> NAK.
>
> 1. you haven't tested this - if you select DEBUG_BCM2835 in the choice,
>     it will select DEBUG_UART_PL01X, which, with your patch, will not
>     have its new dependencies satisfied.

I see your point.
>
> 2. there is nothing to select this debug option on the platforms which
>     you make it depend upon.

>
> The real solution here is to convert these (and the other) remaining
> platforms to the choice mechanism.  Same for the 8250 one.
>
Yes, I can see the mess here,

Choice menu already provides options for selecting pl01x and 8250 via 
DEBUG_LL_UART_PL01X and DEBUG_LL_UART_8250 options. so I don't see point 
of having dependencies or def_bool options for PL01X and 8250. Other 
then getting them selected automatically and overriding DEBUG_LL selections.

This new patch removes those dependencies or default selections and 
forces the platforms to go via choice mechanism.

What do you think?

Subject: [PATCH v2] ARM: debug: fix broken DEBUG_LL_INCLUDE for multi 
platform

On multi_v7_defconfig using def_bool in Kconfig can override the selection
made as part of DEBUG_LL. This is because def_bool will set the config 
to true
if the expression evaluates to true, which is what was happening in
multi_v7_defconfig. ARCH_SPEAR13XX selects DEBUG_UART_PL01X overiding any
previous DEBUG_LL selections.

Making the def_bool to bool made sense in this case, and fixes the issue 
too.
This means that the platforms should go via DEBUG_LL choice mechanism to
select PL01X or 8250 debug uart.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
  arch/arm/Kconfig.debug | 17 +++--------------
  1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 40ee328..ce676bb 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -962,22 +962,11 @@ config DEBUG_LL_INCLUDE
  	default "debug/zynq.S" if DEBUG_ZYNQ_UART0 || DEBUG_ZYNQ_UART1
  	default "mach/debug-macro.S"

-# Compatibility options for PL01x
  config DEBUG_UART_PL01X
-	def_bool ARCH_EP93XX || \
-		ARCH_INTEGRATOR || \
-		ARCH_SPEAR3XX || \
-		ARCH_SPEAR6XX || \
-		ARCH_SPEAR13XX || \
-		ARCH_VERSATILE
-
-# Compatibility options for 8250
+	bool
+
  config DEBUG_UART_8250
-	def_bool ARCH_DOVE || ARCH_EBSA110 || \
-		(FOOTBRIDGE && !DEBUG_DC21285_PORT) || \
-		ARCH_GEMINI || ARCH_IOP13XX || ARCH_IOP32X || \
-		ARCH_IOP33X || ARCH_IXP4XX || ARCH_KIRKWOOD || \
-		ARCH_LPC32XX || ARCH_MV78XX0 || ARCH_ORION5X || ARCH_RPC
+	bool

  config DEBUG_UART_PHYS
  	hex "Physical base address of debug UART"
-- 

--srini

  reply	other threads:[~2014-06-10 10:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-10  9:57 [PATCH] ARM: debug: fix broken DEBUG_LL_INCLUDE for multi platform Srinivas Kandagatla
2014-06-10  9:57 ` Srinivas Kandagatla
2014-06-10 10:10 ` Russell King - ARM Linux
2014-06-10 10:10   ` Russell King - ARM Linux
2014-06-10 10:47   ` Srinivas Kandagatla [this message]
2014-06-10 10:47     ` Srinivas Kandagatla
2014-06-18  8:48 ` [PATCH v1] " Srinivas Kandagatla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5396E224.1070806@linaro.org \
    --to=srinivas.kandagatla@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.