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

Pali Rohár pali at kernel.org
Mon Sep 7 09:58:25 CEST 2020


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.

> CC'ed Baruch, since I looked at the clearfog implementation which has the
> same bug.


More information about the U-Boot mailing list