Re: [PATCH 1/3] ARM: EXYNOS: uncompress - print debug messages if DEBUG_LL is defined

[Tushar Behera <tushar.behera@linaro.org>]

On 06/01/2013 09:29 AM, Olof Johansson wrote:
> On Fri, May 31, 2013 at 05:19:02PM +0530, Tushar Behera wrote:
>> Printing low-level debug messages make an assumption that the specified
>> UART port has been preconfigured by the bootloader. Incorrectly
>> specified UART port results in system getting stalled while printing the
>> message "Uncompressing Linux... done, booting the kernel"
>>
>> This UART port number is specified through S3C_LOWLEVEL_UART_PORT. Since
>> the UART port might different for different board, it is not possible to
>> specify it correctly for every board that use a common defconfig file.
>>
>> Calling this print subroutine only when DEBUG_LL fixes the problem. By
>> disabling DEBUG_LL in default config file, we would be able to boot
>> multiple boards with different default UART ports.
>>
>> With this current approach, we miss the print "Uncompressing Linux...
>> done, booting the kernel." when DEBUG_LL is not defined.
>>
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  arch/arm/mach-exynos/include/mach/uncompress.h  |   11 ++++++++---
>>  arch/arm/plat-samsung/include/plat/uncompress.h |   10 +++++++++-
>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/include/mach/uncompress.h b/arch/arm/mach-exynos/include/mach/uncompress.h
>> index 2979995..730f69f 100644
>> --- a/arch/arm/mach-exynos/include/mach/uncompress.h
>> +++ b/arch/arm/mach-exynos/include/mach/uncompress.h
>> @@ -37,11 +37,16 @@ static void arch_detect_cpu(void)
>>  	chip_id >>= 20;
>>  	chip_id &= 0xf;
>>  
>> +#ifdef CONFIG_DEBUG_LL
>>  	if (chip_id == 0x5)
>> -		uart_base = (volatile u8 *)EXYNOS5_PA_UART + (S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
>> +		uart_base = (volatile u8 *)EXYNOS5_PA_UART +
>> +			(S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
>>  	else
>> -		uart_base = (volatile u8 *)EXYNOS4_PA_UART + (S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
>> -
>> +		uart_base = (volatile u8 *)EXYNOS4_PA_UART +
>> +			(S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
>> +#else
>> +	uart_base = NULL;
>> +#endif
> 
> You can do:
> 
> 	if (!config_enabled(CONFIG_DEBUG_LL))
> 		return;
> 

Thanks for the hint.

Or rather, we calculate uart_base unconditionally in mach/uncompress.h
and replace check for uart_base with config_enabled(CONFIG_DEBUG_LL) in
plat/uncompress.h. That would get rid of all the ifdef's in the
mach/uncompress.h files.

Let me spin out another version with this change.

> Since uart_base will be set to 0 by being in BSS anyway.
> 
> 
> -Olof
> 


-- 
Tushar Behera