2013-05-31 12:03:31

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 0/3] Consolidate uncompress code for Samsung platform

The patches are based on v3.10-rc3.

They are build tested for s3c2410_defconfig, s3c6400_defconfig,
s5p64x0_defconfig, s5pc100_defconfig, s5pv210_defconfig,
exynos4_defconfig and exynos_defconfig.

Since they affect all the Samsung boards, testing them on different
machines would be essential. Unfortunately I can only test on Exynos here.

Tested on Exynos5250 based Arndale board.

Tushar Behera (3):
ARM: EXYNOS: uncompress - print debug messages if DEBUG_LL is defined
ARM: SAMSUNG: Consolidate uncompress subroutine
ARM: S5P64X0: Remove duplicate uncompress code

arch/arm/mach-exynos/include/mach/uncompress.h | 14 +-
arch/arm/mach-s3c24xx/include/mach/uncompress.h | 7 +
arch/arm/mach-s3c64xx/include/mach/uncompress.h | 7 +
arch/arm/mach-s5p64x0/include/mach/uncompress.h | 163 ++---------------------
arch/arm/mach-s5pc100/include/mach/uncompress.h | 6 +
arch/arm/mach-s5pv210/include/mach/uncompress.h | 6 +
arch/arm/plat-samsung/include/plat/uncompress.h | 16 ++-
7 files changed, 53 insertions(+), 166 deletions(-)

--
1.7.9.5


2013-05-31 12:03:22

by Tushar Behera

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

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 <[email protected]>
---
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
/*
* For preventing FIFO overrun or infinite loop of UART console,
* fifo_max should be the minimum fifo size of all of the UART channels
diff --git a/arch/arm/plat-samsung/include/plat/uncompress.h b/arch/arm/plat-samsung/include/plat/uncompress.h
index 438b248..350032b 100644
--- a/arch/arm/plat-samsung/include/plat/uncompress.h
+++ b/arch/arm/plat-samsung/include/plat/uncompress.h
@@ -66,6 +66,9 @@ uart_rd(unsigned int reg)

static void putc(int ch)
{
+ if (!uart_base)
+ return;
+
if (uart_rd(S3C2410_UFCON) & S3C2410_UFCON_FIFOMODE) {
int level;

@@ -118,7 +121,12 @@ static void arch_decomp_error(const char *x)
#ifdef CONFIG_S3C_BOOT_UART_FORCE_FIFO
static inline void arch_enable_uart_fifo(void)
{
- u32 fifocon = uart_rd(S3C2410_UFCON);
+ u32 fifocon;
+
+ if (!uart_base)
+ return;
+
+ fifocon = uart_rd(S3C2410_UFCON);

if (!(fifocon & S3C2410_UFCON_FIFOMODE)) {
fifocon |= S3C2410_UFCON_RESETBOTH;
--
1.7.9.5

2013-05-31 12:03:56

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 3/3] ARM: S5P64X0: Remove duplicate uncompress code

The uncompress code in S5P64X0 is almost same as the uncompress code
defined in plat-samsung. Better to reuse that code.

Signed-off-by: Tushar Behera <[email protected]>
---
arch/arm/mach-s5p64x0/include/mach/uncompress.h | 163 ++---------------------
1 file changed, 8 insertions(+), 155 deletions(-)

diff --git a/arch/arm/mach-s5p64x0/include/mach/uncompress.h b/arch/arm/mach-s5p64x0/include/mach/uncompress.h
index 19e0d64..b28a551 100644
--- a/arch/arm/mach-s5p64x0/include/mach/uncompress.h
+++ b/arch/arm/mach-s5p64x0/include/mach/uncompress.h
@@ -14,171 +14,24 @@
#define __ASM_ARCH_UNCOMPRESS_H

#include <mach/map.h>
+#include <plat/uncompress.h>

-/*
- * cannot use commonly <plat/uncompress.h>
- * because uart base of S5P6440 and S5P6450 is different
- */
-
-typedef unsigned int upf_t; /* cannot include linux/serial_core.h */
-
-/* uart setup */
-
-unsigned int fifo_mask;
-unsigned int fifo_max;
-
-/* forward declerations */
-
-static void arch_detect_cpu(void);
-
-/* defines for UART registers */
-
-#include <plat/regs-serial.h>
-#include <plat/regs-watchdog.h>
-
-/* working in physical space... */
-#undef S3C2410_WDOGREG
-#define S3C2410_WDOGREG(x) ((S3C24XX_PA_WATCHDOG + (x)))
-
-/* how many bytes we allow into the FIFO at a time in FIFO mode */
-#define FIFO_MAX (14)
-
-unsigned long uart_base;
-
-static __inline__ void get_uart_base(void)
+static void arch_detect_cpu(void)
{
unsigned int chipid;

chipid = *(const volatile unsigned int __force *) 0xE0100118;

- uart_base = S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT;
-
+#ifdef DEBUG_LL
if ((chipid & 0xff000) == 0x50000)
- uart_base += 0xEC800000;
+ uart_base = S5P6450_PA_UART(CONFIG_S3C_LOWLEVEL_UART_PORT);
else
- uart_base += 0xEC000000;
-}
-
-static __inline__ void uart_wr(unsigned int reg, unsigned int val)
-{
- volatile unsigned int *ptr;
-
- get_uart_base();
- ptr = (volatile unsigned int *)(reg + uart_base);
- *ptr = val;
-}
-
-static __inline__ unsigned int uart_rd(unsigned int reg)
-{
- volatile unsigned int *ptr;
-
- get_uart_base();
- ptr = (volatile unsigned int *)(reg + uart_base);
- return *ptr;
-}
-
-/*
- * we can deal with the case the UARTs are being run
- * in FIFO mode, so that we don't hold up our execution
- * waiting for tx to happen...
- */
-
-static void putc(int ch)
-{
- if (uart_rd(S3C2410_UFCON) & S3C2410_UFCON_FIFOMODE) {
- int level;
-
- while (1) {
- level = uart_rd(S3C2410_UFSTAT);
- level &= fifo_mask;
-
- if (level < fifo_max)
- break;
- }
-
- } else {
- /* not using fifos */
-
- while ((uart_rd(S3C2410_UTRSTAT) & S3C2410_UTRSTAT_TXE) != S3C2410_UTRSTAT_TXE)
- barrier();
- }
-
- /* write byte to transmission register */
- uart_wr(S3C2410_UTXH, ch);
-}
-
-static inline void flush(void)
-{
-}
-
-#define __raw_writel(d, ad) \
- do { \
- *((volatile unsigned int __force *)(ad)) = (d); \
- } while (0)
-
-
-#ifdef CONFIG_S3C_BOOT_ERROR_RESET
-
-static void arch_decomp_error(const char *x)
-{
- putstr("\n\n");
- putstr(x);
- putstr("\n\n -- System resetting\n");
-
- __raw_writel(0x4000, S3C2410_WTDAT);
- __raw_writel(0x4000, S3C2410_WTCNT);
- __raw_writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV128 | S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x40), S3C2410_WTCON);
-
- while(1);
-}
-
-#define arch_error arch_decomp_error
-#endif
-
-#ifdef CONFIG_S3C_BOOT_UART_FORCE_FIFO
-static inline void arch_enable_uart_fifo(void)
-{
- u32 fifocon = uart_rd(S3C2410_UFCON);
-
- if (!(fifocon & S3C2410_UFCON_FIFOMODE)) {
- fifocon |= S3C2410_UFCON_RESETBOTH;
- uart_wr(S3C2410_UFCON, fifocon);
-
- /* wait for fifo reset to complete */
- while (1) {
- fifocon = uart_rd(S3C2410_UFCON);
- if (!(fifocon & S3C2410_UFCON_RESETBOTH))
- break;
- }
- }
-}
+ uart_base = S5P6440_PA_UART(CONFIG_S3C_LOWLEVEL_UART_PORT);
#else
-#define arch_enable_uart_fifo() do { } while(0)
+ uart_base = NULL;
#endif

-static void arch_decomp_setup(void)
-{
- /*
- * we may need to setup the uart(s) here if we are not running
- * on an BAST... the BAST will have left the uarts configured
- * after calling linux.
- */
-
- arch_detect_cpu();
-
- /*
- * Enable the UART FIFOs if they where not enabled and our
- * configuration says we should turn them on.
- */
-
- arch_enable_uart_fifo();
+ fifo_mask = S3C2440_UFSTAT_TXMASK;
+ fifo_max = 63 << S3C2440_UFSTAT_TXSHIFT;
}
-
-
-
-static void arch_detect_cpu(void)
-{
- /* we do not need to do any cpu detection here at the moment. */
-}
-
#endif /* __ASM_ARCH_UNCOMPRESS_H */
--
1.7.9.5

2013-05-31 12:04:10

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 2/3] ARM: SAMSUNG: Consolidate uncompress subroutine

For mach-exynos, uart_base is a pointer and the value is calculated
in the machine folder. For other machines, uart_base is defined as
a macro in platform directory. For symmetry, the uart_base macro
definition is removed and the uart_base calculation is moved to
specific machine folders.

This would help us consolidating uncompress subroutine for s5p64x0.

Signed-off-by: Tushar Behera <[email protected]>
---
arch/arm/mach-exynos/include/mach/uncompress.h | 3 ---
arch/arm/mach-s3c24xx/include/mach/uncompress.h | 7 +++++++
arch/arm/mach-s3c64xx/include/mach/uncompress.h | 7 +++++++
arch/arm/mach-s5pc100/include/mach/uncompress.h | 6 ++++++
arch/arm/mach-s5pv210/include/mach/uncompress.h | 6 ++++++
arch/arm/plat-samsung/include/plat/uncompress.h | 6 ++----
6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-exynos/include/mach/uncompress.h b/arch/arm/mach-exynos/include/mach/uncompress.h
index 730f69f..30592fd 100644
--- a/arch/arm/mach-exynos/include/mach/uncompress.h
+++ b/arch/arm/mach-exynos/include/mach/uncompress.h
@@ -15,9 +15,6 @@
#include <asm/mach-types.h>

#include <mach/map.h>
-
-volatile u8 *uart_base;
-
#include <plat/uncompress.h>

static unsigned int __raw_readl(unsigned int ptr)
diff --git a/arch/arm/mach-s3c24xx/include/mach/uncompress.h b/arch/arm/mach-s3c24xx/include/mach/uncompress.h
index 8b283f8..638893e 100644
--- a/arch/arm/mach-s3c24xx/include/mach/uncompress.h
+++ b/arch/arm/mach-s3c24xx/include/mach/uncompress.h
@@ -49,6 +49,13 @@ static void arch_detect_cpu(void)
fifo_mask = S3C2410_UFSTAT_TXMASK;
fifo_max = 15 << S3C2410_UFSTAT_TXSHIFT;
}
+
+#ifdef CONFIG_DEBUG_LL
+ uart_base = (volatile u8 *) S3C_PA_UART +
+ (S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
+#else
+ uart_base = NULL;
+#endif
}

#endif /* __ASM_ARCH_UNCOMPRESS_H */
diff --git a/arch/arm/mach-s3c64xx/include/mach/uncompress.h b/arch/arm/mach-s3c64xx/include/mach/uncompress.h
index c6a82a2..d6f9188 100644
--- a/arch/arm/mach-s3c64xx/include/mach/uncompress.h
+++ b/arch/arm/mach-s3c64xx/include/mach/uncompress.h
@@ -23,6 +23,13 @@ static void arch_detect_cpu(void)
/* we do not need to do any cpu detection here at the moment. */
fifo_mask = S3C2440_UFSTAT_TXMASK;
fifo_max = 63 << S3C2440_UFSTAT_TXSHIFT;
+
+#ifdef CONFIG_DEBUG_LL
+ uart_base = (volatile u8 *)S3C_PA_UART +
+ (S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT);
+#else
+ uart_base = NULL;
+#endif
}

#endif /* __ASM_ARCH_UNCOMPRESS_H */
diff --git a/arch/arm/mach-s5pc100/include/mach/uncompress.h b/arch/arm/mach-s5pc100/include/mach/uncompress.h
index 01ccf53..c1242df 100644
--- a/arch/arm/mach-s5pc100/include/mach/uncompress.h
+++ b/arch/arm/mach-s5pc100/include/mach/uncompress.h
@@ -23,6 +23,12 @@ static void arch_detect_cpu(void)
/* we do not need to do any cpu detection here at the moment. */
fifo_mask = S3C2440_UFSTAT_TXMASK;
fifo_max = 63 << S3C2440_UFSTAT_TXSHIFT;
+
+#ifdef CONFIG_DEBUG_LL
+ uart_base = (volatile u8 *)S5P_PA_UART(CONFIG_S3C_LOWLEVEL_UART_PORT);
+#else
+ uart_base = NULL;
+#endif
}

#endif /* __ASM_ARCH_UNCOMPRESS_H */
diff --git a/arch/arm/mach-s5pv210/include/mach/uncompress.h b/arch/arm/mach-s5pv210/include/mach/uncompress.h
index ef977ea..2193812 100644
--- a/arch/arm/mach-s5pv210/include/mach/uncompress.h
+++ b/arch/arm/mach-s5pv210/include/mach/uncompress.h
@@ -21,6 +21,12 @@ static void arch_detect_cpu(void)
/* we do not need to do any cpu detection here at the moment. */
fifo_mask = S5PV210_UFSTAT_TXMASK;
fifo_max = 63 << S5PV210_UFSTAT_TXSHIFT;
+
+#ifdef CONFIG_DEBUG_LL
+ uart_base = (volatile u8 *)S5P_PA_UART(CONFIG_S3C_LOWLEVEL_UART_PORT);
+#else
+ uart_base = NULL;
+#endif
}

#endif /* __ASM_ARCH_UNCOMPRESS_H */
diff --git a/arch/arm/plat-samsung/include/plat/uncompress.h b/arch/arm/plat-samsung/include/plat/uncompress.h
index 350032b..78274eb 100644
--- a/arch/arm/plat-samsung/include/plat/uncompress.h
+++ b/arch/arm/plat-samsung/include/plat/uncompress.h
@@ -21,6 +21,8 @@ typedef unsigned int upf_t; /* cannot include linux/serial_core.h */
unsigned int fifo_mask;
unsigned int fifo_max;

+volatile u8 *uart_base;
+
/* forward declerations */

static void arch_detect_cpu(void);
@@ -37,10 +39,6 @@ static void arch_detect_cpu(void);
/* how many bytes we allow into the FIFO at a time in FIFO mode */
#define FIFO_MAX (14)

-#ifdef S3C_PA_UART
-#define uart_base S3C_PA_UART + (S3C_UART_OFFSET * CONFIG_S3C_LOWLEVEL_UART_PORT)
-#endif
-
static __inline__ void
uart_wr(unsigned int reg, unsigned int val)
{
--
1.7.9.5

2013-06-01 03:59:33

by Olof Johansson

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

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 <[email protected]>
> ---
> 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;

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


-Olof

2013-06-01 04:00:51

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: SAMSUNG: Consolidate uncompress subroutine

On Fri, May 31, 2013 at 05:19:03PM +0530, Tushar Behera wrote:
> For mach-exynos, uart_base is a pointer and the value is calculated
> in the machine folder. For other machines, uart_base is defined as
> a macro in platform directory. For symmetry, the uart_base macro
> definition is removed and the uart_base calculation is moved to
> specific machine folders.
>
> This would help us consolidating uncompress subroutine for s5p64x0.

Sorry, reading these in order and didn't go through all of them.

Same comment here, you can use config_enabled() and return early instead of
adding ifdefs.


-Olof

2013-06-01 08:41:49

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: S5P64X0: Remove duplicate uncompress code

Hi,

On 05/31/2013 01:49 PM, Tushar Behera wrote:
> The uncompress code in S5P64X0 is almost same as the uncompress code
> defined in plat-samsung. Better to reuse that code.
>
> Signed-off-by: Tushar Behera<[email protected]>
> ---
> arch/arm/mach-s5p64x0/include/mach/uncompress.h | 163 ++---------------------
> 1 file changed, 8 insertions(+), 155 deletions(-)

Not sure if you are aware of it, Tomasz has already posted a patch that
removes this duplicated code: https://patchwork.kernel.org/patch/2589331
I think that patch is not yet merged though.

Regards,
Sylwester

2013-06-03 04:18:18

by Tushar Behera

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: S5P64X0: Remove duplicate uncompress code

On 06/01/2013 02:11 PM, Sylwester Nawrocki wrote:
> Hi,
>
> On 05/31/2013 01:49 PM, Tushar Behera wrote:
>> The uncompress code in S5P64X0 is almost same as the uncompress code
>> defined in plat-samsung. Better to reuse that code.
>>
>> Signed-off-by: Tushar Behera<[email protected]>
>> ---
>> arch/arm/mach-s5p64x0/include/mach/uncompress.h | 163
>> ++---------------------
>> 1 file changed, 8 insertions(+), 155 deletions(-)
>
> Not sure if you are aware of it, Tomasz has already posted a patch that
> removes this duplicated code: https://patchwork.kernel.org/patch/2589331
> I think that patch is not yet merged though.
>
> Regards,
> Sylwester

Tomasz,

I had somehow missed that mail.

The patch that I have additionally makes calculation of uart_base
conditional on DEBUG_LL in line with the rest of the patches.

Would you mind re-spining your patch with those changes?

Alternately, I can take your patch and do above modifications in them.

Let me know your opinion on this.

--
Tushar Behera

2013-06-03 04:57:20

by Tushar Behera

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

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 <[email protected]>
>> ---
>> 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

2013-06-03 11:40:54

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: S5P64X0: Remove duplicate uncompress code

Hi Tushar,

On Monday 03 of June 2013 09:48:07 Tushar Behera wrote:
> On 06/01/2013 02:11 PM, Sylwester Nawrocki wrote:
> > Hi,
> >
> > On 05/31/2013 01:49 PM, Tushar Behera wrote:
> >> The uncompress code in S5P64X0 is almost same as the uncompress code
> >> defined in plat-samsung. Better to reuse that code.
> >>
> >> Signed-off-by: Tushar Behera<[email protected]>
> >> ---
> >>
> >> arch/arm/mach-s5p64x0/include/mach/uncompress.h | 163
> >>
> >> ++---------------------
> >>
> >> 1 file changed, 8 insertions(+), 155 deletions(-)
> >
> > Not sure if you are aware of it, Tomasz has already posted a patch that
> > removes this duplicated code: https://patchwork.kernel.org/patch/2589331
> > I think that patch is not yet merged though.
> >
> > Regards,
> > Sylwester
>
> Tomasz,
>
> I had somehow missed that mail.
>
> The patch that I have additionally makes calculation of uart_base
> conditional on DEBUG_LL in line with the rest of the patches.
>
> Would you mind re-spining your patch with those changes?
>
> Alternately, I can take your patch and do above modifications in them.

Unfortunately I'm unlikely to find time to do it myself in nearest future, so
feel free to do it. Thanks.

Keep in mind, however, that this patch was a dependency of

[PATCH 0/6] Samsung watchdog support clean-up
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/18736/focus=18951

so I would like to keep this series applicable.

Best regards,
--
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

2013-06-03 11:56:05

by Tushar Behera

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: S5P64X0: Remove duplicate uncompress code

>>
>> Tomasz,
>>
>> I had somehow missed that mail.
>>
>> The patch that I have additionally makes calculation of uart_base
>> conditional on DEBUG_LL in line with the rest of the patches.
>>
>> Would you mind re-spining your patch with those changes?
>>
>> Alternately, I can take your patch and do above modifications in them.
>
> Unfortunately I'm unlikely to find time to do it myself in nearest future, so
> feel free to do it. Thanks.
>
> Keep in mind, however, that this patch was a dependency of
>
> [PATCH 0/6] Samsung watchdog support clean-up
> http://thread.gmane.org/gmane.linux.kernel.samsung-soc/18736/focus=18951
>
> so I would like to keep this series applicable.
>

Sure. I will ensure that there is minimal change to the original patch
that you submitted.

> Best regards,
>


--
Tushar Behera