From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sebastian Frias <sf84@laposte.net>
Date: Mon, 25 Apr 2016 14:17:50 +0200
Subject: [Buildroot] [PATCH] linux: use $(LINUX_INSTALL_IMAGE_CMDS)
 instead of plain 'cp'
In-Reply-To: <20160425140615.11e86412@free-electrons.com>
References: <571E0683.4080306@laposte.net>
 <20160425140615.11e86412@free-electrons.com>
Message-ID: <571E0AEE.3010002@laposte.net>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

Hi Thomas,

On 04/25/2016 02:06 PM, Thomas Petazzoni wrote:
> Hello,
> 
> Thanks for reporting the problem, and thanks for your patch!

Sure! That's what Open Source is for! :-)

> 
>> diff --git a/linux/linux.mk b/linux/linux.mk
>> index 317587f..1f93074 100644
>> --- a/linux/linux.mk
>> +++ b/linux/linux.mk
>> @@ -330,7 +330,7 @@ ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
>>  # appended DTBs.
>>  define LINUX_INSTALL_IMAGE
>>  	mkdir -p $(1)
>> -	cp $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME).* $(1)
>> +	cp -v $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME).* $(1)
> 
> I think you did this change for debugging (which is OK), but it should
> be kept in your patch.

Ok, I will remove it.

> 
>>  endef
>>  else
>>  # Otherwise, just install the unique image generated by the kernel
>> @@ -447,8 +447,8 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
>> $(LINUX_DIR)/.stamp_target_installed $(LI
>>  	# Build the kernel.
>>  	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D)
>> $(LINUX_TARGET_NAME)

This line is also present on LINUX_BUILD_CMDS, any reason not to reuse it?
LINUX_BUILD_CMDS does a couple of more things than what is done here,
like like running LINUX_BUILD_DTB or 'make modules', and maybe that is
not wanted?

>>  	$(LINUX_APPEND_DTB)
>> -	# Copy the kernel image to its final destination
>> -	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
>> +	# Copy the kernel image(s) to its(their) final destination
>> +	$(LINUX_INSTALL_IMAGES_CMDS)
> 
> I think I would prefer to have:
> 
> 	$(call LINUX_INSTALL_IMAGE,$(BINARIES_DIR))
> 

Ok, just wondering, why? I mean, IIUC, LINUX_INSTALL_IMAGES_CMDS also
handles the DTB copy when APPENDED_DTB is not set, right?

Best regards,

Sebastian