[1/4] Rework support for the device tree

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 linux/Config.in |   46 +++++++++++++++++++++++++++++++++++++++-------
 linux/linux.mk  |   38 +++++++++++++++++++++++---------------
 2 files changed, 62 insertions(+), 22 deletions(-)

On 07/03/12 12:01, Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard<maxime.ripard@free-electrons.com>
> ---
>   linux/Config.in |   46 +++++++++++++++++++++++++++++++++++++++-------
>   linux/linux.mk  |   38 +++++++++++++++++++++++---------------
>   2 files changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/linux/Config.in b/linux/Config.in
> index 4562b1b..6f704b0 100644
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -124,14 +124,46 @@ config BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE
>   	help
>   	  Path to the kernel configuration file
>
> +config BR2_LINUX_KERNEL_DTS_SUPPORT
> +	bool "Device tree support"
> +	help
> +	  Compile a device tree source into a device tree blob.
> +	  Select the dts file to compile in the options below.
> +
> +if BR2_LINUX_KERNEL_DTS_SUPPORT

  Maybe it's better to remove this option, and instead just
detect if BR2_LINUX_KERNEL_DTS_FILE is defined.  Or would
that make things more confusing?


[snip]
> +ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)
> +define LINUX_BUILD_DTB
> +	$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(KERNEL_DTS_NAME).dtb
> +endef
> +endif
> +
>   # Compilation. We make sure the kernel gets rebuilt when the
>   # configuration has changed.
>   define LINUX_BUILD_CMDS
> +	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
> +		mkdir $(KERNEL_ARCH_PATH)/boot/dts

  Should be mkdir -p

> +		cp $(BR2_LINUX_KERNEL_CUSTOM_DTS_FILE) $(KERNEL_ARCH_PATH)/boot/dts/)
>   	$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_IMAGE_NAME)
>   	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
>   		$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
>   	fi
> +	$(LINUX_BUILD_DTB)
>   endef
>
>
> @@ -183,12 +184,19 @@ define LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET
>   endef
>   endif
>
> +ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)
> +define LINUX_INSTALL_DTB
> +	cp $(KERNEL_ARCH_PATH)/boot/$(KERNEL_DTS_NAME).dtb $(BINARIES_DIR)/
> +endef

  I would move this definition to the same place as LINUX_BUILD_DTB.

  Regards,
  Arnout

> +endif
> +
>   define LINUX_INSTALL_IMAGES_CMDS
>   	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
>   endef
>
>   define LINUX_INSTALL_TARGET_CMDS
>   	$(LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET)
> +	$(LINUX_INSTALL_DTB)
>   	# Install modules and remove symbolic links pointing to build
>   	# directories, not relevant on the target
>   	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\

On 07/15/12 14:34, Arnout Vandecappelle wrote:
>> +config BR2_LINUX_KERNEL_DTS_SUPPORT
>> +    bool "Device tree support"
>> +    help
>> +      Compile a device tree source into a device tree blob.
>> +      Select the dts file to compile in the options below.
>> +
>> +if BR2_LINUX_KERNEL_DTS_SUPPORT
>
>   Maybe it's better to remove this option, and instead just
> detect if BR2_LINUX_KERNEL_DTS_FILE is defined.  Or would
> that make things more confusing?

  Forget it, it would make the later patches unnecessarily complex.

  Regards,
  Arnout

Le Tue,  3 Jul 2012 12:01:13 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a écrit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  linux/Config.in |   46 +++++++++++++++++++++++++++++++++++++++-------
>  linux/linux.mk  |   38 +++++++++++++++++++++++---------------
>  2 files changed, 62 insertions(+), 22 deletions(-)
> 
> diff --git a/linux/Config.in b/linux/Config.in
> index 4562b1b..6f704b0 100644
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -124,14 +124,46 @@ config BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE
>  	help
>  	  Path to the kernel configuration file
>  
> +config BR2_LINUX_KERNEL_DTS_SUPPORT
> +	bool "Device tree support"
> +	help
> +	  Compile a device tree source into a device tree blob.
> +	  Select the dts file to compile in the options below.
> +
> +if BR2_LINUX_KERNEL_DTS_SUPPORT
> +
> +choice
> +	prompt "Device tree source"
> +	default BR2_LINUX_KERNEL_USE_DEFAULT_DTS
> +
> +config BR2_LINUX_KERNEL_USE_DEFAULT_DTS

I don't really like "USE_DEFAULT_DTS", it isn't really a "default"
device tree. What about BR2_LINUX_KERNEL_USE_INTREE_DTS or something
like that?

> +	bool "Use a device tree present in the kernel.

Missing " at the end of the line, and the final "." shouldn't be there.

> +	help
> +	   Use a device tree source distributed with
> +	   the kernel sources. The dts files are to be
> +	   found in the arch/<arch>/boot/dts folder.

"are to be found" -> "are located" ?

> +config BR2_LINUX_KERNEL_USE_CUSTOM_DTS
> +	bool "Use a custom device tree file"
> +	help
> +	  Use a custom device tree file.

"i.e, a device tree file that does not belong to the kernel source
tree."

> +
> +endchoice
> +
>  config BR2_LINUX_KERNEL_DTS_FILE
> -    string "Device Tree dts file location"
> -    depends on BR2_microblaze
> -    help
> -      Path from where the dts file has to be copied
> -      The final "custom target" name depends on the
> -      dts file name:
> -          <name>.dts --> simpleImage.<name>
> +	string "DTS name"

	string "Device Tree Source file name" ?

> +	depends on BR2_LINUX_KERNEL_USE_DEFAULT_DTS
> +	help
> +	 Name of the default device tree, without the
> +	 trailing .dts
> +
> +config BR2_LINUX_KERNEL_CUSTOM_DTS_FILE
> +	string "Device tree file path"

	string "Device Tree Source file path" ?

or at least be consistent in the naming, either DTS everywhere, or
Device Tree Source everywhere.

> +	depends on BR2_LINUX_KERNEL_USE_CUSTOM_DTS
> +	help
> +	  Path to the kernel configuration file
> +
> +endif
>  #
>  # Binary format
>  #
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 34f8623..0540aeb 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -46,6 +46,12 @@ LINUX_MAKE_FLAGS = \
>  # going to be installed in the target filesystem.
>  LINUX_VERSION_PROBED = $(shell $(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s kernelrelease)
>  
> +ifeq ($(BR2_LINUX_KERNEL_USE_DEFAULT_DTS),y)
> +KERNEL_DTS_NAME = $(BR2_LINUX_KERNEL_DTS_FILE)
> +else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y)
> +KERNEL_DTS_NAME = $(basename $(notdir $(BR2_LINUX_KERNEL_CUSTOM_DTS_FILE)))
> +endif
> +
>  ifeq ($(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM),y)
>  LINUX_IMAGE_NAME=$(call qstrip,$(BR2_LINUX_KERNEL_IMAGE_TARGET_NAME))
>  else
> @@ -117,19 +123,6 @@ endef
>  
>  LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES
>  
> -ifeq ($(KERNEL_ARCH),microblaze)
> -# on microblaze, we always want mkimage
> -LINUX_DEPENDENCIES+=host-uboot-tools
> -
> -define LINUX_COPY_DTS
> -    if test -f "$(BR2_LINUX_KERNEL_DTS_FILE)" ; then \
> -        cp $(BR2_LINUX_KERNEL_DTS_FILE) $(@D)/arch/microblaze/boot/dts ; \
> -    else \
> -        echo "Cannot copy dts file!" ; \
> -    fi
> -endef
> -endif
> -
>  ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
>  KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
>  else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
> @@ -143,8 +136,6 @@ define LINUX_CONFIGURE_CMDS
>  	$(if $(BR2_ARM_EABI),
>  		$(call KCONFIG_ENABLE_OPT,CONFIG_AEABI,$(@D)/.config),
>  		$(call KCONFIG_DISABLE_OPT,CONFIG_AEABI,$(@D)/.config))
> -    $(if $(BR2_microblaze),
> -        $(call LINUX_COPY_DTS))
>  	# As the kernel gets compiled before root filesystems are
>  	# built, we create a fake cpio file. It'll be
>  	# replaced later by the real cpio archive, and the kernel will be
> @@ -167,13 +158,23 @@ define LINUX_CONFIGURE_CMDS
>  	yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig
>  endef
>  
> +ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)
> +define LINUX_BUILD_DTB
> +	$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(KERNEL_DTS_NAME).dtb
> +endef
> +endif
> +
>  # Compilation. We make sure the kernel gets rebuilt when the
>  # configuration has changed.
>  define LINUX_BUILD_CMDS
> +	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
> +		mkdir $(KERNEL_ARCH_PATH)/boot/dts

Do you really need to create the directory here? I would suspect that
all arches supporting the Device Tree already have their boot/dts/
directory, no?

> +		cp $(BR2_LINUX_KERNEL_CUSTOM_DTS_FILE) $(KERNEL_ARCH_PATH)/boot/dts/)
>  	$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_IMAGE_NAME)
>  	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
>  		$(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
>  	fi
> +	$(LINUX_BUILD_DTB)
>  endef
>  
>  
> @@ -183,12 +184,19 @@ define LINUX_INSTALL_KERNEL_IMAGE_TO_TARGET
>  endef
>  endif
>  
> +ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)
> +define LINUX_INSTALL_DTB
> +	cp $(KERNEL_ARCH_PATH)/boot/$(KERNEL_DTS_NAME).dtb $(BINARIES_DIR)/

Hum, here when I do a make foobar.dtb, the foobar.dtb file ends up in
the main kernel source tree, not in arch/arm/boot/.

Thomas

Le Wed, 18 Jul 2012 18:36:17 +0200,
Arnout Vandecappelle <arnout.vandecappelle@essensium.com> a écrit :

> > Do you really need to create the directory here? I would suspect that
> > all arches supporting the Device Tree already have their boot/dts/
> > directory, no?
> 
>   Nope.  For instance in 3.0 there was preliminary DTS support for ARM,
> but no arch/arm/boot/dts directory.  That said, I'm not entirely sure if the
> foo.dtb target would even work, because I don't see a rule for producing
> it for arm in 3.0...

Realistically, the ARM DT support was so limited (in number of
platforms supported) in 3.0 that I don't think it's worth the effort. I
think we can safely assume that for all practical cases,
arch/<ARCH>/boot/dts exists.

> > Hum, here when I do a make foobar.dtb, the foobar.dtb file ends up in
> > the main kernel source tree, not in arch/arm/boot/.
>   That's strange... For which architecture and kernel version?

That's just complete crap from my side. I got confused by the big
command line I typically use to build the kernel and concatenate the
DTB. The DTB indeed ends up in arch/arm/boot and Maxime's code is
correct in that respect.

Thanks!

Thomas