On 04/13/2016 12:17 PM, Andreas Färber wrote:
Am 13.04.2016 um 19:58 schrieb Stephen Warren:On 04/13/2016 11:42 AM, Andreas Färber wrote:Am 13.04.2016 um 19:00 schrieb Stephen Warren:Anyway, nothing in your benefits-of-EFI statement implies that relying on $fdtfile being set is correct. That's a new requirement that didn't exist before. Either the requirement needs to be removed (e.g. using a default FDT filename such as "${soc}-${board}${boardver}.dts") or only enabling this functionality on boards that do set $fdtfile, since it relies on that.$fdtfile needs to be the Linux filename. It does not always follow the same pattern as the U-Boot variables you suggest here. CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my question to you.That pattern is a good default that at least historically applied to all the systems where the distro bootcmds were enabled. Perhaps the set of systems using the distro bootcmds has increased now so the default isn't always applicable. Boards can set $fdtfile /if/ needed because of that, but I don't think should be forced to in all cases where the default makes sense.I really don't care whether we set fdtfile and use $fdtfile or whether we insert the filename string directly into the appropriate command variable... My point is U-Boot via its jetson-tk1_defconfig / .config knows this (or should know) better than any user.
Yes, U-Boot is a good place to put this information. I disagree that it /has/ to come from the defconfig/.config file.
> And it seemed to me
that variables were not exactly used sparingly in the distro mechanism so far, so I don't see why not to populate that variable _if_ we know what its value needs to be. Do you have any real reason for being against populating fdtfile at whatever level turns out to be suitable?
Yes, $fdtfile} can be auto-populated /at whatver level turns out to be suitable/. I think the only question is what "level" /is/ suitable.
I believe there is no argument that this patch will not be applied.
There's a double negative there so I may not be interpreting you correctly. I believe this patch should not be applied. My objections are:a) This only solves the problem for a single board, yet the issue it solves occurs on many/all boards. We need a solution that solves them all.
b) Boards should not need to define this value given that it can be calculated automatically.
However I am strongly rejecting your attitude that everything is there already with variables and that nothing new is needed. Something needs to be done somewhere - and we need to figure out what exactly and where for minimum impact to the release.
Everything is there.$soc/$board are set by include/env_default.h, with the option for per-board configuration files to override those values if the default CONFIG_SYS_* are incorrect for some reason. Differences between CONFIG_SYS_* and DTB filenames should not be an argument against the default I proposed. So, those values are always available (well, the board must enable CONFIG_ENV_VARS_UBOOT_CONFIG to get them; part of using distro_bootcmd is either enabling that config option or setting $fdtfile some other way).
I put effort into ensuring that "${soc}-${board}${boardver}.dtb" was the correct filename for Tegra boards, and I believe this holds true for a variety of non-Tegra boards as well.
The proposed default $fdtfile value I mentioned above should be the default, since everything required to construct it is already there. Boards that use distro_bootcmd should ensure that they set those variables correctly to allow that default to work; that's the whole point of those variables. Anyone enabling distro_bootcmd on other platforms hopefully ensured those variables were set correctly. If not, we can surely go back and fix those platforms.
Do you have a handle on exactly how many boards don't set those variables in a way that default won't work for them? Can't we fix their $soc/$board values so that the default does work?
I believe the calculation of the default/fallback value for $fdtfile does need to happen at runtime. ${boardver} is by definition a runtime variable since it reflects the HW version. As such, we can't simply put the following into e.g. tegra-common.h:
#define fdtfile "${soc}-${board}${boardver}.dtb"
(or any equivalent of that is actually valid syntax, i.e. using C
pre-processor macros)
(Right now, I believe we don't actually set $boardver at runtime since for the board where it matters we only support one HW revision at all. However, I don't want to break that runtime capability. If we do, we should just override $board for that board, and delete all references to $boardver. I vaguely recall that some non-Tegra boards might use $boardver though?)
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
