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

[Andre Heider]

On 07/09/2020 10:52, Pali Rohár wrote:
> On Monday 07 September 2020 10:46:37 Andre Heider wrote:
>> 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.
> 
> So what about fixing it instead of adding new hacks?

Right, so for the occasional u-boot hacker like me it's not obvious how 
to solve a certain problem. Usually I check how other boards solve it. 
By your definition of hack other boards are full of it.

> Currently default env needs to be known at compile time due to
> assignment to static const storage. But this can be changed and e.g.
> board could could provide weak function which returns additional
> variables/value which uboot main code can use for default settings.

Sounds like you have an idea how to solve it nicely, so please go ahead.

>> Regards,
>> Andre