Hi Simon, Thanks for review.
On 11 June 2013 19:57, Simon Glass <s...@chromium.org> wrote: > Hi, > > On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh > <inderpal.si...@linaro.org>wrote: > >> Not all boards based on exynos5250 have SPI flash, same serial port and >> might >> not require display and the same lds script. Hence move them to board >> specific >> config file. >> > > At least for the serial port this should be controlled by the device tree. > There are quite a lot of pending patches for exynos, one of which enables > this and removes the need for the CONFIG_SERIAL3 define. > > If you want to have a look, I pushed them to: > > http://git.denx.de/u-boot-x86.git in branch 'snow' > I was not aware of this patchset. Thanks for pointing out. I will update my patchset to use DT for serial. > >> >> Signed-off-by: Inderpal Singh <inderpal.si...@linaro.org> >> --- >> v1 was posted as the second patch of [1] >> >> Changes in v2: >> - split from the patchset at [1] >> - moved CONFIG_LCD and CONFIG_SPI_FLASH >> - rebased to latest u-boot-samsung master branch >> >> [1] http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/157101 >> >> include/configs/exynos5250-dt.h | 11 +---------- >> include/configs/smdk5250.h | 16 ++++++++++++++-- >> include/configs/snow.h | 16 ++++++++++++++-- >> 3 files changed, 29 insertions(+), 14 deletions(-) >> >> diff --git a/include/configs/exynos5250-dt.h >> b/include/configs/exynos5250-dt.h >> index 03b07b2..22e47eb 100644 >> --- a/include/configs/exynos5250-dt.h >> +++ b/include/configs/exynos5250-dt.h >> @@ -29,7 +29,6 @@ >> #define CONFIG_SAMSUNG /* in a SAMSUNG core */ >> #define CONFIG_S5P /* S5P Family */ >> #define CONFIG_EXYNOS5 /* which is in a Exynos5 Family */ >> -#define CONFIG_SMDK5250 /* which is in a SMDK5250 >> */ >> > > This is a misnomer - it actually means Exynos 5250 I think. The only thing > it controls is the generation of the SPL packaging tool. So for now it > should be defined for all Exynos5250 boards. > Yes its a misnomer. I will change the name to CONFIG_EXYNOS5250. > > >> >> #include <asm/arch/cpu.h> /* get chip and board defs */ >> >> @@ -78,7 +77,6 @@ >> #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + (4 << 20)) >> >> /* select serial console configuration */ >> -#define CONFIG_SERIAL3 /* use SERIAL 3 */ >> #define CONFIG_BAUDRATE 115200 >> #define EXYNOS5_DEFAULT_UART_OFFSET 0x010000 >> >> @@ -148,8 +146,6 @@ >> #define CONFIG_SPL >> #define COPY_BL2_FNPTR_ADDR 0x02020030 >> >> -/* specific .lds file */ >> -#define CONFIG_SPL_LDSCRIPT >> "board/samsung/smdk5250/smdk5250-uboot-spl.lds" >> > > Again I suspect this is a misnomer. > Since all boards might not need this lds file, so how about moving lds file to board/samsung/common and renaming it to exynos5250-uboot-spl.lds. The boards needing this will have to include in their respective config files. Let me know your opinion. > > >> #define CONFIG_SPL_TEXT_BASE 0x02023400 >> #define CONFIG_SPL_MAX_FOOTPRINT (14 * 1024) >> >> @@ -158,7 +154,7 @@ >> /* Miscellaneous configurable options */ >> #define CONFIG_SYS_LONGHELP /* undef to save memory */ >> #define CONFIG_SYS_HUSH_PARSER /* use "hush" command parser */ >> -#define CONFIG_SYS_PROMPT "SMDK5250 # " >> +#define CONFIG_SYS_PROMPT "EXYNOS5250 # " >> #define CONFIG_SYS_CBSIZE 256 /* Console I/O Buffer >> Size */ >> #define CONFIG_SYS_PBSIZE 384 /* Print Buffer Size */ >> #define CONFIG_SYS_MAXARGS 16 /* max number of command >> args */ >> @@ -198,7 +194,6 @@ >> /* FLASH and environment organization */ >> #define CONFIG_SYS_NO_FLASH >> #undef CONFIG_CMD_IMLS >> -#define CONFIG_IDENT_STRING " for SMDK5250" >> >> #define CONFIG_SYS_MMC_ENV_DEV 0 >> >> @@ -247,9 +242,6 @@ >> #define CONFIG_I2C_EDID >> >> /* SPI */ >> -#define CONFIG_ENV_IS_IN_SPI_FLASH >> -#define CONFIG_SPI_FLASH >> - >> #ifdef CONFIG_SPI_FLASH >> #define CONFIG_EXYNOS_SPI >> #define CONFIG_CMD_SF >> @@ -306,7 +298,6 @@ >> #define CONFIG_SHA256 >> >> /* Display */ >> -#define CONFIG_LCD >> #ifdef CONFIG_LCD >> #define CONFIG_EXYNOS_FB >> #define CONFIG_EXYNOS_DP >> diff --git a/include/configs/smdk5250.h b/include/configs/smdk5250.h >> index 81f83a8..4af1909 100644 >> --- a/include/configs/smdk5250.h >> +++ b/include/configs/smdk5250.h >> @@ -25,9 +25,21 @@ >> #ifndef __CONFIG_SMDK_H >> #define __CONFIG_SMDK_H >> >> -#include <configs/exynos5250-dt.h> >> - >> #undef CONFIG_DEFAULT_DEVICE_TREE >> #define CONFIG_DEFAULT_DEVICE_TREE exynos5250-smdk5250 >> >> +#define CONFIG_SMDK5250 /* which is in a SMDK5250 >> */ >> +#define CONFIG_SERIAL3 /* use SERIAL 3 */ >> + >> +/* specific .lds file */ >> +#define CONFIG_SPL_LDSCRIPT >> "board/samsung/smdk5250/smdk5250-uboot-spl.lds" >> +#define CONFIG_IDENT_STRING " for SMDK5250" >> +#define CONFIG_SPI_FLASH >> +#define CONFIG_ENV_IS_IN_SPI_FLASH >> + >> +/* Display */ >> +#define CONFIG_LCD >> + >> +#include <configs/exynos5250-dt.h> >> + >> #endif /* __CONFIG_SMDK_H */ >> diff --git a/include/configs/snow.h b/include/configs/snow.h >> index b8460fd..e940c69 100644 >> --- a/include/configs/snow.h >> +++ b/include/configs/snow.h >> @@ -25,9 +25,21 @@ >> #ifndef __CONFIG_SNOW_H >> #define __CONFIG_SNOW_H >> >> -#include <configs/exynos5250-dt.h> >> - >> #undef CONFIG_DEFAULT_DEVICE_TREE >> #define CONFIG_DEFAULT_DEVICE_TREE exynos5250-snow >> >> +#define CONFIG_SMDK5250 /* which is in a SMDK5250 >> */ >> +#define CONFIG_SERIAL3 /* use SERIAL 3 */ >> + >> +/* specific .lds file */ >> +#define CONFIG_SPL_LDSCRIPT >> "board/samsung/smdk5250/smdk5250-uboot-spl.lds" >> +#define CONFIG_IDENT_STRING " for SMDK5250" >> +#define CONFIG_SPI_FLASH >> +#define CONFIG_ENV_IS_IN_SPI_FLASH >> + >> +/* Display */ >> +#define CONFIG_LCD >> + >> +#include <configs/exynos5250-dt.h> >> + >> #endif /* __CONFIG_SNOW_H */ >> > > The intent with the exynos5250-dt file is that it supports any board with > that chip, so it should enable any feature used by Exynos5250 boards. > Granted that might not suit all boards, which only need a subset of the > features. Perhaps we should create an exynos5250-dt-base.h, with just the > core options defined. Then other boards can include the base file, and > exynos5250-dt can stay as the 'enable everything/ config? > > So as per you suggestion, there would be 3 files. One exynos5250-dt-base.h, second exynos5250-dt.h and third the board specific config file. How about having core options unconditionally enabled in exynos5250-dt.h and other options with #ifdef. The board specific config files can define the other options. This way only 2 files will do. For example, let exynos5250-dt.h has SPI related configs under #ifdef CONFIG_SPI_FLASH and let smdk5250.h or arndale.h define CONFIG_SPI_FLASH based on the spi flash presence in respective boards. Let me know your opinion. Thanks, Inder Regards, > Simon > >
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
