[Buildroot] buildroot support for OrangePI PC

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Eelco,

On Thu, 24 Dec 2015 10:15:40 +0100, Eelco Chaudron wrote:

> This is the diff;

Thanks for the patch. Can you send it as a proper Git formatted patch?
See the Buildroot manual for instructions on how to do this. I have a
couple more comments below.

> diff --git a/board/orangepi/orangepipc/genimage.cfg b/board/orangepi/orangepipc/genimage.cfg
> new file mode 100644
> index 0000000..43c2249
> --- /dev/null
> +++ b/board/orangepi/orangepipc/genimage.cfg
> @@ -0,0 +1,36 @@
> +# Minimal SD card image for the OrangePi PC
> +#
> +
> +image boot.vfat {
> + vfat {
> + files = {
> + "uImage",
> + "sun8i-h3-orangepi-pc.dtb",
> + "boot.scr"
> + }
> + }
> + size = 10M
> +}

Please fix the indentation here. I think we should use at least two
spaces for indentation, or maybe better one tab.

> diff --git a/board/orangepi/orangepipc/post-build.sh b/board/orangepi/orangepipc/post-build.sh
> new file mode 100755
> index 0000000..9f115cb
> --- /dev/null
> +++ b/board/orangepi/orangepipc/post-build.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +# post-build.sh for OrangePi PC taken from CubieBoard's post-build.sh
> +# 2013, Carlo Caione <<carlo.caione@gmail.com>>
> +
> +BOARD_DIR="$(dirname $0)"
> +MKIMAGE=$HOST_DIR/usr/bin/mkimage
> +BOOT_CMD=$BOARD_DIR/boot.cmd
> +BOOT_CMD_H=$BINARIES_DIR/boot.scr
> +
> +# U-Boot script
> +if [ -e $MKIMAGE -a -e $BOOT_CMD ];

This test is not needed IMO, just run the $MKIMAGE command
unconditionally.

> +then
> +	$MKIMAGE -C none -A arm -T script -d $BOOT_CMD $BOOT_CMD_H
> +fi
> diff --git a/board/orangepi/orangepipc/post-image.sh b/board/orangepi/orangepipc/post-image.sh
> new file mode 100755
> index 0000000..05f6b70
> --- /dev/null
> +++ b/board/orangepi/orangepipc/post-image.sh
> @@ -0,0 +1,16 @@
> +#!/bin/bash

Do we really need this to be a bash script ? #!/bin/sh is probably
sufficient.

> +
> +GENIMAGE_CFG="board/orangepi/orangepipc/genimage.cfg"

Maybe you could use the same BOARD_DIR trick than the one you've used
in the post-build script?

> +GENIMAGE_TMP="${BUILD_DIR}/genimage.tmp"
> +
> +rm -rf "${GENIMAGE_TMP}"
> +
> +genimage \
> + --rootpath "${TARGET_DIR}" \
> + --tmppath "${GENIMAGE_TMP}" \
> + --inputpath "${BINARIES_DIR}" \
> + --outputpath "${BINARIES_DIR}" \
> + --config "${GENIMAGE_CFG}"

Use tab for indentation.

> +
> +RET=${?}
> +exit ${RET}

Just do exit $?

> diff --git a/configs/orangepipc_defconfig b/configs/orangepipc_defconfig
> new file mode 100644
> index 0000000..cd349c5
> --- /dev/null
> +++ b/configs/orangepipc_defconfig
> @@ -0,0 +1,26 @@
> +BR2_arm=y
> +BR2_cortex_a7=y
> +BR2_ARM_FPU_NEON_VFPV4=y

Please use just VFPV4. Using NEON for floating point operations by
default is not a good idea. From the gcc manual:

   If the selected floating-point hardware includes the NEON extension
   (e.g. -mfpu=?neon?), note that floating-point operations are not
   generated by GCC's auto-vectorization pass unless
   -funsafe-math-optimizations is also specified. This is because NEON
   hardware does not fully implement the IEEE 754 standard for
   floating-point arithmetic (in particular denormal values are treated
   as zero), so the use of NEON instructions may lead to a loss of
   precision.


> +BR2_TARGET_GENERIC_HOSTNAME="OrangePi_PC"
> +BR2_TARGET_GENERIC_ISSUE="Welcome to Buildroot for the Orange Pi PC"
> +BR2_ROOTFS_POST_BUILD_SCRIPT="board/orangepi/orangepipc/post-build.sh"
> +BR2_ROOTFS_POST_IMAGE_SCRIPT="board/orangepi/orangepipc/post-image.sh"

Please force the kernel headers version to a fixed version, like is
done in all other defconfig files. For example:

# Lock to 4.3 headers to avoid breaking with newer kernels
BR2_KERNEL_HEADERS_VERSION=y
BR2_DEFAULT_KERNEL_VERSION="4.3"
BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_3=y

(Of course adapt 4.3 to whatever kernel version you're using)

> +BR2_LINUX_KERNEL=y
> +BR2_LINUX_KERNEL_CUSTOM_GIT=y
> +BR2_LINUX_KERNEL_CUSTOM_REPO_URL="<https://github.com/jwrdegoede/linux-sunxi.git>"

Please remove the < and > before and after the URL. I'm not even sure
how it can work with such markers.

> +BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="a516ac6dda21e6f5edceecd08b475b16e360656b"
> +BR2_LINUX_KERNEL_DEFCONFIG="sunxi"
> +BR2_LINUX_KERNEL_UIMAGE_LOADADDR="0x40008000"

The modern way is to boot using a zImage. Can you try using a zImage
instead?

> +BR2_LINUX_KERNEL_DTS_SUPPORT=y
> +BR2_LINUX_KERNEL_INTREE_DTS_NAME="sun8i-h3-orangepi-pc"
> +BR2_TARGET_ROOTFS_EXT2=y
> +BR2_TARGET_ROOTFS_EXT2_4=y
> +# BR2_TARGET_ROOTFS_TAR is not set
> +BR2_TARGET_UBOOT=y
> +BR2_TARGET_UBOOT_BOARDNAME="orangepi_pc"
> +BR2_TARGET_UBOOT_CUSTOM_GIT=y
> +BR2_TARGET_UBOOT_CUSTOM_REPO_URL="<http://git.denx.de/u-boot.git>"

Ditto < and > markers.

> +BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION="4832e17787acb29734d895751bc7a594908aecc6"
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
> +BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-sunxi-with-spl.bin"
> +BR2_PACKAGE_HOST_GENIMAGE=y

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com