Patchwork [1/5] kernel.bbclass: move uImage handling to separate task

login
register
mail settings
Submitter lumag
Date Dec. 18, 2011, 7:47 p.m.
Message ID <1324237652-15618-1-git-send-email-dbaryshkov@gmail.com>
Download mbox | patch
Permalink /patch/17207/
State New
Headers show

Comments

lumag - Dec. 18, 2011, 7:47 p.m.
As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
separate task from do_deploy. This way the do_install task can also
benefit from generated uImage.

The only major feature of oe-core's version (not to recreate uImage
if it exists) is retained in this patch. On the contra, as this version
was merged from meta-oe/org.oe.dev, new function has another feature:
it permits overriding the u-boot entrypoint via u-boot symbol.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 meta/classes/kernel.bbclass |   43 +++++++++++++++++++++++++++----------------
 1 files changed, 27 insertions(+), 16 deletions(-)
Koen Kooi - Dec. 18, 2011, 8:13 p.m.
Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:

> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
> separate task from do_deploy. This way the do_install task can also
> benefit from generated uImage.
> 
> The only major feature of oe-core's version (not to recreate uImage
> if it exists) is retained in this patch.

I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.

> On the contra, as this version
> was merged from meta-oe/org.oe.dev, new function has another feature:
> it permits overriding the u-boot entrypoint via u-boot symbol.

No it doesn't, since it doesn't recreate uImage.
Bruce Ashfield - Dec. 18, 2011, 8:27 p.m.
On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>
> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>
>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>> separate task from do_deploy. This way the do_install task can also
>> benefit from generated uImage.
>>
>> The only major feature of oe-core's version (not to recreate uImage
>> if it exists) is retained in this patch.
>
> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.

In that case, shouldn't people doing u-boot development (or other
non-kernel developers),
be building a uImage via something that isn't in kernel.bbclass ?

Cheers,

Bruce

>
>> On the contra, as this version
>> was merged from meta-oe/org.oe.dev, new function has another feature:
>> it permits overriding the u-boot entrypoint via u-boot symbol.
>
> No it doesn't, since it doesn't recreate uImage.
> -----BEGIN PGP SIGNATURE-----
>
> iEYEARECAAYFAk7uSX8ACgkQMkyGM64RGpGCLwCgtXQaYv3fu3891FMVs9AK8hK7
> z8QAniVSDXosv3RBKp0GYUnqfCXck2bD
> =UYJG
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Koen Kooi - Dec. 18, 2011, 9:19 p.m.
Op 18 dec. 2011, om 21:27 heeft Bruce Ashfield het volgende geschreven:

> On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>> 
>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>> 
>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>> separate task from do_deploy. This way the do_install task can also
>>> benefit from generated uImage.
>>> 
>>> The only major feature of oe-core's version (not to recreate uImage
>>> if it exists) is retained in this patch.
>> 
>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
> 
> In that case, shouldn't people doing u-boot development (or other
> non-kernel developers),
> be building a uImage via something that isn't in kernel.bbclass ?

I use the kernel.bbclass in meta-oe, that does what I need.
Bruce Ashfield - Dec. 19, 2011, 1:39 a.m.
On Sun, Dec 18, 2011 at 4:19 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>
> Op 18 dec. 2011, om 21:27 heeft Bruce Ashfield het volgende geschreven:
>
>> On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>
>>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>>>
>>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>>> separate task from do_deploy. This way the do_install task can also
>>>> benefit from generated uImage.
>>>>
>>>> The only major feature of oe-core's version (not to recreate uImage
>>>> if it exists) is retained in this patch.
>>>
>>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
>>
>> In that case, shouldn't people doing u-boot development (or other
>> non-kernel developers),
>> be building a uImage via something that isn't in kernel.bbclass ?
>
> I use the kernel.bbclass in meta-oe, that does what I need.

ok. I was just trying to wrap my head around the use case, since I'm missing
something, and that would help me understand what is missing in the in kernel
uImage generation scripts. With that, we could see about getting
changes upstream
to address deficiencies.

Cheers,

Bruce

> -----BEGIN PGP SIGNATURE-----
>
> iEYEARECAAYFAk7uWPsACgkQMkyGM64RGpHe8ACdEdFi1Nh17keaiRxAAWQI3Rh6
> 2CYAoKcYow2t+pnGOlJs7teSNB4IQARn
> =3QuN
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Khem Raj - Dec. 19, 2011, 5:06 a.m.
On Sun, Dec 18, 2011 at 5:39 PM, Bruce Ashfield
<bruce.ashfield@gmail.com> wrote:
> On Sun, Dec 18, 2011 at 4:19 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>
>> Op 18 dec. 2011, om 21:27 heeft Bruce Ashfield het volgende geschreven:
>>
>>> On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>>
>>>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>>>>
>>>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>>>> separate task from do_deploy. This way the do_install task can also
>>>>> benefit from generated uImage.
>>>>>
>>>>> The only major feature of oe-core's version (not to recreate uImage
>>>>> if it exists) is retained in this patch.
>>>>
>>>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
>>>
>>> In that case, shouldn't people doing u-boot development (or other
>>> non-kernel developers),
>>> be building a uImage via something that isn't in kernel.bbclass ?
>>

i think we have UBOOT_ENTRYPOINT, UBOOT_ENTRYSYMBOL and
UBOOT_LOADADDRESS which are then used to generate the uImage
and sometimes defaults from kernel build system may not be usable as
it might be generating the image using some other values and we are not
able to control the image generation. Now is that fixable in kernel I guess
it could be but why not have flexibility of generating the image.

>> I use the kernel.bbclass in meta-oe, that does what I need.
>
Bruce Ashfield - Dec. 19, 2011, 5:23 a.m.
On Mon, Dec 19, 2011 at 12:06 AM, Khem Raj <raj.khem@gmail.com> wrote:
> On Sun, Dec 18, 2011 at 5:39 PM, Bruce Ashfield
> <bruce.ashfield@gmail.com> wrote:
>> On Sun, Dec 18, 2011 at 4:19 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>
>>> Op 18 dec. 2011, om 21:27 heeft Bruce Ashfield het volgende geschreven:
>>>
>>>> On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>>>
>>>>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>>>>>
>>>>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>>>>> separate task from do_deploy. This way the do_install task can also
>>>>>> benefit from generated uImage.
>>>>>>
>>>>>> The only major feature of oe-core's version (not to recreate uImage
>>>>>> if it exists) is retained in this patch.
>>>>>
>>>>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
>>>>
>>>> In that case, shouldn't people doing u-boot development (or other
>>>> non-kernel developers),
>>>> be building a uImage via something that isn't in kernel.bbclass ?
>>>
>
> i think we have UBOOT_ENTRYPOINT, UBOOT_ENTRYSYMBOL and
> UBOOT_LOADADDRESS which are then used to generate the uImage
> and sometimes defaults from kernel build system may not be usable as
> it might be generating the image using some other values and we are not
> able to control the image generation. Now is that fixable in kernel I guess
> it could be but why not have flexibility of generating the image.

I'm all for flexibility (see my comments earlier in the thread), have a variable
or some other construct that specifies how you'd like to construct the uImage
(kernel vs non-kernel, as a basic attempt to differentiate the two).

I'm just driving for the details to see if we can remedy the situation in the
medium to long term, since burying the details of how to construct any sort
of image in multiple places .. is not ideal (but I state the obvious).

Cheers,

Bruce

>
>>> I use the kernel.bbclass in meta-oe, that does what I need.
>>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Koen Kooi - Dec. 19, 2011, 9:03 a.m.
Op 19 dec. 2011, om 02:39 heeft Bruce Ashfield het volgende geschreven:

> On Sun, Dec 18, 2011 at 4:19 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>> 
>> Op 18 dec. 2011, om 21:27 heeft Bruce Ashfield het volgende geschreven:
>> 
>>> On Sun, Dec 18, 2011 at 3:13 PM, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>> 
>>>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>>>> 
>>>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>>>> separate task from do_deploy. This way the do_install task can also
>>>>> benefit from generated uImage.
>>>>> 
>>>>> The only major feature of oe-core's version (not to recreate uImage
>>>>> if it exists) is retained in this patch.
>>>> 
>>>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
>>> 
>>> In that case, shouldn't people doing u-boot development (or other
>>> non-kernel developers),
>>> be building a uImage via something that isn't in kernel.bbclass ?
>> 
>> I use the kernel.bbclass in meta-oe, that does what I need.
> 
> ok. I was just trying to wrap my head around the use case, since I'm missing
> something, and that would help me understand what is missing in the in kernel
> uImage generation scripts. With that, we could see about getting
> changes upstream
> to address deficiencies.


The biggest problem is that "upstream" is $vendor in a lot of cases, so fixing it once in OE is more productive than patching N vendor kernels. I agree that mainline should have the working version, but for my current project mainline doesn't even have support for this SoC and won't have till devicetree has cured worldpeace.

regards,

Koen
lumag - Dec. 19, 2011, 12:47 p.m.
On 12/19/2011 12:13 AM, Koen Kooi wrote:
>
> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>
>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>> separate task from do_deploy. This way the do_install task can also
>> benefit from generated uImage.
>>
>> The only major feature of oe-core's version (not to recreate uImage
>> if it exists) is retained in this patch.
>
> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.

Koen, that was addressed with KERNEL_RECREATE_UIMAGE variable.
Probably I should document it somewhere (in the commit message?
documentation.conf? smwh. else?). Would you agree with this patch + docs?

>
>> On the contra, as this version
>> was merged from meta-oe/org.oe.dev, new function has another feature:
>> it permits overriding the u-boot entrypoint via u-boot symbol.
>
> No it doesn't, since it doesn't recreate uImage.

It does.

BTW: I don't have much experience of uImage usage on ARM SoCs (I used
them only on Atmel boards, where things usually 'just worked' regarding 
Kernel load address & Ko). On PowerPC I also didn't have too much 
problems with upstream kernels (both from Linus'es tree and from 
Freescale's one).

I understand your concern, that for your tasks, you have to recreate 
uImage using your sane values. However for some people sane values are 
ones present in upstream tree. Moreover, if you care about history, it 
was specially changed in oe-core not to recreate uImage, as it caused 
problems for some of the users.
lumag - Dec. 19, 2011, 1:02 p.m.
On 12/19/2011 04:54 PM, Koen Kooi wrote:
>
> Op 19 dec. 2011, om 13:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>
>>
>>
>> On 12/19/2011 12:13 AM, Koen Kooi wrote:
>>>
>>> Op 18 dec. 2011, om 20:47 heeft Dmitry Eremin-Solenikov het volgende geschreven:
>>>
>>>> As per org.oe.dev and meta-oe's kernel.bbclass move uImage creation to
>>>> separate task from do_deploy. This way the do_install task can also
>>>> benefit from generated uImage.
>>>>
>>>> The only major feature of oe-core's version (not to recreate uImage
>>>> if it exists) is retained in this patch.
>>>
>>> I still don't agree with that behaviour. The in-kernel uImage code is just like the in-kernel defconfigs: useless for people who aren't kernel developers.
>>
>> Koen, that was addressed with KERNEL_RECREATE_UIMAGE variable.
>> Probably I should document it somewhere (in the commit message?
>> documentation.conf? smwh. else?). Would you agree with this patch + docs?
>
> No. If the machine.conf sets the UBOOT_* variables I should not need some magic unbreak-me variable to get kernel.bbclass to use them.

I see your point. I'll redo it so.

Patch

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 3f2f75a..8832a77 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -489,6 +489,33 @@  do_sizecheck() {
 
 addtask sizecheck before do_install after do_compile
 
+# Define to 1 to force kernel.bbclass to recreate uImage for you
+KERNEL_RECREATE_UIMAGE ?= "0"
+
+do_uboot_mkimage() {
+    test "x${KERNEL_IMAGETYPE}" = "xuImage" || return 0
+    test "x${KERNEL_RECREATE_UIMAGE}" = "x1" -o ! -e arch/${ARCH}/boot/uImage || return 0
+
+    ENTRYPOINT=${UBOOT_ENTRYPOINT}
+    if test -n "${UBOOT_ENTRYSYMBOL}"; then
+        ENTRYPOINT=`${HOST_PREFIX}nm ${S}/vmlinux | \
+               awk '$3=="${UBOOT_ENTRYSYMBOL}" {print $1}'`
+    fi
+    if test -e arch/${ARCH}/boot/compressed/vmlinux ; then
+        ${OBJCOPY} -O binary -R .note -R .comment -S arch/${ARCH}/boot/compressed/vmlinux linux.bin
+        uboot-mkimage -A ${UBOOT_ARCH} -O linux -T kernel -C none -a ${UBOOT_LOADADDRESS} -e $ENTRYPOINT -n "${DISTRO_NAME}/${PV}/${MACHINE}" -d linux.bin arch/${ARCH}/boot/uImage
+        rm -f linux.bin
+    else
+        ${OBJCOPY} -O binary -R .note -R .comment -S vmlinux linux.bin
+        rm -f linux.bin.gz
+        gzip -9 linux.bin
+        uboot-mkimage -A ${UBOOT_ARCH} -O linux -T kernel -C gzip -a ${UBOOT_LOADADDRESS} -e $ENTRYPOINT -n "${DISTRO_NAME}/${PV}/${MACHINE}" -d linux.bin.gz arch/${ARCH}/boot/uImage
+        rm -f linux.bin.gz
+    fi
+}
+
+addtask uboot_mkimage before do_install after do_compile
+
 KERNEL_IMAGE_BASE_NAME ?= "${KERNEL_IMAGETYPE}-${PV}-${PR}-${MACHINE}-${DATETIME}"
 # Don't include the DATETIME variable in the sstate package signatures
 KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"
@@ -500,22 +527,6 @@  kernel_do_deploy() {
 		tar -cvzf ${DEPLOYDIR}/modules-${KERNEL_VERSION}-${PR}-${MACHINE}.tgz -C ${D} lib
 	fi
 
-	if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then 
-		if test -e arch/${ARCH}/boot/uImage ; then
-			cp arch/${ARCH}/boot/uImage ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
-		elif test -e arch/${ARCH}/boot/compressed/vmlinux ; then
-			${OBJCOPY} -O binary -R .note -R .comment -S arch/${ARCH}/boot/compressed/vmlinux linux.bin
-			uboot-mkimage -A ${ARCH} -O linux -T kernel -C none -a ${UBOOT_ENTRYPOINT} -e ${UBOOT_ENTRYPOINT} -n "${DISTRO_NAME}/${PV}/${MACHINE}" -d linux.bin ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
-			rm -f linux.bin
-		else
-			${OBJCOPY} -O binary -R .note -R .comment -S vmlinux linux.bin
-			rm -f linux.bin.gz
-			gzip -9 linux.bin
-			uboot-mkimage -A ${ARCH} -O linux -T kernel -C gzip -a ${UBOOT_ENTRYPOINT} -e ${UBOOT_ENTRYPOINT} -n "${DISTRO_NAME}/${PV}/${MACHINE}" -d linux.bin.gz ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.bin
-			rm -f linux.bin.gz
-		fi
-	fi
-
 	cd ${DEPLOYDIR}
 	rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.bin
 	ln -sf ${KERNEL_IMAGE_BASE_NAME}.bin ${KERNEL_IMAGE_SYMLINK_NAME}.bin