| Message ID | 20230201195956.1758827-1-gsmecher@threespeedlogic.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] board/zynqmp/post-image.sh: Don't insist on a xilinx/ prefix for .dts files. | expand |
Hi Graeme, On Wed, 1 Feb 2023 11:59:56 -0800 Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from > buildroot into the linux tree, these .dts files are copied to > arch/arm64/boot. Unfortunately, the post-image.sh script expects to find > them in arch/arm64/boot/xilinx. > > This patch does not require the xilinx/ prefix to be present when > symlinking the device-tree to system.dtb where u-boot expects to find > it. > > Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> > --- > board/zynqmp/post-image.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/zynqmp/post-image.sh b/board/zynqmp/post-image.sh > index ed6dbe188c..c5de2db820 100755 > --- a/board/zynqmp/post-image.sh > +++ b/board/zynqmp/post-image.sh > @@ -5,7 +5,7 @@ > # devicetree listed in the config. > > FIRST_DT=$(sed -nr \ > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(xilinx/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ I think the following simpler change would also work. Can you check that please? - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\2|p' \
Hi Luca, On 2023-02-01 13:52, Luca Ceresoli wrote: > Hi Graeme, > > On Wed, 1 Feb 2023 11:59:56 -0800 > Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > >> If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from >> buildroot into the linux tree, these .dts files are copied to >> arch/arm64/boot. Unfortunately, the post-image.sh script expects to find >> them in arch/arm64/boot/xilinx. >> >> This patch does not require the xilinx/ prefix to be present when >> symlinking the device-tree to system.dtb where u-boot expects to find >> it. >> >> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> >> --- >> board/zynqmp/post-image.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/board/zynqmp/post-image.sh b/board/zynqmp/post-image.sh >> index ed6dbe188c..c5de2db820 100755 >> --- a/board/zynqmp/post-image.sh >> +++ b/board/zynqmp/post-image.sh >> @@ -5,7 +5,7 @@ >> # devicetree listed in the config. >> >> FIRST_DT=$(sed -nr \ >> - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ >> + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(xilinx/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ > > I think the following simpler change would also work. Can you check > that please? > > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\2|p' \ Assuming you intended \1, not \2 here - this suggestion breaks behaviour for a couple of defconfigs in the tree. If I run the ZCU106 defconfig through this regexp, I get: $ cat configs/zynqmp_zcu106_defconfig | sed -nr -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\1|p' xilinx/zynqmp-zcu106-revA ...this creates a dangling symlink in output/images (system.dtb -> xilinx/zynqmp-zcu106-revA.dtb). The right behaviour is to strip any prefixes that exist, without requiring any. Something like: - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(.*/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ This is less Xilinx-specific, although it seems a tad greedy. best, Graeme
Hi Graeme, On Wed, 1 Feb 2023 14:16:47 -0800 Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > Hi Luca, > > On 2023-02-01 13:52, Luca Ceresoli wrote: > > Hi Graeme, > > > > On Wed, 1 Feb 2023 11:59:56 -0800 > > Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > > > >> If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from > >> buildroot into the linux tree, these .dts files are copied to > >> arch/arm64/boot. Unfortunately, the post-image.sh script expects to find > >> them in arch/arm64/boot/xilinx. > >> > >> This patch does not require the xilinx/ prefix to be present when > >> symlinking the device-tree to system.dtb where u-boot expects to find > >> it. > >> > >> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> > >> --- > >> board/zynqmp/post-image.sh | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/board/zynqmp/post-image.sh b/board/zynqmp/post-image.sh > >> index ed6dbe188c..c5de2db820 100755 > >> --- a/board/zynqmp/post-image.sh > >> +++ b/board/zynqmp/post-image.sh > >> @@ -5,7 +5,7 @@ > >> # devicetree listed in the config. > >> > >> FIRST_DT=$(sed -nr \ > >> - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > >> + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(xilinx/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ > > > > I think the following simpler change would also work. Can you check > > that please? > > > > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\2|p' \ > > Assuming you intended \1, not \2 here - this suggestion breaks behaviour for a couple of defconfigs in the tree. If I run the ZCU106 defconfig through this regexp, I get: > > $ cat configs/zynqmp_zcu106_defconfig | sed -nr -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\1|p' > xilinx/zynqmp-zcu106-revA > > ...this creates a dangling symlink in output/images (system.dtb -> xilinx/zynqmp-zcu106-revA.dtb). Right. > The right behaviour is to strip any prefixes that exist, without requiring any. Something like: > > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(.*/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ > > This is less Xilinx-specific, although it seems a tad greedy. Yes, better keeping the xilinx prefix as this is a xilinx-specific script anyway. Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Hi Graeme, > Hi Luca, > > On 2023-02-01 13:52, Luca Ceresoli wrote: > > Hi Graeme, > > > > On Wed, 1 Feb 2023 11:59:56 -0800 > > Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > > > >> If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from > >> buildroot into the linux tree, these .dts files are copied to > >> arch/arm64/boot. Unfortunately, the post-image.sh script expects to > >> find them in arch/arm64/boot/xilinx. > >> > >> This patch does not require the xilinx/ prefix to be present when > >> symlinking the device-tree to system.dtb where u-boot expects to > >> find it. > >> > >> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> > >> --- > >> board/zynqmp/post-image.sh | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/board/zynqmp/post-image.sh > >> b/board/zynqmp/post-image.sh index ed6dbe188c..c5de2db820 100755 > >> --- a/board/zynqmp/post-image.sh > >> +++ b/board/zynqmp/post-image.sh > >> @@ -5,7 +5,7 @@ > >> # devicetree listed in the config. > >> > >> FIRST_DT=$(sed -nr \ > >> - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > >> + -e > >> + 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(xilinx/)?([-_/[:alnum:]\\. > >> + ]*).*"$|\2|p' \ > > > > I think the following simpler change would also work. Can you check > > that please? > > > > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > > + -e > > + 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\2| > > + p' \ > > Assuming you intended \1, not \2 here - this suggestion breaks behaviour for a couple of defconfigs in the tree. If I run the ZCU106 defconfig through this regexp, I get: > > $ cat configs/zynqmp_zcu106_defconfig | sed -nr -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="([-_/[:alnum:]\\.]*).*"$|\1|p' > xilinx/zynqmp-zcu106-revA > > ...this creates a dangling symlink in output/images (system.dtb -> xilinx/zynqmp-zcu106-revA.dtb). > Right. > The right behaviour is to strip any prefixes that exist, without requiring any. Something like: > > - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ > + -e > + 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(.*/)?([-_/[:alnum:]\\.]*).*"$ > + |\2|p' \ > > This is less Xilinx-specific, although it seems a tad greedy. > Yes, better keeping the xilinx prefix as this is a xilinx-specific script anyway. > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Thank you for your patch. Reviewed-by: Neal Frager <neal.frager@amd.com> Best regards, Neal Frager AMD
On Wed, 1 Feb 2023 11:59:56 -0800 Graeme Smecher <gsmecher@threespeedlogic.com> wrote: > If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from > buildroot into the linux tree, these .dts files are copied to > arch/arm64/boot. Unfortunately, the post-image.sh script expects to find > them in arch/arm64/boot/xilinx. > > This patch does not require the xilinx/ prefix to be present when > symlinking the device-tree to system.dtb where u-boot expects to find > it. > > Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> > --- > board/zynqmp/post-image.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to master, thanks. Thomas
diff --git a/board/zynqmp/post-image.sh b/board/zynqmp/post-image.sh index ed6dbe188c..c5de2db820 100755 --- a/board/zynqmp/post-image.sh +++ b/board/zynqmp/post-image.sh @@ -5,7 +5,7 @@ # devicetree listed in the config. FIRST_DT=$(sed -nr \ - -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/([-_/[:alnum:]\\.]*).*"$|\1|p' \ + -e 's|^BR2_LINUX_KERNEL_INTREE_DTS_NAME="(xilinx/)?([-_/[:alnum:]\\.]*).*"$|\2|p' \ ${BR2_CONFIG}) [ -z "${FIRST_DT}" ] || ln -fs ${FIRST_DT}.dtb ${BINARIES_DIR}/system.dtb
If using BR2_LINUX_KERNEL_CUSTOM_DTS_PATH to copy .dts files from buildroot into the linux tree, these .dts files are copied to arch/arm64/boot. Unfortunately, the post-image.sh script expects to find them in arch/arm64/boot/xilinx. This patch does not require the xilinx/ prefix to be present when symlinking the device-tree to system.dtb where u-boot expects to find it. Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com> --- board/zynqmp/post-image.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)