Re: [Buildroot] [PATCH v3 3/3] board/microchip/mpfs_icicle: update post-image script

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Jamie, All,

On 2023-08-18 11:43 +0100, Jamie Gibbons via buildroot spake thusly:
> The hss-payload-generator could not find where u-boot.bin was when
> looking for it using the config.yaml. This needs to be ran in the
> same directory as the u-boot.bin file to enable it to find it. Fix the
> post image script to allow an image to get built.
> 
> Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
> ---
>  board/microchip/mpfs_icicle/post-image.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/board/microchip/mpfs_icicle/post-image.sh b/board/microchip/mpfs_icicle/post-image.sh
> index 04b34bbd40..2368aa42f6 100755
> --- a/board/microchip/mpfs_icicle/post-image.sh
> +++ b/board/microchip/mpfs_icicle/post-image.sh
> @@ -2,7 +2,9 @@
>  HSS_PAYLOAD_GENERATOR=${HOST_DIR}/bin/hss-payload-generator
>  MKIMAGE=${HOST_DIR}/bin/mkimage

The HOST_DIR expansions here are not quoted, and it's weird that
shellcheck does not whine about it...

Can you fix those, please?

> -"${HSS_PAYLOAD_GENERATOR}" -c board/microchip/mpfs_icicle/config.yaml "${BINARIES_DIR}"/payload.bin
> -cp board/microchip/mpfs_icicle/mpfs_icicle.its "${BINARIES_DIR}"/mpfs_icicle.its
> +cd "${BINARIES_DIR}" || exit

BINARIES does exist when a post-image sctript is run.

But if you really want to exit on error, just add 'set -e' at the top of
the script (below the shabang).

> +"${HSS_PAYLOAD_GENERATOR}" -c "${CONFIG_DIR}"/board/microchip/mpfs_icicle/config.yaml payload.bin

Ditto, this is CONFIG_DIR is incorrect, because it does not work for
out-of-tree builds.

Since that yaml file is in the same directory as the post-image.sh, it
is better to write;

    #!/bin/bash
    set -e

    BOARD_DIR="${0%/*}"

    "${HSS_PAYLOAD_GENERATOR}" -c "${BOARD_DIR}"/config.yaml payload.bin

> +cd "${CONFIG_DIR}" || exit

CONFIG_DIR is not the correct location to return to. Indeed, it is the
directory where the .config file is, not necessarily the top directory
of the buildroot tree, like out-of-tree builds.

Instead you should probably use the pushd/popd construct:

    pushd "${BINARIES_DIR}"
    "${HSS_PAYLOAD_GENERATOR}" -c "${BOARD_DIR}"/config.yaml payload.bin
    popd

> +cp "${CONFIG_DIR}"/board/microchip/mpfs_icicle/mpfs_icicle.its "${BINARIES_DIR}"/mpfs_icicle.its
>  (cd "${BINARIES_DIR}" && "${MKIMAGE}" -f mpfs_icicle.its mpfs_icicle.itb)

Maybe we can include this MKIMAGE call in the pushd/popd section:

    pushd "${BINARIES_DIR}"
    "${HSS_PAYLOAD_GENERATOR}" -c "${BOARD_DIR}"/config.yaml payload.bin
    cp "${BOARD_DIR}"/mpfs_icicle.its "${BINARIES_DIR}"/mpfs_icicle.its
    "${MKIMAGE}" -f mpfs_icicle.its mpfs_icicle.itb)
    popd

I've marked this patch as changes requested (too much changes that I
can't test without spawning a build, which takes too long here...)

Regards,
Yann E. MORIN.

>  support/scripts/genimage.sh -c board/microchip/mpfs_icicle/genimage.cfg
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot