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

Ole Kristian Lona ole.kristian at lona.name
Thu Apr 28 23:54:12 PDT 2022


Hello.

I sent the first patch (support for differently named partitions) now.

Will follow up with the other patches as soon as we agree on this one. As stated in the commit message, I believe the "secondary" partitions (which are actually named xxxx at 0, while the primaries are "@1") are populated by the OEM firmware. I may be mistaken, but at least I can confirm that with a non-working software upgrade, the next boot resulted in the device reverting to the previous firmware version I had, which was OEM.

-And auto-split probably  works, I just hadn't read through that part of the code, and seen that I needed to handle the specific partition names there as well. I am testing this now.


Regards,

Ole Kristian

On 24/04/2022, 10:28, "Sander Vanheule" <sander at svanheule.net> wrote:

    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





More information about the openwrt-devel mailing list