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

[Pali Rohár]

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?

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.

> Regards,
> Andre