[U-Boot] [PATCH 1/2] spl: Remove bogus GD_FLG_SPL_INIT check

[U-Boot] [PATCH 1/2] spl: Remove bogus GD_FLG_SPL_INIT check

[Marek Vasut]

Remove the check for GD_FLG_SPL_INIT in spl_relocate_stack_gd().
The check will always fail. This is because spl_relocate_stack_gd()
is called from ARM's crt0.S and it is called before board_init_r().
The board_init_r() calls spl_init(), which sets the GD_FLG_SPL_INIT
flag.

Note that reserving the malloc area in RAM is not a problem even
if the GD_FLG_SPL_INIT flag is not set.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Pavel Machek <pavel@denx.de>
Cc: Stefan Roese <sr@denx.de>
Cc: Stephen Warren <swarren@nvidia.com>
---
 common/spl/spl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index bdde716..8b10bdf 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -486,9 +486,6 @@ ulong spl_relocate_stack_gd(void)
 
 #ifdef CONFIG_SPL_SYS_MALLOC_SIMPLE
 	if (CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN) {
-		if (!(gd->flags & GD_FLG_SPL_INIT))
-			panic_str("spl_init must be called before heap reloc");
-
 		ptr -= CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN;
 		gd->malloc_base = ptr;
 		gd->malloc_limit = CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN;
-- 
2.7.0

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Marek Vasut]

Enable both features to reduce the SPL size by 6 kiB.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pavel Machek <pavel@denx.de>
Cc: Stefan Roese <sr@denx.de>
---
 configs/socfpga_arria5_defconfig       | 2 ++
 configs/socfpga_cyclone5_defconfig     | 2 ++
 configs/socfpga_de0_nano_soc_defconfig | 2 ++
 configs/socfpga_mcvevk_defconfig       | 2 ++
 configs/socfpga_sockit_defconfig       | 2 ++
 configs/socfpga_socrates_defconfig     | 2 ++
 configs/socfpga_sr1500_defconfig       | 2 ++
 configs/socfpga_vining_fpga_defconfig  | 2 ++
 include/configs/socfpga_common.h       | 3 ---
 9 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/configs/socfpga_arria5_defconfig b/configs/socfpga_arria5_defconfig
index a662e72..ec40ec7 100644
--- a/configs/socfpga_arria5_defconfig
+++ b/configs/socfpga_arria5_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_ARRIA5_SOCDK=y
diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig
index b2933f7..8e5c527 100644
--- a/configs/socfpga_cyclone5_defconfig
+++ b/configs/socfpga_cyclone5_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_CYCLONE5_SOCDK=y
diff --git a/configs/socfpga_de0_nano_soc_defconfig b/configs/socfpga_de0_nano_soc_defconfig
index f197b6d..034a215 100644
--- a/configs/socfpga_de0_nano_soc_defconfig
+++ b/configs/socfpga_de0_nano_soc_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_TERASIC_DE0_NANO=y
diff --git a/configs/socfpga_mcvevk_defconfig b/configs/socfpga_mcvevk_defconfig
index 6624f9e..133a6eb 100644
--- a/configs/socfpga_mcvevk_defconfig
+++ b/configs/socfpga_mcvevk_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_DENX_MCVEVK=y
diff --git a/configs/socfpga_sockit_defconfig b/configs/socfpga_sockit_defconfig
index c6414f8..8b1bcfc 100644
--- a/configs/socfpga_sockit_defconfig
+++ b/configs/socfpga_sockit_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_TERASIC_SOCKIT=y
diff --git a/configs/socfpga_socrates_defconfig b/configs/socfpga_socrates_defconfig
index b47a560..56284a1 100644
--- a/configs/socfpga_socrates_defconfig
+++ b/configs/socfpga_socrates_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_EBV_SOCRATES=y
diff --git a/configs/socfpga_sr1500_defconfig b/configs/socfpga_sr1500_defconfig
index aab4498..d66f7c6 100644
--- a/configs/socfpga_sr1500_defconfig
+++ b/configs/socfpga_sr1500_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_SR1500=y
diff --git a/configs/socfpga_vining_fpga_defconfig b/configs/socfpga_vining_fpga_defconfig
index bfce9b6..b5a0814 100644
--- a/configs/socfpga_vining_fpga_defconfig
+++ b/configs/socfpga_vining_fpga_defconfig
@@ -1,6 +1,8 @@
 CONFIG_ARM=y
 CONFIG_ARCH_SOCFPGA=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_DM=y
 CONFIG_DM_GPIO=y
 CONFIG_TARGET_SOCFPGA_SAMTEC_VINING_FPGA=y
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
index f657766..aee4012 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -324,9 +324,6 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
 #define CONFIG_SPL_RAM_DEVICE
 #define CONFIG_SPL_TEXT_BASE		CONFIG_SYS_INIT_RAM_ADDR
 #define CONFIG_SPL_MAX_SIZE		(64 * 1024)
-#ifdef CONFIG_SPL_BUILD
-#define CONFIG_SYS_MALLOC_SIMPLE
-#endif
 
 #define CONFIG_SPL_LIBCOMMON_SUPPORT
 #define CONFIG_SPL_LIBGENERIC_SUPPORT
-- 
2.7.0

[U-Boot] [PATCH 1/2] spl: Remove bogus GD_FLG_SPL_INIT check

[Stefan Roese]

On 30.05.2016 17:22, Marek Vasut wrote:
> Remove the check for GD_FLG_SPL_INIT in spl_relocate_stack_gd().
> The check will always fail. This is because spl_relocate_stack_gd()
> is called from ARM's crt0.S and it is called before board_init_r().
> The board_init_r() calls spl_init(), which sets the GD_FLG_SPL_INIT
> flag.
>
> Note that reserving the malloc area in RAM is not a problem even
> if the GD_FLG_SPL_INIT flag is not set.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Stephen Warren <swarren@nvidia.com>

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

Thanks,
Stefan

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Stefan Roese]

On 30.05.2016 17:22, Marek Vasut wrote:
> Enable both features to reduce the SPL size by 6 kiB.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Stefan Roese <sr@denx.de>

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

Thanks,
Stefan

[U-Boot] [PATCH 1/2] spl: Remove bogus GD_FLG_SPL_INIT check

[Chin Liang See]

On Mon, 2016-05-30 at 18:39 +0200, Stefan Roese wrote:
> On 30.05.2016 17:22, Marek Vasut wrote:
> > Remove the check for GD_FLG_SPL_INIT in spl_relocate_stack_gd().
> > The check will always fail. This is because spl_relocate_stack_gd()
> > is called from ARM's crt0.S and it is called before board_init_r().
> > The board_init_r() calls spl_init(), which sets the GD_FLG_SPL_INIT
> > flag.
> > 
> > Note that reserving the malloc area in RAM is not a problem even
> > if the GD_FLG_SPL_INIT flag is not set.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Chin Liang See <clsee@altera.com>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Pavel Machek <pavel@denx.de>
> > Cc: Stefan Roese <sr@denx.de>
> > Cc: Stephen Warren <swarren@nvidia.com>
> 
> Reviewed-by: Stefan Roese <sr@denx.de>
> 

Reviewed-by: Chin Liang See <clsee@altera.com>

Thanks
Chin Liang

> Thanks,
> Stefan

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Chin Liang See]

On Mon, 2016-05-30 at 18:39 +0200, Stefan Roese wrote:
> On 30.05.2016 17:22, Marek Vasut wrote:
> > Enable both features to reduce the SPL size by 6 kiB.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Chin Liang See <clsee@altera.com>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Pavel Machek <pavel@denx.de>
> > Cc: Stefan Roese <sr@denx.de>
> 
> Reviewed-by: Stefan Roese <sr@denx.de>
> 

Acked-by: Chin Liang See <clsee@altera.com>

Thanks
Chin Liang

> Thanks,
> Stefan

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Sylvain Lesne]

Hi,

On 05/30/2016 05:22 PM, Marek Vasut wrote:
> Enable both features to reduce the SPL size by 6 kiB.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Stefan Roese <sr@denx.de>

I tried to use the raw MMC boot (on the sockit, with current master
+ your patches), so I changed the following:

--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -349,9 +349,10 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img"
#define CONFIG_SPL_LIBDISK_SUPPORT
#else
-#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 3
-#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0xa00 /* offset 2560
sect (1M+256k) */
-#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION 3
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200 /* offset 512
sect (256k) */
+#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 1000 /* 512 KB */
+#define CONFIG_SPL_LIBDISK_SUPPORT
#endif
#endif

AFAIK, there were two mistakes:
1) FS_BOOT_PARTITION instead of RAW_MODE_U_BOOT
2) CONFIG_SPL_LIBDISK_SUPPORT was missing

(I edited the offset to be able to use u-boot-with-spl.sfp directly)

With these settings, I think we run into the size problem reported
previously in the ML.

So, enabling tiny printf could help, but we now get:

disk/built-in.o: In function `part_get_info_extended':
...u-boot/disk/part_dos.c:236: undefined reference to `sprintf'

So I think we can just put "info->name[0] = 0;" instead of
the snprintf() calls, when using tiny printf.

I can submit those changes as two patches if it makes sense.

Thanks,
Sylvain

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Marek Vasut]

On 05/31/2016 04:58 PM, Sylvain Lesne wrote:
> Hi,

Hi,

> On 05/30/2016 05:22 PM, Marek Vasut wrote:
>> Enable both features to reduce the SPL size by 6 kiB.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Chin Liang See <clsee@altera.com>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Pavel Machek <pavel@denx.de>
>> Cc: Stefan Roese <sr@denx.de>
> 
> I tried to use the raw MMC boot (on the sockit, with current master
> + your patches), so I changed the following:
> 
> --- a/include/configs/socfpga_common.h
> +++ b/include/configs/socfpga_common.h
> @@ -349,9 +349,10 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
> #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img"
> #define CONFIG_SPL_LIBDISK_SUPPORT
> #else
> -#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 3
> -#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0xa00 /* offset 2560
> sect (1M+256k) */
> -#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION 3
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200 /* offset 512
> sect (256k) */
> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 1000 /* 512 KB */

This should be 0x800 (was probably a typo in the original).
Otherwise this should not break anything, yeah.

> +#define CONFIG_SPL_LIBDISK_SUPPORT
> #endif
> #endif
> 
> AFAIK, there were two mistakes:
> 1) FS_BOOT_PARTITION instead of RAW_MODE_U_BOOT
> 2) CONFIG_SPL_LIBDISK_SUPPORT was missing

In fact, LIBDISK support was not used before you added the
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION into the board config.
The libdisk functions are needed for determining the start of the
partition. Looking through the SPL MMC, the code originally loaded the
u-boot image from a fixed offset on the card (2560 sectors), which is
indeed not optimal.

So yes, please send the above patch, it'd be a nice improvement.

> (I edited the offset to be able to use u-boot-with-spl.sfp directly)
> 
> With these settings, I think we run into the size problem reported
> previously in the ML.
> 
> So, enabling tiny printf could help, but we now get:
> 
> disk/built-in.o: In function `part_get_info_extended':
> ...u-boot/disk/part_dos.c:236: undefined reference to `sprintf'
>
> So I think we can just put "info->name[0] = 0;" instead of
> the snprintf() calls, when using tiny printf.

There is more of that stuff in disk/ , at least part_efi.c and
part_iso.c suffer from the exact same problem. I am not sure if
setting the name to '\0' wouldn't break anything, CCing Simon
as he was recently digging in those areas.

Also, take a look at this patch:
https://patchwork.ozlabs.org/patch/626760/

> I can submit those changes as two patches if it makes sense.

Please do.

> Thanks,
> Sylvain
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Sylvain Lesne]

On 05/31/2016 05:42 PM, Marek Vasut wrote:
> On 05/31/2016 04:58 PM, Sylvain Lesne wrote:
>> Hi,
> 
> Hi,
> 
>> On 05/30/2016 05:22 PM, Marek Vasut wrote:
>>> Enable both features to reduce the SPL size by 6 kiB.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Chin Liang See <clsee@altera.com>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Pavel Machek <pavel@denx.de>
>>> Cc: Stefan Roese <sr@denx.de>
>>
>> I tried to use the raw MMC boot (on the sockit, with current master
>> + your patches), so I changed the following:
>>
>> --- a/include/configs/socfpga_common.h
>> +++ b/include/configs/socfpga_common.h
>> @@ -349,9 +349,10 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
>> #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img"
>> #define CONFIG_SPL_LIBDISK_SUPPORT
>> #else
>> -#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 3
>> -#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0xa00 /* offset 2560
>> sect (1M+256k) */
>> -#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */
>> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION 3
>> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200 /* offset 512
>> sect (256k) */
>> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 1000 /* 512 KB */
> 
> This should be 0x800 (was probably a typo in the original).
> Otherwise this should not break anything, yeah.
> 

I might be missing something obvious, but it actually seems that this
define isn't used outside of the include/configs/ folder...

>> +#define CONFIG_SPL_LIBDISK_SUPPORT
>> #endif
>> #endif
>>
>> AFAIK, there were two mistakes:
>> 1) FS_BOOT_PARTITION instead of RAW_MODE_U_BOOT
>> 2) CONFIG_SPL_LIBDISK_SUPPORT was missing
> 
> In fact, LIBDISK support was not used before you added the
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION into the board config.
> The libdisk functions are needed for determining the start of the
> partition. Looking through the SPL MMC, the code originally loaded the
> u-boot image from a fixed offset on the card (2560 sectors), which is
> indeed not optimal.
> 

I agree, but I thought that the CONFIG_SYS_MMCSD_FS_BOOT_PARTITION
option was a bit confusing in this context! And I did not realise
the behaviour would be to load from a fixed offset.

As soon as the tiny printf "issue" is solved, I'll send this patch
(as I don't want to break the build).

> So yes, please send the above patch, it'd be a nice improvement.
> 
>> (I edited the offset to be able to use u-boot-with-spl.sfp directly)
>>
>> With these settings, I think we run into the size problem reported
>> previously in the ML.
>>
>> So, enabling tiny printf could help, but we now get:
>>
>> disk/built-in.o: In function `part_get_info_extended':
>> ...u-boot/disk/part_dos.c:236: undefined reference to `sprintf'
>>
>> So I think we can just put "info->name[0] = 0;" instead of
>> the snprintf() calls, when using tiny printf.
> 
> There is more of that stuff in disk/ , at least part_efi.c and
> part_iso.c suffer from the exact same problem. I am not sure if
> setting the name to '\0' wouldn't break anything, CCing Simon
> as he was recently digging in those areas.
> 
> Also, take a look at this patch:
> https://patchwork.ozlabs.org/patch/626760/

Ah yes, I naively only looked at the part_dos.c file. Actually
a tiny snprintf implementation by Simon was already merged!

http://git.denx.de/?p=u-boot.git;a=commit;h=5c411d88be8df5f6a8a1ea0c961f7c35ba82c064

If I implement a naive sprintf() using this as a basis, I can boot
from MMC without tweaking anything else, but it adds ~730 bytes
to tiny-printf.o.

Thanks!
Sylvain

> 
>> I can submit those changes as two patches if it makes sense.
> 
> Please do.
> 

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Marek Vasut]

On 05/31/2016 07:43 PM, Sylvain Lesne wrote:
> On 05/31/2016 05:42 PM, Marek Vasut wrote:
>> On 05/31/2016 04:58 PM, Sylvain Lesne wrote:
>>> Hi,
>>
>> Hi,
>>
>>> On 05/30/2016 05:22 PM, Marek Vasut wrote:
>>>> Enable both features to reduce the SPL size by 6 kiB.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Chin Liang See <clsee@altera.com>
>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>> Cc: Pavel Machek <pavel@denx.de>
>>>> Cc: Stefan Roese <sr@denx.de>
>>>
>>> I tried to use the raw MMC boot (on the sockit, with current master
>>> + your patches), so I changed the following:
>>>
>>> --- a/include/configs/socfpga_common.h
>>> +++ b/include/configs/socfpga_common.h
>>> @@ -349,9 +349,10 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
>>> #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img"
>>> #define CONFIG_SPL_LIBDISK_SUPPORT
>>> #else
>>> -#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 3
>>> -#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0xa00 /* offset 2560
>>> sect (1M+256k) */
>>> -#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */
>>> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION 3
>>> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x200 /* offset 512
>>> sect (256k) */
>>> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 1000 /* 512 KB */
>>
>> This should be 0x800 (was probably a typo in the original).
>> Otherwise this should not break anything, yeah.
>>
> 
> I might be missing something obvious, but it actually seems that this
> define isn't used outside of the include/configs/ folder...

Oh heh, good catch. If you want to nuke that one in include/configs/, be
my guest. The mmc spl is now parsing the uImage header to load the
right amount of data.

>>> +#define CONFIG_SPL_LIBDISK_SUPPORT
>>> #endif
>>> #endif
>>>
>>> AFAIK, there were two mistakes:
>>> 1) FS_BOOT_PARTITION instead of RAW_MODE_U_BOOT
>>> 2) CONFIG_SPL_LIBDISK_SUPPORT was missing
>>
>> In fact, LIBDISK support was not used before you added the
>> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION into the board config.
>> The libdisk functions are needed for determining the start of the
>> partition. Looking through the SPL MMC, the code originally loaded the
>> u-boot image from a fixed offset on the card (2560 sectors), which is
>> indeed not optimal.
>>
> 
> I agree, but I thought that the CONFIG_SYS_MMCSD_FS_BOOT_PARTITION
> option was a bit confusing in this context! And I did not realise
> the behaviour would be to load from a fixed offset.
> 
> As soon as the tiny printf "issue" is solved, I'll send this patch
> (as I don't want to break the build).

I think I will just pick these patches here, you can apply them and send
whatever you have.

>> So yes, please send the above patch, it'd be a nice improvement.
>>
>>> (I edited the offset to be able to use u-boot-with-spl.sfp directly)
>>>
>>> With these settings, I think we run into the size problem reported
>>> previously in the ML.
>>>
>>> So, enabling tiny printf could help, but we now get:
>>>
>>> disk/built-in.o: In function `part_get_info_extended':
>>> ...u-boot/disk/part_dos.c:236: undefined reference to `sprintf'
>>>
>>> So I think we can just put "info->name[0] = 0;" instead of
>>> the snprintf() calls, when using tiny printf.
>>
>> There is more of that stuff in disk/ , at least part_efi.c and
>> part_iso.c suffer from the exact same problem. I am not sure if
>> setting the name to '\0' wouldn't break anything, CCing Simon
>> as he was recently digging in those areas.
>>
>> Also, take a look at this patch:
>> https://patchwork.ozlabs.org/patch/626760/
> 
> Ah yes, I naively only looked at the part_dos.c file. Actually
> a tiny snprintf implementation by Simon was already merged!
> 
> http://git.denx.de/?p=u-boot.git;a=commit;h=5c411d88be8df5f6a8a1ea0c961f7c35ba82c064
> 
> If I implement a naive sprintf() using this as a basis, I can boot
> from MMC without tweaking anything else, but it adds ~730 bytes
> to tiny-printf.o.

I suppose the naive implementation would be calling snprintf() with size
= ~0 ? Go for it.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Pavel Machek]

On Mon 2016-05-30 17:22:34, Marek Vasut wrote:
> Enable both features to reduce the SPL size by 6 kiB.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>

Acked-by: Pavel Machek <pavel@denx.de>
Tested-by: Pavel Machek <pavel@denx.de>

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH 1/2] spl: Remove bogus GD_FLG_SPL_INIT check

[Pavel Machek]

On Mon 2016-05-30 17:22:33, Marek Vasut wrote:
> Remove the check for GD_FLG_SPL_INIT in spl_relocate_stack_gd().
> The check will always fail. This is because spl_relocate_stack_gd()
> is called from ARM's crt0.S and it is called before board_init_r().
> The board_init_r() calls spl_init(), which sets the GD_FLG_SPL_INIT
> flag.
> 
> Note that reserving the malloc area in RAM is not a problem even
> if the GD_FLG_SPL_INIT flag is not set.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Stephen Warren <swarren@nvidia.com>

Acked-by: Pavel Machek <pavel@denx.de>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH 2/2] arm: socfpga: Enable tiny printf and simple malloc in SPL

[Marek Vasut]

On 05/31/2016 02:34 AM, Chin Liang See wrote:
> On Mon, 2016-05-30 at 18:39 +0200, Stefan Roese wrote:
>> On 30.05.2016 17:22, Marek Vasut wrote:
>>> Enable both features to reduce the SPL size by 6 kiB.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Chin Liang See <clsee@altera.com>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Pavel Machek <pavel@denx.de>
>>> Cc: Stefan Roese <sr@denx.de>
>>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>>
> 
> Acked-by: Chin Liang See <clsee@altera.com>

Applied both to u-boot-socfpga/master

-- 
Best regards,
Marek Vasut