| Message ID | 1527771495-27289-1-git-send-email-luca@lucaceresoli.net |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE | expand |
On 31.5.2018 14:58, Luca Ceresoli wrote: > The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus > forcing it to be a relative path inside the U-Boot source tree. Since > the PMUFW is a binary file generated outside of U-Boot, the PMUFW > binary must be copied inside the U-Boot source tree before the > build. > > This generates a few problems: > > * if the source tree is shared among different out-of-tree builds, > they will pollute (and potentially corrupt) each other > * the source tree cannot be read-only > * any buildsystem must add a command to copy the PMUFW binary > * putting an externally-generated binary in the source tree is ugly > as hell > > Avoid these problems by accepting an absolute path for > PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" > prefix, but in order to keep backward compatibility we rather use the > shell and readlink to get the absolute path even when starting from a > relative path. > > Tested in the 9 possible combinations of: > - PMUFW_INIT_FILE empty, relative or absolute > - building in-tree, in subdir, in other directory > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Simon Glass <sjg@chromium.org> > --- > scripts/Makefile.spl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > index 057389997de6..ef1681ef4bf5 100644 > --- a/scripts/Makefile.spl > +++ b/scripts/Makefile.spl > @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) > endif > ifdef CONFIG_ARCH_ZYNQMP > MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ > - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) > + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" > endif > > spl/boot.bin: $(obj)/u-boot-spl.bin FORCE > looks good. Tom: Do we still have people who build u-boot on BSD system? I would like to know if this readlink -m works there. Thanks, Michal
On Thu, 31 May 2018 16:28:46 +0200 Michal Simek <michal.simek@xilinx.com> wrote: > On 31.5.2018 14:58, Luca Ceresoli wrote: > > The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus > > forcing it to be a relative path inside the U-Boot source tree. Since > > the PMUFW is a binary file generated outside of U-Boot, the PMUFW > > binary must be copied inside the U-Boot source tree before the > > build. > > > > This generates a few problems: > > > > * if the source tree is shared among different out-of-tree builds, > > they will pollute (and potentially corrupt) each other > > * the source tree cannot be read-only > > * any buildsystem must add a command to copy the PMUFW binary > > * putting an externally-generated binary in the source tree is ugly > > as hell > > > > Avoid these problems by accepting an absolute path for > > PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" > > prefix, but in order to keep backward compatibility we rather use the > > shell and readlink to get the absolute path even when starting from a > > relative path. > > > > Tested in the 9 possible combinations of: > > - PMUFW_INIT_FILE empty, relative or absolute > > - building in-tree, in subdir, in other directory > > > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > > Cc: Michal Simek <michal.simek@xilinx.com> > > Cc: Simon Glass <sjg@chromium.org> > > --- > > scripts/Makefile.spl | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > > index 057389997de6..ef1681ef4bf5 100644 > > --- a/scripts/Makefile.spl > > +++ b/scripts/Makefile.spl > > @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) > > endif > > ifdef CONFIG_ARCH_ZYNQMP > > MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ > > - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) > > + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" > > endif > > > > spl/boot.bin: $(obj)/u-boot-spl.bin FORCE > > > > looks good. > > Tom: Do we still have people who build u-boot on BSD system? > I would like to know if this readlink -m works there. > > Thanks, > Michal > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot We don't have -m on FreeBSD and after checking they don't either on OpenBSD and NetBSD. Cheers,
Hi Michal, Emmanuel, On 31/05/2018 17:14, Emmanuel Vadot wrote: > On Thu, 31 May 2018 16:28:46 +0200 > Michal Simek <michal.simek@xilinx.com> wrote: > >> On 31.5.2018 14:58, Luca Ceresoli wrote: >>> The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus >>> forcing it to be a relative path inside the U-Boot source tree. Since >>> the PMUFW is a binary file generated outside of U-Boot, the PMUFW >>> binary must be copied inside the U-Boot source tree before the >>> build. >>> >>> This generates a few problems: >>> >>> * if the source tree is shared among different out-of-tree builds, >>> they will pollute (and potentially corrupt) each other >>> * the source tree cannot be read-only >>> * any buildsystem must add a command to copy the PMUFW binary >>> * putting an externally-generated binary in the source tree is ugly >>> as hell >>> >>> Avoid these problems by accepting an absolute path for >>> PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" >>> prefix, but in order to keep backward compatibility we rather use the >>> shell and readlink to get the absolute path even when starting from a >>> relative path. >>> >>> Tested in the 9 possible combinations of: >>> - PMUFW_INIT_FILE empty, relative or absolute >>> - building in-tree, in subdir, in other directory >>> >>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> >>> Cc: Michal Simek <michal.simek@xilinx.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> scripts/Makefile.spl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl >>> index 057389997de6..ef1681ef4bf5 100644 >>> --- a/scripts/Makefile.spl >>> +++ b/scripts/Makefile.spl >>> @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) >>> endif >>> ifdef CONFIG_ARCH_ZYNQMP >>> MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ >>> - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) >>> + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" >>> endif >>> >>> spl/boot.bin: $(obj)/u-boot-spl.bin FORCE >>> >> >> looks good. >> >> Tom: Do we still have people who build u-boot on BSD system? >> I would like to know if this readlink -m works there. >> >> Thanks, >> Michal >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot > > We don't have -m on FreeBSD and after checking they don't either on > OpenBSD and NetBSD. Ok, then we need a portable version. Thanks for the heads up. The problem with 'readlink -f' is that for a mistyped filename it will call ./tools/mkimage <...> -n "" <...> and exit successfully producing a boot.bin without a pmufw. :( I'll have to find out where to check for file existence, which cannot be done where the readlink is in the present patch.
Hi Emmanuel, On 31/05/2018 17:14, Emmanuel Vadot wrote: > On Thu, 31 May 2018 16:28:46 +0200 > Michal Simek <michal.simek@xilinx.com> wrote: > >> On 31.5.2018 14:58, Luca Ceresoli wrote: >>> The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus >>> forcing it to be a relative path inside the U-Boot source tree. Since >>> the PMUFW is a binary file generated outside of U-Boot, the PMUFW >>> binary must be copied inside the U-Boot source tree before the >>> build. >>> >>> This generates a few problems: >>> >>> * if the source tree is shared among different out-of-tree builds, >>> they will pollute (and potentially corrupt) each other >>> * the source tree cannot be read-only >>> * any buildsystem must add a command to copy the PMUFW binary >>> * putting an externally-generated binary in the source tree is ugly >>> as hell >>> >>> Avoid these problems by accepting an absolute path for >>> PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" >>> prefix, but in order to keep backward compatibility we rather use the >>> shell and readlink to get the absolute path even when starting from a >>> relative path. >>> >>> Tested in the 9 possible combinations of: >>> - PMUFW_INIT_FILE empty, relative or absolute >>> - building in-tree, in subdir, in other directory >>> >>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> >>> Cc: Michal Simek <michal.simek@xilinx.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> scripts/Makefile.spl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl >>> index 057389997de6..ef1681ef4bf5 100644 >>> --- a/scripts/Makefile.spl >>> +++ b/scripts/Makefile.spl >>> @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) >>> endif >>> ifdef CONFIG_ARCH_ZYNQMP >>> MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ >>> - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) >>> + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" >>> endif >>> >>> spl/boot.bin: $(obj)/u-boot-spl.bin FORCE >>> >> >> looks good. >> >> Tom: Do we still have people who build u-boot on BSD system? >> I would like to know if this readlink -m works there. >> >> Thanks, >> Michal >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot > > We don't have -m on FreeBSD and after checking they don't either on > OpenBSD and NetBSD. I just sent v2, where readlink -f is used. However it also uses 'test -r' and 'false', can you confirm those commands portable on BSDs? Thanks,
Hi Luca, On Mon, 4 Jun 2018 12:23:17 +0200 Luca Ceresoli <luca@lucaceresoli.net> wrote: > Hi Emmanuel, > > On 31/05/2018 17:14, Emmanuel Vadot wrote: > > On Thu, 31 May 2018 16:28:46 +0200 > > Michal Simek <michal.simek@xilinx.com> wrote: > > > >> On 31.5.2018 14:58, Luca Ceresoli wrote: > >>> The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus > >>> forcing it to be a relative path inside the U-Boot source tree. Since > >>> the PMUFW is a binary file generated outside of U-Boot, the PMUFW > >>> binary must be copied inside the U-Boot source tree before the > >>> build. > >>> > >>> This generates a few problems: > >>> > >>> * if the source tree is shared among different out-of-tree builds, > >>> they will pollute (and potentially corrupt) each other > >>> * the source tree cannot be read-only > >>> * any buildsystem must add a command to copy the PMUFW binary > >>> * putting an externally-generated binary in the source tree is ugly > >>> as hell > >>> > >>> Avoid these problems by accepting an absolute path for > >>> PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" > >>> prefix, but in order to keep backward compatibility we rather use the > >>> shell and readlink to get the absolute path even when starting from a > >>> relative path. > >>> > >>> Tested in the 9 possible combinations of: > >>> - PMUFW_INIT_FILE empty, relative or absolute > >>> - building in-tree, in subdir, in other directory > >>> > >>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > >>> Cc: Michal Simek <michal.simek@xilinx.com> > >>> Cc: Simon Glass <sjg@chromium.org> > >>> --- > >>> scripts/Makefile.spl | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > >>> index 057389997de6..ef1681ef4bf5 100644 > >>> --- a/scripts/Makefile.spl > >>> +++ b/scripts/Makefile.spl > >>> @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) > >>> endif > >>> ifdef CONFIG_ARCH_ZYNQMP > >>> MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ > >>> - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) > >>> + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" > >>> endif > >>> > >>> spl/boot.bin: $(obj)/u-boot-spl.bin FORCE > >>> > >> > >> looks good. > >> > >> Tom: Do we still have people who build u-boot on BSD system? > >> I would like to know if this readlink -m works there. > >> > >> Thanks, > >> Michal > >> _______________________________________________ > >> U-Boot mailing list > >> U-Boot@lists.denx.de > >> https://lists.denx.de/listinfo/u-boot > > > > We don't have -m on FreeBSD and after checking they don't either on > > OpenBSD and NetBSD. > > I just sent v2, where readlink -f is used. However it also uses 'test > -r' and 'false', can you confirm those commands portable on BSDs? > > Thanks, > -- > Luca No problem for this two commands, thanks a lot.
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 057389997de6..ef1681ef4bf5 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -168,7 +168,7 @@ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) endif ifdef CONFIG_ARCH_ZYNQMP MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) + -n "$(shell cd $(srctree); readlink -m $(CONFIG_PMUFW_INIT_FILE))" endif spl/boot.bin: $(obj)/u-boot-spl.bin FORCE
The value of PMUFW_INIT_FILE is prefixed with "$(srctree)/", thus forcing it to be a relative path inside the U-Boot source tree. Since the PMUFW is a binary file generated outside of U-Boot, the PMUFW binary must be copied inside the U-Boot source tree before the build. This generates a few problems: * if the source tree is shared among different out-of-tree builds, they will pollute (and potentially corrupt) each other * the source tree cannot be read-only * any buildsystem must add a command to copy the PMUFW binary * putting an externally-generated binary in the source tree is ugly as hell Avoid these problems by accepting an absolute path for PMUFW_INIT_FILE. This would be as simple as removing the "$(srctree)/" prefix, but in order to keep backward compatibility we rather use the shell and readlink to get the absolute path even when starting from a relative path. Tested in the 9 possible combinations of: - PMUFW_INIT_FILE empty, relative or absolute - building in-tree, in subdir, in other directory Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> Cc: Michal Simek <michal.simek@xilinx.com> Cc: Simon Glass <sjg@chromium.org> --- scripts/Makefile.spl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)