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

Piotr Dymacz pepe2k at gmail.com
Sun Jul 9 22:50:32 PDT 2017


Hello Matthias,

On 10.07.2017 01:27, Matthias Schiffer wrote:
> 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.

Rework combined mode in mktplinkfw and then drop the mktplinkfw-kernel 
tool is my target plan, already done in my staging tree [0]. This patch 
is just a first step - I wanted to make sure that there are no other use 
cases of the combined mode in mktplinkfw I'm not aware of.

[0] 
https://git.lede-project.org/?p=lede/pepe2k/staging.git;a=shortlog;h=refs/heads/tools-build_rework-mktplink-combined_20170710

--
Cheers,
Piotr

> 
> 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;
>> 
> 
> 




More information about the Lede-dev mailing list