[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 at denx.de>
>> Cc: Chin Liang See <clsee at altera.com>
>> Cc: Dinh Nguyen <dinguyen at opensource.altera.com>
>> Cc: Pavel Machek <pavel at denx.de>
>> Cc: Stefan Roese <sr at 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