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

Pali Rohár
2020-09-07T07:58:25Z

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.