[U-Boot] [PATCH v2] exynos5250: move board specific configs to board specific config file
Inderpal Singh
inderpal.singh at linaro.org
Fri Jun 28 12:27:29 CEST 2013
Hi Simon,
On 20 June 2013 12:40, 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.
>
>
>>
>>
>>>
>>> #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
>>
>>
>
More information about the U-Boot
mailing list