diff mbox

[v4,2/3] Add LINUX_DIR to EXTRA_ENV for use in post-build scripts

Message ID 1489664602-32596-2-git-send-email-abhimanyu.vishwakarma@imgtec.com
State Changes Requested
Headers show

Commit Message

Abhimanyu V March 16, 2017, 11:43 a.m. UTC
From: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>

To build fitImage in post-build scripts, we need compressed
kernel binary which is intermediate target, hence doesnt get
copied to output/images folder.

Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
Reviewed-by: Rahul Bedarkar <Rahul.Bedarkar@imgtec.com>
---
 Changes v3->v4 (Suggested by Arnout)
   - Drop exporting LINUX_DIR instead add to EXTRA_ENV var

 package/Makefile.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Arnout Vandecappelle March 17, 2017, 10:39 p.m. UTC | #1
On 16-03-17 12:43, Abhimanyu V wrote:
> From: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
> 
> To build fitImage in post-build scripts, we need compressed
> kernel binary which is intermediate target, hence doesnt get
> copied to output/images folder.
> 
> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
> Reviewed-by: Rahul Bedarkar <Rahul.Bedarkar@imgtec.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  Changes v3->v4 (Suggested by Arnout)
>    - Drop exporting LINUX_DIR instead add to EXTRA_ENV var
> 
>  package/Makefile.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 4a3eb26..b1962ed 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -310,7 +310,8 @@ HOST_CONFIGURE_OPTS = \
>  EXTRA_ENV = \
>  	PATH=$(BR_PATH) \
>  	BR2_DL_DIR=$(BR2_DL_DIR) \
> -	BUILD_DIR=$(BUILD_DIR)
> +	BUILD_DIR=$(BUILD_DIR) \
> +	LINUX_DIR=$(LINUX_DIR)
>  
>  ################################################################################
>  # settings we need to pass to configure
>
Thomas Petazzoni March 18, 2017, 1:23 p.m. UTC | #2
Hello,

On Fri, 17 Mar 2017 23:39:46 +0100, Arnout Vandecappelle wrote:
> On 16-03-17 12:43, Abhimanyu V wrote:
> > From: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
> > 
> > To build fitImage in post-build scripts, we need compressed
> > kernel binary which is intermediate target, hence doesnt get
> > copied to output/images folder.
> > 
> > Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
> > Reviewed-by: Rahul Bedarkar <Rahul.Bedarkar@imgtec.com>  
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

I am not sure about this one. Why would we pass in the environment
specifically LINUX_DIR, and not the <pkg>_DIR of the other ~2000
packages we have in Buildroot ?

What about instead using BR2_LINUX_KERNEL_VMLINUX_BIN, to get
vmlinux.bin copied to output/images, and then compress it in the
post-build script ?

Or alternatively, if there is a vmlinux.bin.gz target, add support for
it in our linux package?

Best regards,

Thomas
Arnout Vandecappelle March 18, 2017, 1:59 p.m. UTC | #3
On 18-03-17 14:23, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 17 Mar 2017 23:39:46 +0100, Arnout Vandecappelle wrote:
>> On 16-03-17 12:43, Abhimanyu V wrote:
>>> From: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
>>>
>>> To build fitImage in post-build scripts, we need compressed
>>> kernel binary which is intermediate target, hence doesnt get
>>> copied to output/images folder.
>>>
>>> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
>>> Reviewed-by: Rahul Bedarkar <Rahul.Bedarkar@imgtec.com>  
>>
>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> I am not sure about this one. Why would we pass in the environment
> specifically LINUX_DIR, and not the <pkg>_DIR of the other ~2000
> packages we have in Buildroot ?

 Because LINUX_DIR is *much* more likely to be used in a post-build/image
script. E.g. if the initramfs wouldn't be supported directly by Buildroot, you'd
need it. Same for mxs-bootlets.

 I agree though that it's not great, but could find no better way. Although,
actually, it could use a recursive 'make printvars VARS=LINUX_DIR' (which will
be even better when [1] gets committed).

> What about instead using BR2_LINUX_KERNEL_VMLINUX_BIN, to get
> vmlinux.bin copied to output/images, and then compress it in the
> post-build script ?

 I considered that in one of my reviews, but vmlinux.bin doesn't contain the
load and start address. The post-build script would have to retrieve those in
some other way.

 However, Abhimanyu reported that there is upstream support for generating a
fitImage in the kernel build itself. That's of course preferable...


 Abhimanyu, could you rework using either 'make printvars' in the post-build
script (optionally incorporating [1] in your series so 'make printvars' actually
does the right thing), or using the in-kernel fitImage support?

 Regards,
 Arnout

[1] http://patchwork.ozlabs.org/patch/721102/

> 
> Or alternatively, if there is a vmlinux.bin.gz target, add support for
> it in our linux package?
> 
> Best regards,
> 
> Thomas
>
Thomas Petazzoni March 18, 2017, 2:31 p.m. UTC | #4
Hello,

On Sat, 18 Mar 2017 14:59:35 +0100, Arnout Vandecappelle wrote:
> > What about instead using BR2_LINUX_KERNEL_VMLINUX_BIN, to get
> > vmlinux.bin copied to output/images, and then compress it in the
> > post-build script ?  
> 
>  I considered that in one of my reviews, but vmlinux.bin doesn't contain the
> load and start address. The post-build script would have to retrieve those in
> some other way.

vmlinux.bin.gz also does not contain the load/start address. Only
uImage does, and they get them from uImage.

So one possibility would be to make the Linux package behave like the
U-Boot package: instead of having a choice...endchoice for the image
format to install, you simply have a list of options so that you can
select several of them.

>  However, Abhimanyu reported that there is upstream support for generating a
> fitImage in the kernel build itself. That's of course preferable...

Yes, it would be much better obviously.

Otherwise, hardcoding the load/start address in the .its.in file is
also good enough, this file is anyway board-specific.

Best regards,

Thomas
Abhimanyu V March 20, 2017, 7:08 a.m. UTC | #5
Hi Arnout, Thomas,


On Saturday 18 March 2017 07:29 PM, Arnout Vandecappelle wrote:
>
> On 18-03-17 14:23, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Fri, 17 Mar 2017 23:39:46 +0100, Arnout Vandecappelle wrote:
>>> On 16-03-17 12:43, Abhimanyu V wrote:
>>>> From: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
>>>>
>>>> To build fitImage in post-build scripts, we need compressed
>>>> kernel binary which is intermediate target, hence doesnt get
>>>> copied to output/images folder.
>>>>
>>>> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.Vishwakarma@imgtec.com>
>>>> Reviewed-by: Rahul Bedarkar <Rahul.Bedarkar@imgtec.com>
>>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>> I am not sure about this one. Why would we pass in the environment
>> specifically LINUX_DIR, and not the <pkg>_DIR of the other ~2000
>> packages we have in Buildroot ?
>   Because LINUX_DIR is *much* more likely to be used in a post-build/image
> script. E.g. if the initramfs wouldn't be supported directly by Buildroot, you'd
> need it. Same for mxs-bootlets.
>
>   I agree though that it's not great, but could find no better way. Although,
> actually, it could use a recursive 'make printvars VARS=LINUX_DIR' (which will
> be even better when [1] gets committed).
>
>> What about instead using BR2_LINUX_KERNEL_VMLINUX_BIN, to get
>> vmlinux.bin copied to output/images, and then compress it in the
>> post-build script ?
>   I considered that in one of my reviews, but vmlinux.bin doesn't contain the
> load and start address. The post-build script would have to retrieve those in
> some other way.
When working I also had 2 other way to get it working:

1. Using vmlinux.bin and use gzip in build script to generate 
vmlinux.bin.gz and use fixed ENTRY_ADDR which is
how openwrt version also does.

2. Extracting vmlinux.bin.gz from uImage directly.

Out of 3 i thought using what is already generated is best way to use it 
and it also reduces extra processing. If out of above 2 look good let me 
know i can propose the patch with
solution.
>
>   However, Abhimanyu reported that there is upstream support for generating a
> fitImage in the kernel build itself. That's of course preferable...
fitImage support would need some more time, i would prefer if we get it 
pushed before fitImage support is ready.

>
>
>   Abhimanyu, could you rework using either 'make printvars' in the post-build
> script (optionally incorporating [1] in your series so 'make printvars' actually
> does the right thing), or using the in-kernel fitImage support?
>
>   Regards,
>   Arnout
>
> [1] http://patchwork.ozlabs.org/patch/721102/
>
Thanks, i will work on patch.


>> Or alternatively, if there is a vmlinux.bin.gz target, add support for
>> it in our linux package?
>>
>> Best regards,
>>
>> Thomas
>>
Regards
Abhimanyu
Abhimanyu V March 20, 2017, 7:13 a.m. UTC | #6
On Saturday 18 March 2017 08:01 PM, Thomas Petazzoni wrote:
> Hello,
>
> On Sat, 18 Mar 2017 14:59:35 +0100, Arnout Vandecappelle wrote:
>>> What about instead using BR2_LINUX_KERNEL_VMLINUX_BIN, to get
>>> vmlinux.bin copied to output/images, and then compress it in the
>>> post-build script ?
>>   I considered that in one of my reviews, but vmlinux.bin doesn't contain the
>> load and start address. The post-build script would have to retrieve those in
>> some other way.
> vmlinux.bin.gz also does not contain the load/start address. Only
> uImage does, and they get them from uImage.
>
> So one possibility would be to make the Linux package behave like the
> U-Boot package: instead of having a choice...endchoice for the image
> format to install, you simply have a list of options so that you can
> select several of them.
>
>>   However, Abhimanyu reported that there is upstream support for generating a
>> fitImage in the kernel build itself. That's of course preferable...
> Yes, it would be much better obviously.
>
> Otherwise, hardcoding the load/start address in the .its.in file is
> also good enough, this file is anyway board-specific.
Damn, i didnt see this mail before replying to previous version, I will 
prepare the patch with this.
> Best regards,
>
> Thomas
Regards
Abhimanyu
diff mbox

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 4a3eb26..b1962ed 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -310,7 +310,8 @@  HOST_CONFIGURE_OPTS = \
 EXTRA_ENV = \
 	PATH=$(BR_PATH) \
 	BR2_DL_DIR=$(BR2_DL_DIR) \
-	BUILD_DIR=$(BUILD_DIR)
+	BUILD_DIR=$(BUILD_DIR) \
+	LINUX_DIR=$(LINUX_DIR)
 
 ################################################################################
 # settings we need to pass to configure