Hi Ole Kristian,
Could you CC Rafał and me (and other reviewers) on revised patches? At least we
should get
the patch without DMARC wrapping in that case.
On Thu, 2022-05-05 at 17:41 +0200, Ole Kristian Lona wrote:
> Some devices, specifically Deco M4R-v3 / M5 have partition names that
> deviate from the scheme other devices use. They have an added "@0"
> and "@1" for some patition names. The devices have
> fallback partitions which will be used in case the device
> determines that the primary partition set is unbootable.
>
> This patch introduces an option to set these alternate partition
> names in the device definition of tplink-safeloader.
>
> Signed-off-by: Ole Kristian Lona <o...@oklona.net>
> ---
> src/tplink-safeloader.c | 58 ++++++++++++++++++++++++++++++++---------
> 1 file changed, 45 insertions(+), 13 deletions(-)
>
> diff --git a/src/tplink-safeloader.c b/src/tplink-safeloader.c
> index 1c08a33..2adcaf6 100644
> --- a/src/tplink-safeloader.c
> +++ b/src/tplink-safeloader.c
> @@ -48,11 +48,20 @@ struct image_partition_entry {
>
> /** A flash partition table entry */
> struct flash_partition_entry {
> - char *name;
> + const char *name;
> uint32_t base;
> uint32_t size;
> };
>
> +/** Flash partition names table entry */
> +struct factory_partition_names {
> + const char *partition_table;
> + const char *soft_ver;
> + const char *os_image;
> + const char *file_system;
> + const char *extra_para;
> +};
> +
> /** Partition trailing padding definitions
> * Values 0x00 to 0xff are reserved to indicate the padding value
> * Values from 0x100 are reserved to indicate other behaviour */
> @@ -89,6 +98,7 @@ struct device_info {
> struct flash_partition_entry partitions[MAX_PARTITIONS+1];
> const char *first_sysupgrade_partition;
> const char *last_sysupgrade_partition;
> + struct factory_partition_names partition_names;
> };
>
> #define SOFT_VER_TEXT(_t) {.type = SOFT_VER_TYPE_TEXT, .text = _t}
> @@ -2962,6 +2972,21 @@ static struct image_partition_entry
> alloc_image_partition(const
> char *name, size
> return entry;
> }
>
> +/** Sets up default partition names whenever custom names aren't specified */
> +static void set_partition_names(struct device_info *info)
> +{
> + if (!info->partition_names.partition_table)
> + info->partition_names.partition_table = "partition-table";
> + if (!info->partition_names.soft_ver)
> + info->partition_names.soft_ver = "soft-version";
> + if (!info->partition_names.os_image)
> + info->partition_names.os_image = "os-image";
> + if (!info->partition_names.file_system)
> + info->partition_names.file_system = "file-system";
> + if (!info->partition_names.extra_para)
> + info->partition_names.extra_para = "extra-para";
> +}
> +
> /** Frees an image partition */
> static void free_image_partition(struct image_partition_entry entry) {
> free(entry.data);
> @@ -2982,8 +3007,10 @@ 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)
> +{
> + struct image_partition_entry entry = alloc_image_partition(name,
> 0x800);
If you pass a device_info reference instead of the flash_partition_entry list,
you can
draw all the required information from that struct and don't need to pass an
additional
string. make_soft_version(), make_support_list(), and make_extra_para() already
take a
struct device_info * argument.
>
> char *s = (char *)entry.data, *end = (char *)(s+entry.size);
>
> @@ -3018,14 +3045,14 @@ static inline uint8_t bcd(uint8_t v) {
>
>
> /** Generates the soft-version partition */
> -static struct image_partition_entry make_soft_version(
> +static struct image_partition_entry make_soft_version(const char *name,
> const struct device_info *info, uint32_t rev)
> {
> /** If an info string is provided, use this instead of
> * the structured data, and include the null-termination */
> if (info->soft_ver.type == SOFT_VER_TYPE_TEXT) {
> uint32_t len = strlen(info->soft_ver.text) + 1;
> - return init_meta_partition_entry("soft-version",
> + return init_meta_partition_entry(name,
You should be able to draw the partition name from device_info, which you've
updated to
contain the required value(s).
> info->soft_ver.text, len, info->part_trail);
> }
>
> @@ -3055,11 +3082,11 @@ static struct image_partition_entry make_soft_version(
> };
>
> if (info->soft_ver_compat_level == 0)
> - return init_meta_partition_entry("soft-version", &s,
> + return init_meta_partition_entry(name, &s,
> (uint8_t *)(&s.compat_level) - (uint8_t *)(&s),
> info->part_trail);
> else
> - return init_meta_partition_entry("soft-version", &s,
> + return init_meta_partition_entry(name, &s,
> sizeof(s), info->part_trail);
> }
>
Shouldn't you also modify make_support_list() and make_extra_para()?
> @@ -3307,6 +3334,8 @@ static void build_image(const char *output,
> struct flash_partition_entry *file_system_partition = NULL;
> size_t firmware_partition_index = 0;
>
> + set_partition_names(info);
> +
> for (i = 0; info->partitions[i].name; i++) {
> if (!strcmp(info->partitions[i].name, "firmware"))
> {
> @@ -3330,7 +3359,8 @@ static void build_image(const char *output,
> for (i = MAX_PARTITIONS-1; i >= firmware_partition_index + 1;
> i--)
> info->partitions[i+1] = info->partitions[i];
>
> - file_system_partition->name = "file-system";
> + file_system_partition->name =
> info->partition_names.file_system;
> +
You can keep these two lines together, no need to add an empty line.
> file_system_partition->base = firmware_partition->base +
> kernel.st_size;
>
> /* Align partition start to erase blocks for factory images
> only */
> @@ -3339,15 +3369,17 @@ static void build_image(const char *output,
>
> file_system_partition->size = firmware_partition->size -
> file_system_partition->base;
>
> - os_image_partition->name = "os-image";
> + os_image_partition->name = info->partition_names.os_image;
> +
Same here.
> os_image_partition->size = kernel.st_size;
> }
>
> - parts[0] = make_partition_table(info->partitions);
> - parts[1] = make_soft_version(info, rev);
> + parts[0] =
> make_partition_table(info->partition_names.partition_table, info-
> >partitions);
> + parts[1] = make_soft_version(info->partition_names.soft_ver, 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(info->partition_names.os_image, kernel_image,
> false, NULL);
> + parts[4] = read_file(info->partition_names.file_system, rootfs_image,
> add_jffs2_eof, file_system_partition);
> +
No need to add a (second) empty line here
>
> /* Some devices need the extra-para partition to accept the firmware
> */
> if (strcasecmp(info->id, "ARCHER-A6-V3") == 0 ||
Best,
Sander
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
