Re: [PATCH v1 3/4] ARM: boards: add support for Samsung Galaxy S8 (dreamlte)

[Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>]

On 7/30/25 11:31, Ahmad Fatoum wrote:
> Hello Ivaylo,
>
> Thanks for your patch.
>
> On 7/29/25 22:36, Ivaylo Ivanov wrote:
>> Phones utilizing an exynos SoC boot android with samsung's proprietary
>> bootloader, called s-boot (s-lk on newer devices). However, not only is
>> it closed source, it also enforces some limitations that prevent us from
>> booting mainline linux cleanly on them, such as an applied overlay device
>> tree, disabled framebuffer refreshing, misaligned kernel image at boot.
> misaligned kernel image might be a bit problematic for barebox too, but
> we can fix it for PBL if needed.
>
> What misalignment are we speaking of?

On exynos 8890 up to exynos990, the bootloader takes a random number via
hwrng in a specific range after 80000000. If I recall correctly, when I was booting
mainline linux without any shim on my galaxy S8 (it was 5 years ago), the kernel
was complaining about misaligment, but I'd need to recreate that to be able
to provide logs. Anyways, I've tested barebox on 4 exynos devices and so far
there haven't been any inconsistencies at boot.

>
>> Therefore, having another stage bootloader, loaded as a linux kernel
>> image by s-boot, is best.
>>
>> Add support for Samsung Galaxy S8, utilizing the exynos 8895 SoC. Support
>> is modelled to be as reusable on other devices as possible, requiring
>> only a minimal set of changes to boot - a barebox device tree, which in
>> this case is basically imported torvalds tree for dreamlte, that is then
>> matched from the downstream device tree, provided by s-boot at x0.
>>
>> For some reason, on certain devices the stack set up by the previous
>> bootloader is not enough. Since the idea of this board support is to be
>> as generic as possible, setting a fixed stack top via
>> ENTRY_FUNCTION_WITHSTACK does not make sense, due to different exynos
>> devices having different memory layouts - exynos8895's dram starts at
>> 0x80000000, whereas exynos7870's starts at 0x40000000. Instead, set the
>> SP as early as possible in the entry C function by taking the memory base
>> from the downstream fdt + (SZ_8M - SZ_64K).
> naked functions are not support on ARM64, so setting a stack pointer in
> C code is always an unsafe operation that could mess up codegen.
>
> If you need to do this dynamically, this needs to be done in assembly.

hm, so I could just have a C entry that messes with fdt, and then jump to
the assembly function for SP relocation, after which we go back to C?

>
>> Barebox has to be packaged as an android boot image:
>>
>> mkbootimg --kernel images/barebox-exynos.img \
>> --ramdisk ramdisk.bin \
>> --dt stock.dtb
>> --cmdline "buildvariant=eng" \
>> --base 0x10000000 \
>> --kernel_offset 0x00008000 \
>> --ramdisk_offset 0x01000000 \
>> --second_offset 0x00f00000 \
>> --tags_offset 0x00000100 \
>> --os_version 9.0.0 \
>> --os_patch_level 2019-10 \
>> --pagesize 2048 \
>> --hash sha1 \
>> --output boot.img
>>
>> And then flashed to the boot partition:
>>
>> heimdall flash --BOOT boot.img
>>
>> Currently, only a minimal set of features work. An image can be booted by
>> barebox by configuring barebox to jump to the address where ramdisk gets
>> loaded by s-boot, and packaging that payload as a ramdisk with mkbootimg.
> Nice. Would be good to have this info in Documentation/

I was gonna do that in a separate patch after the patchset gets merged.

>
>> +postcore_platform_driver(exynos_board_driver);
>> diff --git a/arch/arm/boards/samsung-exynos/lowlevel.c b/arch/arm/boards/samsung-exynos/lowlevel.c
>> new file mode 100644
>> index 00000000..9c4a0297
>> --- /dev/null
>> +++ b/arch/arm/boards/samsung-exynos/lowlevel.c
>> @@ -0,0 +1,78 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2025 Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> + */
>> +#include <common.h>
>> +#include <pbl.h>
>> +#include <linux/sizes.h>
>> +#include <asm/barebox-arm-head.h>
>> +#include <asm/barebox-arm.h>
>> +#include <asm/sections.h>
>> +#include <asm/cache.h>
>> +#include <asm/mmu.h>
>> +
>> +extern char __dtb_exynos8895_dreamlte_start[];
> As you don't need the DT in PBL, you can embed it in compress form by
> using __dtb_z. This will be especially useful when we start to ship
> multiple device trees.
>
>> +
>> +static bool is_compat(const void *fdt, const char *prefix)
>> +{
>> +	int node, len;
>> +	const char *compat;
>> +
>> +	node = fdt_path_offset(fdt, "/");
>> +	if (node < 0)
>> +		return false;
>> +
>> +	compat = fdt_getprop(fdt, node, "model", &len);
>> +	if (!compat)
>> +		return false;
> Why compare the model and not the compatible as the compat variable name
> would suggest?

Because Samsung are more consistent at coming up with model name
props than compatibles. For example:

compatible = "samsung,X1S EUR OPEN 05\0samsung,EXYNOS990";
compatible = "samsung, DREAMLTE KOR rev01", "samsung,EXYNOS8895";
compatible = "samsung, J7VE LTE LTN OPEN 00", "samsung,exynos7870";

there are inconsistencies in spacing, capitalizing etc etc. But yeah, I will
change the variable name to is_model.

>
>> +
>> +	while (*prefix) {
>> +		if (*compat++ != *prefix++)
>> +			return false;
>> +	}
>> +	return true;
>> +}
>> +
>> +static noinline void exynos_continue(void *downstream_fdt)
>> +{
>> +	void *fdt;
>> +	unsigned long membase, memsize;
>> +	char *__dtb_start;
>> +
>> +	/* select device tree dynamically */
>> +	if (is_compat(downstream_fdt, "Samsung DREAMLTE")) {
>> +		__dtb_start = __dtb_exynos8895_dreamlte_start;
>> +	} else {
>> +		/* we didn't match any device */
>> +		return;
>> +	}
>> +	fdt = __dtb_start + get_runtime_offset();
>> +	fdt_find_mem(fdt, &membase, &memsize);
> Ah, so you do need the FDT in uncompressed form..

Mhm. I tried taking mem from downstream DT, but it's so broken it
doesn't work.

>
>> +
>> +	barebox_arm_entry(membase, memsize, fdt);
>> +}
>> +
>> +ENTRY_FUNCTION(start_exynos, x0, x1, x2)
>> +{
>> +	void *downstream_fdt = (void *)x0;
>> +	unsigned long mem_base, mem_size;
>> +
>> +	if (!downstream_fdt || fdt_check_header(downstream_fdt))
>> +		return;
>> +
>> +	/*
>> +	 * The previous bootloader has a stack set up, but it seems to not be
>> +	 * enough as we can't get past the relocation on some devices. Set up
>> +	 * a stack determined by the memory node from the downstream fdt.
>> +	 */
>> +	fdt_find_mem(downstream_fdt, &mem_base, &mem_size);
> It's not a good idea to call this here before having set up the C
> environment. This makes regressions after compile updates much more
> likely. Can't we instead grow down from the start of the barebox binary?
>
> That's what start_dt_2nd in board-dt-2nd-aarch64.S is doing.

iirc this is not my first attempt at barebox, I had some work a few months ago,
and the board-dt-2nd wasn't booting. I suppose it's the SP stuff that was borked.
I will test it again though and report back.

>
> In future, we should make it easier to reuse the dt-2nd entry pointer,
> maybe with a macro that can be used instead of ENTRY_FUNCTION.
>
>> +	asm volatile("mov sp, %0" : : "r"(mem_base + SZ_8M - SZ_64K));
>> +
>> +	arm_cpu_lowlevel_init();
>> +
>> +	relocate_to_current_adr();
>> +
>> +	setup_c();
>> +
>> +	exynos_continue(downstream_fdt);
>> +}
>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>> index 6612a514..a53834f7 100644
>> --- a/arch/arm/dts/Makefile
>> +++ b/arch/arm/dts/Makefile
>> @@ -13,6 +13,7 @@ lwl-$(CONFIG_MACH_BEAGLEPLAY) += k3-am625-beagleplay.dtb.o k3-am625-r5-beaglepla
>>  lwl-$(CONFIG_MACH_CLEP7212) += ep7212-clep7212.dtb.o
>>  lwl-$(CONFIG_MACH_CM_FX6) += imx6dl-cm-fx6.dtb.o imx6q-cm-fx6.dtb.o imx6q-utilite.dtb.o
>>  lwl-$(CONFIG_MACH_DFI_FS700_M60) += imx6q-dfi-fs700-m60-6q.dtb.o imx6dl-dfi-fs700-m60-6s.dtb.o
>> +lwl-$(CONFIG_MACH_EXYNOS) += exynos8895-dreamlte.dtb.o
> It's on my todo list to add a way to specify device trees externally to
> make. Still need to figure out how it will look like though.

Yeah, I was thinking that it'd be nice to just compile it directly from
dts/src/.

>
>>  lwl-$(CONFIG_MACH_DUCKBILL) += imx28-duckbill.dtb.o
>>  lwl-$(CONFIG_MACH_KINDLE_MX50) += imx50-kindle-d01100.dtb.o imx50-kindle-d01200.dtb.o imx50-kindle-ey21.dtb.o
>>  lwl-$(CONFIG_MACH_EFIKA_MX_SMARTBOOK) += imx51-genesi-efika-sb.dtb.o
>> diff --git a/arch/arm/dts/exynos8895-dreamlte.dts b/arch/arm/dts/exynos8895-dreamlte.dts
>> new file mode 100644
>> index 00000000..36b5271e
>> --- /dev/null
>> +++ b/arch/arm/dts/exynos8895-dreamlte.dts
>> @@ -0,0 +1,13 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +/*
>> + * Samsung Galaxy S8 (dreamlte/SM-G950F) barebox device tree source
>> + *
>> + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> + */
>> +
>> +/dts-v1/;
>> +#include <arm64/exynos/exynos8895-dreamlte.dts>
>> +
>> +/ {
>> +	barebox,disable-deep-probe;
> Why not enable it?

Honestly, I other devices doing that and didn't dig too deep into what
it's meant for. Will test.

Thanks and best regards,
Ivaylo

>
>
> Cheers,
> Ahmad
>