[U-Boot] [PATCH v4 0/3] Add a config for AM335x High Security EVM

[U-Boot] [PATCH v4 0/3] Add a config for AM335x High Security EVM

[Andrew F. Davis]

Hello all,

This series is [0] and [1] together due to a dependency. Both are
modified as suggested in the comments. 

Thanks,
Andrew

[0]: https://patchwork.ozlabs.org/patch/713945/
[1]: https://patchwork.ozlabs.org/patch/715820/

Changes from v3:
 - Reworked v2 as per Tom's comments

Andrew F. Davis (3):
  malloc_simple: Add debug statements to memalign_simple
  spl: Remove overwrite of relocated malloc limit
  defconfig: Add a config for AM335x High Security EVM

 MAINTAINERS                     |  1 +
 common/malloc_simple.c          |  6 +++-
 common/spl/spl.c                |  7 ++++-
 configs/am335x_hs_evm_defconfig | 62 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 configs/am335x_hs_evm_defconfig

-- 
2.11.0

[U-Boot] [PATCH v4 1/3] malloc_simple: Add debug statements to memalign_simple

[Andrew F. Davis]

Add debug statements to memalign_simple to match malloc_simple.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 common/malloc_simple.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c
index 0f6bcbcc71..611400265b 100644
--- a/common/malloc_simple.c
+++ b/common/malloc_simple.c
@@ -39,10 +39,14 @@ void *memalign_simple(size_t align, size_t bytes)
 
 	addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align);
 	new_ptr = addr + bytes - gd->malloc_base;
-	if (new_ptr > gd->malloc_limit)
+	if (new_ptr > gd->malloc_limit) {
+		debug("space exhausted\n");
 		return NULL;
+	}
+
 	ptr = map_sysmem(addr, bytes);
 	gd->malloc_ptr = ALIGN(new_ptr, sizeof(new_ptr));
+	debug("%lx\n", (ulong)ptr);
 
 	return ptr;
 }
-- 
2.11.0

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Andrew F. Davis]

spl_init on some boards is called after stack and heap relocation, on
some platforms spl_relocate_stack_gd is called to handle setting the
limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
SPL malloc is enabled during relocation. spl_init should then not
re-assign the old pre-relocation limit when this is defined.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 common/spl/spl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 462c3a2b97..a3808a988b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -183,7 +183,12 @@ int spl_init(void)
 	int ret;
 
 	debug("spl_init()\n");
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+/*
+ * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
+ * malloc_limit in spl_relocate_stack_gd
+ */
+#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
+	!defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
 #ifdef CONFIG_MALLOC_F_ADDR
 	gd->malloc_base = CONFIG_MALLOC_F_ADDR;
 #endif
-- 
2.11.0

[U-Boot] [PATCH v4 3/3] defconfig: Add a config for AM335x High Security EVM

[Andrew F. Davis]

Add a new defconfig file for the AM335x High Security EVM. This config
is specific for the case of memory device booting. Memory device booting
is handled separatly from peripheral booting on HS devices as the load
address changes.

This defconfig is the same as for the non-secure part, except for:
	CONFIG_TI_SECURE_DEVICE option set to 'y'
	CONFIG_ISW_ENTRY_ADDR updated for secure images.
	CONFIG_FIT_IMAGE_POST_PROCESS option set to 'y'
	CONFIG_SPL_FIT_IMAGE_POST_PROCESS option set to 'y'
	CONFIG_USE_TINY_PRINTF option set to 'y' to reduce SPL size
	CONFIG_SPL_SYS_MALLOC_SIMPLE set to 'y' to reduce SPL size

Signed-off-by: Andrew F. Davis <afd@ti.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 MAINTAINERS                     |  1 +
 configs/am335x_hs_evm_defconfig | 62 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 configs/am335x_hs_evm_defconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index 0e05c0ecff..eaa2c3bbb8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,7 @@ F:	arch/arm/mach-omap2/omap5/sec_entry_cpu1.S
 F:	arch/arm/mach-omap2/omap5/sec-fxns.c
 F:	arch/arm/mach-omap2/sec-common.c
 F:	arch/arm/mach-omap2/config_secure.mk
+F:	configs/am335x_hs_evm_defconfig
 F:	configs/am43xx_hs_evm_defconfig
 F:	configs/am57xx_hs_evm_defconfig
 F:	configs/dra7xx_hs_evm_defconfig
diff --git a/configs/am335x_hs_evm_defconfig b/configs/am335x_hs_evm_defconfig
new file mode 100644
index 0000000000..d6224bcdfb
--- /dev/null
+++ b/configs/am335x_hs_evm_defconfig
@@ -0,0 +1,62 @@
+CONFIG_ARM=y
+CONFIG_AM33XX=y
+CONFIG_TI_SECURE_DEVICE=y
+# CONFIG_SPL_EXT_SUPPORT is not set
+# CONFIG_SPL_NAND_SUPPORT is not set
+CONFIG_TARGET_AM335X_EVM=y
+CONFIG_ISW_ENTRY_ADDR=0x40300350
+CONFIG_SPL_STACK_R_ADDR=0x82000000
+# CONFIG_SPL_YMODEM_SUPPORT is not set
+CONFIG_DEFAULT_DEVICE_TREE="am335x-evm"
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_FIT=y
+CONFIG_SYS_EXTRA_OPTIONS="NAND"
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+CONFIG_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_VERSION_VARIABLE=y
+CONFIG_SPL=y
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_MTD_SUPPORT=y
+# CONFIG_CMD_IMLS is not set
+CONFIG_CMD_ASKENV=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_DFU=y
+CONFIG_CMD_GPIO=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_OF_CONTROL=y
+CONFIG_OF_LIST="am335x-evm"
+# CONFIG_BLK is not set
+CONFIG_DFU_MMC=y
+CONFIG_DFU_NAND=y
+CONFIG_DFU_RAM=y
+CONFIG_DM_I2C=y
+CONFIG_DM_MMC=y
+# CONFIG_DM_MMC_OPS is not set
+CONFIG_MMC_OMAP_HS=y
+CONFIG_SPI_FLASH=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_DM_ETH=y
+CONFIG_SYS_NS16550=y
+CONFIG_TIMER=y
+CONFIG_OMAP_TIMER=y
+CONFIG_USB=y
+CONFIG_USB_MUSB_HOST=y
+CONFIG_USB_MUSB_GADGET=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_DOWNLOAD=y
+CONFIG_G_DNL_MANUFACTURER="Texas Instruments"
+CONFIG_G_DNL_VENDOR_NUM=0x0451
+CONFIG_G_DNL_PRODUCT_NUM=0xd022
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_RSA=y
+CONFIG_SPL_OF_LIBFDT=y
-- 
2.11.0

[U-Boot] [PATCH v4 1/3] malloc_simple: Add debug statements to memalign_simple

[Tom Rini]

On Fri, Jan 27, 2017 at 10:39:18AM -0600, Andrew F. Davis wrote:

> Add debug statements to memalign_simple to match malloc_simple.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170127/9c0033d9/attachment.sig>

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Tom Rini]

On Fri, Jan 27, 2017 at 10:39:19AM -0600, Andrew F. Davis wrote:

> spl_init on some boards is called after stack and heap relocation, on
> some platforms spl_relocate_stack_gd is called to handle setting the
> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
> SPL malloc is enabled during relocation. spl_init should then not
> re-assign the old pre-relocation limit when this is defined.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170127/00cfb235/attachment.sig>

[U-Boot] [U-Boot, v4, 1/3] malloc_simple: Add debug statements to memalign_simple

[Tom Rini]

On Fri, Jan 27, 2017 at 10:39:18AM -0600, Andrew F. Davis wrote:

> Add debug statements to memalign_simple to match malloc_simple.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170128/808905cb/attachment.sig>

[U-Boot] [U-Boot, v4, 2/3] spl: Remove overwrite of relocated malloc limit

[Tom Rini]

On Fri, Jan 27, 2017 at 10:39:19AM -0600, Andrew F. Davis wrote:

> spl_init on some boards is called after stack and heap relocation, on
> some platforms spl_relocate_stack_gd is called to handle setting the
> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
> SPL malloc is enabled during relocation. spl_init should then not
> re-assign the old pre-relocation limit when this is defined.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170128/72697987/attachment.sig>

[U-Boot] [U-Boot, v4, 3/3] defconfig: Add a config for AM335x High Security EVM

[Tom Rini]

On Fri, Jan 27, 2017 at 10:39:20AM -0600, Andrew F. Davis wrote:

> Add a new defconfig file for the AM335x High Security EVM. This config
> is specific for the case of memory device booting. Memory device booting
> is handled separatly from peripheral booting on HS devices as the load
> address changes.
> 
> This defconfig is the same as for the non-secure part, except for:
> 	CONFIG_TI_SECURE_DEVICE option set to 'y'
> 	CONFIG_ISW_ENTRY_ADDR updated for secure images.
> 	CONFIG_FIT_IMAGE_POST_PROCESS option set to 'y'
> 	CONFIG_SPL_FIT_IMAGE_POST_PROCESS option set to 'y'
> 	CONFIG_USE_TINY_PRINTF option set to 'y' to reduce SPL size
> 	CONFIG_SPL_SYS_MALLOC_SIMPLE set to 'y' to reduce SPL size
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170128/6d7bbba5/attachment.sig>

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[André Przywara]

On 27/01/17 16:39, Andrew F. Davis wrote:

Hi,

> spl_init on some boards is called after stack and heap relocation, on
> some platforms spl_relocate_stack_gd is called to handle setting the
> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
> SPL malloc is enabled during relocation. spl_init should then not
> re-assign the old pre-relocation limit when this is defined.

I am sorry to say this, but this very patch breaks SPL boot on sunxi
boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).

U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
DRAM: 1024 MiB
Trying to boot from MMC1
MMC Device 0 not found
spl: could not find mmc device. error: -19
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.

At the moment the code there is beyond me, so I can't really say how
this breaks, but it clearly does: reverting this patch makes U-Boot
happy again.

Any ideas?

Cheers,
Andre.

> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  common/spl/spl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 462c3a2b97..a3808a988b 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -183,7 +183,12 @@ int spl_init(void)
>  	int ret;
>  
>  	debug("spl_init()\n");
> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
> +/*
> + * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
> + * malloc_limit in spl_relocate_stack_gd
> + */
> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
> +	!defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
>  #ifdef CONFIG_MALLOC_F_ADDR
>  	gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>  #endif
> 

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Tom Rini]

On Wed, Feb 01, 2017 at 11:48:11PM +0000, Andr? Przywara wrote:
> On 27/01/17 16:39, Andrew F. Davis wrote:
> 
> Hi,
> 
> > spl_init on some boards is called after stack and heap relocation, on
> > some platforms spl_relocate_stack_gd is called to handle setting the
> > limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
> > SPL malloc is enabled during relocation. spl_init should then not
> > re-assign the old pre-relocation limit when this is defined.
> 
> I am sorry to say this, but this very patch breaks SPL boot on sunxi
> boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
> SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).
> 
> U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
> DRAM: 1024 MiB
> Trying to boot from MMC1
> MMC Device 0 not found
> spl: could not find mmc device. error: -19
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
> 
> sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.
> 
> At the moment the code there is beyond me, so I can't really say how
> this breaks, but it clearly does: reverting this patch makes U-Boot
> happy again.
> 
> Any ideas?

Arg.  I'll try and poke at this but my A20-OLinuXino-Lime2 is (FEL at
least) booting so I hadn't seen this.  Can you confirm that pine64, or
these other boards, also fail when FEL booting?  I ask since if they
pass that way I need to pencil in some time to figure out another way to
test that HW, or at least make additional testing of that hardware
easier.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170201/3b566edd/attachment.sig>

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[André Przywara]

On 02/02/17 00:08, Tom Rini wrote:

Hi Tom,

thanks for the quick reply!

> On Wed, Feb 01, 2017 at 11:48:11PM +0000, Andr? Przywara wrote:
>> On 27/01/17 16:39, Andrew F. Davis wrote:
>>
>> Hi,
>>
>>> spl_init on some boards is called after stack and heap relocation, on
>>> some platforms spl_relocate_stack_gd is called to handle setting the
>>> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
>>> SPL malloc is enabled during relocation. spl_init should then not
>>> re-assign the old pre-relocation limit when this is defined.
>>
>> I am sorry to say this, but this very patch breaks SPL boot on sunxi
>> boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
>> SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).
>>
>> U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
>> DRAM: 1024 MiB
>> Trying to boot from MMC1
>> MMC Device 0 not found
>> spl: could not find mmc device. error: -19
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>>
>> sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.
>>
>> At the moment the code there is beyond me, so I can't really say how
>> this breaks, but it clearly does: reverting this patch makes U-Boot
>> happy again.
>>
>> Any ideas?
> 
> Arg.  I'll try and poke at this but my A20-OLinuXino-Lime2 is (FEL at
> least) booting so I hadn't seen this.  Can you confirm that pine64, or
> these other boards, also fail when FEL booting?  I ask since if they
> pass that way I need to pencil in some time to figure out another way to
> test that HW, or at least make additional testing of that hardware
> easier.  Thanks!

Yeah, sorry, but FEL boot seems to work, with or without this patch. I
guess it's due to the MMC driver malloc'ing something, which would
happen after DRAM availability. FEL boot just returns to the caller, so
no malloc.
And I agree it's nasty to do boot testing with SD cards, I use FEL
mostly as well.

Cheers,
Andre.

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Lokesh Vutla]

On 2/2/2017 5:18 AM, Andr? Przywara wrote:
> On 27/01/17 16:39, Andrew F. Davis wrote:
> 
> Hi,
> 
>> spl_init on some boards is called after stack and heap relocation, on
>> some platforms spl_relocate_stack_gd is called to handle setting the
>> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
>> SPL malloc is enabled during relocation. spl_init should then not
>> re-assign the old pre-relocation limit when this is defined.
> 
> I am sorry to say this, but this very patch breaks SPL boot on sunxi
> boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
> SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).
> 
> U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
> DRAM: 1024 MiB
> Trying to boot from MMC1
> MMC Device 0 not found
> spl: could not find mmc device. error: -19
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
> 
> sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.

In your case, before this patch malloc_limit is MALLOC_F_LEN(0x400),
after this patch malloc_limit is SPL_STACK_R_MALLOC_SIMPLE_LEN(0x100000)
. Stack relocated address is 0x4fe00000(is this in DDR?). SPL_STACK_R is
mainly targetted for relocating SPL stack to DDR.
- Is 1MB available at 0x4fe00000?
- Can you reduce the size or change SPL_STACK_R to DDR and verify?

Thanks and regards,
Lokesh

> 
> At the moment the code there is beyond me, so I can't really say how
> this breaks, but it clearly does: reverting this patch makes U-Boot
> happy again.
> 
> Any ideas?
> 
> Cheers,
> Andre.
> 
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> ---
>>  common/spl/spl.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 462c3a2b97..a3808a988b 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -183,7 +183,12 @@ int spl_init(void)
>>  	int ret;
>>  
>>  	debug("spl_init()\n");
>> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
>> +/*
>> + * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
>> + * malloc_limit in spl_relocate_stack_gd
>> + */
>> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
>> +	!defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
>>  #ifdef CONFIG_MALLOC_F_ADDR
>>  	gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>>  #endif
>>
> 

[U-Boot] [PATCH v4 2/3] spl: Remove overwrite of relocated malloc limit

[Andre Przywara]

Hi Lokesh,

thanks for having a look!

On 02/02/17 05:35, Lokesh Vutla wrote:
> 
> 
> On 2/2/2017 5:18 AM, Andr? Przywara wrote:
>> On 27/01/17 16:39, Andrew F. Davis wrote:
>>
>> Hi,
>>
>>> spl_init on some boards is called after stack and heap relocation, on
>>> some platforms spl_relocate_stack_gd is called to handle setting the
>>> limit to its value CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN when simple
>>> SPL malloc is enabled during relocation. spl_init should then not
>>> re-assign the old pre-relocation limit when this is defined.
>>
>> I am sorry to say this, but this very patch breaks SPL boot on sunxi
>> boards (tested on the Pine64, the Orangepi PC2 (with a similar ARMv8
>> SoC) and the OrangePi Zero (ARMv7 Allwinner H3 SoC).
>>
>> U-Boot SPL 2017.03-rc1-00022-gf77309d (Feb 01 2017 - 23:27:19)
>> DRAM: 1024 MiB
>> Trying to boot from MMC1
>> MMC Device 0 not found
>> spl: could not find mmc device. error: -19
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>>
>> sunxi boards both use the default CONFIG_SYS_MALLOC_F_LEN=0x400 and
>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000.
> 
> In your case, before this patch malloc_limit is MALLOC_F_LEN(0x400),
> after this patch malloc_limit is SPL_STACK_R_MALLOC_SIMPLE_LEN(0x100000)
> . Stack relocated address is 0x4fe00000(is this in DDR?). SPL_STACK_R is
> mainly targetted for relocating SPL stack to DDR.

Yes, this is at DDR (which starts at 1G).

> - Is 1MB available at 0x4fe00000?

Yes, the smallest board I have has 256MB, though I've seen this on
boards with one GB as well. But even in the worst case this is 2 MB
below end of DDR.
The BSS is at 0x4ff80000, but that shouldn't conflict then.

Is this address actually the _starting_ address for the malloc heap? So
malloc uses memory above this address, the stack below? Or does malloc
use some stack allocation for its purpose?

> - Can you reduce the size or change SPL_STACK_R to DDR and verify?

I will check tonight (no boards here at the moment).

Thanks for the explanation and the pointers, this gives me something to
debug on.

Cheers,
Andre


> 
> Thanks and regards,
> Lokesh
> 
>>
>> At the moment the code there is beyond me, so I can't really say how
>> this breaks, but it clearly does: reverting this patch makes U-Boot
>> happy again.
>>
>> Any ideas?
>>
>> Cheers,
>> Andre.
>>
>>>
>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>> ---
>>>  common/spl/spl.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index 462c3a2b97..a3808a988b 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -183,7 +183,12 @@ int spl_init(void)
>>>  	int ret;
>>>  
>>>  	debug("spl_init()\n");
>>> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
>>> +/*
>>> + * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
>>> + * malloc_limit in spl_relocate_stack_gd
>>> + */
>>> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
>>> +	!defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
>>>  #ifdef CONFIG_MALLOC_F_ADDR
>>>  	gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>>>  #endif
>>>
>>