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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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?

> 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.

> 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/

> +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?

> +
> +	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..

> +
> +	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.

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.

>  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?


Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |