common: build ymodem and s_record only on need

Message ID 1518342820-2970-1-git-send-email-jun.nie@linaro.org
State New
Headers show
Series
  • common: build ymodem and s_record only on need
Related show

Commit Message

Jun Nie Feb. 11, 2018, 9:53 a.m.
Build ymodem and s_record only on need to shrink
spl image size.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 common/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andre Przywara Feb. 12, 2018, 12:54 a.m. | #1
Hi,

On 11/02/18 09:53, Jun Nie wrote:
> Build ymodem and s_record only on need to shrink
> spl image size.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  common/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/Makefile b/common/Makefile
> index c7bde23..8e1569f 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -28,6 +28,8 @@ obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o
>  obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o
>  obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
>  obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
> +obj-$(CONFIG_CMD_LOADS) += s_record.o

That sounds like a good idea, and indeed we need s_record.o only for
CMD_LOADS, but in my case this _increases_ the binary size by 6 bytes.

> +obj-$(CONFIG_CMD_LOADB) += xyzModem.o

I don't believe CMD_LOADB is enough. xyzModem.c exports xyzModem_*
functions, which are not only used in load.c, but also in
common/spl/spl_ymodem.c.
So you would need to check for config options enabling this, too.

But same as above, applying this change does not decrease the size of
the SPL binary.
Mine goes from (current master, pine64_plus_defconfig):
   text    data     bss     dec     hex filename
  30038     360     440   30838    7876 spl/u-boot-spl
to:
   text    data     bss     dec     hex filename
  30044     360     440   30844    787c spl/u-boot-spl

It seems like xyzModem_* functions and srec_decode() were removed by the
linker as dead code already.

So what does this save you? Can you share your numbers?

Cheers,
Andre.

>  
>  obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
>  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
> @@ -132,5 +134,3 @@ obj-$(CONFIG_CMD_DFU) += dfu.o
>  obj-y += command.o
>  obj-$(CONFIG_$(SPL_)LOG) += log.o
>  obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
> -obj-y += s_record.o
> -obj-y += xyzModem.o
>
Jun Nie Feb. 12, 2018, 4:37 a.m. | #2
2018-02-12 8:54 GMT+08:00 André Przywara <andre.przywara@arm.com>:
> Hi,
>
> On 11/02/18 09:53, Jun Nie wrote:
>> Build ymodem and s_record only on need to shrink
>> spl image size.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  common/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/Makefile b/common/Makefile
>> index c7bde23..8e1569f 100644
>> --- a/common/Makefile
>> +++ b/common/Makefile
>> @@ -28,6 +28,8 @@ obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o
>>  obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o
>>  obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
>>  obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
>> +obj-$(CONFIG_CMD_LOADS) += s_record.o
>
> That sounds like a good idea, and indeed we need s_record.o only for
> CMD_LOADS, but in my case this _increases_ the binary size by 6 bytes.
>
>> +obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>
> I don't believe CMD_LOADB is enough. xyzModem.c exports xyzModem_*
> functions, which are not only used in load.c, but also in
> common/spl/spl_ymodem.c.
> So you would need to check for config options enabling this, too.

Thanks for reminder! I will add more check for this.
>
> But same as above, applying this change does not decrease the size of
> the SPL binary.
> Mine goes from (current master, pine64_plus_defconfig):
>    text    data     bss     dec     hex filename
>   30038     360     440   30838    7876 spl/u-boot-spl
> to:
>    text    data     bss     dec     hex filename
>   30044     360     440   30844    787c spl/u-boot-spl
>
> It seems like xyzModem_* functions and srec_decode() were removed by the
> linker as dead code already.
>
> So what does this save you? Can you share your numbers?
>
As you said, s_record.o does not impact on SPL size. For xyzModem, SPL
can save 136 bytes on bananapi zero board when I enable FIT
configuration. Otherwise, it does not impact SPL size neither.

No optimization:
   text   data    bss    dec    hex filename
  24229    440    268  24937   6169 spl/u-boot-spl

Add optimization for xyzmodem.c
   text   data    bss    dec    hex filename
  24091    440    268  24799   60df spl/u-boot-spl



> Cheers,
> Andre.
>
>>
>>  obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
>>  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
>> @@ -132,5 +134,3 @@ obj-$(CONFIG_CMD_DFU) += dfu.o
>>  obj-y += command.o
>>  obj-$(CONFIG_$(SPL_)LOG) += log.o
>>  obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
>> -obj-y += s_record.o
>> -obj-y += xyzModem.o
>>
>

Patch

diff --git a/common/Makefile b/common/Makefile
index c7bde23..8e1569f 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -28,6 +28,8 @@  obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o
 obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
+obj-$(CONFIG_CMD_LOADS) += s_record.o
+obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 
 obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
 obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
@@ -132,5 +134,3 @@  obj-$(CONFIG_CMD_DFU) += dfu.o
 obj-y += command.o
 obj-$(CONFIG_$(SPL_)LOG) += log.o
 obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
-obj-y += s_record.o
-obj-y += xyzModem.o