diff mbox series

[1/2] ARM: at91: Enable SPL_SEPARATE_BSS by default

Message ID 20200109163013.601223-1-gregory.clement@bootlin.com
State Superseded
Headers show
Series [1/2] ARM: at91: Enable SPL_SEPARATE_BSS by default | expand

Commit Message

Gregory CLEMENT Jan. 9, 2020, 4:30 p.m. UTC
According to the linker script for both armv7 and arm926ejs based SoC,
BSS section was all the time separated for SPL but this symbol was
only enabled on some boards. However, it is necessary to have it
enabled for OF_SEPARATE configuration where DTB is appended to u-boot
with DTB.

Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Stefan Roese Jan. 10, 2020, 6:32 a.m. UTC | #1
On 09.01.20 17:30, Gregory CLEMENT wrote:
> According to the linker script for both armv7 and arm926ejs based SoC,
> BSS section was all the time separated for SPL but this symbol was
> only enabled on some boards. However, it is necessary to have it
> enabled for OF_SEPARATE configuration where DTB is appended to u-boot
> with DTB.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
> ---
>   arch/arm/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index f9dab073ea..e558024652 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -492,6 +492,7 @@ choice
>   config ARCH_AT91
>   	bool "Atmel AT91"
>   	select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
> +	select SPL_SEPARATE_BSS if SPL
>   
>   config TARGET_EDB93XX
>   	bool "Support edb93xx"
> 

Reviewed-by: Stefan Roese <sr at denx.de>

Thanks,
Stefan
Eugen Hristev Jan. 23, 2020, 8:12 a.m. UTC | #2
On 10.01.2020 08:32, Stefan Roese wrote:

> On 09.01.20 17:30, Gregory CLEMENT wrote:
>> According to the linker script for both armv7 and arm926ejs based SoC,
>> BSS section was all the time separated for SPL but this symbol was
>> only enabled on some boards. However, it is necessary to have it
>> enabled for OF_SEPARATE configuration where DTB is appended to u-boot
>> with DTB.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
>> ---
>>   arch/arm/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index f9dab073ea..e558024652 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -492,6 +492,7 @@ choice
>>   config ARCH_AT91
>>       bool "Atmel AT91"
>>       select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
>> +     select SPL_SEPARATE_BSS if SPL
>>
>>   config TARGET_EDB93XX
>>       bool "Support edb93xx"
>>
> 
> Reviewed-by: Stefan Roese <sr at denx.de>
> 
> Thanks,
> Stefan
> 

Hi,

With these changes, for all the A5 boards , SEPARATE_BSS was already 
enabled, but for the rest of the platforms, it was not.
As I see from the linker script, the BSS area is configured into DRAM area.
Is there any chance that we have accesses to this section before the 
DRAM is initialized on these platforms ? Could cause the boards to hang.

Eugen
Gregory CLEMENT June 5, 2020, 7:49 a.m. UTC | #3
Hi Eugen,

> On 10.01.2020 08:32, Stefan Roese wrote:
>
>> On 09.01.20 17:30, Gregory CLEMENT wrote:
>>> According to the linker script for both armv7 and arm926ejs based SoC,
>>> BSS section was all the time separated for SPL but this symbol was
>>> only enabled on some boards. However, it is necessary to have it
>>> enabled for OF_SEPARATE configuration where DTB is appended to u-boot
>>> with DTB.
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
>>> ---
>>> ? arch/arm/Kconfig | 1 +
>>> ? 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index f9dab073ea..e558024652 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -492,6 +492,7 @@ choice
>>> ? config ARCH_AT91
>>> ????? bool "Atmel AT91"
>>> ????? select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
>>> +???? select SPL_SEPARATE_BSS if SPL
>>>
>>> ? config TARGET_EDB93XX
>>> ????? bool "Support edb93xx"
>>>
>> 
>> Reviewed-by: Stefan Roese <sr at denx.de>
>> 
>> Thanks,
>> Stefan
>> 
>
> Hi,
>
> With these changes, for all the A5 boards , SEPARATE_BSS was already 
> enabled, but for the rest of the platforms, it was not.
> As I see from the linker script, the BSS area is configured into DRAM area.
> Is there any chance that we have accesses to this section before the 
> DRAM is initialized on these platforms ? Could cause the boards to
> hang.

It is explicitly stated that it BSS is not available before setting up
the DRAM: it is forbidden to use global/static variables. So it should
be OK.

Gregory

>
> Eugen
Eugen Hristev June 5, 2020, 8:35 a.m. UTC | #4
On 05.06.2020 10:49, Gregory CLEMENT wrote:
> Hi Eugen,
> 
>> On 10.01.2020 08:32, Stefan Roese wrote:
>>
>>> On 09.01.20 17:30, Gregory CLEMENT wrote:
>>>> According to the linker script for both armv7 and arm926ejs based SoC,
>>>> BSS section was all the time separated for SPL but this symbol was
>>>> only enabled on some boards. However, it is necessary to have it
>>>> enabled for OF_SEPARATE configuration where DTB is appended to u-boot
>>>> with DTB.
>>>>
>>>> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
>>>> ---
>>>>    arch/arm/Kconfig | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index f9dab073ea..e558024652 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -492,6 +492,7 @@ choice
>>>>    config ARCH_AT91
>>>>        bool "Atmel AT91"
>>>>        select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
>>>> +     select SPL_SEPARATE_BSS if SPL
>>>>
>>>>    config TARGET_EDB93XX
>>>>        bool "Support edb93xx"
>>>>
>>>
>>> Reviewed-by: Stefan Roese <sr at denx.de>
>>>
>>> Thanks,
>>> Stefan
>>>
>>
>> Hi,
>>
>> With these changes, for all the A5 boards , SEPARATE_BSS was already
>> enabled, but for the rest of the platforms, it was not.
>> As I see from the linker script, the BSS area is configured into DRAM area.
>> Is there any chance that we have accesses to this section before the
>> DRAM is initialized on these platforms ? Could cause the boards to
>> hang.
> 
> It is explicitly stated that it BSS is not available before setting up
> the DRAM: it is forbidden to use global/static variables. So it should
> be OK.
> 

Thanks for the clarification. Will queue the patches for next merge window.

Eugen

> Gregory
> 
>>
>> Eugen
> 
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
>
Gregory CLEMENT June 5, 2020, 8:45 a.m. UTC | #5
Hi Eugen,

> On 05.06.2020 10:49, Gregory CLEMENT wrote:
>> Hi Eugen,
>> 
>>> On 10.01.2020 08:32, Stefan Roese wrote:
>>>
>>>> On 09.01.20 17:30, Gregory CLEMENT wrote:
>>>>> According to the linker script for both armv7 and arm926ejs based SoC,
>>>>> BSS section was all the time separated for SPL but this symbol was
>>>>> only enabled on some boards. However, it is necessary to have it
>>>>> enabled for OF_SEPARATE configuration where DTB is appended to u-boot
>>>>> with DTB.
>>>>>
>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
>>>>> ---
>>>>>    arch/arm/Kconfig | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>>> index f9dab073ea..e558024652 100644
>>>>> --- a/arch/arm/Kconfig
>>>>> +++ b/arch/arm/Kconfig
>>>>> @@ -492,6 +492,7 @@ choice
>>>>>    config ARCH_AT91
>>>>>        bool "Atmel AT91"
>>>>>        select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
>>>>> +     select SPL_SEPARATE_BSS if SPL
>>>>>
>>>>>    config TARGET_EDB93XX
>>>>>        bool "Support edb93xx"
>>>>>
>>>>
>>>> Reviewed-by: Stefan Roese <sr at denx.de>
>>>>
>>>> Thanks,
>>>> Stefan
>>>>
>>>
>>> Hi,
>>>
>>> With these changes, for all the A5 boards , SEPARATE_BSS was already
>>> enabled, but for the rest of the platforms, it was not.
>>> As I see from the linker script, the BSS area is configured into DRAM area.
>>> Is there any chance that we have accesses to this section before the
>>> DRAM is initialized on these platforms ? Could cause the boards to
>>> hang.
>> 
>> It is explicitly stated that it BSS is not available before setting up
>> the DRAM: it is forbidden to use global/static variables. So it should
>> be OK.
>> 
>
> Thanks for the clarification. Will queue the patches for next merge
> window.

Thanks, actually I've just sent a v2 because in the meantime an other
defconfig needed to be updated.

Gregory

>
> Eugen
>
>> Gregory
>> 
>>>
>>> Eugen
>> 
>> --
>> Gregory Clement, Bootlin
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
>> 
>
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f9dab073ea..e558024652 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -492,6 +492,7 @@  choice
 config ARCH_AT91
 	bool "Atmel AT91"
 	select SPL_BOARD_INIT if SPL && !TARGET_SMARTWEB
+	select SPL_SEPARATE_BSS if SPL
 
 config TARGET_EDB93XX
 	bool "Support edb93xx"