On 20/03/2020 15.27, Rasmus Villemoes wrote:
> On 20/03/2020 15.11, Wolfgang Denk wrote:
>> Dear Rasmus,
>>
>> In message <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
