From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andre Heider <a.heider@gmail.com>
Date: Mon, 7 Sep 2020 10:46:37 +0200
Subject: [PATCH] arm: mvebu: Espressobin: Set environment variable fdtfile
In-Reply-To: <20200907075825.ssdecydumyjwdajf@pali>
References: <20200905120744.634300-1-a.heider@gmail.com>
 <20200906093247.hr6w5svqxkjdimuy@pali>
 <c6282bc7-089a-7e50-3fb7-a58d0ffad833@gmail.com>
 <20200907075825.ssdecydumyjwdajf@pali>
Message-ID: <610fa51f-aa5f-a5e4-306b-ab0851310d3d@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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