[PATCH firmware-utils] tplink-safeloader: TP-Link Deco M4R-V3 and Deco M5 support

[Sander Vanheule]

Hi Ole Kristian,


On Tue, 2022-04-19 at 15:46 +0200, Ole Kristian Lona wrote:
> 
> Support for creating images for TP-Link Deco M4R version 3, and for M5.

Please provide separate patches for:
 * image generation code changes
 * Deco M4R v3
 * Deco M5 

> 
> Partition table and supportlists were extracted from newest OEM image.
> 
> Both devices have partition tables with fallback partitions, like:
> os-image at 0 and os-image at 1, file-system at 0 and file-system at 1
> 
> This is assumed to be a fail-safe built into the firmware of the 
> devices.

These devices are not explicitly dual boot then? How does the bootloader decide which os-
image will be loaded? If the device alternates between @0 and @1 on firmware upgrades, you
will need to specify in the factory install instructions how to select the correct image
set.

> Therefore, this naming scheme has been kept, and tplink-safeloader.c
> has been slightly modified to account for this.
> 
> I have tested without these "@1" and "@0" names. This causes the 
> firmware to be
> makred as invalid. Therefore, I made changes to the logic in the app, 
> (re?)adding a name
> parameter to functions, and (last part of the patch) making sure these 
> names are
> sent to the functions correctly.
> 
> Also tested merging os-image and file-system into one "firmware" 
> partition. This rendered
> the firmware invalid, so reverted back to original TP-Link layout.

Is this true when "firmware" is auto-split into "os-image at 1" and "file-system at 1" too?
Typically, when you keep the fixed split, an update will be required in the future because
the kernel has outgrown the small (4MB in this case) os-image partition.

> 
> Signed-off-by: Ole Kristian Lona <oklo at oklona.net>
> ---
>   src/tplink-safeloader.c | 169 +++++++++++++++++++++++++++++++++++++---
>   1 file changed, 159 insertions(+), 10 deletions(-)
> 
> diff --git a/src/tplink-safeloader.c b/src/tplink-safeloader.c
> index c4727df..7b5d2d4 100644
> --- a/src/tplink-safeloader.c
> +++ b/src/tplink-safeloader.c
> 

...

> @@ -2895,8 +3026,8 @@ static void set_source_date_epoch() {
>   }
> 
>   /** Generates the partition-table partition */
> -static struct image_partition_entry make_partition_table(const struct 
> flash_partition_entry *p) {
> -       struct image_partition_entry entry = 
> alloc_image_partition("partition-table", 0x800);
> +static struct image_partition_entry make_partition_table(const char 
> *name,const struct flash_partition_entry *p) {

Add a space after the comma

> +       struct image_partition_entry entry = alloc_image_partition(name, 
> 0x800);
> 
>         char *s = (char *)entry.data, *end = (char *)(s+entry.size);

...

> 
> @@ -3213,6 +3344,11 @@ static void build_image(const char *output,
> 
>         size_t i;
> 
> +       char *part_table_name;
> +       char *soft_ver_name;
> +       char *os_im_name;
> +       char *fs_name;
> +
>         struct image_partition_entry parts[7] = {};
> 
>         struct flash_partition_entry *firmware_partition = NULL;
> @@ -3256,11 +3392,24 @@ static void build_image(const char *output,
>                 os_image_partition->size = kernel.st_size;
>         }
> 
> -       parts[0] = make_partition_table(info->partitions);
> -       parts[1] = make_soft_version(info, rev);
> +       if (strcasecmp(info->id, "DECO-M4R-V3") == 0 ||
> +               strcasecmp(info->id, "DECO-M5") == 0 ) {
> +               part_table_name="partition-table at 1";
> +               soft_ver_name="soft-version at 1";
> +               os_im_name="os-image at 1";
> +               fs_name="file-system at 1";
> +       } else {
> +               part_table_name="partition-table";
> +               soft_ver_name="soft-version";
> +               os_im_name="os-image";
> +               fs_name="file-system";
> +       }
> +
> +       parts[0] = make_partition_table(part_table_name,info->partitions);
> +       parts[1] = make_soft_version(soft_ver_name,info, rev);
>         parts[2] = make_support_list(info);
> -       parts[3] = read_file("os-image", kernel_image, false, NULL);
> -       parts[4] = read_file("file-system", rootfs_image, add_jffs2_eof, 
> file_system_partition);
> +       parts[3] = read_file(os_im_name, kernel_image, false, NULL);
> +       parts[4] = read_file(fs_name, rootfs_image, add_jffs2_eof, 
> file_system_partition);
> 
>         /* Some devices need the extra-para partition to accept the firmware 
> */
>         if (strcasecmp(info->id, "ARCHER-A6-V3") == 0 ||

I'm aware a strcasecmp() is performed for extra-para partition, but I don't particularly
like that either. This puts device-specific details in different places, and after a while
you don't know where to look anymore to add support for a new device.

The list of generated partition for factory images is fixed. You could create a new struct
(e.g. 'factory_partition_names') that contains names for all required partitions:

struct factory_partition_names {
const char *partition_table;
const char *soft_ver;
const char *support_list;
const char *os_image;
const char *file_system;
const char *extra_para;
};

Then, add a member to the device_info struct that points to a factory_partition_names
object. For example:

static const struct factory_partition_names PARTITION_NAMES_DUALBOOT_DECO_M = {
"partition-table at 1",
"soft-version at 1",
"support-list",
"os-image at 1",
"file-system at 1",
NULL /* extra-para is not used on these devices */
};

If the partition names pointer is NULL (e.g. when left unspecified), then you can default
to current set of names. If the pointer is not NULL, take the names from the pointed-to
struct. Then all you need to pass around is the device_info pointer, and the partition
generator functions can grab all the required info from there.

Also bear in mind that the partition names are not only used to generate OpenWrt factory
images, but also to convert OEM firmware images to sysupgrade-compatible files. But
solving that problem can be a separate patch, so let's do factory images first.


Best,
Sander