All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
@ 2022-05-22 13:15 Neal Frager via buildroot
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf Neal Frager via buildroot
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Neal Frager via buildroot @ 2022-05-22 13:15 UTC (permalink / raw)
  To: buildroot; +Cc: wesley, Neal Frager, luca, giulio.benetti, michal.simek

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf
  2022-05-22 13:15 [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Neal Frager via buildroot
@ 2022-05-22 13:15 ` Neal Frager via buildroot
  2022-05-25 12:14   ` Luca Ceresoli
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 3/4] configs/zynqmp_zcu106_defconfig: " Neal Frager via buildroot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Neal Frager via buildroot @ 2022-05-22 13:15 UTC (permalink / raw)
  To: buildroot; +Cc: wesley, Neal Frager, luca, giulio.benetti, michal.simek

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH v1 3/4] configs/zynqmp_zcu106_defconfig: add parameters to generate extlinux.conf
  2022-05-22 13:15 [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Neal Frager via buildroot
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf Neal Frager via buildroot
@ 2022-05-22 13:15 ` Neal Frager via buildroot
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 4/4] configs/zynqmp_kria_kv260_defconfig: " Neal Frager via buildroot
  2022-05-25  8:57 ` [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Yann E. MORIN
  3 siblings, 0 replies; 12+ messages in thread
From: Neal Frager via buildroot @ 2022-05-22 13:15 UTC (permalink / raw)
  To: buildroot; +Cc: wesley, Neal Frager, luca, giulio.benetti, michal.simek

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH v1 4/4] configs/zynqmp_kria_kv260_defconfig: add parameters to generate extlinux.conf
  2022-05-22 13:15 [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Neal Frager via buildroot
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf Neal Frager via buildroot
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 3/4] configs/zynqmp_zcu106_defconfig: " Neal Frager via buildroot
@ 2022-05-22 13:15 ` Neal Frager via buildroot
  2022-05-25  8:57 ` [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Yann E. MORIN
  3 siblings, 0 replies; 12+ messages in thread
From: Neal Frager via buildroot @ 2022-05-22 13:15 UTC (permalink / raw)
  To: buildroot; +Cc: wesley, Neal Frager, luca, giulio.benetti, michal.simek

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-22 13:15 [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Neal Frager via buildroot
                   ` (2 preceding siblings ...)
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 4/4] configs/zynqmp_kria_kv260_defconfig: " Neal Frager via buildroot
@ 2022-05-25  8:57 ` Yann E. MORIN
  2022-05-25 12:12   ` Luca Ceresoli
  2022-05-25 13:31   ` Neal Frager
  3 siblings, 2 replies; 12+ messages in thread
From: Yann E. MORIN @ 2022-05-25  8:57 UTC (permalink / raw)
  To: Neal Frager; +Cc: luca, giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-25  8:57 ` [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Yann E. MORIN
@ 2022-05-25 12:12   ` Luca Ceresoli
  2022-05-25 13:33     ` Neal Frager
  2022-05-25 13:31   ` Neal Frager
  1 sibling, 1 reply; 12+ messages in thread
From: Luca Ceresoli @ 2022-05-25 12:12 UTC (permalink / raw)
  To: Yann E. MORIN, Neal Frager
  Cc: giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf
  2022-05-22 13:15 ` [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf Neal Frager via buildroot
@ 2022-05-25 12:14   ` Luca Ceresoli
  2022-05-25 13:34     ` Neal Frager
  0 siblings, 1 reply; 12+ messages in thread
From: Luca Ceresoli @ 2022-05-25 12:14 UTC (permalink / raw)
  To: Neal Frager, buildroot; +Cc: giulio.benetti, michal.simek, wesley

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-25  8:57 ` [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Yann E. MORIN
  2022-05-25 12:12   ` Luca Ceresoli
@ 2022-05-25 13:31   ` Neal Frager
  1 sibling, 0 replies; 12+ messages in thread
From: Neal Frager @ 2022-05-25 13:31 UTC (permalink / raw)
  To: Yann E. MORIN, Neal Frager
  Cc: luca, giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-25 12:12   ` Luca Ceresoli
@ 2022-05-25 13:33     ` Neal Frager
  2022-05-25 13:59       ` Luca Ceresoli
  0 siblings, 1 reply; 12+ messages in thread
From: Neal Frager @ 2022-05-25 13:33 UTC (permalink / raw)
  To: Luca Ceresoli, Yann E. MORIN, Neal Frager
  Cc: giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf
  2022-05-25 12:14   ` Luca Ceresoli
@ 2022-05-25 13:34     ` Neal Frager
  0 siblings, 0 replies; 12+ messages in thread
From: Neal Frager @ 2022-05-25 13:34 UTC (permalink / raw)
  To: Luca Ceresoli, Neal Frager, buildroot
  Cc: giulio.benetti, michal.simek, wesley

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-25 13:33     ` Neal Frager
@ 2022-05-25 13:59       ` Luca Ceresoli
  2022-05-25 14:02         ` Neal Frager
  0 siblings, 1 reply; 12+ messages in thread
From: Luca Ceresoli @ 2022-05-25 13:59 UTC (permalink / raw)
  To: Neal Frager, Yann E. MORIN, Neal Frager
  Cc: giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf
  2022-05-25 13:59       ` Luca Ceresoli
@ 2022-05-25 14:02         ` Neal Frager
  0 siblings, 0 replies; 12+ messages in thread
From: Neal Frager @ 2022-05-25 14:02 UTC (permalink / raw)
  To: Luca Ceresoli, Yann E. MORIN, Neal Frager
  Cc: giulio.benetti, wesley, michal.simek, buildroot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-05-25 14:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-22 13:15 [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Neal Frager via buildroot
2022-05-22 13:15 ` [Buildroot] [PATCH v1 2/4] configs/zynqmp_zcu102_defconfig: add parameters to generate extlinux.conf Neal Frager via buildroot
2022-05-25 12:14   ` Luca Ceresoli
2022-05-25 13:34     ` Neal Frager
2022-05-22 13:15 ` [Buildroot] [PATCH v1 3/4] configs/zynqmp_zcu106_defconfig: " Neal Frager via buildroot
2022-05-22 13:15 ` [Buildroot] [PATCH v1 4/4] configs/zynqmp_kria_kv260_defconfig: " Neal Frager via buildroot
2022-05-25  8:57 ` [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf Yann E. MORIN
2022-05-25 12:12   ` Luca Ceresoli
2022-05-25 13:33     ` Neal Frager
2022-05-25 13:59       ` Luca Ceresoli
2022-05-25 14:02         ` Neal Frager
2022-05-25 13:31   ` Neal Frager

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.