[PATCH] arm: mvebu: Espressobin: Set environment variable fdtfile

[Andre Heider]

Hi Pali,

On 07/09/2020 09:58, Pali Rohár wrote:
> On Sunday 06 September 2020 20:44:50 Andre Heider wrote:
>> On 06/09/2020 11:32, Pali Rohár wrote:
>>> On Saturday 05 September 2020 14:07:44 Andre Heider wrote:
>>>> +
>>>> +	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
>>>> +
>>>> +	if (ddr4 && emmc)
>>>> +		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
>>>> +	else if (ddr4)
>>>> +		env_set("fdtfile", "marvell/armada-3720-espressobin-v7.dtb");
>>>> +	else if (emmc)
>>>> +		env_set("fdtfile", "marvell/armada-3720-espressobin-emmc.dtb");
>>>> +	else
>>>> +		env_set("fdtfile", "marvell/armada-3720-espressobin.dtb");
>>>
>>> This code would overwrite user's value of fdtfile variable. And any
>>> change done by saveenv for fdtfile would be lost. I do not think this is
>>> correct way as it would break booting other distributions/forks (e.g.
>>> Marvell one) which DTB file has different name.
>>>
>>> Also user would not be able to adjust usage of its own DTB file if this
>>> code would overwrite it at every boot.
>>
>> Indeed, it shouldn't wipe a saved $fdtfile. Fixed locally with adding this
>> hunk to the start of the function:
>> +	if (env_get("fdtfile"))
>> +		return 0;
> 
> This has still one issue: 'env default -a' does not restore original
> value. I would expect that 'env default -a' resets these values to
> correct default.

there doesn't seem to be a way to archive that programmatically?
The default argument needs to be known at compile time.

Regards,
Andre