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