diff mbox

arm: Fix DEBUG_LL for multi-platform kernels (without PL01X)

Message ID 1396610269-11580-1-git-send-email-daniel.thompson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Thompson April 4, 2014, 11:17 a.m. UTC
When building a multi_v7_defconfig kernel it is not possible to configure
DEBUG_LL to use any serial device except a ARM Primecell PL01X, or more
accurately and worse, it is possible to configure a different serial
device but KConfig does not honour this request.

The happens because the multi-platform mode may include ARCH_SPEAR13XX
and this forcibly engages DEBUG_UART_PL01X to provide some kind of
compatibility with single platform builds (SPEAr supports both single and
multi-platform). This in turn causes DEBUG_LL_INCLUDE to wedge at
debug/pl01x.S.

Problem is fixed by only deploying the compatibility options for SPEAr
when ARCH_MULTIPLATFORM is not set.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm/Kconfig.debug | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Arnd Bergmann April 4, 2014, 11:39 a.m. UTC | #1
On Friday 04 April 2014 12:17:49 Daniel Thompson wrote:
> When building a multi_v7_defconfig kernel it is not possible to configure
> DEBUG_LL to use any serial device except a ARM Primecell PL01X, or more
> accurately and worse, it is possible to configure a different serial
> device but KConfig does not honour this request.
> 
> The happens because the multi-platform mode may include ARCH_SPEAR13XX
> and this forcibly engages DEBUG_UART_PL01X to provide some kind of
> compatibility with single platform builds (SPEAr supports both single and
> multi-platform). This in turn causes DEBUG_LL_INCLUDE to wedge at
> debug/pl01x.S.
> 
> Problem is fixed by only deploying the compatibility options for SPEAr
> when ARCH_MULTIPLATFORM is not set.

The assumption is actually wrong: SPEAr does not support single
platform builds any more. The PLAT_SPEAR_SINGLE symbol in Kconfig
is a leftover from an earlier patch I did that never got merged,
and we should just remove that.

> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  arch/arm/Kconfig.debug | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 0531da8..f10c784 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -991,9 +991,9 @@ config DEBUG_LL_INCLUDE
>  config DEBUG_UART_PL01X
>         def_bool ARCH_EP93XX || \
>                 ARCH_INTEGRATOR || \
> -               ARCH_SPEAR3XX || \
> -               ARCH_SPEAR6XX || \
> -               ARCH_SPEAR13XX || \
> +               (ARCH_SPEAR3XX && !ARCH_MULTIPLATFORM) || \
> +               (ARCH_SPEAR6XX && !ARCH_MULTIPLATFORM) || \
> +               (ARCH_SPEAR13XX && !ARCH_MULTIPLATFORM) || \
>                 ARCH_VERSATILE

Removing the SPEAR lines seems correct here, but the lines
you add don't make any sense.

	Arnd
Daniel Thompson April 4, 2014, 2:02 p.m. UTC | #2
On 04/04/14 12:39, Arnd Bergmann wrote:
>> Problem is fixed by only deploying the compatibility options for SPEAr
>> when ARCH_MULTIPLATFORM is not set.
> 
> The assumption is actually wrong: SPEAr does not support single
> platform builds any more. The PLAT_SPEAR_SINGLE symbol in Kconfig
> is a leftover from an earlier patch I did that never got merged,
> and we should just remove that.

Thanks. I did make a wrong conclusion based on that symbol.


>> ---
>>  arch/arm/Kconfig.debug | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index 0531da8..f10c784 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -991,9 +991,9 @@ config DEBUG_LL_INCLUDE
>>  config DEBUG_UART_PL01X
>>         def_bool ARCH_EP93XX || \
>>                 ARCH_INTEGRATOR || \
>> -               ARCH_SPEAR3XX || \
>> -               ARCH_SPEAR6XX || \
>> -               ARCH_SPEAR13XX || \
>> +               (ARCH_SPEAR3XX && !ARCH_MULTIPLATFORM) || \
>> +               (ARCH_SPEAR6XX && !ARCH_MULTIPLATFORM) || \
>> +               (ARCH_SPEAR13XX && !ARCH_MULTIPLATFORM) || \
>>                 ARCH_VERSATILE
> 
> Removing the SPEAR lines seems correct here, but the lines
> you add don't make any sense.

Will fix.
Paul Bolle April 4, 2014, 2:14 p.m. UTC | #3
On Fri, 2014-04-04 at 13:39 +0200, Arnd Bergmann wrote:
> The PLAT_SPEAR_SINGLE symbol in Kconfig
> is a leftover from an earlier patch I did that never got merged,
> and we should just remove that.

Correct, see http://thread.gmane.org/gmane.linux.kernel/1646112 . It
seems nothing happened after that. I'm planning on sending a reminder if
PLAT_SPEAR_SINGLE is still mentioned in v3.15-rc1.


Paul Bolle
Russell King - ARM Linux April 4, 2014, 2:41 p.m. UTC | #4
On Fri, Apr 04, 2014 at 12:17:49PM +0100, Daniel Thompson wrote:
> When building a multi_v7_defconfig kernel it is not possible to configure
> DEBUG_LL to use any serial device except a ARM Primecell PL01X, or more
> accurately and worse, it is possible to configure a different serial
> device but KConfig does not honour this request.
> 
> The happens because the multi-platform mode may include ARCH_SPEAR13XX
> and this forcibly engages DEBUG_UART_PL01X to provide some kind of
> compatibility with single platform builds (SPEAr supports both single and
> multi-platform). This in turn causes DEBUG_LL_INCLUDE to wedge at
> debug/pl01x.S.
> 
> Problem is fixed by only deploying the compatibility options for SPEAr
> when ARCH_MULTIPLATFORM is not set.

I think it's probably about time we killed the def_bool and just made
this a plain 'bool'.  The affected platforms should be using
DEBUG_LL_UART_PL01X by now.
Paul Bolle April 4, 2014, 2:52 p.m. UTC | #5
On Fri, 2014-04-04 at 16:14 +0200, Paul Bolle wrote:
> Correct, see http://thread.gmane.org/gmane.linux.kernel/1646112 . It
> seems nothing happened after that. I'm planning on sending a reminder if
> PLAT_SPEAR_SINGLE is still mentioned in v3.15-rc1.

It turns out my patch was incorrect. This chunk

@@ -95,7 +94,6 @@ config MACH_SPEAR600
     	  Supports ST SPEAr600 boards configured via the device-treesource "arch/arm/mach-spear6xx/Kconfig"
 
 config ARCH_SPEAR_AUTO
-	def_bool PLAT_SPEAR_SINGLE
 	depends on !ARCH_SPEAR13XX && !ARCH_SPEAR6XX
 	select ARCH_SPEAR3XX
 

is bogus. It leaves ARCH_SPEAR_AUTO without a type. So I'll have to
redo, but I'll wait until v3.15-rc1.


Paul Bolle
Daniel Thompson April 4, 2014, 3:18 p.m. UTC | #6
On 04/04/14 15:41, Russell King - ARM Linux wrote:
>> The happens because the multi-platform mode may include ARCH_SPEAR13XX
>> and this forcibly engages DEBUG_UART_PL01X to provide some kind of
>> compatibility with single platform builds (SPEAr supports both single and
>> multi-platform). This in turn causes DEBUG_LL_INCLUDE to wedge at
>> debug/pl01x.S.
>>
>> Problem is fixed by only deploying the compatibility options for SPEAr
>> when ARCH_MULTIPLATFORM is not set.
> 
> I think it's probably about time we killed the def_bool and just made
> this a plain 'bool'.  The affected platforms should be using
> DEBUG_LL_UART_PL01X by now.

Ok.

Looking closely I think this will actually be better even for the single
platforms since today KConfig will not honour targeting DEBUG_LL at
EmbeddedICE or semihosting on the platforms that remain in the list.

Expect a v3...



Daniel.
diff mbox

Patch

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 0531da8..f10c784 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -991,9 +991,9 @@  config DEBUG_LL_INCLUDE
 config DEBUG_UART_PL01X
 	def_bool ARCH_EP93XX || \
 		ARCH_INTEGRATOR || \
-		ARCH_SPEAR3XX || \
-		ARCH_SPEAR6XX || \
-		ARCH_SPEAR13XX || \
+		(ARCH_SPEAR3XX && !ARCH_MULTIPLATFORM) || \
+		(ARCH_SPEAR6XX && !ARCH_MULTIPLATFORM) || \
+		(ARCH_SPEAR13XX && !ARCH_MULTIPLATFORM) || \
 		ARCH_VERSATILE
 
 # Compatibility options for 8250