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.