Re: [PATCH] of: build dbts with symbols when CONFIG_OF_OVERLAY is set

[Frank Rowand]

On 12/16/17 09:25, Andre Heider wrote:
> Hi Frank,
> 
> On 15/12/17 22:06, Frank Rowand wrote:
>> On 12/14/17 07:12, Andre Heider wrote:
>>> The overlay feature requires the base dtb to be built with symbols, so
>>> lets build the dtbs with symbols when overlay support was explicitly
>>> enabled.
>>>
>>> With CONFIG_OF_ALL_DTBS on ARCH=arm the 989 dtb files grow about ~38% on
>>> average.
>>>
>>> Totals in bytes with the 3 biggest ones:
>>>
>>> Before:
>>> ÂÂÂÂ90471ÂÂÂ arch/arm/boot/dts/am57xx-beagle-x15-revc.dtb
>>> ÂÂÂÂ90521ÂÂÂ arch/arm/boot/dts/am57xx-beagle-x15-revb1.dtb
>>> ÂÂÂÂ92639ÂÂÂ arch/arm/boot/dts/dra7-evm.dtb
>>> ÂÂÂÂ25731296ÂÂÂ total
>>>
>>> After:
>>> ÂÂÂÂ133203ÂÂÂ arch/arm/boot/dts/am57xx-beagle-x15-revc.dtb
>>> ÂÂÂÂ133237ÂÂÂ arch/arm/boot/dts/am57xx-beagle-x15-revb1.dtb
>>> ÂÂÂÂ134545ÂÂÂ arch/arm/boot/dts/dra7-evm.dtb
>>> ÂÂÂÂ35464440ÂÂÂ total
>>>
>>> Signed-off-by: Andre Heider <a.heider@xxxxxxxxx>
>>> ---
>>>
>>> Hi,
>>>
>>> while playing around with overlays I noticed that I needed to rebuilt
>>> my distro's device trees because they didn't come with symbols.
>>>
>>> Is that for a reason, maybe the not so minor increase in size?
>>
>> Yes, size is the issue.
>>
>>
>>>
>>> Thanks,
>>> Andre
>>>
>>> Â drivers/of/unittest-data/Makefile | 7 -------
>>> Â scripts/Makefile.libÂÂÂÂÂÂÂÂÂÂÂÂÂ | 5 +++++
>>> Â 2 files changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
>>> index 32389acfa616..b65061013512 100644
>>> --- a/drivers/of/unittest-data/Makefile
>>> +++ b/drivers/of/unittest-data/Makefile
>>> @@ -15,13 +15,6 @@ targets += overlay.dtb overlay.dtb.S
>>> Â targets += overlay_bad_phandle.dtb overlay_bad_phandle.dtb.S
>>> Â targets += overlay_bad_symbol.dtb overlay_bad_symbol.dtb.S
>>> Â targets += overlay_base.dtb overlay_base.dtb.S
>>> -
>>> -# enable creation of __symbols__ node
>>> -DTC_FLAGS_overlay := -@
>>> -DTC_FLAGS_overlay_bad_phandle := -@
>>> -DTC_FLAGS_overlay_bad_symbol := -@
>>> -DTC_FLAGS_overlay_base := -@
>>> -
>>> Â endif
>>> Â Â .PRECIOUS: \
>>
>> No. The unittests require these to be set unconditionally.
>>
>>
>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>> index 1ca4dcd2d500..c7ba4aa8a07a 100644
>>> --- a/scripts/Makefile.lib
>>> +++ b/scripts/Makefile.lib
>>> @@ -278,6 +278,11 @@ DTC_FLAGS += -Wnode_name_chars_strict \
>>> ÂÂÂÂÂ -Wproperty_name_chars_strict
>>> Â endif
>>> Â +ifeq ($(CONFIG_OF_OVERLAY),y)
>>> +# enable creation of __symbols__ node
>>> +DTC_FLAGS += -@
>>> +endif
>>> +
>>> Â DTC_FLAGS += $(DTC_FLAGS_$(basetarget))
>>> Â Â # Generate an assembly file to wrap the output of the device tree compiler
>>>
>>
>> Not needed. Instead set DTC_FLAGS in the make command. For example:
>>
>> ÂÂÂ DTC_FLAGS=-@ make qcom-apq8074-dragonboard.dtb
>>
>> There are a few architecture Makefiles that need to be fixed to not unconditionally
>> set DTC_FLAGS.
> 
> Rebuilding the dts files manually with symbols isn't the problem. The
> idea was to enable it with a switch which distros simply can flip.
> 
> Passing DTC_FLAGS to `make` would work too I guess, but on multi
> platform builds that's still the same issue if there're boards which
> can't handle that because of the size increase.
> 
> Would a solution which enables symbols per family work? E.g.
> CONFIG_MACH_SUN7I chooses to use -@ because it can handle it.
> 
> Thanks,
> Andre

Making it easy for distros to increase the size of .dtb files is
not the correct answer.  We are trying to decrease the footprint
of devicetree, not increase it.

-Frank