* [PATCH] linux-dtb: Use kernel build system to generate the dtb files
@ 2013-08-12 15:37 Otavio Salvador
2013-08-12 15:39 ` Otavio Salvador
2013-08-13 3:40 ` Bruce Ashfield
0 siblings, 2 replies; 7+ messages in thread
From: Otavio Salvador @ 2013-08-12 15:37 UTC (permalink / raw)
To: OpenEmbedded Core Mailing List
Cc: Evan Kotara, Otavio Salvador, Daiane Angolini, Lauren Post
As the Linux kernel, unconditionally, builds the dtc application and
it is the compatible version with the DeviceTree files shipped within
the kernel it is better to use it and the kernel build system to
generate the dtb files.
Some DeviceTree files rely on CPP and kernel headers to be able to
generate the dtb binary contents and it is harder to replicate it
outside of Linux kernel build system so we /use/ it.
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with /${KERNEL_IMAGEDEST}/' patch
meta/recipes-kernel/linux/linux-dtb.inc | 59 +++++++++++++++------------------
1 file changed, 27 insertions(+), 32 deletions(-)
diff --git a/meta/recipes-kernel/linux/linux-dtb.inc b/meta/recipes-kernel/linux/linux-dtb.inc
index 41dd599..a65f8bd 100644
--- a/meta/recipes-kernel/linux/linux-dtb.inc
+++ b/meta/recipes-kernel/linux/linux-dtb.inc
@@ -1,44 +1,39 @@
# Support for device tree generation
FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*"
-KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000"
python __anonymous () {
- devicetree = d.getVar("KERNEL_DEVICETREE", True) or ''
- if devicetree:
- depends = d.getVar("DEPENDS", True)
- d.setVar("DEPENDS", "%s dtc-native" % depends)
- packages = d.getVar("PACKAGES", True)
- d.setVar("PACKAGES", "%s kernel-devicetree" % packages)
+ d.appendVar("PACKAGES", " kernel-devicetree")
}
do_install_append() {
+ bbwarn "ARRG"
if test -n "${KERNEL_DEVICETREE}"; then
- for DTS_FILE in ${KERNEL_DEVICETREE}; do
- if [ ! -f ${DTS_FILE} ]; then
- echo "Warning: ${DTS_FILE} is not available!"
- continue
+ for DTB in ${KERNEL_DEVICETREE}; do
+ if echo ${DTB} | grep -q '/dts/'; then
+ bbwarn "${DTB} points to the full path dts file while it should have the target for use."
+ DTB=`basename ${DTB} | sed 's,\.dts$,.dtb,g'`
fi
- DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
- DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
- DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
- dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o ${DTS_BASE_NAME} ${DTS_FILE}
- install -m 0644 ${DTS_BASE_NAME} ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb
+ DTB_BASE_NAME=`basename ${DTB} .dtb`
+ DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
+ DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
+ oe_runmake ${DTB}
+ install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb
done
fi
}
do_deploy_append() {
if test -n "${KERNEL_DEVICETREE}"; then
- for DTS_FILE in ${KERNEL_DEVICETREE}; do
- if [ ! -f ${DTS_FILE} ]; then
- echo "Warning: ${DTS_FILE} is not available!"
- continue
+ for DTB in ${KERNEL_DEVICETREE}; do
+ if echo ${DTB} | grep -q '/dts/'; then
+ bbwarn "${DTB} points to the full path dts file while it should have the target for use. "
+ DTB=`basename ${DTB} | sed 's,\.dts$,.dtb,g'`
fi
- DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
- DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
- DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
+ DTB_BASE_NAME=`basename ${DTB} .dtb`
+ DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
+ DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
install -d ${DEPLOYDIR}
- install -m 0644 ${B}/${DTS_BASE_NAME} ${DEPLOYDIR}/${DTB_NAME}.dtb
+ install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} ${DEPLOYDIR}/${DTB_NAME}.dtb
cd ${DEPLOYDIR}
ln -sf ${DTB_NAME}.dtb ${DTB_SYMLINK_NAME}.dtb
cd -
@@ -48,20 +43,20 @@ do_deploy_append() {
pkg_postinst_kernel-devicetree () {
cd /${KERNEL_IMAGEDEST}
- for DTS_FILE in ${KERNEL_DEVICETREE}
+ for DTB_FILE in ${KERNEL_DEVICETREE}
do
- DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
- DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
- update-alternatives --install /${KERNEL_IMAGEDEST}/${DTS_BASE_NAME}.dtb ${DTS_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true
+ DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print $1}'`
+ DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
+ update-alternatives --install /${KERNEL_IMAGEDEST}/${DTB_BASE_NAME}.dtb ${DTB_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true
done
}
pkg_postrm_kernel-devicetree () {
cd /${KERNEL_IMAGEDEST}
- for DTS_FILE in ${KERNEL_DEVICETREE}
+ for DTB_FILE in ${KERNEL_DEVICETREE}
do
- DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
- DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"`
- update-alternatives --remove ${DTS_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true
+ DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print $1}'`
+ DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"`
+ update-alternatives --remove ${DTB_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true
done
}
--
1.8.4.rc1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-12 15:37 [PATCH] linux-dtb: Use kernel build system to generate the dtb files Otavio Salvador @ 2013-08-12 15:39 ` Otavio Salvador 2013-08-13 3:40 ` Bruce Ashfield 1 sibling, 0 replies; 7+ messages in thread From: Otavio Salvador @ 2013-08-12 15:39 UTC (permalink / raw) To: OpenEmbedded Core Mailing List Cc: Evan Kotara, Otavio Salvador, Daiane Angolini, Lauren Post On Mon, Aug 12, 2013 at 12:37 PM, Otavio Salvador <otavio@ossystems.com.br> wrote: > As the Linux kernel, unconditionally, builds the dtc application and > it is the compatible version with the DeviceTree files shipped within > the kernel it is better to use it and the kernel build system to > generate the dtb files. > > Some DeviceTree files rely on CPP and kernel headers to be able to > generate the dtb binary contents and it is harder to replicate it > outside of Linux kernel build system so we /use/ it. > > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with /${KERNEL_IMAGEDEST}/' patch > > meta/recipes-kernel/linux/linux-dtb.inc | 59 +++++++++++++++------------------ > 1 file changed, 27 insertions(+), 32 deletions(-) > > diff --git a/meta/recipes-kernel/linux/linux-dtb.inc b/meta/recipes-kernel/linux/linux-dtb.inc > index 41dd599..a65f8bd 100644 > --- a/meta/recipes-kernel/linux/linux-dtb.inc > +++ b/meta/recipes-kernel/linux/linux-dtb.inc > @@ -1,44 +1,39 @@ > # Support for device tree generation > FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" > -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" > > python __anonymous () { > - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' > - if devicetree: > - depends = d.getVar("DEPENDS", True) > - d.setVar("DEPENDS", "%s dtc-native" % depends) > - packages = d.getVar("PACKAGES", True) > - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) > + d.appendVar("PACKAGES", " kernel-devicetree") > } > > do_install_append() { > + bbwarn "ARRG" Kept when testing. I will await for more feedback before sending v2. -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://projetos.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-12 15:37 [PATCH] linux-dtb: Use kernel build system to generate the dtb files Otavio Salvador 2013-08-12 15:39 ` Otavio Salvador @ 2013-08-13 3:40 ` Bruce Ashfield 2013-08-13 12:46 ` Otavio Salvador 1 sibling, 1 reply; 7+ messages in thread From: Bruce Ashfield @ 2013-08-13 3:40 UTC (permalink / raw) To: Otavio Salvador Cc: Evan Kotara, Lauren Post, Daiane Angolini, OpenEmbedded Core Mailing List On 13-08-12 11:37 AM, Otavio Salvador wrote: > As the Linux kernel, unconditionally, builds the dtc application and > it is the compatible version with the DeviceTree files shipped within > the kernel it is better to use it and the kernel build system to > generate the dtb files. From my point of view, I agree with this. I've always pushed for using all of the artifacts from the kernel built (uImage, dtb, etc) and this is consistent with that approach. > > Some DeviceTree files rely on CPP and kernel headers to be able to > generate the dtb binary contents and it is harder to replicate it > outside of Linux kernel build system so we /use/ it. > > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with /${KERNEL_IMAGEDEST}/' patch > > meta/recipes-kernel/linux/linux-dtb.inc | 59 +++++++++++++++------------------ > 1 file changed, 27 insertions(+), 32 deletions(-) > > diff --git a/meta/recipes-kernel/linux/linux-dtb.inc b/meta/recipes-kernel/linux/linux-dtb.inc > index 41dd599..a65f8bd 100644 > --- a/meta/recipes-kernel/linux/linux-dtb.inc > +++ b/meta/recipes-kernel/linux/linux-dtb.inc > @@ -1,44 +1,39 @@ > # Support for device tree generation > FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" > -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" > > python __anonymous () { > - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' > - if devicetree: > - depends = d.getVar("DEPENDS", True) > - d.setVar("DEPENDS", "%s dtc-native" % depends) > - packages = d.getVar("PACKAGES", True) > - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) > + d.appendVar("PACKAGES", " kernel-devicetree") What if someone, somewhere needs a custom dtc. Can we still trigger a dependency on dtc-native is a different flag is set ? > } > > do_install_append() { > + bbwarn "ARRG" > if test -n "${KERNEL_DEVICETREE}"; then > - for DTS_FILE in ${KERNEL_DEVICETREE}; do > - if [ ! -f ${DTS_FILE} ]; then > - echo "Warning: ${DTS_FILE} is not available!" > - continue > + for DTB in ${KERNEL_DEVICETREE}; do > + if echo ${DTB} | grep -q '/dts/'; then > + bbwarn "${DTB} points to the full path dts file while it should have the target for use." This isn't reading right to me. What are you trying to say ? That KERNEL_DEVICE_TREE should be the name of the dts and not the path to that dts ? I'd say 'name' versus target. What about testing for it's existence ? The old code used to do that. > + DTB=`basename ${DTB} | sed 's,\.dts$,.dtb,g'` From the description of the patch .. this part is unexpected. You are changing more than what is used to compile the dtb, but the processing of the options. > fi > - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'` > - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > - dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o ${DTS_BASE_NAME} ${DTS_FILE} > - install -m 0644 ${DTS_BASE_NAME} ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb > + DTB_BASE_NAME=`basename ${DTB} .dtb` > + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > + oe_runmake ${DTB} The change log should explain this as well. You are switching the naming to DTB from the old DTS .. because ? The kernel build requires it ? something else ? You tell me :) > + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb > done > fi > } > > do_deploy_append() { > if test -n "${KERNEL_DEVICETREE}"; then > - for DTS_FILE in ${KERNEL_DEVICETREE}; do > - if [ ! -f ${DTS_FILE} ]; then > - echo "Warning: ${DTS_FILE} is not available!" > - continue > + for DTB in ${KERNEL_DEVICETREE}; do > + if echo ${DTB} | grep -q '/dts/'; then > + bbwarn "${DTB} points to the full path dts file while it should have the target for use. " > + DTB=`basename ${DTB} | sed 's,\.dts$,.dtb,g'` Given that the old and new code repeats this block .. is there an opportunity to factor it out into a function ? > fi > - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'` > - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > + DTB_BASE_NAME=`basename ${DTB} .dtb` > + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > install -d ${DEPLOYDIR} > - install -m 0644 ${B}/${DTS_BASE_NAME} ${DEPLOYDIR}/${DTB_NAME}.dtb > + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} ${DEPLOYDIR}/${DTB_NAME}.dtb Same question, we switch to DTB from DTS for technical or clarity/cosmetic reasons ? > cd ${DEPLOYDIR} > ln -sf ${DTB_NAME}.dtb ${DTB_SYMLINK_NAME}.dtb > cd - > @@ -48,20 +43,20 @@ do_deploy_append() { > > pkg_postinst_kernel-devicetree () { > cd /${KERNEL_IMAGEDEST} > - for DTS_FILE in ${KERNEL_DEVICETREE} > + for DTB_FILE in ${KERNEL_DEVICETREE} > do > - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'` > - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > - update-alternatives --install /${KERNEL_IMAGEDEST}/${DTS_BASE_NAME}.dtb ${DTS_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true > + DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print $1}'` > + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > + update-alternatives --install /${KERNEL_IMAGEDEST}/${DTB_BASE_NAME}.dtb ${DTB_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true The DTS -> DTB name change is making the patch look bigger than it is, and really should be separate from the switch to the in tree dtc. Bruce > done > } > > pkg_postrm_kernel-devicetree () { > cd /${KERNEL_IMAGEDEST} > - for DTS_FILE in ${KERNEL_DEVICETREE} > + for DTB_FILE in ${KERNEL_DEVICETREE} > do > - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'` > - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` > - update-alternatives --remove ${DTS_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true > + DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print $1}'` > + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` > + update-alternatives --remove ${DTB_BASE_NAME}.dtb devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true > done > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-13 3:40 ` Bruce Ashfield @ 2013-08-13 12:46 ` Otavio Salvador 2013-08-13 13:06 ` Bruce Ashfield 0 siblings, 1 reply; 7+ messages in thread From: Otavio Salvador @ 2013-08-13 12:46 UTC (permalink / raw) To: Bruce Ashfield Cc: Evan Kotara, Lauren Post, Daiane Angolini, OpenEmbedded Core Mailing List On Tue, Aug 13, 2013 at 12:40 AM, Bruce Ashfield <bruce.ashfield@windriver.com> wrote: > On 13-08-12 11:37 AM, Otavio Salvador wrote: >> >> As the Linux kernel, unconditionally, builds the dtc application and >> it is the compatible version with the DeviceTree files shipped within >> the kernel it is better to use it and the kernel build system to >> generate the dtb files. > > > From my point of view, I agree with this. I've always pushed for using > all of the artifacts from the kernel built (uImage, dtb, etc) and this > is consistent with that approach. Good. >> Some DeviceTree files rely on CPP and kernel headers to be able to >> generate the dtb binary contents and it is harder to replicate it >> outside of Linux kernel build system so we /use/ it. >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >> --- >> NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with >> /${KERNEL_IMAGEDEST}/' patch >> >> meta/recipes-kernel/linux/linux-dtb.inc | 59 >> +++++++++++++++------------------ >> 1 file changed, 27 insertions(+), 32 deletions(-) >> >> diff --git a/meta/recipes-kernel/linux/linux-dtb.inc >> b/meta/recipes-kernel/linux/linux-dtb.inc >> index 41dd599..a65f8bd 100644 >> --- a/meta/recipes-kernel/linux/linux-dtb.inc >> +++ b/meta/recipes-kernel/linux/linux-dtb.inc >> @@ -1,44 +1,39 @@ >> # Support for device tree generation >> FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" >> -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" >> >> python __anonymous () { >> - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' >> - if devicetree: >> - depends = d.getVar("DEPENDS", True) >> - d.setVar("DEPENDS", "%s dtc-native" % depends) >> - packages = d.getVar("PACKAGES", True) >> - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) >> + d.appendVar("PACKAGES", " kernel-devicetree") > > > What if someone, somewhere needs a custom dtc. Can we still trigger > a dependency on dtc-native is a different flag is set ? The Kernel does not provide a way to 'override' it; it always use it from scripts/dtc and it is the compatible version with the DeviceTree files shipped within the kernel, so it is better to use it and the kernel build system to generate the dtb files. >> } >> >> do_install_append() { >> + bbwarn "ARRG" >> if test -n "${KERNEL_DEVICETREE}"; then >> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >> - if [ ! -f ${DTS_FILE} ]; then >> - echo "Warning: ${DTS_FILE} is not >> available!" >> - continue >> + for DTB in ${KERNEL_DEVICETREE}; do >> + if echo ${DTB} | grep -q '/dts/'; then >> + bbwarn "${DTB} points to the full path dts >> file while it should have the target for use." > > > This isn't reading right to me. What are you trying to say ? That > KERNEL_DEVICE_TREE should be the name of the dts and not the path > to that dts ? I'd say 'name' versus target. You should name it 'imx6q-sabresd.dtb' for example. > What about testing for it's existence ? The old code used to do that. It will fail if you ask it to build an invalid dtb file. >> + DTB=`basename ${DTB} | sed >> 's,\.dts$,.dtb,g'` > > > From the description of the patch .. this part is unexpected. > You are changing more than what is used to compile the dtb, > but the processing of the options. It is not; I need to call it with the target. This is to support converting from 'current' use to 'new' use and keep backward compatibility. >> fi >> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >> '{print $1}'` >> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >> - DTB_SYMLINK_NAME=`echo >> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >> - dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o >> ${DTS_BASE_NAME} ${DTS_FILE} >> - install -m 0644 ${DTS_BASE_NAME} >> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >> + DTB_BASE_NAME=`basename ${DTB} .dtb` >> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >> + DTB_SYMLINK_NAME=`echo >> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >> + oe_runmake ${DTB} > > > The change log should explain this as well. You are switching the > naming to DTB from the old DTS .. because ? The kernel build > requires it ? something else ? You tell me :) This is to support converting from 'current' use to 'new' use and keep backward compatibility. >> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >> done >> fi >> } >> >> do_deploy_append() { >> if test -n "${KERNEL_DEVICETREE}"; then >> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >> - if [ ! -f ${DTS_FILE} ]; then >> - echo "Warning: ${DTS_FILE} is not >> available!" >> - continue >> + for DTB in ${KERNEL_DEVICETREE}; do >> + if echo ${DTB} | grep -q '/dts/'; then >> + bbwarn "${DTB} points to the full path dts >> file while it should have the target for use. " >> + DTB=`basename ${DTB} | sed >> 's,\.dts$,.dtb,g'` > > > Given that the old and new code repeats this block .. is there an > opportunity to factor it out into a function ? In a new patch? >> fi >> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >> '{print $1}'` >> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >> - DTB_SYMLINK_NAME=`echo >> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >> + DTB_BASE_NAME=`basename ${DTB} .dtb` >> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >> + DTB_SYMLINK_NAME=`echo >> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >> install -d ${DEPLOYDIR} >> - install -m 0644 ${B}/${DTS_BASE_NAME} >> ${DEPLOYDIR}/${DTB_NAME}.dtb >> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >> ${DEPLOYDIR}/${DTB_NAME}.dtb > > > Same question, we switch to DTB from DTS for technical or clarity/cosmetic > reasons ? This is to support converting from 'current' use to 'new' use and keep backward compatibility. >> cd ${DEPLOYDIR} >> ln -sf ${DTB_NAME}.dtb ${DTB_SYMLINK_NAME}.dtb >> cd - >> @@ -48,20 +43,20 @@ do_deploy_append() { >> >> pkg_postinst_kernel-devicetree () { >> cd /${KERNEL_IMAGEDEST} >> - for DTS_FILE in ${KERNEL_DEVICETREE} >> + for DTB_FILE in ${KERNEL_DEVICETREE} >> do >> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print >> $1}'` >> - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed >> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >> - update-alternatives --install >> /${KERNEL_IMAGEDEST}/${DTS_BASE_NAME}.dtb ${DTS_BASE_NAME}.dtb >> devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true >> + DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print >> $1}'` >> + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed >> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >> + update-alternatives --install >> /${KERNEL_IMAGEDEST}/${DTB_BASE_NAME}.dtb ${DTB_BASE_NAME}.dtb >> devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true > > > The DTS -> DTB name change is making the patch look bigger than it is, > and really should be separate from the switch to the in tree dtc. This is to support converting from 'current' use to 'new' use and keep backward compatibility. We need to use the .dtb file when calling make. -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://projetos.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-13 12:46 ` Otavio Salvador @ 2013-08-13 13:06 ` Bruce Ashfield 2013-08-13 13:35 ` Otavio Salvador 0 siblings, 1 reply; 7+ messages in thread From: Bruce Ashfield @ 2013-08-13 13:06 UTC (permalink / raw) To: Otavio Salvador Cc: Evan Kotara, Lauren Post, Daiane Angolini, OpenEmbedded Core Mailing List On 13-08-13 08:46 AM, Otavio Salvador wrote: > On Tue, Aug 13, 2013 at 12:40 AM, Bruce Ashfield > <bruce.ashfield@windriver.com> wrote: >> On 13-08-12 11:37 AM, Otavio Salvador wrote: >>> >>> As the Linux kernel, unconditionally, builds the dtc application and >>> it is the compatible version with the DeviceTree files shipped within >>> the kernel it is better to use it and the kernel build system to >>> generate the dtb files. >> >> >> From my point of view, I agree with this. I've always pushed for using >> all of the artifacts from the kernel built (uImage, dtb, etc) and this >> is consistent with that approach. > > Good. > >>> Some DeviceTree files rely on CPP and kernel headers to be able to >>> generate the dtb binary contents and it is harder to replicate it >>> outside of Linux kernel build system so we /use/ it. >>> >>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >>> --- >>> NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with >>> /${KERNEL_IMAGEDEST}/' patch >>> >>> meta/recipes-kernel/linux/linux-dtb.inc | 59 >>> +++++++++++++++------------------ >>> 1 file changed, 27 insertions(+), 32 deletions(-) >>> >>> diff --git a/meta/recipes-kernel/linux/linux-dtb.inc >>> b/meta/recipes-kernel/linux/linux-dtb.inc >>> index 41dd599..a65f8bd 100644 >>> --- a/meta/recipes-kernel/linux/linux-dtb.inc >>> +++ b/meta/recipes-kernel/linux/linux-dtb.inc >>> @@ -1,44 +1,39 @@ >>> # Support for device tree generation >>> FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" >>> -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" >>> >>> python __anonymous () { >>> - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' >>> - if devicetree: >>> - depends = d.getVar("DEPENDS", True) >>> - d.setVar("DEPENDS", "%s dtc-native" % depends) >>> - packages = d.getVar("PACKAGES", True) >>> - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) >>> + d.appendVar("PACKAGES", " kernel-devicetree") >> >> >> What if someone, somewhere needs a custom dtc. Can we still trigger >> a dependency on dtc-native is a different flag is set ? > > The Kernel does not provide a way to 'override' it; it always use it > from scripts/dtc and it is the compatible version with the DeviceTree > files shipped within the kernel, so it is better to use it and the > kernel build system to generate the dtb files. What I meant was not to override the kernel's dtc, but to simply fall back to the old dtc-native reaching into the tree and compiling the dts. That way we have flexibility as a fall back. > >>> } >>> >>> do_install_append() { >>> + bbwarn "ARRG" >>> if test -n "${KERNEL_DEVICETREE}"; then >>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>> - if [ ! -f ${DTS_FILE} ]; then >>> - echo "Warning: ${DTS_FILE} is not >>> available!" >>> - continue >>> + for DTB in ${KERNEL_DEVICETREE}; do >>> + if echo ${DTB} | grep -q '/dts/'; then >>> + bbwarn "${DTB} points to the full path dts >>> file while it should have the target for use." >> >> >> This isn't reading right to me. What are you trying to say ? That >> KERNEL_DEVICE_TREE should be the name of the dts and not the path >> to that dts ? I'd say 'name' versus target. > > You should name it 'imx6q-sabresd.dtb' for example. right, that's what I always do myself. So I'm really just suggesting that the bbwarn message be a bit more clear. i.e. bbwarn "${DTB} contains the full path to the the dts file, but only the dtb name should be used." > >> What about testing for it's existence ? The old code used to do that. > > It will fail if you ask it to build an invalid dtb file. > >>> + DTB=`basename ${DTB} | sed >>> 's,\.dts$,.dtb,g'` >> >> >> From the description of the patch .. this part is unexpected. >> You are changing more than what is used to compile the dtb, >> but the processing of the options. > > It is not; I need to call it with the target. This is to support > converting from 'current' use to 'new' use and keep backward > compatibility. Right, so you are switching to dtb, which the kernel expects, not dts. The commit message should make this clear, right now it doesn't. You need to understand what the kernel is looking for to know why the change is being made. > >>> fi >>> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >>> '{print $1}'` >>> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>> - DTB_SYMLINK_NAME=`echo >>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>> - dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o >>> ${DTS_BASE_NAME} ${DTS_FILE} >>> - install -m 0644 ${DTS_BASE_NAME} >>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>> + DTB_BASE_NAME=`basename ${DTB} .dtb` >>> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>> + DTB_SYMLINK_NAME=`echo >>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>> + oe_runmake ${DTB} >> >> >> The change log should explain this as well. You are switching the >> naming to DTB from the old DTS .. because ? The kernel build >> requires it ? something else ? You tell me :) > > This is to support converting from 'current' use to 'new' use and keep > backward compatibility. > >>> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>> done >>> fi >>> } >>> >>> do_deploy_append() { >>> if test -n "${KERNEL_DEVICETREE}"; then >>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>> - if [ ! -f ${DTS_FILE} ]; then >>> - echo "Warning: ${DTS_FILE} is not >>> available!" >>> - continue >>> + for DTB in ${KERNEL_DEVICETREE}; do >>> + if echo ${DTB} | grep -q '/dts/'; then >>> + bbwarn "${DTB} points to the full path dts >>> file while it should have the target for use. " >>> + DTB=`basename ${DTB} | sed >>> 's,\.dts$,.dtb,g'` >> >> >> Given that the old and new code repeats this block .. is there an >> opportunity to factor it out into a function ? > > In a new patch? Could be, or just as part of this one. Either way works. Cheers, Bruce > >>> fi >>> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >>> '{print $1}'` >>> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>> - DTB_SYMLINK_NAME=`echo >>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>> + DTB_BASE_NAME=`basename ${DTB} .dtb` >>> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>> + DTB_SYMLINK_NAME=`echo >>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>> install -d ${DEPLOYDIR} >>> - install -m 0644 ${B}/${DTS_BASE_NAME} >>> ${DEPLOYDIR}/${DTB_NAME}.dtb >>> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >>> ${DEPLOYDIR}/${DTB_NAME}.dtb >> >> >> Same question, we switch to DTB from DTS for technical or clarity/cosmetic >> reasons ? > > This is to support converting from 'current' use to 'new' use and keep > backward compatibility. > >>> cd ${DEPLOYDIR} >>> ln -sf ${DTB_NAME}.dtb ${DTB_SYMLINK_NAME}.dtb >>> cd - >>> @@ -48,20 +43,20 @@ do_deploy_append() { >>> >>> pkg_postinst_kernel-devicetree () { >>> cd /${KERNEL_IMAGEDEST} >>> - for DTS_FILE in ${KERNEL_DEVICETREE} >>> + for DTB_FILE in ${KERNEL_DEVICETREE} >>> do >>> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print >>> $1}'` >>> - DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed >>> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>> - update-alternatives --install >>> /${KERNEL_IMAGEDEST}/${DTS_BASE_NAME}.dtb ${DTS_BASE_NAME}.dtb >>> devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true >>> + DTB_BASE_NAME=`basename ${DTB_FILE} | awk -F "." '{print >>> $1}'` >>> + DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed >>> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>> + update-alternatives --install >>> /${KERNEL_IMAGEDEST}/${DTB_BASE_NAME}.dtb ${DTB_BASE_NAME}.dtb >>> devicetree-${DTB_SYMLINK_NAME}.dtb ${KERNEL_PRIORITY} || true >> >> >> The DTS -> DTB name change is making the patch look bigger than it is, >> and really should be separate from the switch to the in tree dtc. > > This is to support converting from 'current' use to 'new' use and keep > backward compatibility. We need to use the .dtb file when calling > make. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-13 13:06 ` Bruce Ashfield @ 2013-08-13 13:35 ` Otavio Salvador 2013-08-13 13:40 ` Bruce Ashfield 0 siblings, 1 reply; 7+ messages in thread From: Otavio Salvador @ 2013-08-13 13:35 UTC (permalink / raw) To: Bruce Ashfield Cc: Evan Kotara, Lauren Post, Daiane Angolini, OpenEmbedded Core Mailing List On Tue, Aug 13, 2013 at 10:06 AM, Bruce Ashfield <bruce.ashfield@windriver.com> wrote: > On 13-08-13 08:46 AM, Otavio Salvador wrote: >> >> On Tue, Aug 13, 2013 at 12:40 AM, Bruce Ashfield >> <bruce.ashfield@windriver.com> wrote: >>> >>> On 13-08-12 11:37 AM, Otavio Salvador wrote: >>>> >>>> >>>> As the Linux kernel, unconditionally, builds the dtc application and >>>> it is the compatible version with the DeviceTree files shipped within >>>> the kernel it is better to use it and the kernel build system to >>>> generate the dtb files. >>> >>> >>> >>> From my point of view, I agree with this. I've always pushed for using >>> all of the artifacts from the kernel built (uImage, dtb, etc) and this >>> is consistent with that approach. >> >> >> Good. >> >>>> Some DeviceTree files rely on CPP and kernel headers to be able to >>>> generate the dtb binary contents and it is harder to replicate it >>>> outside of Linux kernel build system so we /use/ it. >>>> >>>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >>>> --- >>>> NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with >>>> /${KERNEL_IMAGEDEST}/' patch >>>> >>>> meta/recipes-kernel/linux/linux-dtb.inc | 59 >>>> +++++++++++++++------------------ >>>> 1 file changed, 27 insertions(+), 32 deletions(-) >>>> >>>> diff --git a/meta/recipes-kernel/linux/linux-dtb.inc >>>> b/meta/recipes-kernel/linux/linux-dtb.inc >>>> index 41dd599..a65f8bd 100644 >>>> --- a/meta/recipes-kernel/linux/linux-dtb.inc >>>> +++ b/meta/recipes-kernel/linux/linux-dtb.inc >>>> @@ -1,44 +1,39 @@ >>>> # Support for device tree generation >>>> FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" >>>> -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" >>>> >>>> python __anonymous () { >>>> - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' >>>> - if devicetree: >>>> - depends = d.getVar("DEPENDS", True) >>>> - d.setVar("DEPENDS", "%s dtc-native" % depends) >>>> - packages = d.getVar("PACKAGES", True) >>>> - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) >>>> + d.appendVar("PACKAGES", " kernel-devicetree") >>> >>> >>> >>> What if someone, somewhere needs a custom dtc. Can we still trigger >>> a dependency on dtc-native is a different flag is set ? >> >> >> The Kernel does not provide a way to 'override' it; it always use it >> from scripts/dtc and it is the compatible version with the DeviceTree >> files shipped within the kernel, so it is better to use it and the >> kernel build system to generate the dtb files. > > > What I meant was not to override the kernel's dtc, but to simply fall > back to the old dtc-native reaching into the tree and compiling the > dts. That way we have flexibility as a fall back. I think it will make the code much harder to follow and I am unsure we will have it in a flexible enough way; for example if user uses kernel headers for pins and like, it won't work, and it will be hard to come up with something which could comply with all this. As this .inc file is intended for 'in kernel build' it does not seems worth it. >>>> } >>>> >>>> do_install_append() { >>>> + bbwarn "ARRG" >>>> if test -n "${KERNEL_DEVICETREE}"; then >>>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>>> - if [ ! -f ${DTS_FILE} ]; then >>>> - echo "Warning: ${DTS_FILE} is not >>>> available!" >>>> - continue >>>> + for DTB in ${KERNEL_DEVICETREE}; do >>>> + if echo ${DTB} | grep -q '/dts/'; then >>>> + bbwarn "${DTB} points to the full path >>>> dts >>>> file while it should have the target for use." >>> >>> >>> >>> This isn't reading right to me. What are you trying to say ? That >>> KERNEL_DEVICE_TREE should be the name of the dts and not the path >>> to that dts ? I'd say 'name' versus target. >> >> >> You should name it 'imx6q-sabresd.dtb' for example. > > > right, that's what I always do myself. So I'm really just suggesting that > the bbwarn message be a bit more clear. i.e. > > bbwarn "${DTB} contains the full path to the the dts file, but only the > dtb name should be used." Great; changed for v2 >>> What about testing for it's existence ? The old code used to do that. >> >> >> It will fail if you ask it to build an invalid dtb file. >> >>>> + DTB=`basename ${DTB} | sed >>>> 's,\.dts$,.dtb,g'` >>> >>> >>> >>> From the description of the patch .. this part is unexpected. >>> You are changing more than what is used to compile the dtb, >>> but the processing of the options. >> >> >> It is not; I need to call it with the target. This is to support >> converting from 'current' use to 'new' use and keep backward >> compatibility. > > > Right, so you are switching to dtb, which the kernel expects, not > dts. The commit message should make this clear, right now it > doesn't. You need to understand what the kernel is looking for to > know why the change is being made. Ok; Changed for v2. >>>> fi >>>> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >>>> '{print $1}'` >>>> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>>> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>>> - DTB_SYMLINK_NAME=`echo >>>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>>> - dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o >>>> ${DTS_BASE_NAME} ${DTS_FILE} >>>> - install -m 0644 ${DTS_BASE_NAME} >>>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>>> + DTB_BASE_NAME=`basename ${DTB} .dtb` >>>> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>>> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>>> + DTB_SYMLINK_NAME=`echo >>>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>>> + oe_runmake ${DTB} >>> >>> >>> >>> The change log should explain this as well. You are switching the >>> naming to DTB from the old DTS .. because ? The kernel build >>> requires it ? something else ? You tell me :) >> >> >> This is to support converting from 'current' use to 'new' use and keep >> backward compatibility. >> >>>> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >>>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>>> done >>>> fi >>>> } >>>> >>>> do_deploy_append() { >>>> if test -n "${KERNEL_DEVICETREE}"; then >>>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>>> - if [ ! -f ${DTS_FILE} ]; then >>>> - echo "Warning: ${DTS_FILE} is not >>>> available!" >>>> - continue >>>> + for DTB in ${KERNEL_DEVICETREE}; do >>>> + if echo ${DTB} | grep -q '/dts/'; then >>>> + bbwarn "${DTB} points to the full path >>>> dts >>>> file while it should have the target for use. " >>>> + DTB=`basename ${DTB} | sed >>>> 's,\.dts$,.dtb,g'` >>> >>> >>> >>> Given that the old and new code repeats this block .. is there an >>> opportunity to factor it out into a function ? >> >> >> In a new patch? > > > Could be, or just as part of this one. Either way works. Ok; I will take a look on that. -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://projetos.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-dtb: Use kernel build system to generate the dtb files 2013-08-13 13:35 ` Otavio Salvador @ 2013-08-13 13:40 ` Bruce Ashfield 0 siblings, 0 replies; 7+ messages in thread From: Bruce Ashfield @ 2013-08-13 13:40 UTC (permalink / raw) To: Otavio Salvador Cc: Evan Kotara, Lauren Post, Daiane Angolini, OpenEmbedded Core Mailing List On 13-08-13 09:35 AM, Otavio Salvador wrote: > On Tue, Aug 13, 2013 at 10:06 AM, Bruce Ashfield > <bruce.ashfield@windriver.com> wrote: >> On 13-08-13 08:46 AM, Otavio Salvador wrote: >>> >>> On Tue, Aug 13, 2013 at 12:40 AM, Bruce Ashfield >>> <bruce.ashfield@windriver.com> wrote: >>>> >>>> On 13-08-12 11:37 AM, Otavio Salvador wrote: >>>>> >>>>> >>>>> As the Linux kernel, unconditionally, builds the dtc application and >>>>> it is the compatible version with the DeviceTree files shipped within >>>>> the kernel it is better to use it and the kernel build system to >>>>> generate the dtb files. >>>> >>>> >>>> >>>> From my point of view, I agree with this. I've always pushed for using >>>> all of the artifacts from the kernel built (uImage, dtb, etc) and this >>>> is consistent with that approach. >>> >>> >>> Good. >>> >>>>> Some DeviceTree files rely on CPP and kernel headers to be able to >>>>> generate the dtb binary contents and it is harder to replicate it >>>>> outside of Linux kernel build system so we /use/ it. >>>>> >>>>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> >>>>> --- >>>>> NOTE: This depends on 'linux-dtb.inc: Replace /boot/ with >>>>> /${KERNEL_IMAGEDEST}/' patch >>>>> >>>>> meta/recipes-kernel/linux/linux-dtb.inc | 59 >>>>> +++++++++++++++------------------ >>>>> 1 file changed, 27 insertions(+), 32 deletions(-) >>>>> >>>>> diff --git a/meta/recipes-kernel/linux/linux-dtb.inc >>>>> b/meta/recipes-kernel/linux/linux-dtb.inc >>>>> index 41dd599..a65f8bd 100644 >>>>> --- a/meta/recipes-kernel/linux/linux-dtb.inc >>>>> +++ b/meta/recipes-kernel/linux/linux-dtb.inc >>>>> @@ -1,44 +1,39 @@ >>>>> # Support for device tree generation >>>>> FILES_kernel-devicetree = "/${KERNEL_IMAGEDEST}/devicetree*" >>>>> -KERNEL_DEVICETREE_FLAGS ?= "-R 8 -p 0x3000" >>>>> >>>>> python __anonymous () { >>>>> - devicetree = d.getVar("KERNEL_DEVICETREE", True) or '' >>>>> - if devicetree: >>>>> - depends = d.getVar("DEPENDS", True) >>>>> - d.setVar("DEPENDS", "%s dtc-native" % depends) >>>>> - packages = d.getVar("PACKAGES", True) >>>>> - d.setVar("PACKAGES", "%s kernel-devicetree" % packages) >>>>> + d.appendVar("PACKAGES", " kernel-devicetree") >>>> >>>> >>>> >>>> What if someone, somewhere needs a custom dtc. Can we still trigger >>>> a dependency on dtc-native is a different flag is set ? >>> >>> >>> The Kernel does not provide a way to 'override' it; it always use it >>> from scripts/dtc and it is the compatible version with the DeviceTree >>> files shipped within the kernel, so it is better to use it and the >>> kernel build system to generate the dtb files. >> >> >> What I meant was not to override the kernel's dtc, but to simply fall >> back to the old dtc-native reaching into the tree and compiling the >> dts. That way we have flexibility as a fall back. > > I think it will make the code much harder to follow and I am unsure we > will have it in a flexible enough way; for example if user uses kernel > headers for pins and like, it won't work, and it will be hard to come > up with something which could comply with all this. As this .inc file > is intended for 'in kernel build' it does not seems worth it. I have to agree. I knew it was a bit of a stretch. ultimate flexibility is not always a good thing. If this causes a problem for someone, we can consider their patches to fall back to an out of tree build :) Cheers, Bruce > >>>>> } >>>>> >>>>> do_install_append() { >>>>> + bbwarn "ARRG" >>>>> if test -n "${KERNEL_DEVICETREE}"; then >>>>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>>>> - if [ ! -f ${DTS_FILE} ]; then >>>>> - echo "Warning: ${DTS_FILE} is not >>>>> available!" >>>>> - continue >>>>> + for DTB in ${KERNEL_DEVICETREE}; do >>>>> + if echo ${DTB} | grep -q '/dts/'; then >>>>> + bbwarn "${DTB} points to the full path >>>>> dts >>>>> file while it should have the target for use." >>>> >>>> >>>> >>>> This isn't reading right to me. What are you trying to say ? That >>>> KERNEL_DEVICE_TREE should be the name of the dts and not the path >>>> to that dts ? I'd say 'name' versus target. >>> >>> >>> You should name it 'imx6q-sabresd.dtb' for example. >> >> >> right, that's what I always do myself. So I'm really just suggesting that >> the bbwarn message be a bit more clear. i.e. >> >> bbwarn "${DTB} contains the full path to the the dts file, but only the >> dtb name should be used." > > Great; changed for v2 > >>>> What about testing for it's existence ? The old code used to do that. >>> >>> >>> It will fail if you ask it to build an invalid dtb file. >>> >>>>> + DTB=`basename ${DTB} | sed >>>>> 's,\.dts$,.dtb,g'` >>>> >>>> >>>> >>>> From the description of the patch .. this part is unexpected. >>>> You are changing more than what is used to compile the dtb, >>>> but the processing of the options. >>> >>> >>> It is not; I need to call it with the target. This is to support >>> converting from 'current' use to 'new' use and keep backward >>> compatibility. >> >> >> Right, so you are switching to dtb, which the kernel expects, not >> dts. The commit message should make this clear, right now it >> doesn't. You need to understand what the kernel is looking for to >> know why the change is being made. > > Ok; Changed for v2. > >>>>> fi >>>>> - DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." >>>>> '{print $1}'` >>>>> - DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>>>> "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>>>> - DTB_SYMLINK_NAME=`echo >>>>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTS_BASE_NAME}/g"` >>>>> - dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o >>>>> ${DTS_BASE_NAME} ${DTS_FILE} >>>>> - install -m 0644 ${DTS_BASE_NAME} >>>>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>>>> + DTB_BASE_NAME=`basename ${DTB} .dtb` >>>>> + DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed >>>>> "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>>>> + DTB_SYMLINK_NAME=`echo >>>>> ${KERNEL_IMAGE_SYMLINK_NAME} | sed "s/${MACHINE}/${DTB_BASE_NAME}/g"` >>>>> + oe_runmake ${DTB} >>>> >>>> >>>> >>>> The change log should explain this as well. You are switching the >>>> naming to DTB from the old DTS .. because ? The kernel build >>>> requires it ? something else ? You tell me :) >>> >>> >>> This is to support converting from 'current' use to 'new' use and keep >>> backward compatibility. >>> >>>>> + install -m 0644 ${B}/arch/${ARCH}/boot/${DTB} >>>>> ${D}/${KERNEL_IMAGEDEST}/devicetree-${DTB_SYMLINK_NAME}.dtb >>>>> done >>>>> fi >>>>> } >>>>> >>>>> do_deploy_append() { >>>>> if test -n "${KERNEL_DEVICETREE}"; then >>>>> - for DTS_FILE in ${KERNEL_DEVICETREE}; do >>>>> - if [ ! -f ${DTS_FILE} ]; then >>>>> - echo "Warning: ${DTS_FILE} is not >>>>> available!" >>>>> - continue >>>>> + for DTB in ${KERNEL_DEVICETREE}; do >>>>> + if echo ${DTB} | grep -q '/dts/'; then >>>>> + bbwarn "${DTB} points to the full path >>>>> dts >>>>> file while it should have the target for use. " >>>>> + DTB=`basename ${DTB} | sed >>>>> 's,\.dts$,.dtb,g'` >>>> >>>> >>>> >>>> Given that the old and new code repeats this block .. is there an >>>> opportunity to factor it out into a function ? >>> >>> >>> In a new patch? >> >> >> Could be, or just as part of this one. Either way works. > > Ok; I will take a look on that. > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-08-13 13:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-12 15:37 [PATCH] linux-dtb: Use kernel build system to generate the dtb files Otavio Salvador 2013-08-12 15:39 ` Otavio Salvador 2013-08-13 3:40 ` Bruce Ashfield 2013-08-13 12:46 ` Otavio Salvador 2013-08-13 13:06 ` Bruce Ashfield 2013-08-13 13:35 ` Otavio Salvador 2013-08-13 13:40 ` Bruce Ashfield
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.