[U-Boot] [PATCH v2] exynos5250: move board specific configs to board specific config file

Inderpal Singh inderpal.singh at linaro.org
Thu Jun 20 09:10:29 CEST 2013


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.


>
>
>>
>>  #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
>
>


More information about the U-Boot mailing list