<div dir="ltr">The refactoring is already in the 3 previous patches from these 4. This is just the final fix, what do you want to separate from this?</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 4, 2018 at 10:54 AM Paulo Neves <<a href="mailto:ptsneves@gmail.com">ptsneves@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Great work!<br>
<br>
The only issue I have with your changes is that this is basically a<br>
rewrite and you mix fixes with refactoring. Would it be possible to<br>
split the patch into smaller patches? This would make it much easier<br>
analyze.<br>
<br>
<br>
<br>
On Wed, Jul 4, 2018 at 9:59 AM, Martin Jansa <<a href="mailto:martin.jansa@gmail.com" target="_blank">martin.jansa@gmail.com</a>> wrote:<br>
> * add 2 new variables:<br>
> KERNEL_DTB_BASE_NAME<br>
> KERNEL_DTB_SYMLINK_NAME<br>
> instead of reusing KERNEL_IMAGE_SYMLINK_NAME and than expecting that<br>
> default value ${MACHINE} was being used in e.g.:<br>
> DTB_SYMLINK_NAME=`echo ${symlink_name} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`<br>
><br>
> * install normal DTB files only once even if there is multiple entries<br>
> in KERNEL_IMAGETYPE_FOR_MAKE and don't prefix them with the type of<br>
> the kernel image, use the KERNEL_IMAGETYPE_FOR_MAKE as a prefix only<br>
> when installing them bundled with kernel or initramfs image.<br>
><br>
> * deploy the files from ${D}/${KERNEL_IMAGEDEST}/ instead of kernel<br>
> build directory, so that we don't need to call<br>
> DTB_PATH=`get_real_dtb_path_in_kernel "${DTB}"`<br>
> again in do_deploy<br>
><br>
> * create all links in do_deploy task, because default KERNEL_DTB_BASE_NAME<br>
> like KERNEL_IMAGE_BASE_NAME contains PKGR and PKGR is different in<br>
> do_install and do_deploy, because kernel.bbclass calls<br>
> meta/classes/kernel.bbclass:do_install[prefuncs] += "package_get_auto_pr"<br>
> meta/classes/kernel.bbclass:do_deploy[prefuncs] += "package_get_auto_pr"<br>
><br>
> * the filenames are a bit different, but with separate variable it<br>
> should be easier for other bbclasses which use these DTB files to<br>
> find them correctly, just use either the cannonical name<br>
> $dtb_base_name.$dtb_ext or $dtb_base_name-${KERNEL_DTB_SYMLINK_NAME}.$dtb_ext<br>
> because PKGR (and other PKG* variables) might be different in your<br>
> task and kernel.do_deploy task.<br>
><br>
> * fix DTB files being deployed with incorrect filenames when<br>
> KERNEL_IMAGE_SYMLINK_NAME isn't set to ${MACHINE}, e.g. instead of<br>
> the default:<br>
> -rw-r--r-- 2 bitbake bitbake 1.4K Nov 20 07:41 deploy/images/raspberrypi3-64/Image-1-4.9.59+git0+e7976b2aff-r0.2-lirc-rpi-20171120043031.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 64 Nov 20 07:41 deploy/images/raspberrypi3-64/Image-lirc-rpi.dtbo -> Image-1-4.9.59+git0+e7976b2aff-r0.2-lirc-rpi-20171120043031.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 64 Nov 20 07:41 deploy/images/raspberrypi3-64/lirc-rpi.dtbo -> Image-1-4.9.59+git0+e7976b2aff-r0.2-lirc-rpi-20171120043031.dtbo<br>
> I was getting:<br>
> -rw-r--r-- 2 bitbake bitbake 1348 Nov 20 10:28 deploy/images/raspberrypi3-64/Image-linux-raspberrypi-lirc-rpi.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 37 Nov 20 10:28 deploy/images/raspberrypi3-64/Image-linux-raspberrypi-lirc-rpi-master-20171120102653.dtbo -> Image-linux-raspberrypi-lirc-rpi.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 37 Nov 20 10:28 deploy/images/raspberrypi3-64/lirc-rpi.dtbo -> Image-linux-raspberrypi-lirc-rpi.dtbo<br>
> and e.g. sdcard_image-rpi.bbclass from meta-raspberrypi:<br>
> <a href="https://github.com/agherzan/meta-raspberrypi/blob/37e4e18f4a745ce8dc11f7e40a29da0859ff13c6/classes/sdcard_image-rpi.bbclass" rel="noreferrer" target="_blank">https://github.com/agherzan/meta-raspberrypi/blob/37e4e18f4a745ce8dc11f7e40a29da0859ff13c6/classes/sdcard_image-rpi.bbclass</a><br>
> was failing in:<br>
> mcopy -i ${WORKDIR}/boot.img -s ${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGETYPE}-${DTB_BASE_NAME}.dtb ::${DTB_BASE_NAME}.dtb<br>
> because ${KERNEL_IMAGETYPE}-${DTB_BASE_NAME}.dtb doesn't exist in my<br>
> build, due to<br>
> DTB_SYMLINK_NAME=`echo ${symlink_name} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`<br>
> not replacing whole "${KERNEL_IMAGE_SYMLINK_NAME}" (read ${MACHINE})<br>
> with just ${DTB_BASE_NAME}<br>
><br>
> * with this change applied the deploy dir looks like this:<br>
> -rw-r--r-- 2 bitbake bitbake 1.4K Nov 20 15:49 deploy/images/raspberrypi3-64/lirc-rpi-1-4.9.59+git0+e7976b2aff-r0.8-raspberrypi3-64-20171120154716.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 74 Nov 20 15:49 deploy/images/raspberrypi3-64/lirc-rpi.dtbo -> lirc-rpi-1-4.9.59+git0+e7976b2aff-r0.8-raspberrypi3-64-20171120154716.dtbo<br>
> lrwxrwxrwx 2 bitbake bitbake 74 Nov 20 15:49 deploy/images/raspberrypi3-64/lirc-rpi-raspberrypi3-64.dtbo -> lirc-rpi-1-4.9.59+git0+e7976b2aff-r0.8-raspberrypi3-64-20171120154716.dtbo<br>
> and works correctly even with DISTRO using different naming scheme<br>
><br>
> * the sdcard_image-rpi.bbclass still needs to be modified, I've provided<br>
> updated version here:<br>
> <a href="https://github.com/agherzan/meta-raspberrypi/pull/159" rel="noreferrer" target="_blank">https://github.com/agherzan/meta-raspberrypi/pull/159</a><br>
><br>
> Signed-off-by: Martin Jansa <<a href="mailto:Martin.Jansa@gmail.com" target="_blank">Martin.Jansa@gmail.com</a>><br>
> ---<br>
> meta/classes/kernel-devicetree.bbclass | 47 ++++++++------------------<br>
> meta/classes/kernel.bbclass | 3 ++<br>
> 2 files changed, 18 insertions(+), 32 deletions(-)<br>
><br>
> diff --git a/meta/classes/kernel-devicetree.bbclass b/meta/classes/kernel-devicetree.bbclass<br>
> index 5d38d3760d..7faa869493 100644<br>
> --- a/meta/classes/kernel-devicetree.bbclass<br>
> +++ b/meta/classes/kernel-devicetree.bbclass<br>
> @@ -60,20 +60,9 @@ do_install_append() {<br>
> for dtbf in ${KERNEL_DEVICETREE}; do<br>
> dtb=`normalize_dtb "$dtbf"`<br>
> dtb_ext=${dtb##*.}<br>
> + dtb_base_name=`basename $dtb .$dtb_ext`<br>
> dtb_path=`get_real_dtb_path_in_kernel "$dtb"`<br>
> - dtb_base_name=`basename $dtb ."$dtb_ext"`<br>
> install -m 0644 $dtb_path ${D}/${KERNEL_IMAGEDEST}/$dtb_base_name.$dtb_ext<br>
> - for type in ${KERNEL_IMAGETYPE_FOR_MAKE}; do<br>
> - symlink_name=${type}"-"${KERNEL_IMAGE_SYMLINK_NAME}<br>
> - dtb_symlink_name=`echo ${symlink_name} | sed "s/${MACHINE}/$dtb_base_name/g"`<br>
> - ln -sf $dtb_base_name.$dtb_ext ${D}/${KERNEL_IMAGEDEST}/devicetree-$dtb_symlink_name.$dtb_ext<br>
> -<br>
> - if [ "$type" = "zImage" ] && [ "${KERNEL_DEVICETREE_BUNDLE}" = "1" ]; then<br>
> - cat ${D}/${KERNEL_IMAGEDEST}/$type \<br>
> - ${D}/${KERNEL_IMAGEDEST}/$dtb_base_name.$dtb_ext \<br>
> - > ${D}/${KERNEL_IMAGEDEST}/$type-$dtb_base_name.$dtb_ext.bin<br>
> - fi<br>
> - done<br>
> done<br>
> }<br>
><br>
> @@ -81,30 +70,24 @@ do_deploy_append() {<br>
> for dtbf in ${KERNEL_DEVICETREE}; do<br>
> dtb=`normalize_dtb "$dtbf"`<br>
> dtb_ext=${dtb##*.}<br>
> - dtb_base_name=`basename $dtb ."$dtb_ext"`<br>
> + dtb_base_name=`basename $dtb .$dtb_ext`<br>
> + install -d ${DEPLOYDIR}<br>
> + install -m 0644 ${D}/${KERNEL_IMAGEDEST}/$dtb_base_name.$dtb_ext ${DEPLOYDIR}/$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext<br>
> + ln -sf $dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext ${DEPLOYDIR}/$dtb_base_name.$dtb_ext<br>
> + ln -sf $dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext ${DEPLOYDIR}/$dtb_base_name-${KERNEL_DTB_SYMLINK_NAME}.$dtb_ext<br>
> for type in ${KERNEL_IMAGETYPE_FOR_MAKE}; do<br>
> - base_name=${type}"-"${KERNEL_IMAGE_BASE_NAME}<br>
> - symlink_name=${type}"-"${KERNEL_IMAGE_SYMLINK_NAME}<br>
> - dtb_name=`echo ${base_name} | sed "s/${MACHINE}/$dtb_base_name/g"`<br>
> - dtb_symlink_name=`echo ${symlink_name} | sed "s/${MACHINE}/$dtb_base_name/g"`<br>
> - dtb_path=`get_real_dtb_path_in_kernel "$dtb"`<br>
> - install -d ${DEPLOYDIR}<br>
> - install -m 0644 $dtb_path ${DEPLOYDIR}/$dtb_name.$dtb_ext<br>
> - ln -sf $dtb_name.$dtb_ext ${DEPLOYDIR}/$dtb_symlink_name.$dtb_ext<br>
> - ln -sf $dtb_name.$dtb_ext ${DEPLOYDIR}/$dtb_base_name.$dtb_ext<br>
> -<br>
> if [ "$type" = "zImage" ] && [ "${KERNEL_DEVICETREE_BUNDLE}" = "1" ]; then<br>
> - cat ${DEPLOYDIR}/$type \<br>
> - ${DEPLOYDIR}/$dtb_name.$dtb_ext \<br>
> - > ${DEPLOYDIR}/$dtb_name.$dtb_ext.bin<br>
> - ln -sf $dtb_name.$dtb_ext.bin ${DEPLOYDIR}/$type-$dtb_base_name.$dtb_ext.bin<br>
> -<br>
> + cat ${D}/${KERNEL_IMAGEDEST}/$type \<br>
> + ${DEPLOYDIR}/$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext \<br>
> + > ${DEPLOYDIR}/$type-$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext.bin<br>
> + ln -sf $type-$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext.bin \<br>
> + ${DEPLOYDIR}/$type-$dtb_base_name-${KERNEL_DTB_SYMLINK_NAME}.$dtb_ext.bin<br>
> if [ -e "${KERNEL_OUTPUT_DIR}/${type}.initramfs" ]; then<br>
> cat ${KERNEL_OUTPUT_DIR}/${type}.initramfs \<br>
> - ${DEPLOYDIR}/$dtb_name.$dtb_ext \<br>
> - > ${DEPLOYDIR}/${type}-${INITRAMFS_BASE_NAME}-$dtb_base_name.$dtb_ext.bin<br>
> - ln -sf ${type}-${INITRAMFS_BASE_NAME}-$dtb_base_name.$dtb_ext.bin \<br>
> - ${DEPLOYDIR}/${type}-initramfs-$dtb_base_name.$dtb_ext-${MACHINE}.bin<br>
> + ${DEPLOYDIR}/$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext<br>
> + > ${DEPLOYDIR}/${type}-${INITRAMFS_BASE_NAME}-$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext.bin<br>
> + ln -sf ${type}-${INITRAMFS_BASE_NAME}-$dtb_base_name-${KERNEL_DTB_BASE_NAME}.$dtb_ext.bin \<br>
> + ${DEPLOYDIR}/${type}-${INITRAMFS_BASE_NAME}-$dtb_base_name-${KERNEL_DTB_SYMLINK_NAME}.$dtb_ext.bin<br>
> fi<br>
> fi<br>
> done<br>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass<br>
> index 3213b932bf..7ce64fd19f 100644<br>
> --- a/meta/classes/kernel.bbclass<br>
> +++ b/meta/classes/kernel.bbclass<br>
> @@ -663,6 +663,9 @@ KERNEL_IMAGE_BASE_NAME ?= "${PKGE}-${PKGV}-${PKGR}-${MACHINE}-${DATETIME}"<br>
> # Don't include the DATETIME variable in the sstate package signatures<br>
> KERNEL_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"<br>
> KERNEL_IMAGE_SYMLINK_NAME ?= "${MACHINE}"<br>
> +KERNEL_DTB_BASE_NAME ?= "${PKGE}-${PKGV}-${PKGR}-${MACHINE}-${DATETIME}"<br>
> +KERNEL_DTB_BASE_NAME[vardepsexclude] = "DATETIME"<br>
> +KERNEL_DTB_SYMLINK_NAME ?= "${MACHINE}"<br>
> MODULE_IMAGE_BASE_NAME ?= "modules-${PKGE}-${PKGV}-${PKGR}-${MACHINE}-${DATETIME}"<br>
> MODULE_IMAGE_BASE_NAME[vardepsexclude] = "DATETIME"<br>
> MODULE_TARBALL_BASE_NAME ?= "${MODULE_IMAGE_BASE_NAME}.tgz"<br>
> --<br>
> 2.17.1<br>
><br>
> --<br>
> _______________________________________________<br>
> Openembedded-core mailing list<br>
> <a href="mailto:Openembedded-core@lists.openembedded.org" target="_blank">Openembedded-core@lists.openembedded.org</a><br>
> <a href="http://lists.openembedded.org/mailman/listinfo/openembedded-core" rel="noreferrer" target="_blank">http://lists.openembedded.org/mailman/listinfo/openembedded-core</a><br>
</blockquote></div>