[U-Boot] [PATCH v2] exynos5250: move board specific configs to board specific config file
Simon Glass
sjg at chromium.org
Fri Jun 28 17:50:58 CEST 2013
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.
>
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
More information about the U-Boot
mailing list