[U-Boot] [PATCH v2] exynos5250: move board specific configs to board specific config file
Simon Glass
sjg at chromium.org
Mon Jul 1 06:20:48 CEST 2013
Hi,
On Mon, Jul 1, 2013 at 1:08 PM, Chander Kashyap
<chander.kashyap at linaro.org>wrote:
> On 28 June 2013 21:20, Simon Glass <sjg at chromium.org> wrote:
> > Hi Inderpal,
> >
> > On Thu, Jun 20, 2013 at 12:10 AM, Inderpal Singh
> > <inderpal.singh at linaro.org>wrote:
> >
> >> Hi Simon,
> >>
> >> Thanks for review.
> >>
> >>
> >> On 11 June 2013 19:57, Simon Glass <sjg at chromium.org> wrote:
> >>
> >>> Hi,
> >>>
> >>> On Fri, Jun 7, 2013 at 4:56 AM, Inderpal Singh <
> inderpal.singh at 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 at 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
>
>
>
> --
> with warm regards,
> Chander Kashyap
>
More information about the U-Boot
mailing list