[U-Boot] [PATCH 2/2] tools/env: allow negative offsets

[Andreas Fenkart]

Hi,

2016-07-14 2:14 GMT+02:00 Stefan Agner <stefan at agner.ch>:
> From: Stefan Agner <stefan.agner at toradex.com>
>
> A negative value for the offset is treated as a backwards offset for
> from the end of the device/partition for block devices. This aligns
> the behavior of the config file with the syntax of CONFIG_ENV_OFFSET
> where the functionality has been introduced with
> commit 5c088ee841f9 ("env_mmc: allow negative CONFIG_ENV_OFFSET").
>
> Signed-off-by: Stefan Agner <stefan.agner at toradex.com>
lgtm

> ---
>
>  tools/env/fw_env.c      | 39 ++++++++++++++++++++++++++++++---------
>  tools/env/fw_env.config |  5 +++++
>  2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 06d6865..be338a5b 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -14,6 +14,7 @@
>  #include <errno.h>
>  #include <env_flags.h>
>  #include <fcntl.h>
> +#include <linux/fs.h>
>  #include <linux/stringify.h>
>  #include <ctype.h>
>  #include <stdio.h>
> @@ -51,7 +52,7 @@ struct env_opts default_opts = {
>
>  struct envdev_s {
>         const char *devname;            /* Device name */
> -       ulong devoff;                   /* Device offset */
> +       long long devoff;               /* Device offset */
>         ulong env_size;                 /* environment size */
>         ulong erase_size;               /* device erase size */
>         ulong env_sectors;              /* number of environment sectors */
> @@ -994,7 +995,7 @@ static int flash_write (int fd_current, int fd_target, int dev_target)
>         }
>
>  #ifdef DEBUG
> -       fprintf(stderr, "Writing new environment at 0x%lx on %s\n",
> +       fprintf(stderr, "Writing new environment at 0x%llx on %s\n",
>                 DEVOFFSET (dev_target), DEVNAME (dev_target));
>  #endif
>
> @@ -1010,7 +1011,7 @@ static int flash_write (int fd_current, int fd_target, int dev_target)
>                         offsetof (struct env_image_redundant, flags);
>  #ifdef DEBUG
>                 fprintf(stderr,
> -                       "Setting obsolete flag in environment at 0x%lx on %s\n",
> +                       "Setting obsolete flag in environment at 0x%llx on %s\n",
>                         DEVOFFSET (dev_current), DEVNAME (dev_current));
>  #endif
>                 flash_flag_obsolete (dev_current, fd_current, offset);
> @@ -1335,7 +1336,27 @@ static int check_device_config(int dev)
>                 }
>                 DEVTYPE(dev) = mtdinfo.type;
>         } else {
> +               uint64_t size;
>                 DEVTYPE(dev) = MTD_ABSENT;
> +
> +               /*
> +                * Check for negative offsets, treat it as backwards offset
> +                * from the end of the block device
> +                */
> +               if (DEVOFFSET(dev) < 0) {
> +                       rc = ioctl(fd, BLKGETSIZE64, &size);
> +                       if (rc < 0) {
> +                               fprintf(stderr, "Could not get block device size on %s\n",
> +                                       DEVNAME(dev));
> +                               goto err;
> +                       }
> +
> +                       DEVOFFSET(dev) = DEVOFFSET(dev) + size;

I wonder if we need a check, that the redundant areas don't overlap.
But we can add that later


> +#ifdef DEBUG
> +                       fprintf(stderr, "Calculated device offset 0x%llx on %s\n",
> +                               DEVOFFSET(dev), DEVNAME(dev));
> +#endif
> +               }
>         }
>
>  err:
> @@ -1434,12 +1455,12 @@ static int get_config (char *fname)
>                 if (dump[0] == '#')
>                         continue;
>
> -               rc = sscanf (dump, "%ms %lx %lx %lx %lx",
> -                            &devname,
> -                            &DEVOFFSET (i),
> -                            &ENVSIZE (i),
> -                            &DEVESIZE (i),
> -                            &ENVSECTORS (i));
> +               rc = sscanf(dump, "%ms %lli %lx %lx %lx",
> +                           &devname,
> +                           &DEVOFFSET(i),
> +                           &ENVSIZE(i),
> +                           &DEVESIZE(i),
> +                           &ENVSECTORS(i));
>
>                 if (rc < 3)
>                         continue;
> diff --git a/tools/env/fw_env.config b/tools/env/fw_env.config
> index 6f216f9..f0f66aa 100644
> --- a/tools/env/fw_env.config
> +++ b/tools/env/fw_env.config
> @@ -4,6 +4,7 @@
>  # Notice, that the "Number of sectors" is not required on NOR and SPI-dataflash.
>  # Futhermore, if the Flash sector size is ommitted, this value is assumed to
>  # be the same as the Environment size, which is valid for NOR and SPI-dataflash
> +# Device offset must be prefixed with 0x to be parsed as a hexadecimal value.
>
>  # NOR example
>  # MTD device name      Device offset   Env. size       Flash sector size       Number of sectors
> @@ -18,8 +19,12 @@
>  # NAND example
>  #/dev/mtd0             0x4000          0x4000          0x20000                 2
>
> +# On a block device a negative offset is treated as a backwards offset from the
> +# end of the device/partition, rather than a forwards offset from the start.
> +
>  # Block device example
>  #/dev/mmcblk0          0xc0000         0x20000
> +#/dev/mmcblk0          -0x20000        0x20000
>
>  # VFAT example
>  #/boot/uboot.env       0x0000          0x4000
> --
> 2.9.0
>