[LEDE-DEV] [PATCH] firmware-utils: mktplinkfw: rework combined image option

Matthias Schiffer mschiffer at universe-factory.net
Sun Jul 9 16:27:27 PDT 2017


On 07/10/2017 01:02 AM, Piotr Dymacz wrote:
> We use combined option in "mktplinkfw" tool for generating initramfs
> kernel images and header for kernel inside "safeloader" image type (in
> fact, only for TL-WR1043ND v4 at this moment).
> 
> There is also "mktplinkfw-kernel" tool, a stripped-down version, used
> only for generating "simple" header, for safeloader image types.

I haven't had a detailed look at your patch yet, but I think we should
unify this: either build the 1043v4 using mktplinkfw-kernel, or get rid of
mktplinkfw-kernel and use mktplinkfw-combined instead.

Matthias


> 
> This changes how "mktplinkfw" handles combined images (which then will
> allow us to drop the stripped-down version of the tool):
> 
> - drop "ignore size" command line option (it was used only for combined
>   images anyway)
> - don't require "flash layout id" for combined images (we don't need and
>   shouldn't limit size of the initramfs kernel and for kernels inside
>   safeloader images, the "tplink-safeloader" tool does the size check)
> - require kernel address and entry point in command line parameters for
>   combined images (consequence of previous point)
> - don't include md5 sum and firmware length values in header (they are
>   needed only for update from vendor GUI and are ingored in case of
>   initramfs and "tplink-safeloader" images)
> - drop "fake" flash layout for TL-WR1043ND v4 as it's no longer needed
> 
> Also, adjust "mktplinkfw-combined" command in ar71xx/image/tp-link.mk to
> match introduced changes in "mktplinkfw" tool.
> 
> Signed-off-by: Piotr Dymacz <pepe2k at gmail.com>
> ---
>  target/linux/ar71xx/image/tp-link.mk  |  7 ++-
>  tools/firmware-utils/src/mktplinkfw.c | 99 +++++++++++++++--------------------
>  2 files changed, 44 insertions(+), 62 deletions(-)
> 
> diff --git a/target/linux/ar71xx/image/tp-link.mk b/target/linux/ar71xx/image/tp-link.mk
> index f717707..f393d15 100644
> --- a/target/linux/ar71xx/image/tp-link.mk
> +++ b/target/linux/ar71xx/image/tp-link.mk
> @@ -40,11 +40,11 @@ endef
>  # -c combined image
>  define Build/mktplinkfw-combined
>  	$(STAGING_DIR_HOST)/bin/mktplinkfw \
> -		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -F $(TPLINK_FLASHLAYOUT) -N OpenWrt -V $(REVISION) $(1) \
> -		-m $(TPLINK_HEADER_VERSION) \
> +		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -N OpenWrt -V $(REVISION) $(1) \
> +		-L $(KERNEL_LOADADDR) -m $(TPLINK_HEADER_VERSION) \
> +		-E $(if $(KERNEL_ENTRY),$(KERNEL_ENTRY),$(KERNEL_LOADADDR)) \
>  		-k $@ \
>  		-o $@.new \
> -		-s -S \
>  		-c
>  	@mv $@.new $@
>  endef
> @@ -707,7 +707,6 @@ define Device/tl-wr1043nd-v4
>    BOARDNAME := TL-WR1043ND-v4
>    DEVICE_PROFILE := TLWR1043
>    TPLINK_HWID :=  0x10430004
> -  TPLINK_FLASHLAYOUT := 16Msafeloader
>    MTDPARTS := spi0.0:128k(u-boot)ro,1536k(kernel),14016k(rootfs),128k(product-info)ro,320k(config)ro,64k(partition-table)ro,128k(logs)ro,64k(ART)ro,15552k at 0x20000(firmware)
>    IMAGE_SIZE := 15552k
>    TPLINK_BOARD_ID := TLWR1043NDV4
> diff --git a/tools/firmware-utils/src/mktplinkfw.c b/tools/firmware-utils/src/mktplinkfw.c
> index 93db441..c537862 100644
> --- a/tools/firmware-utils/src/mktplinkfw.c
> +++ b/tools/firmware-utils/src/mktplinkfw.c
> @@ -117,7 +117,6 @@ static uint32_t rootfs_align;
>  static struct file_info boot_info;
>  static int combined;
>  static int strip_padding;
> -static int ignore_size;
>  static int add_jffs2_eof;
>  static unsigned char jffs2_eof_mark[4] = {0xde, 0xad, 0xc0, 0xde};
>  static uint32_t fw_max_len;
> @@ -181,20 +180,6 @@ static struct flash_layout layouts[] = {
>  		.kernel_ep	= 0xc0000000,
>  		.rootfs_ofs	= 0x2a0000,
>  	}, {
> -		/*
> -			Some devices (e.g. TL-WR1043 v4) use a mktplinkfw kernel image
> -			embedded in a tplink-safeloader image as os-image partition.
> -
> -			We use a 1.5MB partition for the compressed kernel, which should
> -			be sufficient, but not too wasteful (the flash of the TL-WR1043 v4
> -			has 16MB in total).
> -		*/
> -		.id		= "16Msafeloader",
> -		.fw_max_len	= 0x180000,
> -		.kernel_la	= 0x80060000,
> -		.kernel_ep	= 0x80060000,
> -		.rootfs_ofs	= 0,
> -	}, {
>  		/* terminating entry */
>  	}
>  };
> @@ -272,7 +257,6 @@ static void usage(int status)
>  "  -R <offset>     overwrite rootfs offset with <offset> (hexval prefixed with 0x)\n"
>  "  -o <file>       write output to the file <file>\n"
>  "  -s              strip padding from the end of the image\n"
> -"  -S              ignore firmware size limit (only for combined images)\n"
>  "  -j              add jffs2 end-of-filesystem markers\n"
>  "  -N <vendor>     set image vendor to <vendor>\n"
>  "  -V <version>    set image version to <version>\n"
> @@ -362,7 +346,7 @@ static int check_options(void)
>  	}
>  	hw_id = strtoul(opt_hw_id, NULL, 0);
>  
> -	if (layout_id == NULL) {
> +	if (!combined && layout_id == NULL) {
>  		ERR("flash layout is not specified");
>  		return -1;
>  	}
> @@ -380,26 +364,31 @@ static int check_options(void)
>  		}
>  	}
>  
> -	layout = find_layout(layout_id);
> -	if (layout == NULL) {
> -		ERR("unknown flash layout \"%s\"", layout_id);
> -		return -1;
> -	}
> +	if (combined) {
> +		if (!kernel_la || !kernel_ep) {
> +			ERR("kernel loading address and entry point must be specified for combined image");
> +			return -1;
> +		}
> +	} else {
> +		layout = find_layout(layout_id);
> +		if (layout == NULL) {
> +			ERR("unknown flash layout \"%s\"", layout_id);
> +			return -1;
> +		}
>  
> -	if (!kernel_la)
> -		kernel_la = layout->kernel_la;
> -	if (!kernel_ep)
> -		kernel_ep = layout->kernel_ep;
> -	if (!rootfs_ofs)
> -		rootfs_ofs = layout->rootfs_ofs;
> +		if (!kernel_la)
> +			kernel_la = layout->kernel_la;
> +		if (!kernel_ep)
> +			kernel_ep = layout->kernel_ep;
> +		if (!rootfs_ofs)
> +			rootfs_ofs = layout->rootfs_ofs;
>  
> -	if (reserved_space > layout->fw_max_len) {
> -		ERR("reserved space is not valid");
> -		return -1;
> +		if (reserved_space > layout->fw_max_len) {
> +			ERR("reserved space is not valid");
> +			return -1;
> +		}
>  	}
>  
> -	fw_max_len = layout->fw_max_len - reserved_space;
> -
>  	if (kernel_info.file_name == NULL) {
>  		ERR("no kernel image specified");
>  		return -1;
> @@ -411,18 +400,9 @@ static int check_options(void)
>  
>  	kernel_len = kernel_info.file_size;
>  
> -	if (combined) {
> -		exceed_bytes = kernel_info.file_size - (fw_max_len - sizeof(struct fw_header));
> -		if (exceed_bytes > 0) {
> -			if (!ignore_size) {
> -				ERR("kernel image is too big by %i bytes", exceed_bytes);
> -				return -1;
> -			}
> -			layout->fw_max_len = sizeof(struct fw_header) +
> -					     kernel_info.file_size +
> -					     reserved_space;
> -		}
> -	} else {
> +	if (!combined) {
> +		fw_max_len = layout->fw_max_len - reserved_space;
> +
>  		if (rootfs_info.file_name == NULL) {
>  			ERR("no rootfs image specified");
>  			return -1;
> @@ -494,17 +474,18 @@ static void fill_header(char *buf, int len)
>  	hdr->hw_id = htonl(hw_id);
>  	hdr->hw_rev = htonl(hw_rev);
>  
> -	if (boot_info.file_size == 0)
> -		memcpy(hdr->md5sum1, md5salt_normal, sizeof(hdr->md5sum1));
> -	else
> -		memcpy(hdr->md5sum1, md5salt_boot, sizeof(hdr->md5sum1));
> -
>  	hdr->kernel_la = htonl(kernel_la);
>  	hdr->kernel_ep = htonl(kernel_ep);
> -	hdr->fw_length = htonl(layout->fw_max_len);
>  	hdr->kernel_ofs = htonl(sizeof(struct fw_header));
>  	hdr->kernel_len = htonl(kernel_len);
> +
>  	if (!combined) {
> +		if (boot_info.file_size == 0)
> +			memcpy(hdr->md5sum1, md5salt_normal, sizeof(hdr->md5sum1));
> +		else
> +			memcpy(hdr->md5sum1, md5salt_boot, sizeof(hdr->md5sum1));
> +
> +		hdr->fw_length = htonl(layout->fw_max_len);
>  		hdr->rootfs_ofs = htonl(rootfs_ofs);
>  		hdr->rootfs_len = htonl(rootfs_info.file_size);
>  	}
> @@ -530,7 +511,8 @@ static void fill_header(char *buf, int len)
>  		hdr->kernel_ep = bswap_32(hdr->kernel_ep);
>  	}
>  
> -	get_md5(buf, len, hdr->md5sum1);
> +	if (!combined)
> +		get_md5(buf, len, hdr->md5sum1);
>  }
>  
>  static int pad_jffs2(char *buf, int currlen)
> @@ -607,7 +589,12 @@ static int build_fw(void)
>  	int ret = EXIT_FAILURE;
>  	int writelen = 0;
>  
> -	buflen = layout->fw_max_len;
> +	writelen = sizeof(struct fw_header) + kernel_len;
> +
> +	if (combined)
> +		buflen = writelen;
> +	else
> +		buflen = layout->fw_max_len;
>  
>  	buf = malloc(buflen);
>  	if (!buf) {
> @@ -621,7 +608,6 @@ static int build_fw(void)
>  	if (ret)
>  		goto out_free_buf;
>  
> -	writelen = sizeof(struct fw_header) + kernel_len;
>  
>  	if (!combined) {
>  		if (rootfs_align)
> @@ -814,7 +800,7 @@ int main(int argc, char *argv[])
>  	while ( 1 ) {
>  		int c;
>  
> -		c = getopt(argc, argv, "a:H:E:F:L:m:V:N:W:C:ci:k:r:R:o:xX:ehsSjv:");
> +		c = getopt(argc, argv, "a:H:E:F:L:m:V:N:W:C:ci:k:r:R:o:xX:ehsjv:");
>  		if (c == -1)
>  			break;
>  
> @@ -870,9 +856,6 @@ int main(int argc, char *argv[])
>  		case 's':
>  			strip_padding = 1;
>  			break;
> -		case 'S':
> -			ignore_size = 1;
> -			break;
>  		case 'i':
>  			inspect_info.file_name = optarg;
>  			break;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/lede-dev/attachments/20170710/5a0d6d0a/attachment-0001.sig>


More information about the Lede-dev mailing list