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

Stephen Warren
2015-02-24T16:45:18Z

On 02/23/2015 10:05 PM, Masahiro Yamada wrote:
> Hi Simon, Stephen,
>
>
>
> On Mon, 23 Feb 2015 10:44:54 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> 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.
>>
>
>
> Yes, Stephen explained all about my intention.
>
> I think CONFIG_USE_PRIVATE_LIBGCC is also necessary for Raspberry Pi 1 for example.

I can't remember if it's already set for the Pi. If not, the toolchains
I use happen not to need it:-)

> Moreover, I had already posted this patch:
> http://patchwork.ozlabs.org/patch/438360/
>
> I'd like to expand the private library to all the ARM boards.
>
>
> Linux Kernel includes the library in its source tree.
>
> I think it is generally a good idea to reduce the depencendy on particular toolchains.
> Agree?

I tend to agree. However, in the past, Wolfgang Denk has argued against
(ever?) using that option, claiming people should just use the correct
toolchain.