On 28 June 2013 21:20, Simon Glass <s...@chromium.org> wrote: > Hi Inderpal, > > On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh > <inderpal.si...@linaro.org>wrote: > >> 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. >> > > Sounds good. > > >> >> >>> >>> >>>> >>>> #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. >> > > OK, so long has you know of boards that don't need 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.SPI >> > > Well the problem is who sets CONFIG_SPI_FLASH IIUC the CONFIG_SPI_FLASH will be set in respective board config file. Hence only boards want to have SPI boot support will define it. > > For us at least, exynos5250-dt is a good upstream target, since we can add > all features into it and it will should boot on all the different boards. > It helps to make sure that other boards don't get non-device-tree config > that breaks this approach. > > So I think you do need a base config file. But I think a better name might > be exynos5250-dt-common.h instead of exynos5250-dt-base.h. > > Regards, > Simon > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
-- with warm regards, Chander Kashyap _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
