[U-Boot] [RFC: v2] tools/env: ensure environment starts at erase block boundary

[Stefan Agner]

On 2016-08-11 12:39, Andreas Fenkart wrote:
> 56086921 added support for unaligned environments access.
> U-boot itself does not support this:
> - env_nand.c fails when using an unaligned offset. It produces an
>   error in nand_erase_opts{drivers/mtd/nand/nand_util.c}
> - in env_sf/env_flash the unused space at the end is preserved, but
>   not in the beginning. block alignment is assumed
> - env_sata/env_mmc aligns offset/length to the block size of the
>   underlying device. data is silently redirected to the beginning of
>   a block
> 
> There is seems no use case for unaligned environment. If there is
> some useful data at the beginning of the the block (e.g. end of u-boot)
> that would be very unsafe. If the redundant environments are hosted by
> the same erase block then that invalidates the idea of double buffering.
> It might be that unaligned access was allowed in the past, and that
> people with legacy u-boot are trapped. But at the time of 56086921
> it wasn't supported and due to reasons above I guess it was never
> introduced.
> I prefer to remove that (unused) feature in favor of simplicity

I also don't see any value supporting unaligned environment, so FWIW:

Acked-by: Stefan Agner <stefan.agner at toradex.com>

Small nit below:

> 
> Signed-off-by: Andreas Fenkart <andreas.fenkart at digitalstrom.com>
> ---
>  tools/env/fw_env.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 6b0dcaa..d2b167d 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -1294,6 +1294,18 @@ static int check_device_config(int dev)
>  	struct stat st;
>  	int fd, rc = 0;
>  
> +	if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
> +		fprintf(stderr, "Environment does not start on erase block boundary\n");

Erase block is sometwhat confusing in the MMC context, maybe
parenthesize "erase"?

--
Stefan

> +		errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {
> +		fprintf(stderr, "Environment does not fit into available sectors\n");
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
>  	fd = open(DEVNAME(dev), O_RDONLY);
>  	if (fd < 0) {
>  		fprintf(stderr,