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

Simon Glass
2015-02-23T17:44:54Z

Hi Stephen,

On 23 February 2015 at 10:33, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/23/2015 07:02 AM, Simon Glass wrote:
>>
>> Hi Masahiro,
>>
>> On 20 February 2015 at 19:37, Masahiro YAMADA <yamada.m@jp.panasonic.com>
>> wrote:
>>>
>>> Hi Simon,
>>>
>>>
>>> 2015-02-21 11:28 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Masahiro,
>>>>
>>>> On 20 February 2015 at 17:54, Masahiro YAMADA
>>>> <yamada.m@jp.panasonic.com> wrote:
>>>>>
>>>>> 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 I saw that, I understand now. So SPL_BUILD is no longer available
>>>> in Kconfig, but is still available in Makefiles, right?
>>>
>>>
>>> Yes, exactly!
>>
>>
>> This all works fine on Tegra for me. However I like to suggest
>> dropping a few patches in this series.
>>
>> I don't think it is worth using ARCH_MALLOC_F_LEN. In fact for me the
>> Tegra defconfig looks OK and SPL is built correctly.
>>
>> My remaining question is about that Tegra seems to want
>> USE_PRIVATE_LIBGCC for SPL but not for U-Boot. I'm not sure why, nor
>> whether it matters. It seems to work find using it for both.
>
>
> Depending on the toolchain, we actively need USE_PRIVATE_LIBGCC for SPL, and
> don't /need/ it for non-SPL. However, enabling USE_PRIVATE_LIBGCC for
> non-SPL likely won't hurt.
>
> The issue is that the libgcc bundled with most compilers is for ARMv7 (since
> we tend to use ARMv7 compilers, since the main U-boot is built for ARMv7).
> That bundled libgcc won't work on the ARMv4 that runs the SPL, so we need
> USE_PRIVATE_LIBGCC there. The private libgcc bundled with U-Boot should work
> fine when built for either CPU, so it is OK to always use it, rather than
> only use it when strictly needed.

Ah yes, I think I knew that once. So in short Masahiro's patch here
should be fine.

Regards,
Simon