[OpenWrt-Devel] [PATCH] gemini: Support sysupgrade on DIR-685

[Petr Štetiar]

Linus Walleij <linus.walleij at linaro.org> [2019-05-05 14:23:27]:

Hi,

I'm wondering, if it would be possible to hook (adding just a short
references, there are enough examples in the tree already) metadata into the
upgrade process instead, it would mean adding SUPPORTED_DEVICES to image/Makefile:

 define Device/Default
   ...
   SUPPORTED_DEVICES := $(subst _,$(comma),$(1))
   ...
 endef

where SUPPORTED_DEVICES needs to match DT compatible (which probably does in
DIR-685 case).

> diff --git a/target/linux/gemini/base-files/lib/upgrade/platform.sh b/target/linux/gemini/base-files/lib/upgrade/platform.sh
> new file mode 100644
> index 000000000000..0c6fc44ee725
> --- /dev/null
> +++ b/target/linux/gemini/base-files/lib/upgrade/platform.sh

Needs REQUIRE_IMAGE_METADATA=1

> @@ -0,0 +1,54 @@
> +wrgg_get_image_magic() {
> +	get_image "$@" | dd bs=4 count=1 skip=8 2>/dev/null | hexdump -v -n 4 -e '1/1 "%02x"'
> +}
> +
> +platform_find_part_size() {
> +	local first dev size erasesize name
> +	while read dev size erasesize name; do
> +		name=${name#'"'}; name=${name%'"'}
> +		[ "$name" = "$1" ] && {
> +			echo "$size"
> +			break
> +		}
> +	done < /proc/mtd
> +}
> +
> +platform_check_image() {
> +	local board=$(board_name)
> +	# TODO: check size of sysupgrade image

I would leave only useful comments.

> +
> +	case "$board" in
> +	dlink,dir-685 )
> +		local magic=$(wrgg_get_image_magic "$1")

with metadata in the image, this additional checking is not necessary anymore,
so you can just return 0 here.

> +		[ "$magic" != "21030820" ] && {
> +			echo "Invalid image. Bad magic for DIR-685."
> +			return 1
> +		}
> +		echo "Image looks OK"
> +		return 0
> +		;;
> +	esac
> +
> +	echo "Sysupgrade is not yet supported on $board."
> +	return 1
> +}
> +
> +platform_do_upgrade() {
> +	local board=$(board_name)
> +
> +	v "board=$board"
> +	case "$board" in
> +	dlink,dir-685 )
> +		# This will overwrite "firmware" which means that the
> +		# partitions "kernel", "rootfs" and "rootfs_data" get
> +		# overwritten in the flash. The "kernel" image has the
> +		# necessary boot loader magic attached to it, and after
> +		# the "rootfs" there is an empty space marker for "rootfs_data"

I find it quite misleading(rootfs_data would get probably overwritten only if
rootfs is bigger then the previous one) and I would simply remove this comment.
All the glory details are included in the image generation code already, so
this comment lack any value.

> +		PART_NAME=firmware
> +		default_do_upgrade "$ARGV"
> +		;;
> +	*)
> +
> +		;;
> +	esac
> +}
> diff --git a/target/linux/gemini/image/Makefile b/target/linux/gemini/image/Makefile
> index 8fec250f186a..a7001eb0068a 100644
> --- a/target/linux/gemini/image/Makefile
> +++ b/target/linux/gemini/image/Makefile
> @@ -143,9 +143,10 @@ define Device/dlink_dir-685
>  	DEVICE_PACKAGES := $(GEMINI_NAS_PACKAGES) \
>  			kmod-switch-rtl8366rb swconfig \
>  			kmod-rt2800-pci
> -	IMAGES := factory.bin
> +	IMAGES := factory.bin sysupgrade.bin
>  	# Pad to 128k erase blocks with 160 bytes WRGG header

>  	IMAGE/factory.bin := append-kernel | pad-offset 128k 160 | append-rootfs | dir685-pad-rootfs | dir685-image
> +	IMAGE/sysupgrade.bin := append-kernel | pad-offset 128k 160 | dir685-image | append-rootfs | dir685-pad-rootfs

IMAGE/sysupgrade.bin := append-kernel | pad-offset 128k 160 | dir685-image | append-rootfs | dir685-pad-rootfs | append-metadata

FYI images with metadata are going to play well in the future, when (if?) we
switch to signed images[1].

1. https://github.com/openwrt/openwrt/pull/1992

-- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel