Message ID | 1528107661-25391-1-git-send-email-luca@lucaceresoli.net |
---|---|
State | Accepted |
Commit | c7df098a71e05dc81cee818747759e8060b59626 |
Delegated to: | Michal Simek |
Headers | show |
Series | [U-Boot,v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE | expand |
On 4.6.2018 12:21, 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. > > Since 'readlink -f' produces an empty string if the file does not > exist, we also add a check to ensure the file configured in > PMUFW_INIT_FILE exists. Otherwise the build would exit successfully, > but produce a boot.bin without PMUFW as if PMUFW_INIT_FILE were empty. > > Tested in the 12 possible combinations of: > - PMUFW_INIT_FILE empty, relative, absolute, non-existing > - 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> > Cc: Emmanuel Vadot <manu@bidouilliste.com> > > --- > > Changes v1 -> v2: > - avoid non-portable 'readlink -m' by adding an explicit check to > ensure the file exists > - test also the case where PMUFW_INIT_FILE points to a non-existing > file > --- > scripts/Makefile.spl | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > index 057389997de6..08660878b49e 100644 > --- a/scripts/Makefile.spl > +++ b/scripts/Makefile.spl > @@ -167,8 +167,14 @@ ifdef CONFIG_ARCH_ZYNQ > MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) > endif > ifdef CONFIG_ARCH_ZYNQMP > +ifneq ($(CONFIG_PMUFW_INIT_FILE),"") > +spl/boot.bin: zynqmp-check-pmufw > +zynqmp-check-pmufw: FORCE > + ( cd $(srctree) && test -r $(CONFIG_PMUFW_INIT_FILE) ) \ > + || ( echo "Cannot read $(CONFIG_PMUFW_INIT_FILE)" && false ) > +endif > MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ > - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) > + -n "$(shell cd $(srctree); readlink -f $(CONFIG_PMUFW_INIT_FILE))" > endif > > spl/boot.bin: $(obj)/u-boot-spl.bin FORCE > Emmanuel: Can you please try it and let me know if this works on BSD? Thanks, Michal
On Fri, 8 Jun 2018 08:28:02 +0200 Michal Simek <michal.simek@xilinx.com> wrote: > On 4.6.2018 12:21, 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. > > > > Since 'readlink -f' produces an empty string if the file does not > > exist, we also add a check to ensure the file configured in > > PMUFW_INIT_FILE exists. Otherwise the build would exit successfully, > > but produce a boot.bin without PMUFW as if PMUFW_INIT_FILE were empty. > > > > Tested in the 12 possible combinations of: > > - PMUFW_INIT_FILE empty, relative, absolute, non-existing > > - 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> > > Cc: Emmanuel Vadot <manu@bidouilliste.com> > > > > --- > > > > Changes v1 -> v2: > > - avoid non-portable 'readlink -m' by adding an explicit check to > > ensure the file exists > > - test also the case where PMUFW_INIT_FILE points to a non-existing > > file > > --- > > scripts/Makefile.spl | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > > index 057389997de6..08660878b49e 100644 > > --- a/scripts/Makefile.spl > > +++ b/scripts/Makefile.spl > > @@ -167,8 +167,14 @@ ifdef CONFIG_ARCH_ZYNQ > > MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) > > endif > > ifdef CONFIG_ARCH_ZYNQMP > > +ifneq ($(CONFIG_PMUFW_INIT_FILE),"") > > +spl/boot.bin: zynqmp-check-pmufw > > +zynqmp-check-pmufw: FORCE > > + ( cd $(srctree) && test -r $(CONFIG_PMUFW_INIT_FILE) ) \ > > + || ( echo "Cannot read $(CONFIG_PMUFW_INIT_FILE)" && false ) > > +endif > > MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ > > - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) > > + -n "$(shell cd $(srctree); readlink -f $(CONFIG_PMUFW_INIT_FILE))" > > endif > > > > spl/boot.bin: $(obj)/u-boot-spl.bin FORCE > > > > Emmanuel: Can you please try it and let me know if this works on BSD? > > Thanks, > Michal Yes no problem for readlink -f on BSD, I already answered to Luca. I don't know if you need a acked-by or something (and how to properly send it) since I'm in CC. Cheers,
On 8.6.2018 15:02, Emmanuel Vadot wrote: > On Fri, 8 Jun 2018 08:28:02 +0200 > Michal Simek <michal.simek@xilinx.com> wrote: > >> On 4.6.2018 12:21, 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. >>> >>> Since 'readlink -f' produces an empty string if the file does not >>> exist, we also add a check to ensure the file configured in >>> PMUFW_INIT_FILE exists. Otherwise the build would exit successfully, >>> but produce a boot.bin without PMUFW as if PMUFW_INIT_FILE were empty. >>> >>> Tested in the 12 possible combinations of: >>> - PMUFW_INIT_FILE empty, relative, absolute, non-existing >>> - 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> >>> Cc: Emmanuel Vadot <manu@bidouilliste.com> >>> >>> --- >>> >>> Changes v1 -> v2: >>> - avoid non-portable 'readlink -m' by adding an explicit check to >>> ensure the file exists >>> - test also the case where PMUFW_INIT_FILE points to a non-existing >>> file >>> --- >>> scripts/Makefile.spl | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl >>> index 057389997de6..08660878b49e 100644 >>> --- a/scripts/Makefile.spl >>> +++ b/scripts/Makefile.spl >>> @@ -167,8 +167,14 @@ ifdef CONFIG_ARCH_ZYNQ >>> MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) >>> endif >>> ifdef CONFIG_ARCH_ZYNQMP >>> +ifneq ($(CONFIG_PMUFW_INIT_FILE),"") >>> +spl/boot.bin: zynqmp-check-pmufw >>> +zynqmp-check-pmufw: FORCE >>> + ( cd $(srctree) && test -r $(CONFIG_PMUFW_INIT_FILE) ) \ >>> + || ( echo "Cannot read $(CONFIG_PMUFW_INIT_FILE)" && false ) >>> +endif >>> MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ >>> - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) >>> + -n "$(shell cd $(srctree); readlink -f $(CONFIG_PMUFW_INIT_FILE))" >>> endif >>> >>> spl/boot.bin: $(obj)/u-boot-spl.bin FORCE >>> >> >> Emmanuel: Can you please try it and let me know if this works on BSD? >> >> Thanks, >> Michal > > Yes no problem for readlink -f on BSD, I already answered to Luca. > I don't know if you need a acked-by or something (and how to properly > send it) since I'm in CC. ok. Then applied. Thanks, Michal
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 057389997de6..08660878b49e 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -167,8 +167,14 @@ ifdef CONFIG_ARCH_ZYNQ MKIMAGEFLAGS_boot.bin = -T zynqimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) endif ifdef CONFIG_ARCH_ZYNQMP +ifneq ($(CONFIG_PMUFW_INIT_FILE),"") +spl/boot.bin: zynqmp-check-pmufw +zynqmp-check-pmufw: FORCE + ( cd $(srctree) && test -r $(CONFIG_PMUFW_INIT_FILE) ) \ + || ( echo "Cannot read $(CONFIG_PMUFW_INIT_FILE)" && false ) +endif MKIMAGEFLAGS_boot.bin = -T zynqmpimage -R $(srctree)/$(CONFIG_BOOT_INIT_FILE) \ - -n $(srctree)/$(CONFIG_PMUFW_INIT_FILE) + -n "$(shell cd $(srctree); readlink -f $(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. Since 'readlink -f' produces an empty string if the file does not exist, we also add a check to ensure the file configured in PMUFW_INIT_FILE exists. Otherwise the build would exit successfully, but produce a boot.bin without PMUFW as if PMUFW_INIT_FILE were empty. Tested in the 12 possible combinations of: - PMUFW_INIT_FILE empty, relative, absolute, non-existing - 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> Cc: Emmanuel Vadot <manu@bidouilliste.com> --- Changes v1 -> v2: - avoid non-portable 'readlink -m' by adding an explicit check to ensure the file exists - test also the case where PMUFW_INIT_FILE points to a non-existing file --- scripts/Makefile.spl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)