[U-Boot] [PATCH v2] exynos5250: move board specific configs to board specific config file
Simon Glass
sjg at chromium.org
Tue Jul 2 04:53:46 CEST 2013
Hi,
On Mon, Jul 1, 2013 at 7:28 PM, Inderpal Singh <inderpal.singh at linaro.org>wrote:
> Hi Simon,
>
>
> 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.
>>>
>>
>> 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
More information about the U-Boot
mailing list