| Message ID | 1472133887-34746-3-git-send-email-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Thu, 25 Aug 2016 17:04:40 +0300, Andy Shevchenko wrote: > There are several x86 boards that can utilize same image creation > infrastructure. > > Add common configuration, cmdline, post-image.sh, post-build.sh scripts and > placeholders for their extensions. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for working on this, it's great to see contributions from Intel for those platforms. However, I'm afraid, quite a few things in there really aren't done in the Buildroot spirit/philosophy. See some comments below. > diff --git a/board/intel/common/README.rst b/board/intel/common/README.rst > new file mode 100644 > index 0000000..e427067 > --- /dev/null > +++ b/board/intel/common/README.rst We don't use the .rst format anywhere else in the tree, so please stick to a regular readme.txt, like we have for all other platforms. > +Building an image > +~~~~~~~~~~~~~~~~~ > + > +For building a normal bootable image, you need to do following steps: > + > +1) Build your own kernel. > + > +2) Configure Buildroot. > + > +The Buildroot configuration would be done by running:: > + > + % make <BOARD>_defconfig > + > +For most of the boards it's good enough to use generic [intel_defconfig]_. > + > +3) Build Buildroot. > + > +Build the necessary Buildroot packages and resulting image:: > + > + % make KERNEL_SRC=~/linux What is this magic KERNEL_SRC variable? In appearance at least, it completely circumvents the Buildroot config options for specifying where the kernel sources are located. So this is clearly a no-go. > +Supported environment variables > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +The scripts under ``board/intel/common`` accept several environment variables > +that can be used to alter the default behaviour. Typically you do something > +like:: > + > + % make KERNEL_SRC=~/linux Should not exist. > +in order to take advantage of these. > + > +BOARD_INTEL_CUSTOM_CMDLINE > + provides a custom appendix to the command line. > + > +BOARD_INTEL_DIR > + points to a specific board directory. > + > +KERNEL_SRC > + path to your kernel output folder. I'm not sure any of these should exist. KERNEL_SRC is already something provided by Buildroot. The board directory can be known by other ways (see how the various raspberrypi variants are handled, for example). > +Alterate console Alterate, or alternate ? > +~~~~~~~~~~~~~~~~ > + > +By default ``ttyS0`` is used as a default cosole for both kernel and userspace. cosole -> console > +The **BR2_TARGET_GENERIC_GETTY_PORT** variable could be used to alterate this > +setting. Why not, but this is general Buildroot usage, not related to Intel boards specifically. > +Supported boards > +~~~~~~~~~~~~~~~~ > + > +.. [intel_defconfig] Generic config for most of Intel SoCs. This part is pretty uninformative :) > +Examples > +~~~~~~~~ > + > +- ASuS T100TA and the rest with ``ttyUSB0``:: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyUSB0 \ > + BOARD_INTEL_CUSTOM_CMDLINE="reboot=h,p" > + > +- Galileo (Quark):: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS1 > + > +- Joule (Broxton), Edison (Merrifield):: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS2 > + > +- Minnowboard [#]_:: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyPCH0 > + > +.. [#] Minnowboard MAX goes the standard way with ``ttyS0``. Please instead provide different defconfigs for those platforms: galileo_defconfig minnowboard_defconfig joule_defconfig etc. > diff --git a/board/intel/common/post-build.d/README b/board/intel/common/post-build.d/README > new file mode 100644 > index 0000000..5bcb10b > --- /dev/null > +++ b/board/intel/common/post-build.d/README > @@ -0,0 +1,4 @@ > +# > +# This folder contains the shell scripts which will be executed in alphabetical > +# order on post build stage. > +# I don't think we want an Intel specific mechanism for directories containing post-build scripts. I'd prefer to extend the current BR2_ROOTFS_POST_BUILD_SCRIPT logic (which allows to pass multiple space-separated post-build script) to automatically run all scripts in a directory if one entry in BR2_ROOTFS_POST_BUILD_SCRIPT is a directory. Or maybe, with the small number of post-build scripts and post-image scripts that you have, just call them from the relevant board defconfig. > +# Read cmdline and modify console parameter > +sed -e "s#$console_default#$console#" "$cmdline" > "$BINARIES_DIR/cmdline" What is this cmdline file doing in the end? > diff --git a/board/intel/common/post-image.d/40-prepare-initrd b/board/intel/common/post-image.d/40-prepare-initrd > new file mode 100755 > index 0000000..7bf7769 > --- /dev/null > +++ b/board/intel/common/post-image.d/40-prepare-initrd > @@ -0,0 +1,14 @@ > +#!/bin/sh -e > + > +# > +# Copyright (c) 2016 Intel Corp. > +# > + > +# > +# Environment: > +# > +# None > +# That's really a whole lot of nothing for just a single line of useful code :-) > diff --git a/configs/intel_defconfig b/configs/intel_defconfig > new file mode 100644 > index 0000000..8aff9e1 > --- /dev/null > +++ b/configs/intel_defconfig We don't have the concept of such generic defconfigs. We have defconfig per identified platform, and I'm not really sure to understand the logic behind this generic defconfig that then needs to be passed through custom environment variables to be customized depending on the platform. It's clearly not the direction I would like to take for the defconfigs. > +# Packages > +BR2_PACKAGE_KEXEC=y > +BR2_PACKAGE_KEXEC_ZLIB=y > +BR2_PACKAGE_LRZSZ=y > +BR2_PACKAGE_SCREEN=y We normally don't enable specific packages in our defconfig, since they are meant to be minimal, i.e just the toolchain, kernel, bootloader, and Busybox userspace. Best regards, Thomas
Hi Andy, I generally agree with Thomas's comments, and have a few more. On 25-08-16 16:04, Andy Shevchenko wrote: [snip] > +3) Build Buildroot. > + > +Build the necessary Buildroot packages and resulting image:: > + > + % make KERNEL_SRC=~/linux > + > +where ``~/linux`` is whatever the path to the kernel output folder is. > + > +4) Get the resulting image. > + > +The resulting image is placed under output/images and is called either > +``rootfs.cpio.bz2`` or ``initrd``. ``initrd`` is the link to the last modified > +image since some scripts may alter it on post image stage. Why are you not building a kernel or bootloader in this defconfig? Why is it called 'initrd'? All bootloaders allow to pass an initrd with a different name. And it's this initrd name that caused confusion in our understanding of the two cpio archives: if it would have said 'cat rootfs.cpio.bz2 >> $initrd' it would have been a lot more obvious. > + > +Supported environment variables Don't pass things through environment variables, pass them on the command line. The same arguments are passed to all scripts, so you have to parse them with some advanced getopt, but that's easy. E.g. the following converts args to vars: for arg; do [[ $arg == *=* ]] && eval "$arg" done > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +The scripts under ``board/intel/common`` accept several environment variables > +that can be used to alter the default behaviour. Typically you do something > +like:: > + > + % make KERNEL_SRC=~/linux > + > +in order to take advantage of these. > + > +BOARD_INTEL_CUSTOM_CMDLINE > + provides a custom appendix to the command line. > + > +BOARD_INTEL_DIR > + points to a specific board directory. > + > +KERNEL_SRC > + path to your kernel output folder. As mentioned by Thomas: scripts that are part of buildroot should only support a buildroot-built kernel. And the defconfigs should build that kernel. > + > +Alterate console > +~~~~~~~~~~~~~~~~ > + > +By default ``ttyS0`` is used as a default cosole for both kernel and userspace. > +The **BR2_TARGET_GENERIC_GETTY_PORT** variable could be used to alterate this > +setting. > + > +Supported boards > +~~~~~~~~~~~~~~~~ > + > +.. [intel_defconfig] Generic config for most of Intel SoCs. > + > +Examples > +~~~~~~~~ > + > +- ASuS T100TA and the rest with ``ttyUSB0``:: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyUSB0 \ > + BOARD_INTEL_CUSTOM_CMDLINE="reboot=h,p" > + > +- Galileo (Quark):: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS1 > + > +- Joule (Broxton), Edison (Merrifield):: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS2 > + > +- Minnowboard [#]_:: > + > + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyPCH0 As mentioned by Thomas, each of these should have their own defconfig. To show the commonality, you could make them start with intel_*. > + > +.. [#] Minnowboard MAX goes the standard way with ``ttyS0``. > diff --git a/board/intel/common/cmdline b/board/intel/common/cmdline > new file mode 100644 > index 0000000..54465ef > --- /dev/null > +++ b/board/intel/common/cmdline > @@ -0,0 +1 @@ > +console=tty1 console=ttyS0,115200n8 > diff --git a/board/intel/common/libshell-intel b/board/intel/common/libshell-intel > new file mode 100644 > index 0000000..f834ecb > --- /dev/null > +++ b/board/intel/common/libshell-intel > @@ -0,0 +1,26 @@ > +# > +# Copyright (c) 2016 Intel Corp. > +# > + > +# > +# Environment: > +# > +# BOARD_INTEL_DIR - Directory holding board specific > +# configuration > +# > + > +# Use this folder to provide board specific files > +BOARD_DIR_DEFAULT="$(dirname $(dirname "$0"))" > +BOARD_DIR="$(readlink -mnq "${BOARD_INTEL_DIR:-$BOARD_DIR_DEFAULT}")" > + > +[ -d "$BOARD_DIR" ] || echo "Warning: $BOARD_INTEL_DIR does not exist" > + > +# Looking up for a custom file if provided, otherwise fallback to the original name > +intel_file_lookup() { > + [ -f "$BOARD_DIR/$1" ] && echo "$BOARD_DIR/$1" || echo "$BOARD_DIR_DEFAULT/$1" > +} > + > +# Looking up for a custom folder if provided, otherwise fallback to the original name > +intel_folder_lookup() { > + [ -d "$BOARD_DIR/$1" ] && echo "$BOARD_DIR/$1" || echo "$BOARD_DIR_DEFAULT/$1" > +} I don't see why we would need this fallback approach. The defconfigs and related board stuff are not meant to be used for real development. Instead, they should be copied and adapted for real development. Regards, Arnout [snip]
diff --git a/board/intel/common/README.rst b/board/intel/common/README.rst new file mode 100644 index 0000000..e427067 --- /dev/null +++ b/board/intel/common/README.rst @@ -0,0 +1,96 @@ +=========== + Intel IoT +=========== + +---------------------------------------- + Common infrastructure for Intel boards +---------------------------------------- + +:Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> +:Date: 2016-08-17 + +Quick instructions +------------------ + +Building an image +~~~~~~~~~~~~~~~~~ + +For building a normal bootable image, you need to do following steps: + +1) Build your own kernel. + +2) Configure Buildroot. + +The Buildroot configuration would be done by running:: + + % make <BOARD>_defconfig + +For most of the boards it's good enough to use generic [intel_defconfig]_. + +3) Build Buildroot. + +Build the necessary Buildroot packages and resulting image:: + + % make KERNEL_SRC=~/linux + +where ``~/linux`` is whatever the path to the kernel output folder is. + +4) Get the resulting image. + +The resulting image is placed under output/images and is called either +``rootfs.cpio.bz2`` or ``initrd``. ``initrd`` is the link to the last modified +image since some scripts may alter it on post image stage. + +Supported environment variables +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The scripts under ``board/intel/common`` accept several environment variables +that can be used to alter the default behaviour. Typically you do something +like:: + + % make KERNEL_SRC=~/linux + +in order to take advantage of these. + +BOARD_INTEL_CUSTOM_CMDLINE + provides a custom appendix to the command line. + +BOARD_INTEL_DIR + points to a specific board directory. + +KERNEL_SRC + path to your kernel output folder. + +Alterate console +~~~~~~~~~~~~~~~~ + +By default ``ttyS0`` is used as a default cosole for both kernel and userspace. +The **BR2_TARGET_GENERIC_GETTY_PORT** variable could be used to alterate this +setting. + +Supported boards +~~~~~~~~~~~~~~~~ + +.. [intel_defconfig] Generic config for most of Intel SoCs. + +Examples +~~~~~~~~ + +- ASuS T100TA and the rest with ``ttyUSB0``:: + + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyUSB0 \ + BOARD_INTEL_CUSTOM_CMDLINE="reboot=h,p" + +- Galileo (Quark):: + + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS1 + +- Joule (Broxton), Edison (Merrifield):: + + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyS2 + +- Minnowboard [#]_:: + + % make KERNEL_SRC=~/linux BR2_TARGET_GENERIC_GETTY_PORT=ttyPCH0 + +.. [#] Minnowboard MAX goes the standard way with ``ttyS0``. diff --git a/board/intel/common/cmdline b/board/intel/common/cmdline new file mode 100644 index 0000000..54465ef --- /dev/null +++ b/board/intel/common/cmdline @@ -0,0 +1 @@ +console=tty1 console=ttyS0,115200n8 diff --git a/board/intel/common/libshell-intel b/board/intel/common/libshell-intel new file mode 100644 index 0000000..f834ecb --- /dev/null +++ b/board/intel/common/libshell-intel @@ -0,0 +1,26 @@ +# +# Copyright (c) 2016 Intel Corp. +# + +# +# Environment: +# +# BOARD_INTEL_DIR - Directory holding board specific +# configuration +# + +# Use this folder to provide board specific files +BOARD_DIR_DEFAULT="$(dirname $(dirname "$0"))" +BOARD_DIR="$(readlink -mnq "${BOARD_INTEL_DIR:-$BOARD_DIR_DEFAULT}")" + +[ -d "$BOARD_DIR" ] || echo "Warning: $BOARD_INTEL_DIR does not exist" + +# Looking up for a custom file if provided, otherwise fallback to the original name +intel_file_lookup() { + [ -f "$BOARD_DIR/$1" ] && echo "$BOARD_DIR/$1" || echo "$BOARD_DIR_DEFAULT/$1" +} + +# Looking up for a custom folder if provided, otherwise fallback to the original name +intel_folder_lookup() { + [ -d "$BOARD_DIR/$1" ] && echo "$BOARD_DIR/$1" || echo "$BOARD_DIR_DEFAULT/$1" +} diff --git a/board/intel/common/post-build.d/10-install-modules b/board/intel/common/post-build.d/10-install-modules new file mode 100755 index 0000000..4f88d8f --- /dev/null +++ b/board/intel/common/post-build.d/10-install-modules @@ -0,0 +1,23 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +# +# Environment: +# +# BOARD_INTEL_LEAVE_MODULES - If defined, the modules from previous kernel +# will be left untouched. Otherwise cleans up +# /lib/modules folder completely. +# +# KERNEL_SRC - Directory holding output of Linux kernel +# build. If defined, it will be used to install +# kernel modules from. +# + +# Leave old modules in the /lib/modules folder if asked +[ -z "$BOARD_INTEL_LEAVE_MODULES" ] && rm -rf $TARGET_DIR/lib/modules/* + +# Install kernel modules +[ -d "$KERNEL_SRC" ] && make -C "$KERNEL_SRC" INSTALL_MOD_PATH="$1" modules_install diff --git a/board/intel/common/post-build.d/README b/board/intel/common/post-build.d/README new file mode 100644 index 0000000..5bcb10b --- /dev/null +++ b/board/intel/common/post-build.d/README @@ -0,0 +1,4 @@ +# +# This folder contains the shell scripts which will be executed in alphabetical +# order on post build stage. +# diff --git a/board/intel/common/post-build.sh b/board/intel/common/post-build.sh new file mode 100755 index 0000000..28cf638 --- /dev/null +++ b/board/intel/common/post-build.sh @@ -0,0 +1,16 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +SCRIPT_LOCATION="$(dirname $0)" +SCRIPT_DIR="$SCRIPT_LOCATION/post-build.d" + +find "$SCRIPT_DIR" -maxdepth 1 -type f -perm -u+x | sort -u | while read script; do + name=$(basename $script) + + echo "Executing $name..." + + $script "$@" +done diff --git a/board/intel/common/post-image.d/10-prepare-cmdline b/board/intel/common/post-image.d/10-prepare-cmdline new file mode 100755 index 0000000..a222a1e --- /dev/null +++ b/board/intel/common/post-image.d/10-prepare-cmdline @@ -0,0 +1,26 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +# +# Environment: +# +# BR2_TARGET_GENERIC_GETTY_PORT - set console port +# + +PROG_NAME="${0##*/}" +PROG_DIR="${0%/*}" + +. $PROG_DIR/../libshell-intel + +# Assign default console which should be the same as provided in cmdline +console_default="ttyS0" +console="${BR2_TARGET_GENERIC_GETTY_PORT:-$console_default}" + +# Get initial cmdline +cmdline="$(intel_file_lookup "cmdline")" + +# Read cmdline and modify console parameter +sed -e "s#$console_default#$console#" "$cmdline" > "$BINARIES_DIR/cmdline" diff --git a/board/intel/common/post-image.d/30-append-custom-cmdline b/board/intel/common/post-image.d/30-append-custom-cmdline new file mode 100755 index 0000000..cbc07da --- /dev/null +++ b/board/intel/common/post-image.d/30-append-custom-cmdline @@ -0,0 +1,17 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +# +# Environment: +# +# BOARD_INTEL_CUSTOM_CMDLINE - Provides an addition to the default kernel +# command line +# + +[ -n "$BOARD_INTEL_CUSTOM_CMDLINE" ] || exit 0 + +# Append custom part of cmdline +echo "$(cat "$BINARIES_DIR/cmdline") $BOARD_INTEL_CUSTOM_CMDLINE" > "$BINARIES_DIR/cmdline" diff --git a/board/intel/common/post-image.d/40-prepare-initrd b/board/intel/common/post-image.d/40-prepare-initrd new file mode 100755 index 0000000..7bf7769 --- /dev/null +++ b/board/intel/common/post-image.d/40-prepare-initrd @@ -0,0 +1,14 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +# +# Environment: +# +# None +# + +# Link to the initial ramfs image as stated in intel_defconfig +ln -sf "rootfs.cpio.bz2" "$BINARIES_DIR/initrd" diff --git a/board/intel/common/post-image.d/README b/board/intel/common/post-image.d/README new file mode 100644 index 0000000..d214a4e --- /dev/null +++ b/board/intel/common/post-image.d/README @@ -0,0 +1,4 @@ +# +# This folder contains the shell scripts which will be executed in alphabetical +# order on post image stage. +# diff --git a/board/intel/common/post-image.sh b/board/intel/common/post-image.sh new file mode 100755 index 0000000..4166845 --- /dev/null +++ b/board/intel/common/post-image.sh @@ -0,0 +1,16 @@ +#!/bin/sh -e + +# +# Copyright (c) 2016 Intel Corp. +# + +SCRIPT_LOCATION="$(dirname $0)" +SCRIPT_DIR="$SCRIPT_LOCATION/post-image.d" + +find "$SCRIPT_DIR" -maxdepth 1 -type f -perm -u+x | sort -u | while read script; do + name=$(basename $script) + + echo "Executing $name..." + + $script "$@" +done diff --git a/configs/intel_defconfig b/configs/intel_defconfig new file mode 100644 index 0000000..8aff9e1 --- /dev/null +++ b/configs/intel_defconfig @@ -0,0 +1,27 @@ +# Architecture +BR2_i386=y +BR2_x86_i586=y + +# Misc +BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV=y +BR2_TARGET_GENERIC_GETTY_PORT="ttyS0" +BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200=y + +# Root FS +# BR2_TARGET_ROOTFS_TAR is not set +BR2_TARGET_ROOTFS_CPIO=y +BR2_TARGET_ROOTFS_CPIO_BZIP2=y + +# Root FS hooks +BR2_ROOTFS_POST_BUILD_SCRIPT="board/intel/common/post-build.sh" +BR2_ROOTFS_POST_IMAGE_SCRIPT="board/intel/common/post-image.sh" +BR2_ROOTFS_POST_SCRIPT_ARGS="" + +# Busybox utilities (target) +BR2_PACKAGE_BUSYBOX_WATCHDOG=y + +# Packages +BR2_PACKAGE_KEXEC=y +BR2_PACKAGE_KEXEC_ZLIB=y +BR2_PACKAGE_LRZSZ=y +BR2_PACKAGE_SCREEN=y
There are several x86 boards that can utilize same image creation infrastructure. Add common configuration, cmdline, post-image.sh, post-build.sh scripts and placeholders for their extensions. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- board/intel/common/README.rst | 96 ++++++++++++++++++++++ board/intel/common/cmdline | 1 + board/intel/common/libshell-intel | 26 ++++++ board/intel/common/post-build.d/10-install-modules | 23 ++++++ board/intel/common/post-build.d/README | 4 + board/intel/common/post-build.sh | 16 ++++ board/intel/common/post-image.d/10-prepare-cmdline | 26 ++++++ .../common/post-image.d/30-append-custom-cmdline | 17 ++++ board/intel/common/post-image.d/40-prepare-initrd | 14 ++++ board/intel/common/post-image.d/README | 4 + board/intel/common/post-image.sh | 16 ++++ configs/intel_defconfig | 27 ++++++ 12 files changed, 270 insertions(+) create mode 100644 board/intel/common/README.rst create mode 100644 board/intel/common/cmdline create mode 100644 board/intel/common/libshell-intel create mode 100755 board/intel/common/post-build.d/10-install-modules create mode 100644 board/intel/common/post-build.d/README create mode 100755 board/intel/common/post-build.sh create mode 100755 board/intel/common/post-image.d/10-prepare-cmdline create mode 100755 board/intel/common/post-image.d/30-append-custom-cmdline create mode 100755 board/intel/common/post-image.d/40-prepare-initrd create mode 100644 board/intel/common/post-image.d/README create mode 100755 board/intel/common/post-image.sh create mode 100644 configs/intel_defconfig