On Thu, 21 Feb 2019 12:47:04 +0100 Alexander Graf agraf@suse.de wrote:
Hi Alex,
On 02/21/2019 02:30 AM, Andre Przywara wrote:
Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure. This allows loaders to take the target architecture in account for custom loading procedures. Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.
Signed-off-by: Andre Przywara andre.przywara@arm.com
Reviewed-by: Alexander Graf agraf@suse.de
Thanks for having a look!
I don't fully buy the argument that the generic mapping would be too big though. Realistically you should be able to get away with 1 or 2 branches per case, no? So that would be maybe 40 instructions?
You are apparently not in the Allwinner SPL mindset ;-) I get excited when I find a way of saving 100 bytes (see the writel_relaxed patch), so wasting 160 bytes when we will probably never need to return IH_ARCH_X86_64 doesn't sound right to me.
And Philipp is right: the canonical way would be to use uimage_arch[] from common/image.c, which is quite big.
We could have a big #ifdef cascade, something like: +#ifdef CONFIG_ARM + if (!strcmp(arch_str, "arm")) + return IH_ARCH_ARM; + + if (!strcmp(arch_str, "arm64")) + return IH_ARCH_ARM64; +#endif +#ifdef CONFIG_X86 + if (!strcmp(arch_str, "x86")) + return IH_ARCH_I386; + + if (!strcmp(arch_str, "x86_64")) + return IH_ARCH_X86_64; +#endif ....
I am just not sure it's worth to introduce this out of the blue *now*, without use cases and without proper testing.
Cheers, Andre.