[Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Neal Frager via buildroot]

This patch uses the BR2_ROOTFS_POST_SCRIPT_ARGS to auto-generate the
extlinux.conf file, so developers will only need to modify the
board_defconfig file to change the console and boot file system locations.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 board/zynqmp/post-build.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/board/zynqmp/post-build.sh b/board/zynqmp/post-build.sh
index 9fd8bbf2c8..749784c3f3 100755
--- a/board/zynqmp/post-build.sh
+++ b/board/zynqmp/post-build.sh
@@ -4,5 +4,19 @@
 # in the binaries directory
 
 BOARD_DIR="$(dirname $0)"
+CONSOLE=$2
+ROOT=$3
+
+FILE=$BOARD_DIR/extlinux.conf
+if test -f "$FILE"; then
+    echo "$FILE aready exists."
+else
+  cat << __HEADER_EOF >> $BOARD_DIR/extlinux.conf
+label linux
+  kernel /Image
+  devicetree /system.dtb
+  append console=$CONSOLE root=/dev/$ROOT rw rootwait
+__HEADER_EOF
+fi
 
 install -m 0644 -D $BOARD_DIR/extlinux.conf $BINARIES_DIR/extlinux.conf
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf

[Neal Frager via buildroot]

This patch enables the zynqmp_zcu102_defconfig to auto-generate the
extlinux.conf file.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 configs/zynqmp_zcu102_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/zynqmp_zcu102_defconfig b/configs/zynqmp_zcu102_defconfig
index 5361981f5b..217fa0814b 100644
--- a/configs/zynqmp_zcu102_defconfig
+++ b/configs/zynqmp_zcu102_defconfig
@@ -2,6 +2,7 @@ BR2_aarch64=y
 BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
 BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
+BR2_ROOTFS_POST_SCRIPT_ARGS="ttyPS0,115200 mmcblk0p2"
 BR2_LINUX_KERNEL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,linux-xlnx,xlnx_rebase_v5.15_LTS_2022.1)/xlnx_rebase_v5.15_LTS_2022.1.tar.gz"
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf

[Luca Ceresoli]

Hi Neal,

On 22/05/22 15:15, Neal Frager wrote:
> This patch enables the zynqmp_zcu102_defconfig to auto-generate the
> extlinux.conf file.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  configs/zynqmp_zcu102_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/zynqmp_zcu102_defconfig b/configs/zynqmp_zcu102_defconfig
> index 5361981f5b..217fa0814b 100644
> --- a/configs/zynqmp_zcu102_defconfig
> +++ b/configs/zynqmp_zcu102_defconfig
> @@ -2,6 +2,7 @@ BR2_aarch64=y
>  BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
>  BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/post-build.sh"
>  BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
> +BR2_ROOTFS_POST_SCRIPT_ARGS="ttyPS0,115200 mmcblk0p2"

This patch as is has no effect without patch 3 where the extlinux.conf
file is actually removed. Thus, for the sake of commit atomicity, I
would squash patches 2+3 together.

Otherwise looks good. It's a nice cleanup, thank you!

-- 
Luca
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf

[Neal Frager]

Hi Luca,

> This patch enables the zynqmp_zcu102_defconfig to auto-generate the 
> extlinux.conf file.
>
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  configs/zynqmp_zcu102_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/zynqmp_zcu102_defconfig 
> b/configs/zynqmp_zcu102_defconfig index 5361981f5b..217fa0814b 100644
> --- a/configs/zynqmp_zcu102_defconfig
> +++ b/configs/zynqmp_zcu102_defconfig
> @@ -2,6 +2,7 @@ BR2_aarch64=y
>  BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
>  BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/post-build.sh"
>  BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
> +BR2_ROOTFS_POST_SCRIPT_ARGS="ttyPS0,115200 mmcblk0p2"

> This patch as is has no effect without patch 3 where the extlinux.conf file is actually removed. Thus, for the sake of commit atomicity, I would squash patches 2+3 together.

Yes, no problem.

> Otherwise looks good. It's a nice cleanup, thank you!

Thank you for your review!

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v1 3/4] configs/zynqmp_zcu106_defconfig: add parameters to generate extlinux.conf

[Neal Frager via buildroot]

This patch enables the zynqmp_zcu106_defconfig to auto-generate the
extlinux.conf file.

The board/zynqmp/extlinux.conf has been removed as it is no longer necessary.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 board/zynqmp/extlinux.conf      | 5 -----
 configs/zynqmp_zcu106_defconfig | 1 +
 2 files changed, 1 insertion(+), 5 deletions(-)
 delete mode 100644 board/zynqmp/extlinux.conf

diff --git a/board/zynqmp/extlinux.conf b/board/zynqmp/extlinux.conf
deleted file mode 100644
index ae3ec8614a..0000000000
--- a/board/zynqmp/extlinux.conf
+++ /dev/null
@@ -1,5 +0,0 @@
-label linux
-  kernel /Image
-  devicetree /system.dtb
-  append console=ttyPS0,115200 root=/dev/mmcblk0p2 rw rootwait
-
diff --git a/configs/zynqmp_zcu106_defconfig b/configs/zynqmp_zcu106_defconfig
index e1ea27a496..3d98dffe01 100644
--- a/configs/zynqmp_zcu106_defconfig
+++ b/configs/zynqmp_zcu106_defconfig
@@ -2,6 +2,7 @@ BR2_aarch64=y
 BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
 BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
+BR2_ROOTFS_POST_SCRIPT_ARGS="ttyPS0,115200 mmcblk0p2"
 BR2_LINUX_KERNEL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,linux-xlnx,xlnx_rebase_v5.15_LTS_2022.1)/xlnx_rebase_v5.15_LTS_2022.1.tar.gz"
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v1 4/4] configs/zynqmp_kria_kv260_defconfig: add parameters to generate extlinux.conf

[Neal Frager via buildroot]

This patch enables the zynqmp_kria_kv260_defconfig to auto-generate the
extlinux.conf file.

The board/zynqmp/kria/extlinux.conf and board/zynqmp/kria/post-build.sh have
been removed as they are no longer necessary.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 board/zynqmp/kria/extlinux.conf     | 4 ----
 board/zynqmp/kria/post-build.sh     | 8 --------
 configs/zynqmp_kria_kv260_defconfig | 3 ++-
 3 files changed, 2 insertions(+), 13 deletions(-)
 delete mode 100644 board/zynqmp/kria/extlinux.conf
 delete mode 100755 board/zynqmp/kria/post-build.sh

diff --git a/board/zynqmp/kria/extlinux.conf b/board/zynqmp/kria/extlinux.conf
deleted file mode 100644
index 663aabb7c3..0000000000
--- a/board/zynqmp/kria/extlinux.conf
+++ /dev/null
@@ -1,4 +0,0 @@
-label linux
-  kernel /Image
-  devicetree /system.dtb
-  append console=ttyPS1,115200 root=/dev/mmcblk1p2 rw rootwait
diff --git a/board/zynqmp/kria/post-build.sh b/board/zynqmp/kria/post-build.sh
deleted file mode 100755
index 9fd8bbf2c8..0000000000
--- a/board/zynqmp/kria/post-build.sh
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/sh
-
-# genimage will need to find the extlinux.conf
-# in the binaries directory
-
-BOARD_DIR="$(dirname $0)"
-
-install -m 0644 -D $BOARD_DIR/extlinux.conf $BINARIES_DIR/extlinux.conf
diff --git a/configs/zynqmp_kria_kv260_defconfig b/configs/zynqmp_kria_kv260_defconfig
index d4a72a0d19..c1366b705c 100644
--- a/configs/zynqmp_kria_kv260_defconfig
+++ b/configs/zynqmp_kria_kv260_defconfig
@@ -1,7 +1,8 @@
 BR2_aarch64=y
 BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_15=y
-BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/kria/post-build.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/zynqmp/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
+BR2_ROOTFS_POST_SCRIPT_ARGS="ttyPS1,115200 mmcblk1p2"
 BR2_LINUX_KERNEL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
 BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,Xilinx,linux-xlnx,xlnx_rebase_v5.15_LTS_2022.1)/xlnx_rebase_v5.15_LTS_2022.1.tar.gz"
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Yann E. MORIN]

Neal, All,

On 2022-05-22 07:15 -0600, Neal Frager via buildroot spake thusly:
> This patch uses the BR2_ROOTFS_POST_SCRIPT_ARGS to auto-generate the
> extlinux.conf file, so developers will only need to modify the
> board_defconfig file to change the console and boot file system locations.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  board/zynqmp/post-build.sh | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/board/zynqmp/post-build.sh b/board/zynqmp/post-build.sh
> index 9fd8bbf2c8..749784c3f3 100755
> --- a/board/zynqmp/post-build.sh
> +++ b/board/zynqmp/post-build.sh
> @@ -4,5 +4,19 @@
>  # in the binaries directory
>  
>  BOARD_DIR="$(dirname $0)"
> +CONSOLE=$2
> +ROOT=$3
> +
> +FILE=$BOARD_DIR/extlinux.conf
> +if test -f "$FILE"; then
> +    echo "$FILE aready exists."
> +else
> +  cat << __HEADER_EOF >> $BOARD_DIR/extlinux.conf

Why would you need append-redirection >> ? Afterall, you just tested
that the file did not already exist... Anyway, see below...

> +label linux
> +  kernel /Image
> +  devicetree /system.dtb
> +  append console=$CONSOLE root=/dev/$ROOT rw rootwait
> +__HEADER_EOF
> +fi
> 
>  install -m 0644 -D $BOARD_DIR/extlinux.conf $BINARIES_DIR/extlinux.conf

We try to avoid generating files in the source tree.

Instead, why not just:

    FILE="${BOARD_DIR}/extlinux.conf"
    if test -f "$FILE"; then
        install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
    else
        mkdir -p "${BINARIES_DIR}"
        cat <<-__HEADER_EOF >"${BINARIES_DIR}/extlinux.conf"
    TAB>label linux
    TAB>  kernel /Image
    TAB>  devicetree /system.dtb
    TAB>  append console=${CONSOLE} root=/dev/${ROOT} rw rootwait
    TAB>__HEADER_EOF
    fi

Note: the <<- rediretion removes leading TABs, so it is allows a nicer
indentation (so, replace TAB> above with an actual TAB, of course).

Also, please always evaluate variables using curly braces and quote the
expansion, like so:   "${foo}"   (in the here-document, quotes are not
needed, of course).

Eventualy, at the end of this series, the script will always be used for
boards that do not already have an extlinux.conf, the condition can be
dropped in a final patch, to just only keep the else part of the
condition, don't you think?

Regards,
Yann E. MORIN.

> -- 
> 2.17.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

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Luca Ceresoli]

Hi Neal, Yann,

On 25/05/22 10:57, Yann E. MORIN wrote:
> Neal, All,
> 
> On 2022-05-22 07:15 -0600, Neal Frager via buildroot spake thusly:
>> This patch uses the BR2_ROOTFS_POST_SCRIPT_ARGS to auto-generate the
>> extlinux.conf file, so developers will only need to modify the
>> board_defconfig file to change the console and boot file system locations.
>>
>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>> ---
>>  board/zynqmp/post-build.sh | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/board/zynqmp/post-build.sh b/board/zynqmp/post-build.sh
>> index 9fd8bbf2c8..749784c3f3 100755
>> --- a/board/zynqmp/post-build.sh
>> +++ b/board/zynqmp/post-build.sh
>> @@ -4,5 +4,19 @@
>>  # in the binaries directory
>>  
>>  BOARD_DIR="$(dirname $0)"
>> +CONSOLE=$2
>> +ROOT=$3
>> +
>> +FILE=$BOARD_DIR/extlinux.conf
>> +if test -f "$FILE"; then
>> +    echo "$FILE aready exists."
>> +else
>> +  cat << __HEADER_EOF >> $BOARD_DIR/extlinux.conf
> 
> Why would you need append-redirection >> ? Afterall, you just tested
> that the file did not already exist... Anyway, see below...

Sure, '>' is enough here.

>> +label linux
>> +  kernel /Image
>> +  devicetree /system.dtb
>> +  append console=$CONSOLE root=/dev/$ROOT rw rootwait
>> +__HEADER_EOF
>> +fi
>>
>>  install -m 0644 -D $BOARD_DIR/extlinux.conf $BINARIES_DIR/extlinux.conf
> 
> We try to avoid generating files in the source tree.
> 
> Instead, why not just:
> 
>     FILE="${BOARD_DIR}/extlinux.conf"
>     if test -f "$FILE"; then
>         install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"

Right, but use "${FILE}" after -D.

>     else
>         mkdir -p "${BINARIES_DIR}"
>         cat <<-__HEADER_EOF >"${BINARIES_DIR}/extlinux.conf"
>     TAB>label linux
>     TAB>  kernel /Image
>     TAB>  devicetree /system.dtb
>     TAB>  append console=${CONSOLE} root=/dev/${ROOT} rw rootwait
>     TAB>__HEADER_EOF
>     fi
> 
> Note: the <<- rediretion removes leading TABs, so it is allows a nicer
> indentation (so, replace TAB> above with an actual TAB, of course).

It's amazing how I always have some more shell trick to learn from Yann! :-D

> Also, please always evaluate variables using curly braces and quote the
> expansion, like so:   "${foo}"   (in the here-document, quotes are not
> needed, of course).
> 
> Eventualy, at the end of this series, the script will always be used for
> boards that do not already have an extlinux.conf, the condition can be
> dropped in a final patch, to just only keep the else part of the
> condition, don't you think?

Good point as well.

-- 
Luca
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Neal Frager]

Hi Luca,

>     FILE="${BOARD_DIR}/extlinux.conf"
>     if test -f "$FILE"; then
>         install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"

> Right, but use "${FILE}" after -D.

I just want to make sure I am not missing something.  I have changed the script to the following:

if test -f "${FILE}"; then
         install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"

Is there another -D I need to add somewhere?

>     else
>         mkdir -p "${BINARIES_DIR}"
>         cat <<-__HEADER_EOF >"${BINARIES_DIR}/extlinux.conf"
>     TAB>label linux
>     TAB>  kernel /Image
>     TAB>  devicetree /system.dtb
>     TAB>  append console=${CONSOLE} root=/dev/${ROOT} rw rootwait
>     TAB>__HEADER_EOF
>     fi
>
> Note: the <<- rediretion removes leading TABs, so it is allows a nicer 
> indentation (so, replace TAB> above with an actual TAB, of course).

> It's amazing how I always have some more shell trick to learn from Yann! :-D

Yes, I am learning a lot too!

> Also, please always evaluate variables using curly braces and quote the
> expansion, like so:   "${foo}"   (in the here-document, quotes are not
> needed, of course).
>
> Eventualy, at the end of this series, the script will always be used 
> for boards that do not already have an extlinux.conf, the condition 
> can be dropped in a final patch, to just only keep the else part of 
> the condition, don't you think?

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Luca Ceresoli]

Hi Neal,

On 25/05/22 15:33, Neal Frager wrote:
> Hi Luca,
> 
>>     FILE="${BOARD_DIR}/extlinux.conf"
>>     if test -f "$FILE"; then
>>         install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
> 
>> Right, but use "${FILE}" after -D.
> 
> I just want to make sure I am not missing something.  I have changed the script to the following:
> 
> if test -f "${FILE}"; then
>          install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
> 
> Is there another -D I need to add somewhere?

I didn't explain clearly, sorry. What I mean is quite trivial: you can
simplify this:

  install -m 0644 -D "${BOARD_DIR}/extlinux.conf" ...

with this:

  install -m 0644 -D "${FILE}" ...

-- 
Luca
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Neal Frager]

Hi Luca,

>
>>     FILE="${BOARD_DIR}/extlinux.conf"
>>     if test -f "$FILE"; then
>>         install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
>
>> Right, but use "${FILE}" after -D.
>
> I just want to make sure I am not missing something.  I have changed the script to the following:
>
> if test -f "${FILE}"; then
>          install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
>
> Is there another -D I need to add somewhere?

> I didn't explain clearly, sorry. What I mean is quite trivial: you can simplify this:

>  install -m 0644 -D "${BOARD_DIR}/extlinux.conf" ...

> with this:

>  install -m 0644 -D "${FILE}" ...

Ah, I see.  Yes, that would have been another simplification.
In the v2 patch set, I added the fourth patch that removes the if statement completely.
So in the end, this line is going away anyway.

Please let me know what you think of v2.

Thanks for your review and support!

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

[Neal Frager]

Hi Yann,

>  BOARD_DIR="$(dirname $0)"
> +CONSOLE=$2
> +ROOT=$3
> +
> +FILE=$BOARD_DIR/extlinux.conf
> +if test -f "$FILE"; then
> +    echo "$FILE aready exists."
> +else
> +  cat << __HEADER_EOF >> $BOARD_DIR/extlinux.conf

> Why would you need append-redirection >> ? Afterall, you just tested that the file did not already exist... Anyway, see below...

Thank you for catching this.

> +label linux
> +  kernel /Image
> +  devicetree /system.dtb
> +  append console=$CONSOLE root=/dev/$ROOT rw rootwait __HEADER_EOF fi
>
>  install -m 0644 -D $BOARD_DIR/extlinux.conf 
> $BINARIES_DIR/extlinux.conf

> We try to avoid generating files in the source tree.

> Instead, why not just:

    FILE="${BOARD_DIR}/extlinux.conf"
    if test -f "$FILE"; then
        install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
    else
        mkdir -p "${BINARIES_DIR}"
        cat <<-__HEADER_EOF >"${BINARIES_DIR}/extlinux.conf"
    TAB>label linux
    TAB>  kernel /Image
    TAB>  devicetree /system.dtb
    TAB>  append console=${CONSOLE} root=/dev/${ROOT} rw rootwait
    TAB>__HEADER_EOF
    fi

> Note: the <<- rediretion removes leading TABs, so it is allows a nicer indentation (so, replace TAB> above with an actual TAB, of course).

Very good solution.  I was not aware of the <<- trick!  Thank you!

> Also, please always evaluate variables using curly braces and quote the
> expansion, like so:   "${foo}"   (in the here-document, quotes are not
> needed, of course).

Ok, I will update this.

> Eventualy, at the end of this series, the script will always be used for boards that do not already have an extlinux.conf, the condition can be dropped in a final patch, to just only keep the else part of the condition, don't you think?

Yes, I will add a last patch to the set that removes the if statement leaving a more simple solution at the end.

Thank you for your review!

Best regards,
Neal Frager
AMD

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot