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 <pep...@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@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;
> 


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Reply via email to