| Message ID | 20160714001439.9062-2-stefan@agner.ch |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
Hi, 2016-07-14 2:14 GMT+02:00 Stefan Agner <stefan@agner.ch>: > From: Stefan Agner <stefan.agner@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@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 >
On 2016-07-17 06:37, Andreas Fenkart wrote: > Hi, > > 2016-07-14 2:14 GMT+02:00 Stefan Agner <stefan@agner.ch>: >> From: Stefan Agner <stefan.agner@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@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 We haven't had it before, but yeah I agree it would be a sensible thing to do. And we have now a good place to add such checks... -- Stefan > > >> +#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 >>
On Wed, Jul 13, 2016 at 05:14:38PM -0700, Stefan Agner wrote: > From: Stefan Agner <stefan.agner@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@toradex.com> Applied to u-boot/master, thanks!
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; +#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