[Buildroot] OF FDT compilation support

Grant Likely grant.likely at secretlab.ca
Thu Mar 31 21:40:47 UTC 2011


On Thu, Mar 31, 2011 at 3:31 PM, Bryan Hundven <bryanhundven at gmail.com> wrote:
> On Thu, Mar 31, 2011 at 1:51 PM, Grant Likely <grant.likely at secretlab.ca> wrote:
>> On Tue, Nov 23, 2010 at 10:45:05PM +0100, Thomas Petazzoni wrote:
>>> Hello,
>>>
>>> On Wed, 17 Nov 2010 14:49:06 +0300
>>> Stanislav Bogatyrev <realloc at gmail.com> wrote:
>>>
>>> >  #
>>> > +# Device tree
>>> > +#
>>> > +
>>> > +config BR2_LINUX_BUILD_DT
>>>
>>> I think I'd prefer BR2_LINUX_DEVICE_TREE (ditto for all other options)
>>>
>>> > +  bool "Compile OpenFirmware Flattened Device Tree"
>>>
>>> Indentation before bool, depends on, help, etc. is one tab. Could you
>>> fix everywhere ?
>>>
>>> > +  depends on BR2_powerpc
>>> > +  help
>>> > +    Compile OpenFirmware Flattened Device Tree
>>>
>>> And for help text it's one tab and two spaces. Could you fix
>>> everywhere ?
>>>
>>> A slightly better help text would be useful as well.
>>>
>>> > +config BR2_LINUX_BUILD_DT_FORCE
>>> > +  bool "Force - try to produce output even if the input tree has errors"
>>> > +  depends on BR2_LINUX_BUILD_DT
>>>
>>> We don't really have many "force" options anywhere else in Buildroot.
>>> Is this really needed ?
>>>
>>> Replace all BR2_LINUX_BUILD_DT by an enclosing if BR2_LINUX_DEVICE_TREE
>>>
>>> > +config BR2_LINUX_BUILD_DT_LIST
>>> > +  string "List of .dts files to compile"
>>> > +  depends on BR2_LINUX_BUILD_DT
>>> > +
>>> > +choice
>>> > +   prompt "DTC output format"
>>> > +  depends on BR2_LINUX_BUILD_DT
>>> > +  default BR2_LINUX_BUILD_DT_DTB
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_DTB
>>> > +  bool "dtb"
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_DTS
>>> > +  bool "dts"
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_ASM
>>> > +  bool "asm"
>>> > +endchoice
>>>
>>> What are all those formats for ?
>>
>> Pretty much *nobody* uses the ASM or DTS output options for building
>> deployable images.  DTB is about the only target necessary.
>>
>>>
>>> > +config BR2_LINUX_BUILD_DT_BOOT_CPU
>>> > +  string "CPU number to boot"
>>> > +  default 0
>>> > +  depends on BR2_LINUX_BUILD_DT
>>
>> I don't think anyone specifies this manually either.
>>
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_EXTRA_SPACE
>>> > +  string "Make the blob at least <bytes> long (extra space)"
>>> > +  default 0
>>> > +  depends on BR2_LINUX_BUILD_DT && ( BR2_LINUX_BUILD_DT_DTB || BR2_LINUX_BUILD_DT_ASM )
>>
>> Modern u-boot can handle unpadded .dtbs just fine.  Should not need
>> this options.
>>
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_RESERVED_SPACE
>>> > +  string "Make space for <number> reserve map entries"
>>> > +  default 0
>>> > +  depends on BR2_LINUX_BUILD_DT && ( BR2_LINUX_BUILD_DT_DTB || BR2_LINUX_BUILD_DT_ASM )
>>
>> Ditto.
>>
>>> > +
>>> > +config BR2_LINUX_BUILD_DT_PADDING_SPACE
>>> > +  string "Add padding to the blob of <bytes> long (extra space)"
>>> > +  default 0
>>> > +  depends on BR2_LINUX_BUILD_DT && ( BR2_LINUX_BUILD_DT_DTB || BR2_LINUX_BUILD_DT_ASM )
>>>
>>> I am not very familiar with the device tree, are those options really
>>> useful in general ?
>>
>> Ditto.
>>
>>>
>>> > +# prepare command-line arguments for dtc
>>> > +LINUX26_DTS_PATH=$(LINUX26_DIR)/arch/$(KERNEL_ARCH)/boot/dts
>>> > +LINUX26_DTC=$(LINUX26_DIR)/arch/$(KERNEL_ARCH)/boot/dtc
>>> > +
>>> > +ifeq ($(BR2_LINUX_BUILD_DT_FORCE),y)
>>> > +LINUX26_DTC_ARG+= -f
>>> > +endif
>>> > +
>>> > +ifeq ($(BR2_LINUX_BUILD_DT_DTB),y)
>>> > +LINUX26_DTC_OUTF+=dtb
>>> > +else ifeq ($(BR2_LINUX_BUILD_DT_ASM),y)
>>> > +LINUX26_DTC_OUTF+=asm
>>> > +else ifeq ($(BR2_LINUX_BUILD_DT_DTS),y)
>>> > +LINUX26_DTC_OUTF+=dts
>>> > +endif
>>>
>>> Those lines should probably be LINUX26_DTC_OUTF= and not += since they
>>> are mutually exclusive.
>>>
>>> > +LINUX26_DTC_ARG+= -b $(call qstrip,$(BR2_LINUX_BUILD_DT_BOOT_CPU)) \
>>> > +-R $(call qstrip,$(BR2_LINUX_BUILD_DT_RESERVED_SPACE)) \
>>> > +-S $(call qstrip,$(BR2_LINUX_BUILD_DT_EXTRA_SPACE)) \
>>> > +-p $(call qstrip,$(BR2_LINUX_BUILD_DT_PADDING_SPACE)) \
>>> > +-o $(BINARIES_DIR)/$(file).$(LINUX26_DTC_OUTF) \
>>> > +-O $(LINUX26_DTC_OUTF)
>>>
>>> One tab here before each continuing line.
>>>
>>> > +# Flattened Device Tree compilation
>>> > +$(LINUX26_DIR)/.stamp_dt_compiled: $(LINUX26_DIR)/.stamp_compiled
>>> > +ifeq ($(BR2_LINUX_BUILD_DT),y)
>>> > +   @$(call MESSAGE,"Compiling Flattened Device Tree")
>>> > +   $(foreach file, $(call qstrip,$(BR2_LINUX_BUILD_DT_LIST)), \
>>> > +   $(LINUX26_DTC) $(LINUX26_DTC_ARG) $(LINUX26_DTS_PATH)/$(file).dts;)
>>> > +endif
>>> > +   $(Q)touch $@
>>> > +
>>> >  # Installation
>>> >  $(LINUX26_DIR)/.stamp_installed: $(LINUX26_DIR)/.stamp_compiled
>>> >     @$(call MESSAGE,"Installing kernel")
>>> > @@ -143,7 +176,7 @@ $(LINUX26_DIR)/.stamp_installed: $(LINUX26_DIR)/.stamp_compiled
>>> >     fi
>>> >     $(Q)touch $@
>>> >
>>> > -linux26: host-module-init-tools $(LINUX26_DEPENDENCIES) $(LINUX26_DIR)/.stamp_installed
>>> > +linux26: host-module-init-tools $(LINUX26_DEPENDENCIES) $(LINUX26_DIR)/.stamp_installed $(LINUX26_DIR)/.stamp_dt_compiled
>>>
>>> I think I'd prefer something such as:
>>>
>>> +$(LINUX26_DIR)/.stamp_dt_compiled: $(LINUX26_DIR)/.stamp_compiled
>>> +     @$(call MESSAGE,"Compiling Flattened Device Tree")
>>> +     $(foreach file, $(call qstrip,$(BR2_LINUX_BUILD_DT_LIST)), \
>>> +     $(LINUX26_DTC) $(LINUX26_DTC_ARG) $(LINUX26_DTS_PATH)/$(file).dts;)
>>> +     $(Q)touch $@
>>> +
>>> +ifeq ($(BR2_LINUX_BUILD_DT),y)
>>> +LINUX26_DEVICE_TREE_DEP=$(LINUX26_DIR)/.stamp_dt_compiled
>>> +endif
>>> [...]
>>> -linux26: host-module-init-tools $(LINUX26_DEPENDENCIES) $(LINUX26_DIR)/.stamp_installed
>>> +linux26: host-module-init-tools $(LINUX26_DEPENDENCIES) $(LINUX26_DIR)/.stamp_installed $(LINUX26_DEVICE_TREE_DEP)
>>>
>>> Thanks!
>>>
>>> Thomas
>>> --
>>> Thomas Petazzoni, Free Electrons
>>> Kernel, drivers, real-time and embedded Linux
>>> development, consulting, training and support.
>>> http://free-electrons.com
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot at busybox.net
>>> http://lists.busybox.net/mailman/listinfo/buildroot
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>>
>
> Just curious, would it also make sense to optionally provide the .dtu
> (uboot mkimage wrapped .dtb) as well?
>
> Not sure if .dtu's are even needed/used anymore...

I certainly don't.  I'd prefer not to need them.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.



More information about the buildroot mailing list