On 29.05.2016 21:08, Daniel Gimpelevich wrote: > On Sun, 2016-05-29 at 21:01 +0200, Jonas Gorski wrote: >> On 29.05.2016 20:38, Daniel Gimpelevich wrote: >>> On Sun, 2016-05-29 at 12:53 +0200, Jonas Gorski wrote: >>>> This will break/won't compile for ZBOOT_APPENDED_DTB as __appended_dtb >>>> is >>>> part of the wrapping decompressor, and the kernel has no knowledge of >>>> this >>>> this symbol. >>> >>> If this were true, it wouldn't compile with the code you added to >>> compressed/head.S, either. You're referencing it as an external symbol, >>> which is exactly the same thing I'm doing here. >> >> There are two different __appended_dtb definitions: the one for the >> (uncompressed) kernel appended one that is visible to the kernel (in >> kernel/vmlinux.lds.S), and one in boot/compressed/ld.script that is >> visible only to the wrapping decompressor. >> >> Since the wrapping decompressor is built *after* the kernel was compiled >> and compressed, there is no way to tell the kernel where __appended_dtb is >> relative to the decompressor, so for ZBOOT_APPENDED_DTB you cannot >> reference __appended_dtb from kernel code. >> >> Your proposed >>> alternatives are functionally almost equivalent to your earlier rejected >>> patches: >> >> These weren't rejected, just deemed insufficient (mostly by me myself). >> >> And only to this one is similar: >>> https://patchwork.linux-mips.org/patch/7274/ >> >> But in contrast to this one, it doesn't populate initial_boot_params auto- >> matically, but instead still requires the mach to do that (by calling >> __dt_setup_arch()). I dropped that because IIRC at that time I read that >> initial_boot_params isn't supposed to be directly accessed. >> Also not populating initial_boot_params is IMHO better as just because >> a0 says -2 it doesn't mean a1 references a dtb - that should still be up >> to the mach to say that it expects a dtb to be passed. >> >> >>> https://patchwork.linux-mips.org/patch/7313/ >> >> This one was only missing alignment for the !SMP case but is otherwise >> equivalent to what is in the kernel now. >> >> >> Jonas > > I see, and you are absolutely right in what you now suggest. What > escapes me at the moment, though, is how to reconcile all this with UHI. > UHI bootloaders may pass an external DTB, and it should be > indistinguishable by the boot code from an appended DTB. Thoughts? That's what my proposed code does: if a0 is -2, then store a1 in fw_passed_dtb else if appended dtb support is enabled and there is a valid dtb at __appended_dtb, then store the address of it in fw_passed_dtb. else set fw_passed_dtb to 0. so for the kernel/mach side, there is no difference between a UHI passed dtb and a __appended_dtb, both will populate fw_passed_dtb. That still leaves the question which one should be preferred in case both are present. Jonas