Hi Simon,
On 20 June 2013 12:40, Inderpal Singh <[email protected]> wrote: > Hi Simon, > > Thanks for review. > > > On 11 June 2013 19:57, Simon Glass <[email protected]> wrote: > >> Hi, >> >> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <[email protected] >> > 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 <[email protected]> >>> --- >>> 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. > Any thoughts about it? > > >> >> >>> #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. > Any feedback about the above proposal? Thanks, Inder > > Thanks, > Inder > > > Regards, >> Simon >> >> >
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
