[U-Boot] [PATCH v3 6/7] kconfig: remove unneeded dependency on !SPL_BUILD

Masahiro YAMADA
2015-02-21T00:54:13Z

Hi Simon, Stephen,

2015-02-21 3:39 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Hi,
>
> On 20 February 2015 at 10:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/20/2015 10:06 AM, Simon Glass wrote:
>>>
>>> +Stephen
>>>
>>> Hi Masahiro,
>>>
>>> On 19 February 2015 at 22:25, Masahiro Yamada <yamada.m@jp.panasonic.com>
>>> wrote:
>>>>
>>>> Now CONFIG_SPL_BUILD is not defined in Kconfig, so
>>>> "!depends on SPL_BUILD" and "if !SPL_BUILD" are redundant.
>>
>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index 41f3220..700e2a8 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -739,9 +739,8 @@ config TEGRA
>>>> bool "NVIDIA Tegra"
>>>> select SUPPORT_SPL
>>>> select SPL
>>>> - select OF_CONTROL if !SPL_BUILD
>>>> - select CPU_ARM720T if SPL_BUILD
>>>> - select CPU_V7 if !SPL_BUILD
>>>> + select OF_CONTROL
>>>> + select CPU_V7
>>>
>>>
>>> Sorry if I have missed something here. On Tegra most unfortunately the
>>> SPL uses ARMv4t and U-Boot proper uses ARMv7. In fact that is the only
>>> reason that Tegra has SPL. Doesn't this change with this commit?

No. I think behavior is still the same as before.

In a single .config, we cannot define two CPUs in Kconfig.

So, we only define CPU_V7, for the main processors.

For SPL, we override the "CPU" in config.mk

ifdef CONFIG_SPL_BUILD
ifdef CONFIG_TEGRA
CPU := arm720t
endif
endif

I know what you might be saying is, this is too ugly. Yes.

I think we can do a little better with further rafactoring,
but the basic idea is, SPL of Tegra is a special case.

>> Yes, on Tegra SPL runs on an ARMv4t and main U-Boot on some ARMv7 CPU. The
>> change above is actively incorrect.
>>
>> The OF_CONTROL change is probably incorrect too; we certainly don't intend
>> to use OF_CONTROL in the SPL (there's really nothing to control in the SPL)
>> - I just don't know if enabling that feature will cause any issue. Things to
>> look out for would be bloat of the SPL binary so that it didn't fit into the
>> space before the main binary's TEXT_BASE, since the two get concatenated
>> together into a single binary that's loaded into RAM, and XIP'd.
>
> This at least is not a problem with this patch.
>
> I'll make some time to take a look at this in the next few days.
>

As for CONFIG_OF_CONTROL, I think my answer is already in the git-description.
I wrote as follows:

- Add some entries to include/config_uncmd_spl.h and the new file
scripts/Makefile.uncmd_spl. Some CONFIG options that are not
supported on SPL must be disabled because one .config is shared
between SPL and U-Boot proper going forward. I know this is not
a beautiful solution and I think we can do better, but let's see
how much we will have to describe them.

--
Best Regards
Masahiro Yamada