
Hi Andreas
2016-11-28 10:47 GMT+01:00 Andreas Fenkart andreas.fenkart@digitalstrom.com:
Hi Max,
LGTM, see one nit below, can fixed later
On 11/19/2016 01:58 PM, Max Krummenacher wrote:
commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the environment must start at an erase block boundary.
For block devices the sample fw_env.config does not mandate a erase block size for block devices. A missing setting defaults to the full env size.
tools/env/fw_env.c | 60
...
} DEVTYPE(dev) = mtdinfo.type;
if (DEVESIZE(dev) == 0)
/* Assume the erase size is the same as the
env-size */
DEVESIZE(dev) = ENVSIZE(dev);
Since we already checked for character devices, we could use the mtdinfo.erasesize. I guess we can relay on mtdinfo on new kernels, and if not there is still the fallback to specify it in fw_env.config.
Agreed, however since that changes functionallity I think this must go into a follow up patch. Also, since the config file skeleton mandates a erase size I don't see a pressing need.
} else { uint64_t size; DEVTYPE(dev) = MTD_ABSENT;
if (DEVESIZE(dev) == 0)
/* Assume the erase size to be 512 bytes */
DEVESIZE(dev) = 0x200;
It doesn't even matter. In case of MTD_ABSENT, erasize is never used itself, only the tuple (DEVESIZE * ENVSECTORS) matters.
That is not true. with the test for DEVOFFSET being aligned to DEVESIZE the previous used default broke env with certain legal config files. Other than this new test I agree.
Max