| Message ID | 1370606193-2566-1-git-send-email-inderpal.singh@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <inderpal.singh@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' > > Signed-off-by: Inderpal Singh <inderpal.singh@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. > > #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. > #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? Regards, Simon
Hi Simon, Thanks for review. On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <inderpal.singh@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.singh@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 > >
Hi Simon, On 20 June 2013 12:40, Inderpal Singh <inderpal.singh@linaro.org> wrote: > Hi Simon, > > Thanks for review. > > > On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: > >> Hi, >> >> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <inderpal.singh@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.singh@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. > 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 >> >> >
Hi Inderpal, On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh <inderpal.singh@linaro.org>wrote: > Hi Simon, > > Thanks for review. > > > On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: > >> Hi, >> >> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <inderpal.singh@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.singh@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. > Well the problem is who sets CONFIG_SPI_FLASH 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
On 28 June 2013 21:20, Simon Glass <sjg@chromium.org> wrote: > Hi Inderpal, > > On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh > <inderpal.singh@linaro.org>wrote: > >> Hi Simon, >> >> Thanks for review. >> >> >> On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: >> >>> Hi, >>> >>> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <inderpal.singh@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.singh@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
Hi, On Mon, Jul 1, 2013 at 1:08 PM, Chander Kashyap <chander.kashyap@linaro.org>wrote: > On 28 June 2013 21:20, Simon Glass <sjg@chromium.org> wrote: > > Hi Inderpal, > > > > On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh > > <inderpal.singh@linaro.org>wrote: > > > >> Hi Simon, > >> > >> Thanks for review. > >> > >> > >> On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: > >> > >>> Hi, > >>> > >>> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh < > inderpal.singh@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.singh@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. > Yes, so if those boards include exynos5250-dt-common.h they can then define CONFIG_SPI_FLASH if they want to. But for exynos5250-dt.h, it can define more things (including CONFIG_SPI_FLASH) so that it can boot on any board, and provide a good build test target. Regards, Simon > > > > 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 >
Hi Simon, On 28 June 2013 21:20, Simon Glass <sjg@chromium.org> wrote: > Hi Inderpal, > > On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh < > inderpal.singh@linaro.org> wrote: > >> Hi Simon, >> >> Thanks for review. >> >> >> On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: >> >>> Hi, >>> >>> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh < >>> inderpal.singh@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.singh@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. >> > > Well the problem is who sets CONFIG_SPI_FLASH > > 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. > As per my understanding the exynos5250-dt.h was meant to be common for all exynos5250 based boards. Creating one more base common file looks a bit overdone to me. I will drop this patch. And for Arndale board support, I will just #undef the config options which are not needed as of now. Thanks, Inder > Regards, > Simon > >
Hi, On Mon, Jul 1, 2013 at 7:28 PM, Inderpal Singh <inderpal.singh@linaro.org>wrote: > Hi Simon, > > > On 28 June 2013 21:20, Simon Glass <sjg@chromium.org> wrote: > >> Hi Inderpal, >> >> On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh < >> inderpal.singh@linaro.org> wrote: >> >>> Hi Simon, >>> >>> Thanks for review. >>> >>> >>> On 11 June 2013 19:57, Simon Glass <sjg@chromium.org> wrote: >>> >>>> Hi, >>>> >>>> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh < >>>> inderpal.singh@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.singh@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. >>> >> >> Well the problem is who sets CONFIG_SPI_FLASH >> >> 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. >> > > As per my understanding the exynos5250-dt.h was meant to be common for all > exynos5250 based boards. Creating one more base common file looks a bit > overdone to me. > > Yes it is supposed to be able to boot on all boards. But if you are wanting to #undef options, then you could create a new board. > I will drop this patch. And for Arndale board support, I will just #undef > the config options which are not needed as of now. > OK. Regards, Simon
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 */ #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" #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 */
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. Signed-off-by: Inderpal Singh <inderpal.singh@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(-)