[PATCH] tools: fw_env: use erasesize from MEMGETINFO ioctl

[Rasmus Villemoes]

On 20/03/2020 15.27, Rasmus Villemoes wrote:
> On 20/03/2020 15.11, Wolfgang Denk wrote:
>> Dear Rasmus,
>>
>> In message <20200320140414.19689-1-rasmus.villemoes at prevas.dk> you wrote:
>>>
>>> Having different fw_env.config files for the different revisions is
>>> highly impractical, and the correct information is already available
>>> right at our fingertips. So use the erasesize returned by the
>>> MEMGETINFO ioctl when the fourth column is absent or contains a 0.
>>>
>>> As I'm only testing this on a NOR flash, I'm only changing the logic
>>> for that case, though I think it should be possible for the other
>>> types as well.
>>>
>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>>> ---
>>>  tools/env/fw_env.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
>>> index 381739d28d..87c3504315 100644
>>> --- a/tools/env/fw_env.c
>>> +++ b/tools/env/fw_env.c
>>> @@ -1647,6 +1647,8 @@ static int check_device_config(int dev)
>>>  			goto err;
>>>  		}
>>>  		DEVTYPE(dev) = mtdinfo.type;
>>> +		if (DEVESIZE(dev) == 0 && mtdinfo.type == MTD_NORFLASH)
>>> +			DEVESIZE(dev) = mtdinfo.erasesize;
>>>  		if (DEVESIZE(dev) == 0)
>>>  			/* Assume the erase size is the same as the env-size */
>>>  			DEVESIZE(dev) = ENVSIZE(dev);
>>
>> What happens if you - say - have an environment size of 16 KiB and
>> an erase block size of 4 KiB?
> 
> The right thing - just as in my test case of an environment of 8K and
> erase block size of 4K, ENVSECTORS gets computed as 2 (or 4 in your
> example). And if the environment has a size which is not a multiple of
> the erase block size, all the same logic as usual applies.
> 
> It really works just the same as if one explicitly put in an erase block
> size in fw_env.config of 4K, but as I wrote in the commit log, there are
> cases where one has to support different flashes with different erase
> block sizes.
> 
> Now, if somebody has put in 0 for the erase block size (relying on the
> env size being used) and a 1 in the ENVSECTORS column, then yes, this
> will break.

But, it won't break silently - there's already a check

	if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {

Rasmus