[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[Abhimanyu Vishwakarma]

From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>

When called from BR2_ROOTFS_POST_IMAGE_SCRIPT, this script
ends up with following error:

Error: Missing argument

This is because, extra positional argument is also passed
along with BR2_ROOTFS_POST_SCRIPT_ARGS. genimage.sh didnt
had support to parse positional and optional argument
together.

Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
---
 Changes v7->v8
   - New file

 support/scripts/genimage.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/support/scripts/genimage.sh b/support/scripts/genimage.sh
index 0ed0e8bcc..2b5549fb0 100755
--- a/support/scripts/genimage.sh
+++ b/support/scripts/genimage.sh
@@ -5,13 +5,18 @@ die() {
   exit 1
 }
 
+# Parse arguments and put into argument list of the script
+eval set -- $(getopt -n genimage.sh -o c: -- "$@")
+
 GENIMAGE_TMP="${BUILD_DIR}/genimage.tmp"
 
-while getopts c: OPT ; do
-	case "${OPT}" in
-	c) GENIMAGE_CFG="${OPTARG}";;
-	:) die "option '${OPTARG}' expects a mandatory argument\n";;
-	\?) die "unknown option '${OPTARG}'\n";;
+while true ; do
+	case "$1" in
+	-c) [ ! -z "$2" ] || die "option '${1}' expects a mandatory argument\n";
+	    GENIMAGE_CFG="${2}";
+	    shift 2 ;;
+	--) shift 1; break ;;
+	*) die "unknown option '${1}'\n";;
 	esac
 done
 
-- 
2.11.0

[Buildroot] [PATCH v8 2/2] Add defconfig for MIPS Creator ci40

[Abhimanyu Vishwakarma]

From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>

Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
---
 Changes v1->v2
  - No change
 Changes v2->v3 (Suggested by Arnout)
  - Remove uImage and add fitImage generation
  - Tidy readme.txt
  - Tidy ci40_defconfig, remove custom toolchain and add wifi helper packages
  Some changes are not done:
  - Using git-helper:
    - Custom kernel doesnt provide tar file, so it didnt work for me
 Changes v3->v4 (Suggested by Arnout)
  - Use github helper for getting kernel/u-boot
  - rename fitImage.its -> fitImage.its.in to reflect as template file,
    remove hardcoded dtb filename
  - Find dtb file in output/images directory and use first dtb in fitImage
  - Remove image size from genimage.cfg
  - Tidy readme.txt and add section to explain how to stop u-boot autoboot,
    how to make it persistent, and other grammar
 Changes v4->v5 (Suggested by Thomas)
  - Use fixed kernel entry addr
  - Generate vmlinux.bin.gz from generate vmlinux.bin
  - Remove generating uImage (not required for anything now)
 Changes v5->v6
  - Use kernel generate fitimage
  - bump up u-boot version
  - Change emailid from imgtec.com -> personal id
 Changes v6->v7 (Suggested by Thomas)
  - Use genimage.sh script
  - Improve and fix grammer in readme.txt
 Changes v7->v8 (Suggested by Arnout)
  - Use genimage.sh script from POST_IMAGE_SCRIPT
  - Use uboot kconfig base config
  - Remove dts config from defconfig
  - Some other small cleanup
  - rebase on master

 board/ci40/genimage.cfg  | 12 +++++++++
 board/ci40/post-build.sh |  4 +++
 board/ci40/readme.txt    | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 configs/ci40_defconfig   | 48 ++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 board/ci40/genimage.cfg
 create mode 100755 board/ci40/post-build.sh
 create mode 100644 board/ci40/readme.txt
 create mode 100644 configs/ci40_defconfig

diff --git a/board/ci40/genimage.cfg b/board/ci40/genimage.cfg
new file mode 100644
index 000000000..0ffc91ce8
--- /dev/null
+++ b/board/ci40/genimage.cfg
@@ -0,0 +1,12 @@
+# Minimal SD card image
+#
+
+image sdcard.img {
+  hdimage {
+  }
+
+  partition rootfs {
+    partition-type = 0x83
+    image = "rootfs.ext4"
+  }
+}
diff --git a/board/ci40/post-build.sh b/board/ci40/post-build.sh
new file mode 100755
index 000000000..978ced585
--- /dev/null
+++ b/board/ci40/post-build.sh
@@ -0,0 +1,4 @@
+#!/bin/bash
+
+# Copy to target
+cp ${BINARIES_DIR}/vmlinux.gz.itb ${TARGET_DIR}/fitImage
diff --git a/board/ci40/readme.txt b/board/ci40/readme.txt
new file mode 100644
index 000000000..9d399c850
--- /dev/null
+++ b/board/ci40/readme.txt
@@ -0,0 +1,64 @@
+*********************
+* MIPS Creator CI40 *
+*********************
+
+This document details how to build and run a Buildroot system on the
+MIPS Creator CI40 platform. For more details about the CI40, see
+https://creatordev.io/ci40-iot-hub.html.
+
+How to build
+------------
+
+$ make ci40_defconfig
+$ make
+
+Prepare USB/MMC for boot
+------------------------
+
+On successful build, "sdcard.img" file will be created in 'output/images'
+folder.
+
+Use following command to write image to bootable device
+
+# dd if=./output/images/sdcard.img of=/dev/<your-microsd-or-usb-device>
+
+Booting from USB/MMC
+--------------------
+
+The boot loader is already present in NOR flash. To boot your newly generated
+Linux and root filesystem, you need to interrupt U-Boot autoboot. Current
+U-Boot is configured with 2 seconds of boot-delay, after expiry of this
+boot-delay timeout U-Boot starts booting the default image. To interrupt
+autoboot, press any key before the boot-delay time expires, U-Boot will
+stop the autoboot process and give a U-Boot prompt. You can now boot to
+your preferred boot method as describe below:
+
+From USB
+  pistachio # run usbboot
+
+From SD-Card
+  pistachio # run mmcboot
+
+Persistent boot command
+-----------------------
+
+To boot automatically to your preferred boot method, use following command to
+make it persistent, for example to automatically boot to usb:
+
+  pistachio # setenv bootcmd run usbboot
+  pistachio # saveenv
+
+Flash new bootloader
+--------------------
+
+Bootloader image will be available in 'output/images' folder.
+Use following command to flash new bootloader:
+
+# flashcp -v u-boot-pistachio_marduk-<version>.img /dev/mtd0
+
+Online docs
+-----------
+
+Mostly for OpenWRT but it is applicable to Buildroot
+https://docs.creatordev.io/ci40/guides/openwrt-platform/#overview
+
diff --git a/configs/ci40_defconfig b/configs/ci40_defconfig
new file mode 100644
index 000000000..2ea3afe47
--- /dev/null
+++ b/configs/ci40_defconfig
@@ -0,0 +1,48 @@
+# architecture
+BR2_mipsel=y
+BR2_mips_32r2=y
+
+# linux header same as custom kernel ie 4.4.x
+BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_4=y
+
+# kernel
+BR2_LINUX_KERNEL=y
+BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
+BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,CreatorDev,linux,openwrt-4.4.14)/linux-openwrt-4.4.14.tar.gz"
+BR2_LINUX_KERNEL_DEFCONFIG="pistachio"
+BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM=y
+# vmlinux.gz.itb image includes img/pistachio_marduk device tree
+BR2_LINUX_KERNEL_IMAGE_TARGET_NAME="vmlinux.gz.itb"
+
+# bootloader flash support
+BR2_PACKAGE_MTD=y
+
+# wireless firmware
+BR2_PACKAGE_UCCP420WLAN=y
+
+# wireless package
+BR2_PACKAGE_WIRELESS_TOOLS=y
+BR2_PACKAGE_WPA_SUPPLICANT=y
+BR2_PACKAGE_WPA_SUPPLICANT_PASSPHRASE=y
+
+# bootloader
+BR2_TARGET_UBOOT=y
+BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
+BR2_TARGET_UBOOT_BOARD_DEFCONFIG="pistachio_marduk"
+BR2_TARGET_UBOOT_CUSTOM_TARBALL=y
+BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,CreatorDev,u-boot,v1.0.5)/u-boot-CreatorDev-v1.0.5.tar.gz"
+BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
+BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-pistachio_marduk-2015.10-v1.0.5.img"
+
+# fitimage / image generation
+BR2_PACKAGE_HOST_UBOOT_TOOLS=y
+BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SUPPORT=y
+BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SIGNATURE_SUPPORT=y
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/ci40/post-build.sh"
+
+# image generation
+BR2_TARGET_ROOTFS_EXT2=y
+BR2_TARGET_ROOTFS_EXT2_4=y
+BR2_PACKAGE_HOST_GENIMAGE=y
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
+BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/ci40/genimage.cfg"
-- 
2.11.0

[Buildroot] [PATCH v8 2/2] Add defconfig for MIPS Creator ci40

[Arnout Vandecappelle]

 Hi Abhimanyu,

 I have a few tiny improvement suggestions still, but it looks good already so
on your next iteration (with the improved genimage.sh) you can add my

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

On 16-04-17 19:40, Abhimanyu Vishwakarma wrote:
> From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> 
> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>

[snip]
> diff --git a/board/ci40/post-build.sh b/board/ci40/post-build.sh
> new file mode 100755
> index 000000000..978ced585
> --- /dev/null
> +++ b/board/ci40/post-build.sh
> @@ -0,0 +1,4 @@
> +#!/bin/bash

 It's a bit of a personal preference: when the script doesn't rely on bashisms,
I prefer to use /bin/sh instead of /bin/bash.

> +
> +# Copy to target
> +cp ${BINARIES_DIR}/vmlinux.gz.itb ${TARGET_DIR}/fitImage
> diff --git a/board/ci40/readme.txt b/board/ci40/readme.txt
> new file mode 100644
> index 000000000..9d399c850
> --- /dev/null
> +++ b/board/ci40/readme.txt
> @@ -0,0 +1,64 @@
> +*********************
> +* MIPS Creator CI40 *
> +*********************
> +
> +This document details how to build and run a Buildroot system on the
> +MIPS Creator CI40 platform. For more details about the CI40, see
> +https://creatordev.io/ci40-iot-hub.html.
> +
> +How to build
> +------------
> +
> +$ make ci40_defconfig
> +$ make
> +
> +Prepare USB/MMC for boot
> +------------------------
> +
> +On successful build, "sdcard.img" file will be created in 'output/images'
> +folder.
> +
> +Use following command to write image to bootable device
> +
> +# dd if=./output/images/sdcard.img of=/dev/<your-microsd-or-usb-device>

 You could add here something like:

You usually have to be root to be allowed to do this

of just put sudo in front of the command line example (and start it with $
instead of #, of course).

> +
> +Booting from USB/MMC
> +--------------------
> +
> +The boot loader is already present in NOR flash. To boot your newly generated
> +Linux and root filesystem, you need to interrupt U-Boot autoboot. Current
> +U-Boot is configured with 2 seconds of boot-delay, after expiry of this
> +boot-delay timeout U-Boot starts booting the default image. To interrupt
> +autoboot, press any key before the boot-delay time expires, U-Boot will
> +stop the autoboot process and give a U-Boot prompt. You can now boot to
> +your preferred boot method as describe below:

 A little bit verbose, but very clear!

> +
> +From USB
> +  pistachio # run usbboot
> +
> +From SD-Card
> +  pistachio # run mmcboot
> +
> +Persistent boot command
> +-----------------------
> +
> +To boot automatically to your preferred boot method, use following command to
> +make it persistent, for example to automatically boot to usb:
> +
> +  pistachio # setenv bootcmd run usbboot
> +  pistachio # saveenv
> +
> +Flash new bootloader
> +--------------------
> +
> +Bootloader image will be available in 'output/images' folder.
> +Use following command to flash new bootloader:
> +
> +# flashcp -v u-boot-pistachio_marduk-<version>.img /dev/mtd0

 Although you were asked before to make this section shorter, I think it's a
little bit too terse now. How about

The bootloader image will be available in the 'output/images' folder. To flash
the new bootloader, copy it to the device and use the following command on the
device:

# flashcp -v u-boot-pistachio_marduk-<version>.img /dev/mtd0

> +
> +Online docs
> +-----------
> +
> +Mostly for OpenWRT but it is applicable to Buildroot
> +https://docs.creatordev.io/ci40/guides/openwrt-platform/#overview
> +
> diff --git a/configs/ci40_defconfig b/configs/ci40_defconfig
> new file mode 100644
> index 000000000..2ea3afe47
> --- /dev/null
> +++ b/configs/ci40_defconfig
> @@ -0,0 +1,48 @@
> +# architecture
> +BR2_mipsel=y
> +BR2_mips_32r2=y

 Perhaps it would make sense to add an explicit

BR2_MIPS_SOFT_FLOAT=y

? Although we don't do this for any of the other MIPSes and I doubt we'll change
the default any time soon for 32-bit MIPS.

 Regards,
 Arnout

> +
> +# linux header same as custom kernel ie 4.4.x
> +BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_4=y
> +
> +# kernel
> +BR2_LINUX_KERNEL=y
> +BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
> +BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call github,CreatorDev,linux,openwrt-4.4.14)/linux-openwrt-4.4.14.tar.gz"
> +BR2_LINUX_KERNEL_DEFCONFIG="pistachio"
> +BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM=y
> +# vmlinux.gz.itb image includes img/pistachio_marduk device tree
> +BR2_LINUX_KERNEL_IMAGE_TARGET_NAME="vmlinux.gz.itb"
> +
> +# bootloader flash support
> +BR2_PACKAGE_MTD=y
> +
> +# wireless firmware
> +BR2_PACKAGE_UCCP420WLAN=y
> +
> +# wireless package
> +BR2_PACKAGE_WIRELESS_TOOLS=y
> +BR2_PACKAGE_WPA_SUPPLICANT=y
> +BR2_PACKAGE_WPA_SUPPLICANT_PASSPHRASE=y
> +
> +# bootloader
> +BR2_TARGET_UBOOT=y
> +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
> +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="pistachio_marduk"
> +BR2_TARGET_UBOOT_CUSTOM_TARBALL=y
> +BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call github,CreatorDev,u-boot,v1.0.5)/u-boot-CreatorDev-v1.0.5.tar.gz"
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-pistachio_marduk-2015.10-v1.0.5.img"
> +
> +# fitimage / image generation
> +BR2_PACKAGE_HOST_UBOOT_TOOLS=y
> +BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SUPPORT=y
> +BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SIGNATURE_SUPPORT=y
> +BR2_ROOTFS_POST_BUILD_SCRIPT="board/ci40/post-build.sh"
> +
> +# image generation
> +BR2_TARGET_ROOTFS_EXT2=y
> +BR2_TARGET_ROOTFS_EXT2_4=y
> +BR2_PACKAGE_HOST_GENIMAGE=y
> +BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
> +BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/ci40/genimage.cfg"
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v8 2/2] Add defconfig for MIPS Creator ci40

[abhimanyu.v at gmail.com]

Thankyou again for review Arnout!
Sorry for late reply as was out of station!

On Tue, Apr 18, 2017 at 5:12 PM, Arnout Vandecappelle <arnout@mind.be>
wrote:

>  Hi Abhimanyu,
>
>  I have a few tiny improvement suggestions still, but it looks good
> already so
> on your next iteration (with the improved genimage.sh) you can add my
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> On 16-04-17 19:40, Abhimanyu Vishwakarma wrote:
> > From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> >
> > Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
>
> [snip]
> > diff --git a/board/ci40/post-build.sh b/board/ci40/post-build.sh
> > new file mode 100755
> > index 000000000..978ced585
> > --- /dev/null
> > +++ b/board/ci40/post-build.sh
> > @@ -0,0 +1,4 @@
> > +#!/bin/bash
>
>  It's a bit of a personal preference: when the script doesn't rely on
> bashisms,
> I prefer to use /bin/sh instead of /bin/bash.
>
>
Will do it.


> > +
> > +# Copy to target
> > +cp ${BINARIES_DIR}/vmlinux.gz.itb ${TARGET_DIR}/fitImage
> > diff --git a/board/ci40/readme.txt b/board/ci40/readme.txt
> > new file mode 100644
> > index 000000000..9d399c850
> > --- /dev/null
> > +++ b/board/ci40/readme.txt
> > @@ -0,0 +1,64 @@
> > +*********************
> > +* MIPS Creator CI40 *
> > +*********************
> > +
> > +This document details how to build and run a Buildroot system on the
> > +MIPS Creator CI40 platform. For more details about the CI40, see
> > +https://creatordev.io/ci40-iot-hub.html.
> > +
> > +How to build
> > +------------
> > +
> > +$ make ci40_defconfig
> > +$ make
> > +
> > +Prepare USB/MMC for boot
> > +------------------------
> > +
> > +On successful build, "sdcard.img" file will be created in
> 'output/images'
> > +folder.
> > +
> > +Use following command to write image to bootable device
> > +
> > +# dd if=./output/images/sdcard.img of=/dev/<your-microsd-or-usb-device>
>
>  You could add here something like:
>
> You usually have to be root to be allowed to do this
>
> of just put sudo in front of the command line example (and start it with $
> instead of #, of course).
>
>
Thanks will update.

> +
> > +Booting from USB/MMC
> > +--------------------
> > +
> > +The boot loader is already present in NOR flash. To boot your newly
> generated
> > +Linux and root filesystem, you need to interrupt U-Boot autoboot.
> Current
> > +U-Boot is configured with 2 seconds of boot-delay, after expiry of this
> > +boot-delay timeout U-Boot starts booting the default image. To interrupt
> > +autoboot, press any key before the boot-delay time expires, U-Boot will
> > +stop the autoboot process and give a U-Boot prompt. You can now boot to
> > +your preferred boot method as describe below:
>
>  A little bit verbose, but very clear!
>
>
Thankyou :)


> > +
> > +From USB
> > +  pistachio # run usbboot
> > +
> > +From SD-Card
> > +  pistachio # run mmcboot
> > +
> > +Persistent boot command
> > +-----------------------
> > +
> > +To boot automatically to your preferred boot method, use following
> command to
> > +make it persistent, for example to automatically boot to usb:
> > +
> > +  pistachio # setenv bootcmd run usbboot
> > +  pistachio # saveenv
> > +
> > +Flash new bootloader
> > +--------------------
> > +
> > +Bootloader image will be available in 'output/images' folder.
> > +Use following command to flash new bootloader:
> > +
> > +# flashcp -v u-boot-pistachio_marduk-<version>.img /dev/mtd0
>
>  Although you were asked before to make this section shorter, I think it's
> a
> little bit too terse now. How about
>
> The bootloader image will be available in the 'output/images' folder. To
> flash
> the new bootloader, copy it to the device and use the following command on
> the
> device:
>
> # flashcp -v u-boot-pistachio_marduk-<version>.img /dev/mtd0
>
>
It look good to me. I will also use the above comment to use sudo here too!


> > +
> > +Online docs
> > +-----------
> > +
> > +Mostly for OpenWRT but it is applicable to Buildroot
> > +https://docs.creatordev.io/ci40/guides/openwrt-platform/#overview
> > +
> > diff --git a/configs/ci40_defconfig b/configs/ci40_defconfig
> > new file mode 100644
> > index 000000000..2ea3afe47
> > --- /dev/null
> > +++ b/configs/ci40_defconfig
> > @@ -0,0 +1,48 @@
> > +# architecture
> > +BR2_mipsel=y
> > +BR2_mips_32r2=y
>
>  Perhaps it would make sense to add an explicit
>
> BR2_MIPS_SOFT_FLOAT=y
>
> ? Although we don't do this for any of the other MIPSes and I doubt we'll
> change
> the default any time soon for 32-bit MIPS.
>
>
Since we have never done for other MIPSes i think we should keep it
consistent? So will dropping this one.


>  Regards,
>  Arnout
>
> > +
> > +# linux header same as custom kernel ie 4.4.x
> > +BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_4=y
> > +
> > +# kernel
> > +BR2_LINUX_KERNEL=y
> > +BR2_LINUX_KERNEL_CUSTOM_TARBALL=y
> > +BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION="$(call
> github,CreatorDev,linux,openwrt-4.4.14)/linux-openwrt-4.4.14.tar.gz"
> > +BR2_LINUX_KERNEL_DEFCONFIG="pistachio"
> > +BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM=y
> > +# vmlinux.gz.itb image includes img/pistachio_marduk device tree
> > +BR2_LINUX_KERNEL_IMAGE_TARGET_NAME="vmlinux.gz.itb"
> > +
> > +# bootloader flash support
> > +BR2_PACKAGE_MTD=y
> > +
> > +# wireless firmware
> > +BR2_PACKAGE_UCCP420WLAN=y
> > +
> > +# wireless package
> > +BR2_PACKAGE_WIRELESS_TOOLS=y
> > +BR2_PACKAGE_WPA_SUPPLICANT=y
> > +BR2_PACKAGE_WPA_SUPPLICANT_PASSPHRASE=y
> > +
> > +# bootloader
> > +BR2_TARGET_UBOOT=y
> > +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
> > +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="pistachio_marduk"
> > +BR2_TARGET_UBOOT_CUSTOM_TARBALL=y
> > +BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION="$(call
> github,CreatorDev,u-boot,v1.0.5)/u-boot-CreatorDev-v1.0.5.tar.gz"
> > +BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
> > +BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-pistachio_
> marduk-2015.10-v1.0.5.img"
> > +
> > +# fitimage / image generation
> > +BR2_PACKAGE_HOST_UBOOT_TOOLS=y
> > +BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SUPPORT=y
> > +BR2_PACKAGE_HOST_UBOOT_TOOLS_FIT_SIGNATURE_SUPPORT=y
> > +BR2_ROOTFS_POST_BUILD_SCRIPT="board/ci40/post-build.sh"
> > +
> > +# image generation
> > +BR2_TARGET_ROOTFS_EXT2=y
> > +BR2_TARGET_ROOTFS_EXT2_4=y
> > +BR2_PACKAGE_HOST_GENIMAGE=y
> > +BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
> > +BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/ci40/genimage.cfg"
> >
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>

Regards,
Abhimanyu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170420/a9b2f075/attachment.html>

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[Gaël PORTAY]

Hello Abhimanyu,

On Sun, Apr 16, 2017 at 11:10:47PM +0530, Abhimanyu Vishwakarma wrote:
> From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> 
> When called from BR2_ROOTFS_POST_IMAGE_SCRIPT, this script
> ends up with following error:
> 
> Error: Missing argument
> 
> This is because, extra positional argument is also passed
> along with BR2_ROOTFS_POST_SCRIPT_ARGS. genimage.sh didnt
> had support to parse positional and optional argument
> together.
> 

Indeed, the problem comes from the first argument given to any post-image
scripts [1] (which is the binary directory).

A more simple fix consists in adding the shift instruction before right before
the while getopts.

+ 	shift
 	while getopts c: OPT ; do
 		case "${OPT}" in
 		c) GENIMAGE_CFG="${OPTARG}";;
 		:) die "option '${OPTARG}' expects a mandatory argument\n";;
 		\?) die "unknown option '${OPTARG}'\n";;
 	esac
 	done

> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> ---
>  Changes v7->v8
>    - New file
> 
>  support/scripts/genimage.sh | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/support/scripts/genimage.sh b/support/scripts/genimage.sh
> index 0ed0e8bcc..2b5549fb0 100755
> --- a/support/scripts/genimage.sh
> +++ b/support/scripts/genimage.sh
> @@ -5,13 +5,18 @@ die() {
>    exit 1
>  }
>  
> +# Parse arguments and put into argument list of the script
> +eval set -- $(getopt -n genimage.sh -o c: -- "$@")
> +
>  GENIMAGE_TMP="${BUILD_DIR}/genimage.tmp"
>  
> -while getopts c: OPT ; do
> -	case "${OPT}" in
> -	c) GENIMAGE_CFG="${OPTARG}";;
> -	:) die "option '${OPTARG}' expects a mandatory argument\n";;
> -	\?) die "unknown option '${OPTARG}'\n";;
> +while true ; do
> +	case "$1" in
> +	-c) [ ! -z "$2" ] || die "option '${1}' expects a mandatory argument\n";
> +	    GENIMAGE_CFG="${2}";
> +	    shift 2 ;;
> +	--) shift 1; break ;;
> +	*) die "unknown option '${1}'\n";;
>  	esac
>  done
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Furthermore, I see an error in traces. To display the traces properly, the
optstring required to start with a colon (:), and the trailling \n is not
required.

	while getopts :c: OPT ; do

[1] https://github.com/buildroot/buildroot/blob/2017.02.1/Makefile#L717

Regards,
Ga?l

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[abhimanyu.v at gmail.com]

Thankyou Ga?l for review!

On Mon, Apr 17, 2017 at 3:08 AM, Ga?l PORTAY <
gael.portay@savoirfairelinux.com> wrote:

> Hello Abhimanyu,
>
> On Sun, Apr 16, 2017 at 11:10:47PM +0530, Abhimanyu Vishwakarma wrote:
> > From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> >
> > When called from BR2_ROOTFS_POST_IMAGE_SCRIPT, this script
> > ends up with following error:
> >
> > Error: Missing argument
> >
> > This is because, extra positional argument is also passed
> > along with BR2_ROOTFS_POST_SCRIPT_ARGS. genimage.sh didnt
> > had support to parse positional and optional argument
> > together.
> >
>
> Indeed, the problem comes from the first argument given to any post-image
> scripts [1] (which is the binary directory).
>
> A more simple fix consists in adding the shift instruction before right
> before
> the while getopts.
>
> +       shift
>         while getopts c: OPT ; do
>                 case "${OPT}" in
>                 c) GENIMAGE_CFG="${OPTARG}";;
>                 :) die "option '${OPTARG}' expects a mandatory
> argument\n";;
>                 \?) die "unknown option '${OPTARG}'\n";;
>         esac
>         done
>
>
Yes you are right, this is simple fix. What i had in mind was to make the
script independent of any positional argument. With proposed solution if in
future we add extra argument to POST_SCRIPTS then this script can cope with
it. Please let me know your thought. I will prepare patch accordingly.

<snip>


> Furthermore, I see an error in traces. To display the traces properly, the
> optstring required to start with a colon (:), and the trailling \n is not
> required.
>
>         while getopts :c: OPT ; do
>
>
Thanks, I will fix it.


> [1] https://github.com/buildroot/buildroot/blob/2017.02.1/Makefile#L717
>
> Regards,
> Ga?l
>

Regards
Abhimanyu V
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170417/3e8d2afd/attachment.html>

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[Gaël PORTAY]

Hello Abhimanyu,

On Mon, Apr 17, 2017 at 03:38:27PM +0530, abhimanyu.v at gmail.com wrote:
> Thankyou Ga?l for review!
> 
> [...]
> >
> > Indeed, the problem comes from the first argument given to any post-image
> > scripts [1] (which is the binary directory).
> >
> > A more simple fix consists in adding the shift instruction before right
> > before
> > the while getopts.
> >
> > +       shift
> >         while getopts c: OPT ; do
> >                 case "${OPT}" in
> >                 c) GENIMAGE_CFG="${OPTARG}";;
> >                 :) die "option '${OPTARG}' expects a mandatory
> > argument\n";;
> >                 \?) die "unknown option '${OPTARG}'\n";;
> >         esac
> >         done
> >
> >
> Yes you are right, this is simple fix. What i had in mind was to make the
> script independent of any positional argument. With proposed solution if in
> future we add extra argument to POST_SCRIPTS then this script can cope with
> it. Please let me know your thought. I will prepare patch accordingly.
> 
> <snip>
> 

Afterward, my suggestion about shift does not suit me.

If genimage.sh is run inside a post-image script, this same script have to give
a fake parameter as first argument.

	#!/bin/sh
	#
	# BR post-image script
	#
	# (...)

	support/scripts/genimage.sh "$1" -c path/to/genimage.cfg

Initially, genimage.sh script was NOT intend to be run either as or inside a
post-image. It was a Makefile target [1]. [2] and [3] are use cases.

Maybe, a cleaner solution consists in updating the Makefile to remove this first
argument given to both post-build and post-image scripts. But it breaks the
existing.

Thomas, Arnout, do you have a better idea?

I had a quick look to scripts in-tree; they do not seem to use this parameter.

Instead, they access directly to $TARGET_DIR or $BINARIES_DIR values using the
environment variables.

For extra arguments, they use $2, $3; they need to be updated.

> > Furthermore, I see an error in traces. To display the traces properly, the
> > optstring required to start with a colon (:), and the trailling \n is not
> > required.
> >
> >         while getopts :c: OPT ; do
> >
> >
> Thanks, I will fix it.
> 
> 
> > [1] https://github.com/buildroot/buildroot/blob/2017.02.1/Makefile#L717
> >
> > Regards,
> > Ga?l
> >
> 
> Regards
> Abhimanyu V

> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,
Ga?l

[1] http://patchwork.ozlabs.org/patch/744825/
[2] http://patchwork.ozlabs.org/patch/744826/
[3] http://patchwork.ozlabs.org/patch/744824/> 

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[abhimanyu.v at gmail.com]

Hello Ga?l,

On Mon, Apr 17, 2017 at 7:50 PM, Ga?l PORTAY <
gael.portay@savoirfairelinux.com> wrote:

> Hello Abhimanyu,
>
> On Mon, Apr 17, 2017 at 03:38:27PM +0530, abhimanyu.v at gmail.com wrote:
> > Thankyou Ga?l for review!
> >
>

<snip>


> Afterward, my suggestion about shift does not suit me.
>
> If genimage.sh is run inside a post-image script, this same script have to
> give
> a fake parameter as first argument.
>
>         #!/bin/sh
>         #
>         # BR post-image script
>         #
>         # (...)
>
>         support/scripts/genimage.sh "$1" -c path/to/genimage.cfg
>
>
The proposed changes to script doesn't have above issue.

<snip>

Regards
Abhimanyu V
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170417/67dec21f/attachment.html>

[Buildroot] genimage.sh arguments [was: [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT]

[Arnout Vandecappelle]

 [Adding Etienne to the Cc as the main contributor of genimage.sh]

On 17-04-17 16:20, Ga?l PORTAY wrote:
> Hello Abhimanyu,
> 
[snip]
> Initially, genimage.sh script was NOT intend to be run either as or inside a
> post-image. It was a Makefile target [1]. [2] and [3] are use cases.

 True, but since [1] isn't applied yet and is still under discussion, we really
want to be able to call it as a post-image script directly.


> Maybe, a cleaner solution consists in updating the Makefile to remove this first
> argument given to both post-build and post-image scripts. But it breaks the
> existing.

 I don't think we can do that. Most "interesting" post-* scripts are out of
tree, so we can't change them. Although updating Buildroot is expected to have
some implications on the user integration layer (cfr. the change in br2-external
handling), we try to avoid it.

 It would indeed be good to remove that first argument, but then I think we have
to go through a (long) deprecation period. That means right now: mark it as
legacy in the manual and help text.

> Thomas, Arnout, do you have a better idea?
> 
> I had a quick look to scripts in-tree; they do not seem to use this parameter.
> 
> Instead, they access directly to $TARGET_DIR or $BINARIES_DIR values using the
> environment variables.
> 
> For extra arguments, they use $2, $3; they need to be updated.

 ... which proves that deprecating it is not easy, because it's fairly difficult
to make a script compatible with both the "new" and the "old" way - cfr. the
special care taking in the genimage.sh script, and that's relatively easy
because the argument has a -c added to it.

[snip]
> [1] http://patchwork.ozlabs.org/patch/744825/
> [2] http://patchwork.ozlabs.org/patch/744826/
> [3] http://patchwork.ozlabs.org/patch/744824/


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] genimage.sh arguments [was: [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT]

[Gaël PORTAY]

Arnout,

On Tue, Apr 18, 2017 at 01:51:28PM +0200, Arnout Vandecappelle wrote:
>  [Adding Etienne to the Cc as the main contributor of genimage.sh]
> 
> On 17-04-17 16:20, Ga?l PORTAY wrote:
> > Hello Abhimanyu,
> > 
> [snip]
> > Initially, genimage.sh script was NOT intend to be run either as or inside a
> > post-image. It was a Makefile target [1]. [2] and [3] are use cases.
> 
>  True, but since [1] isn't applied yet and is still under discussion, we really
> want to be able to call it as a post-image script directly.
> 

Okay.

The question is, do we "accept" to run genimage.sh inside a post-image script?

If yes, developpers have to call genimage.sh with a fake first argument (or the
binary directory).

If no, the shift is a fair solution alongside a comment.

A second solution consists in using this first argument in the script:

	# First argument is the binary directory
	bindir="$1"
	shift
	while getopts :c: OPT ; do
		case "${OPT}" in
		c) GENIMAGE_CFG="${OPTARG}";;
		:) die "option '${OPTARG}' expects a mandatory argument";;
		\?) die "unknown option '${OPTARG}'";;
		esac
	done

	# ...

	genimage \
	--rootpath "${TARGET_DIR}"     \
	--tmppath "${GENIMAGE_TMP}"    \
	--inputpath "${bindir}"  \
	--outputpath "${bindir}" \
	--config "${GENIMAGE_CFG}"

But, if we deprecated the first argument, it also impacts the genimage.sh.

> 
> > Maybe, a cleaner solution consists in updating the Makefile to remove this first
> > argument given to both post-build and post-image scripts. But it breaks the
> > existing.
> 
>  I don't think we can do that. Most "interesting" post-* scripts are out of
> tree, so we can't change them. Although updating Buildroot is expected to have
> some implications on the user integration layer (cfr. the change in br2-external
> handling), we try to avoid it.
> 

That is why I added Thomas and you in copy :)

>  It would indeed be good to remove that first argument, but then I think we have
> to go through a (long) deprecation period. That means right now: mark it as
> legacy in the manual and help text.
> 
> > Thomas, Arnout, do you have a better idea?
> > 
> > I had a quick look to scripts in-tree; they do not seem to use this parameter.
> > 
> > Instead, they access directly to $TARGET_DIR or $BINARIES_DIR values using the
> > environment variables.
> > 
> > For extra arguments, they use $2, $3; they need to be updated.
> 
>  ... which proves that deprecating it is not easy, because it's fairly difficult
> to make a script compatible with both the "new" and the "old" way - cfr. the
> special care taking in the genimage.sh script, and that's relatively easy
> because the argument has a -c added to it.

I agree.

> 
> [snip]
> > [1] http://patchwork.ozlabs.org/patch/744825/
> > [2] http://patchwork.ozlabs.org/patch/744826/
> > [3] http://patchwork.ozlabs.org/patch/744824/
> 

Something that would be nice is to allow to give extra arguments to genimage.
genimage.sh is acting as a wrapper.

	BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
	BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/boardname/genimage.cfg -- --loglevel 1"

Regards,
Ga?l

[Buildroot] genimage.sh arguments [was: [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT]

[Arnout Vandecappelle]

On 18-04-17 15:52, Ga?l PORTAY wrote:
> Arnout,
> 
> On Tue, Apr 18, 2017 at 01:51:28PM +0200, Arnout Vandecappelle wrote:
>>  [Adding Etienne to the Cc as the main contributor of genimage.sh]
>>
>> On 17-04-17 16:20, Ga?l PORTAY wrote:
>>> Hello Abhimanyu,
>>>
>> [snip]
>>> Initially, genimage.sh script was NOT intend to be run either as or inside a
>>> post-image. It was a Makefile target [1]. [2] and [3] are use cases.
>>
>>  True, but since [1] isn't applied yet and is still under discussion, we really
>> want to be able to call it as a post-image script directly.
>>
> 
> Okay.
> 
> The question is, do we "accept" to run genimage.sh inside a post-image script?

 Of course we do, because in some cases, genimage.sh cannot replace the
post-image script - even with [1] applied, you still need a post-image script
that calls genimage, and the genimage.sh wrapper is still useful in that case.


> If yes, developpers have to call genimage.sh with a fake first argument (or the
> binary directory).

 Well, Abhimanyu's patch avoids that, by ignoring non-option arguments. And this
was the reason I told Etienne to use the -c option to begin with: to make
handling of options more flexible.

> 
> If no, the shift is a fair solution alongside a comment.
> 
> A second solution consists in using this first argument in the script:
> 
> 	# First argument is the binary directory
> 	bindir="$1"
> 	shift
> 	while getopts :c: OPT ; do
> 		case "${OPT}" in
> 		c) GENIMAGE_CFG="${OPTARG}";;
> 		:) die "option '${OPTARG}' expects a mandatory argument";;
> 		\?) die "unknown option '${OPTARG}'";;
> 		esac
> 	done
> 
> 	# ...
> 
> 	genimage \
> 	--rootpath "${TARGET_DIR}"     \
> 	--tmppath "${GENIMAGE_TMP}"    \
> 	--inputpath "${bindir}"  \
> 	--outputpath "${bindir}" \
> 	--config "${GENIMAGE_CFG}"
> 
> But, if we deprecated the first argument, it also impacts the genimage.sh.

 Exactly. And it's not very useful, since it should *never* be different from
BINARIES_DIR.


>>> Maybe, a cleaner solution consists in updating the Makefile to remove this first
>>> argument given to both post-build and post-image scripts. But it breaks the
>>> existing.
>>
>>  I don't think we can do that. Most "interesting" post-* scripts are out of
>> tree, so we can't change them. Although updating Buildroot is expected to have
>> some implications on the user integration layer (cfr. the change in br2-external
>> handling), we try to avoid it.
>>
> 
> That is why I added Thomas and you in copy :)
> 
>>  It would indeed be good to remove that first argument, but then I think we have
>> to go through a (long) deprecation period. That means right now: mark it as
>> legacy in the manual and help text.
>>
>>> Thomas, Arnout, do you have a better idea?
>>>
>>> I had a quick look to scripts in-tree; they do not seem to use this parameter.
>>>
>>> Instead, they access directly to $TARGET_DIR or $BINARIES_DIR values using the
>>> environment variables.
>>>
>>> For extra arguments, they use $2, $3; they need to be updated.
>>
>>  ... which proves that deprecating it is not easy, because it's fairly difficult
>> to make a script compatible with both the "new" and the "old" way - cfr. the
>> special care taking in the genimage.sh script, and that's relatively easy
>> because the argument has a -c added to it.
> 
> I agree.
> 
>>
>> [snip]
>>> [1] http://patchwork.ozlabs.org/patch/744825/
>>> [2] http://patchwork.ozlabs.org/patch/744826/
>>> [3] http://patchwork.ozlabs.org/patch/744824/
>>
> 
> Something that would be nice is to allow to give extra arguments to genimage.
> genimage.sh is acting as a wrapper.
> 
> 	BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
> 	BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/boardname/genimage.cfg -- --loglevel 1"

 Yes, that could be useful. Although the -- makes is difficult again because
it's not compatible with getopt so Abhimanyu's patch wouldn't work anymore. But
I could accept it with extra quoting:

BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/boardname/genimage.cfg -O '--loglevel 1'"

 However, I don't really see the usefulness of this feature. loglevel is in fact
the only option that you could usefully pass on to genimage. So just adding a -v
option to genimage.sh is probably more convenient.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v8 1/2] genimage.sh: fix calling from BR2_ROOTFS_POST_IMAGE_SCRIPT

[Arnout Vandecappelle]

On 16-04-17 19:40, Abhimanyu Vishwakarma wrote:
> From: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> 
> When called from BR2_ROOTFS_POST_IMAGE_SCRIPT, this script
> ends up with following error:
> 
> Error: Missing argument
> 
> This is because, extra positional argument is also passed
> along with BR2_ROOTFS_POST_SCRIPT_ARGS. genimage.sh didnt
> had support to parse positional and optional argument
> together.
> 
> Signed-off-by: Abhimanyu Vishwakarma <Abhimanyu.V@gmail.com>
> ---
>  Changes v7->v8
>    - New file
> 
>  support/scripts/genimage.sh | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/support/scripts/genimage.sh b/support/scripts/genimage.sh
> index 0ed0e8bcc..2b5549fb0 100755
> --- a/support/scripts/genimage.sh
> +++ b/support/scripts/genimage.sh
> @@ -5,13 +5,18 @@ die() {
>    exit 1
>  }
>  
> +# Parse arguments and put into argument list of the script
> +eval set -- $(getopt -n genimage.sh -o c: -- "$@")

 The proper way is to quote the $(getopt ...) part, because the eval will do an
additional level of unquoting. Otherwise if an option contains a shell meta
character (e.g. $) it will be expanded by eval. Also, instead of explicitly
putting genimage.sh, it's better to pass the basename with which it was called.
Also, getopt will print an error message and return false if something is wrong
with the options, so better handle that.

 So:

opts="$(getopt -n "${0##*/}" -o c: -- "$@")" || exit $?
eval set -- "$opts"


> +
>  GENIMAGE_TMP="${BUILD_DIR}/genimage.tmp"
>  
> -while getopts c: OPT ; do
> -	case "${OPT}" in
> -	c) GENIMAGE_CFG="${OPTARG}";;
> -	:) die "option '${OPTARG}' expects a mandatory argument\n";;
> -	\?) die "unknown option '${OPTARG}'\n";;
> +while true ; do
> +	case "$1" in
> +	-c) [ ! -z "$2" ] || die "option '${1}' expects a mandatory argument\n";

 This check shouldn't be needed since it's already done by getopt.

> +	    GENIMAGE_CFG="${2}";
> +	    shift 2 ;;
> +	--) shift 1; break ;;

 This could use a comment, e.g. # Discard all non-option parameters

 Also, since you use one line per statement above, do it here as well.

 In fact, I'm personally in favour of splitting off the case part (i.e., -c) and
--) ) on a separate line as well.

> +	*) die "unknown option '${1}'\n";;

 This is not strictly needed since getopt will already detect it, but I guess
better to keep it as an emergency fallback.

 Regards,
 Arnout
>  	esac
>  done
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF