[U-Boot] [PATCH v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE

[U-Boot] [PATCH v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE

[Luca Ceresoli]

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
-- 
2.7.4

[U-Boot] [PATCH v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE

[Michal Simek]

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

[U-Boot] [PATCH v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE

[Emmanuel Vadot]

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,

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

[U-Boot] [PATCH v2] arm64: zynqmp: accept an absolute path for PMUFW_INIT_FILE

[Michal Simek]

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


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180611/008f571d/attachment.sig>