[PATCH 1/5] ARM: at91: Add PBL_BREAK support

[PATCH 1/5] ARM: at91: Add PBL_BREAK support

[Alexander Shiyan]

This adds PBL_BREAK support to the barebox at91 header.
Since we're here, let's set the header parameters to the defaults
for the barebox and do some code formatting to improve readability.

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 include/mach/at91/barebox-arm.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/mach/at91/barebox-arm.h b/include/mach/at91/barebox-arm.h
index 3a041b4557..ef4bb185bc 100644
--- a/include/mach/at91/barebox-arm.h
+++ b/include/mach/at91/barebox-arm.h
@@ -34,15 +34,16 @@ static __always_inline void __barebox_at91_head(void)
 		".endr\n"
 		"2:\n"
 	);
+	pbl_barebox_break();
 }
 
 #define SAMA5_ENTRY_FUNCTION(name, stack_top, r4)				\
-	void name (u32 r0, u32 r1, u32 r2, u32 r3);				\
+	void name (u32 r0, u32 r1, u32 r2);					\
 										\
 	static void __##name(u32);						\
 										\
 	void __naked __section(.text_head_entry_##name)	name			\
-				(u32 r0, u32 r1, u32 r2, u32 r3)		\
+				(u32 r0, u32 r1, u32 r2)			\
 		{								\
 			register u32 r4 asm("r4");				\
 			__barebox_at91_head();					\
@@ -50,7 +51,7 @@ static __always_inline void __barebox_at91_head(void)
 				arm_setup_stack(stack_top);			\
 			__##name(r4);						\
 		}								\
-		static void noinline __##name(u32 r4)
+	static void noinline __##name(u32 r4)
 
 /* BootROM already initialized usable stack top */
 #define SAMA5D2_ENTRY_FUNCTION(name, r4)					\
@@ -62,7 +63,7 @@ static __always_inline void __barebox_at91_head(void)
 #define SAMA5D4_ENTRY_FUNCTION(name, r4)					\
 	SAMA5_ENTRY_FUNCTION(name, SAMA5D4_SRAM_BASE + SAMA5D4_SRAM_SIZE, r4)
 
-#define SAM9_ENTRY_FUNCTION(name)	\
+#define SAM9_ENTRY_FUNCTION(name)						\
 	ENTRY_FUNCTION_WITHSTACK_HEAD(name, AT91SAM9261_SRAM_BASE + AT91SAM9261_SRAM_SIZE, \
 				      __barebox_at91_head, r0, r1, r2)
 
-- 
2.39.1

[PATCH 2/5] ARM: at91: Merge sama5_boot_xload() asm code

[Alexander Shiyan]

This merges the sama5_boot_xload() assembler code to prevent
the r4 register from being used in the second line of asm volatile().

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 include/mach/at91/sama5_bootsource.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/mach/at91/sama5_bootsource.h b/include/mach/at91/sama5_bootsource.h
index 2fb65ce10f..aeda1b4114 100644
--- a/include/mach/at91/sama5_bootsource.h
+++ b/include/mach/at91/sama5_bootsource.h
@@ -56,8 +56,13 @@ extern unsigned long at91_bootsource;
 
 static inline void __noreturn sama5_boot_xload(void __noreturn (*bb)(void), u32 r4)
 {
-	asm volatile("mov r4, %0" : : "r"(r4) : );
-	asm volatile("bx  %0"     : : "r"(bb) : );
+	asm volatile (
+		"mov r4, %0\n"
+		"bx  %1\n"
+		:
+		: "r"(r4), "r"(bb)
+		: "r4"
+	);
 	__builtin_unreachable();
 }
 
-- 
2.39.1

[PATCH 3/5] ARM: at91: Rename xload-mmc.c

[Alexander Shiyan]

This will rename xload-mmc.c to the simpler name xload.c indicating
support for booting from more than just MMC for the first stage of
the bootloader.
Also rename the hidden helper configuration option for this.

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 arch/arm/mach-at91/Kconfig                  | 2 +-
 arch/arm/mach-at91/Makefile                 | 3 +--
 arch/arm/mach-at91/{xload-mmc.c => xload.c} | 0
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename arch/arm/mach-at91/{xload-mmc.c => xload.c} (100%)

diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index 2f70e4aac3..bb04c1267e 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -52,7 +52,7 @@ config HAVE_AT91_SDRAMC
 config HAVE_AT91_DDRAMC
 	bool
 
-config AT91_MCI_PBL
+config AT91_PBL
 	def_bool MCI_ATMEL_PBL || MCI_ATMEL_SDHCI_PBL
 
 # Select if board uses the common at91sam926x_board_init
diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index 556e46f181..1c87cab5bb 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -14,8 +14,7 @@ obj-$(CONFIG_BOOTM) += bootm-barebox.o
 obj-y += at91sam9_reset.o
 obj-pbl-$(CONFIG_HAVE_AT91_DDRAMC) += ddramc.o
 obj-pbl-$(CONFIG_HAVE_AT91_SDRAMC) += sdramc.o
-pbl-$(CONFIG_AT91_MCI_PBL) +=  xload-mmc.o
-pbl-$(CONFIG_AT91_MCI_PBL) +=  at91sam9_xload_mmc.o
+pbl-$(CONFIG_AT91_PBL) += at91sam9_xload_mmc.o xload.o
 
 obj-$(CONFIG_AT91SAM9_SMC) += sam9_smc.o
 obj-$(CONFIG_HAVE_AT91SAM9_RST) += at91sam9_rst.o
diff --git a/arch/arm/mach-at91/xload-mmc.c b/arch/arm/mach-at91/xload.c
similarity index 100%
rename from arch/arm/mach-at91/xload-mmc.c
rename to arch/arm/mach-at91/xload.c
-- 
2.39.1

[PATCH 4/5] ARM: at91: sama5: Unify xx_start_image() calls

[Alexander Shiyan]

This unifies the xload.c start_image() code for both sama5d2() and
sama5d3() CPU variants.

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 .../arm/boards/sama5d27-giantboard/lowlevel.c |   2 +-
 arch/arm/boards/sama5d27-som1/lowlevel.c      |   2 +-
 arch/arm/mach-at91/xload.c                    | 102 ++++++++++--------
 include/mach/at91/xload.h                     |   2 +-
 4 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/arch/arm/boards/sama5d27-giantboard/lowlevel.c b/arch/arm/boards/sama5d27-giantboard/lowlevel.c
index 49540bede0..c266af6357 100644
--- a/arch/arm/boards/sama5d27-giantboard/lowlevel.c
+++ b/arch/arm/boards/sama5d27-giantboard/lowlevel.c
@@ -31,7 +31,7 @@ SAMA5D2_ENTRY_FUNCTION(start_sama5d27_giantboard_xload_mmc, r4)
 
 	sama5d2_udelay_init(MASTER_CLOCK);
 	sama5d2_d1g_ddrconf();
-	sama5d2_sdhci_start_image(r4);
+	sama5d2_start_image(r4);
 }
 
 extern char __dtb_z_at91_sama5d27_giantboard_start[];
diff --git a/arch/arm/boards/sama5d27-som1/lowlevel.c b/arch/arm/boards/sama5d27-som1/lowlevel.c
index 67300587fe..e74502244b 100644
--- a/arch/arm/boards/sama5d27-som1/lowlevel.c
+++ b/arch/arm/boards/sama5d27-som1/lowlevel.c
@@ -55,7 +55,7 @@ SAMA5D2_ENTRY_FUNCTION(start_sama5d27_som1_ek_xload_mmc, r4)
 	ek_turn_led(RGB_LED_RED | RGB_LED_GREEN); /* Yellow */
 	sama5d2_udelay_init(MASTER_CLOCK);
 	sama5d2_d1g_ddrconf();
-	sama5d2_sdhci_start_image(r4);
+	sama5d2_start_image(r4);
 }
 
 extern char __dtb_z_at91_sama5d27_som1_ek_start[];
diff --git a/arch/arm/mach-at91/xload.c b/arch/arm/mach-at91/xload.c
index 9c03d2119c..e417223b31 100644
--- a/arch/arm/mach-at91/xload.c
+++ b/arch/arm/mach-at91/xload.c
@@ -11,39 +11,50 @@
 #include <asm/cache.h>
 #include <pbl/bio.h>
 
-static void at91_fat_start_image(struct pbl_bio *bio,
-				 void *buf, unsigned int len,
-				 u32 r4)
+struct xload_instance {
+	void __iomem *base;
+	unsigned id;
+	u8 periph;
+	s8 pins[15];
+};
+
+static void at91_fat_start_image(struct pbl_bio *bio, u32 r4)
 {
-	void __noreturn (*bb)(void);
+	void *buf = (void *)SAMA5_DDRCS;
 	int ret;
 
-	ret = pbl_fat_load(bio, "barebox.bin", buf, len);
+	ret = pbl_fat_load(bio, "barebox.bin", buf, SZ_2M);
 	if (ret < 0) {
 		pr_err("pbl_fat_load: error %d\n", ret);
 		return;
 	}
 
-	bb = buf;
-
 	sync_caches_for_execution();
 
-	sama5_boot_xload(bb, r4);
+	sama5_boot_xload(buf, r4);
 }
 
-static const struct sdhci_instance {
-	void __iomem *base;
-	unsigned id;
-	u8 periph;
-	s8 pins[15];
-} sdhci_instances[] = {
+static const struct xload_instance sama5d2_mci_instances[] = {
 	[0] = {
-		.base = SAMA5D2_BASE_SDHC0, .id = SAMA5D2_ID_SDMMC0, .periph = AT91_MUX_PERIPH_A,
-		.pins = { 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 13, 10, 11, 12, -1 }
+		.base = SAMA5D2_BASE_SDHC0,
+		.id = SAMA5D2_ID_SDMMC0,
+		.periph = AT91_MUX_PERIPH_A,
+		.pins = {
+			AT91_PIN_PA2, AT91_PIN_PA3, AT91_PIN_PA4, AT91_PIN_PA5,
+			AT91_PIN_PA6, AT91_PIN_PA7, AT91_PIN_PA8, AT91_PIN_PA9,
+			AT91_PIN_PA0, AT91_PIN_PA1, AT91_PIN_PA13, AT91_PIN_PA10,
+			AT91_PIN_PA11, AT91_PIN_PA12, -1
+		}
 	},
 	[1] = {
-		.base = SAMA5D2_BASE_SDHC1, .id = SAMA5D2_ID_SDMMC1, .periph = AT91_MUX_PERIPH_E,
-		.pins = { 18, 19, 20, 21, 22, 28, 30, -1 }
+		.base = SAMA5D2_BASE_SDHC1,
+		.id = SAMA5D2_ID_SDMMC1,
+		.periph = AT91_MUX_PERIPH_E,
+		.pins = {
+			AT91_PIN_PA18, AT91_PIN_PA19, AT91_PIN_PA20,
+			AT91_PIN_PA21, AT91_PIN_PA22, AT91_PIN_PA28,
+			AT91_PIN_PA30, -1
+		}
 	},
 };
 
@@ -51,25 +62,23 @@ static const struct sdhci_instance {
  * sama5d2_sdhci_start_image - Load and start an image from FAT-formatted SDHCI
  * @r4: value of r4 passed by BootROM
  */
-void __noreturn sama5d2_sdhci_start_image(u32 r4)
+static void __noreturn sama5d2_sdhci_start_image(u32 r4)
 {
-	void *buf = (void *)SAMA5_DDRCS;
-	const struct sdhci_instance *instance;
+	const struct xload_instance *instance;
 	struct pbl_bio bio;
 	const s8 *pin;
 	int ret;
 
 	ret = sama5_bootsource_instance(r4);
-	if (ret > 1)
+	if (ret > ARRAY_SIZE(sama5d2_mci_instances) - 1)
 		panic("Couldn't determine boot MCI instance\n");
 
-	instance = &sdhci_instances[ret];
+	instance = &sama5d2_mci_instances[ret];
 
 	sama5d2_pmc_enable_periph_clock(SAMA5D2_ID_PIOA);
-	for (pin = instance->pins; *pin >= 0; pin++) {
+	for (pin = instance->pins; *pin >= 0; pin++)
 		at91_mux_pio4_set_periph(SAMA5D2_BASE_PIOA,
 					 BIT(*pin), instance->periph);
-	}
 
 	sama5d2_pmc_enable_periph_clock(instance->id);
 	sama5d2_pmc_enable_generic_clock(instance->id, AT91_PMC_GCKCSS_UPLL_CLK, 1);
@@ -80,18 +89,26 @@ void __noreturn sama5d2_sdhci_start_image(u32 r4)
 
 	/* TODO: eMMC boot partition handling: they are not FAT-formatted */
 
-	at91_fat_start_image(&bio, buf, SZ_16M, r4);
+	at91_fat_start_image(&bio, r4);
 
 out_panic:
 	panic("FAT chainloading failed\n");
 }
 
-static const struct atmci_instance {
-	void __iomem *base;
-	unsigned id;
-	u8 periph;
-	s8 pins[15];
-} sama5d3_atmci_instances[] = {
+void __noreturn sama5d2_start_image(u32 r4)
+{
+	switch (sama5_bootsource(r4)) {
+	case BOOTSOURCE_MMC:
+		sama5d2_sdhci_start_image(r4);
+		break;
+	default:
+		break;
+	}
+
+	panic("Unsupported boot configuration!\n");
+}
+
+static const struct xload_instance sama5d3_mci_instances[] = {
 	[0] = {
 		.base = IOMEM(SAMA5D3_BASE_HSMCI0),
 		.id = SAMA5D3_ID_HSMCI0,
@@ -99,30 +116,29 @@ static const struct atmci_instance {
 		.pins = {
 			AT91_PIN_PD0, AT91_PIN_PD1, AT91_PIN_PD2, AT91_PIN_PD3,
 			AT91_PIN_PD4, AT91_PIN_PD5, AT91_PIN_PD6, AT91_PIN_PD7,
-			AT91_PIN_PD8, AT91_PIN_PD9, -1 }
+			AT91_PIN_PD8, AT91_PIN_PD9, -1
+		}
 	},
 };
 
-void __noreturn sama5d3_atmci_start_image(u32 boot_src, unsigned int clock,
+void __noreturn sama5d3_atmci_start_image(u32 r4, unsigned int clock,
 					  unsigned int slot)
 {
-	void *buf = (void *)SAMA5_DDRCS;
-	const struct atmci_instance *instance;
+	const struct xload_instance *instance;
 	struct pbl_bio bio;
 	const s8 *pin;
 	int ret;
 
-	ret = sama5_bootsource_instance(boot_src);
-	if (ret > ARRAY_SIZE(sama5d3_atmci_instances) - 1)
+	ret = sama5_bootsource_instance(r4);
+	if (ret > ARRAY_SIZE(sama5d3_mci_instances) - 1)
 		panic("Couldn't determine boot MCI instance\n");
 
-	instance = &sama5d3_atmci_instances[boot_src];
+	instance = &sama5d3_mci_instances[r4];
 
 	sama5d3_pmc_enable_periph_clock(SAMA5D3_ID_PIOD);
-	for (pin = instance->pins; *pin >= 0; pin++) {
+	for (pin = instance->pins; *pin >= 0; pin++)
 		at91_mux_pio3_pin(IOMEM(SAMA5D3_BASE_PIOD),
-					 pin_to_mask(*pin), instance->periph, 0);
-	}
+				  pin_to_mask(*pin), instance->periph, 0);
 
 	sama5d3_pmc_enable_periph_clock(instance->id);
 
@@ -130,7 +146,7 @@ void __noreturn sama5d3_atmci_start_image(u32 boot_src, unsigned int clock,
 	if (ret)
 		goto out_panic;
 
-	at91_fat_start_image(&bio, buf, SZ_16M, boot_src);
+	at91_fat_start_image(&bio, r4);
 
 out_panic:
 	panic("FAT chainloading failed\n");
diff --git a/include/mach/at91/xload.h b/include/mach/at91/xload.h
index 2886490246..d5fc7596fd 100644
--- a/include/mach/at91/xload.h
+++ b/include/mach/at91/xload.h
@@ -6,7 +6,7 @@
 #include <linux/compiler.h>
 #include <pbl/bio.h>
 
-void __noreturn sama5d2_sdhci_start_image(u32 r4);
+void __noreturn sama5d2_start_image(u32 r4);
 void __noreturn sama5d3_atmci_start_image(u32 r4, unsigned int clock,
 					  unsigned int slot);
 
-- 
2.39.1

[PATCH 5/5] ARM: at91: sama5d2: Add QSPI boot support

[Alexander Shiyan]

This adds QSPI NOR boot support for sama5d2 CPUs.

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 arch/arm/mach-at91/xload.c | 84 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm/mach-at91/xload.c b/arch/arm/mach-at91/xload.c
index e417223b31..9f9945e96d 100644
--- a/arch/arm/mach-at91/xload.c
+++ b/arch/arm/mach-at91/xload.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <common.h>
+#include <filetype.h>
 #include <mach/at91/xload.h>
 #include <mach/at91/sama5_bootsource.h>
 #include <mach/at91/hardware.h>
@@ -95,12 +96,95 @@ static void __noreturn sama5d2_sdhci_start_image(u32 r4)
 	panic("FAT chainloading failed\n");
 }
 
+static const struct xload_instance sama5d2_qspi_ioset1_instances[] = {
+	[0] = {
+		.base = SAMA5D2_BASE_QSPI0,
+		.id = SAMA5D2_ID_QSPI0,
+		.periph = AT91_MUX_PERIPH_B,
+		.pins = {
+			AT91_PIN_PA0, AT91_PIN_PA1, AT91_PIN_PA2,
+			AT91_PIN_PA3, AT91_PIN_PA4, AT91_PIN_PA5, -1
+		}
+	},
+	[1] = {
+		.base = SAMA5D2_BASE_QSPI1,
+		.id = SAMA5D2_ID_QSPI1,
+		.periph = AT91_MUX_PERIPH_B,
+		.pins = {
+			AT91_PIN_PA6, AT91_PIN_PA7, AT91_PIN_PA8,
+			AT91_PIN_PA9, AT91_PIN_PA10, AT91_PIN_PA11, -1
+		}
+	},
+};
+
+/**
+ * sama5d2_qspi_start_image - Start an image from QSPI NOR flash
+ * @r4: value of r4 passed by BootROM
+ */
+static void __noreturn sama5d2_qspi_start_image(u32 r4)
+{
+	void __iomem *mem, *dest = IOMEM(SAMA5_DDRCS);
+	const struct xload_instance *instance;
+	const s8 *pin;
+	u32 offs;
+	int ret;
+
+	ret = sama5_bootsource_instance(r4);
+	if (ret == 0)
+		mem = SAMA5D2_BASE_QSPI0_MEM;
+	else if (ret == 1)
+		mem = SAMA5D2_BASE_QSPI1_MEM;
+	else
+		panic("Couldn't determine boot QSPI instance\n");
+
+	instance = &sama5d2_qspi_ioset1_instances[ret];
+
+	sama5d2_pmc_enable_periph_clock(SAMA5D2_ID_PIOA);
+	for (pin = instance->pins; *pin >= 0; pin++)
+		at91_mux_pio4_set_periph(SAMA5D2_BASE_PIOA,
+					 BIT(*pin), instance->periph);
+
+	sama5d2_pmc_enable_periph_clock(instance->id);
+
+	/*
+	 * Since we booted from QSPI, we expect the QSPI registers to be
+	 * properly initialized already.
+	 * Let's just read the memory-mapped data.
+	 */
+
+	/* Find barebox pattern first */
+	for (offs = SZ_128K; offs <= SZ_1M; offs += SZ_128K) {
+		/* Fix cache coherency issue by reading each sector only once */
+		memcpy(dest, mem + offs, SZ_128K);
+
+		if (is_barebox_arm_head(dest)) {
+			u32 size = readl(dest + ARM_HEAD_SIZE_OFFSET);
+
+			pr_info("Image found at 0x%08x, size %u\n", offs, size);
+
+			/* Copy remaining barebox code */
+			if (size > SZ_128K)
+				memcpy(dest + SZ_128K, mem + offs + SZ_128K,
+				       size - SZ_128K);
+
+			sync_caches_for_execution();
+
+			sama5_boot_xload(dest, r4);
+		}
+	}
+
+	panic("No barebox image found!\n");
+}
+
 void __noreturn sama5d2_start_image(u32 r4)
 {
 	switch (sama5_bootsource(r4)) {
 	case BOOTSOURCE_MMC:
 		sama5d2_sdhci_start_image(r4);
 		break;
+	case BOOTSOURCE_SPI:
+		sama5d2_qspi_start_image(r4);
+		break;
 	default:
 		break;
 	}
-- 
2.39.1

Re: [PATCH 4/5] ARM: at91: sama5: Unify xx_start_image() calls

[Sascha Hauer]

Hi Alexander,

On Tue, Mar 18, 2025 at 10:43:26AM +0300, Alexander Shiyan wrote:
> This unifies the xload.c start_image() code for both sama5d2() and
> sama5d3() CPU variants.

I have trouble bringing the description together with what the patch
acutally does.

> 
> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> ---
>  .../arm/boards/sama5d27-giantboard/lowlevel.c |   2 +-
>  arch/arm/boards/sama5d27-som1/lowlevel.c      |   2 +-
>  arch/arm/mach-at91/xload.c                    | 102 ++++++++++--------
>  include/mach/at91/xload.h                     |   2 +-
>  4 files changed, 62 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/arm/boards/sama5d27-giantboard/lowlevel.c b/arch/arm/boards/sama5d27-giantboard/lowlevel.c
> index 49540bede0..c266af6357 100644
> --- a/arch/arm/boards/sama5d27-giantboard/lowlevel.c
> +++ b/arch/arm/boards/sama5d27-giantboard/lowlevel.c
> @@ -31,7 +31,7 @@ SAMA5D2_ENTRY_FUNCTION(start_sama5d27_giantboard_xload_mmc, r4)
>  
>  	sama5d2_udelay_init(MASTER_CLOCK);
>  	sama5d2_d1g_ddrconf();
> -	sama5d2_sdhci_start_image(r4);
> +	sama5d2_start_image(r4);
>  }
>  
>  extern char __dtb_z_at91_sama5d27_giantboard_start[];
> diff --git a/arch/arm/boards/sama5d27-som1/lowlevel.c b/arch/arm/boards/sama5d27-som1/lowlevel.c
> index 67300587fe..e74502244b 100644
> --- a/arch/arm/boards/sama5d27-som1/lowlevel.c
> +++ b/arch/arm/boards/sama5d27-som1/lowlevel.c
> @@ -55,7 +55,7 @@ SAMA5D2_ENTRY_FUNCTION(start_sama5d27_som1_ek_xload_mmc, r4)
>  	ek_turn_led(RGB_LED_RED | RGB_LED_GREEN); /* Yellow */
>  	sama5d2_udelay_init(MASTER_CLOCK);
>  	sama5d2_d1g_ddrconf();
> -	sama5d2_sdhci_start_image(r4);
> +	sama5d2_start_image(r4);
>  }
>  
>  extern char __dtb_z_at91_sama5d27_som1_ek_start[];
> diff --git a/arch/arm/mach-at91/xload.c b/arch/arm/mach-at91/xload.c
> index 9c03d2119c..e417223b31 100644
> --- a/arch/arm/mach-at91/xload.c
> +++ b/arch/arm/mach-at91/xload.c
> @@ -11,39 +11,50 @@
>  #include <asm/cache.h>
>  #include <pbl/bio.h>
>  
> -static void at91_fat_start_image(struct pbl_bio *bio,
> -				 void *buf, unsigned int len,
> -				 u32 r4)
> +struct xload_instance {
> +	void __iomem *base;
> +	unsigned id;
> +	u8 periph;
> +	s8 pins[15];
> +};

Original code has struct sdhci_instance and struct atmci_instance which
both have the same members. You unify this to struct xload_instance
which is a good step. I think this should be a separate patch along with
the rename from sdhci_instance[] to sama5d2_mci_instances[] and
sama5d3_atmci_instances[] to sama5d3_mci_instances[].

This will help to better see what the rest of this patch is about.

> +
> +static void at91_fat_start_image(struct pbl_bio *bio, u32 r4)

You remove the *buf and len arguments from at91_fat_start_image()
because both arguments are common for all callers. Again, should be a
separate patch.

>  {
> -	void __noreturn (*bb)(void);
> +	void *buf = (void *)SAMA5_DDRCS;
>  	int ret;
>  
> -	ret = pbl_fat_load(bio, "barebox.bin", buf, len);
> +	ret = pbl_fat_load(bio, "barebox.bin", buf, SZ_2M);
>  	if (ret < 0) {
>  		pr_err("pbl_fat_load: error %d\n", ret);
>  		return;
>  	}
>  
> -	bb = buf;
> -
>  	sync_caches_for_execution();
>  
> -	sama5_boot_xload(bb, r4);
> +	sama5_boot_xload(buf, r4);
>  }
>  
> -static const struct sdhci_instance {
> -	void __iomem *base;
> -	unsigned id;
> -	u8 periph;
> -	s8 pins[15];
> -} sdhci_instances[] = {
> +static const struct xload_instance sama5d2_mci_instances[] = {
>  	[0] = {
> -		.base = SAMA5D2_BASE_SDHC0, .id = SAMA5D2_ID_SDMMC0, .periph = AT91_MUX_PERIPH_A,
> -		.pins = { 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 13, 10, 11, 12, -1 }
> +		.base = SAMA5D2_BASE_SDHC0,
> +		.id = SAMA5D2_ID_SDMMC0,
> +		.periph = AT91_MUX_PERIPH_A,
> +		.pins = {
> +			AT91_PIN_PA2, AT91_PIN_PA3, AT91_PIN_PA4, AT91_PIN_PA5,
> +			AT91_PIN_PA6, AT91_PIN_PA7, AT91_PIN_PA8, AT91_PIN_PA9,
> +			AT91_PIN_PA0, AT91_PIN_PA1, AT91_PIN_PA13, AT91_PIN_PA10,
> +			AT91_PIN_PA11, AT91_PIN_PA12, -1
> +		}

Here the hardcoded values are replaced with defines which is also a good
step but could be a separate patch.

>  	},
>  	[1] = {
> -		.base = SAMA5D2_BASE_SDHC1, .id = SAMA5D2_ID_SDMMC1, .periph = AT91_MUX_PERIPH_E,
> -		.pins = { 18, 19, 20, 21, 22, 28, 30, -1 }
> +		.base = SAMA5D2_BASE_SDHC1,
> +		.id = SAMA5D2_ID_SDMMC1,
> +		.periph = AT91_MUX_PERIPH_E,
> +		.pins = {
> +			AT91_PIN_PA18, AT91_PIN_PA19, AT91_PIN_PA20,
> +			AT91_PIN_PA21, AT91_PIN_PA22, AT91_PIN_PA28,
> +			AT91_PIN_PA30, -1
> +		}
>  	},
>  };
>  
> @@ -51,25 +62,23 @@ static const struct sdhci_instance {
>   * sama5d2_sdhci_start_image - Load and start an image from FAT-formatted SDHCI
>   * @r4: value of r4 passed by BootROM
>   */
> -void __noreturn sama5d2_sdhci_start_image(u32 r4)
> +static void __noreturn sama5d2_sdhci_start_image(u32 r4)
>  {
> -	void *buf = (void *)SAMA5_DDRCS;
> -	const struct sdhci_instance *instance;
> +	const struct xload_instance *instance;
>  	struct pbl_bio bio;
>  	const s8 *pin;
>  	int ret;
>  
>  	ret = sama5_bootsource_instance(r4);
> -	if (ret > 1)
> +	if (ret > ARRAY_SIZE(sama5d2_mci_instances) - 1)
>  		panic("Couldn't determine boot MCI instance\n");
>  
> -	instance = &sdhci_instances[ret];
> +	instance = &sama5d2_mci_instances[ret];
>  
>  	sama5d2_pmc_enable_periph_clock(SAMA5D2_ID_PIOA);
> -	for (pin = instance->pins; *pin >= 0; pin++) {
> +	for (pin = instance->pins; *pin >= 0; pin++)
>  		at91_mux_pio4_set_periph(SAMA5D2_BASE_PIOA,
>  					 BIT(*pin), instance->periph);
> -	}
>  
>  	sama5d2_pmc_enable_periph_clock(instance->id);
>  	sama5d2_pmc_enable_generic_clock(instance->id, AT91_PMC_GCKCSS_UPLL_CLK, 1);
> @@ -80,18 +89,26 @@ void __noreturn sama5d2_sdhci_start_image(u32 r4)
>  
>  	/* TODO: eMMC boot partition handling: they are not FAT-formatted */
>  
> -	at91_fat_start_image(&bio, buf, SZ_16M, r4);
> +	at91_fat_start_image(&bio, r4);
>  
>  out_panic:
>  	panic("FAT chainloading failed\n");
>  }
>  
> -static const struct atmci_instance {
> -	void __iomem *base;
> -	unsigned id;
> -	u8 periph;
> -	s8 pins[15];
> -} sama5d3_atmci_instances[] = {
> +void __noreturn sama5d2_start_image(u32 r4)
> +{
> +	switch (sama5_bootsource(r4)) {
> +	case BOOTSOURCE_MMC:
> +		sama5d2_sdhci_start_image(r4);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	panic("Unsupported boot configuration!\n");
> +}

And this is what the patch really is about: You want to have a function
which is no longer MMC specific, but boots from MMC/QSPI depending on
the bootsource. Please make this a separate patch as well.

Sascha

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

Re: [PATCH 4/5] ARM: at91: sama5: Unify xx_start_image() calls

[Alexander Shiyan]

Hello.

Ok. Can You apply the first patches to reduce the v2 series?
I also want to add another one to rename the sama5d2-related output
images to reflect that one image can now be used to boot from
both MMC and QSPI.

Thanks!

> On Tue, Mar 18, 2025 at 10:43:26AM +0300, Alexander Shiyan wrote:
> > This unifies the xload.c start_image() code for both sama5d2() and
> > sama5d3() CPU variants.
...
> Original code has struct sdhci_instance and struct atmci_instance which
> both have the same members. You unify this to struct xload_instance
> which is a good step. I think this should be a separate patch along with
> the rename from sdhci_instance[] to sama5d2_mci_instances[] and
> sama5d3_atmci_instances[] to sama5d3_mci_instances[].
>
> This will help to better see what the rest of this patch is about.

Re: (subset) [PATCH 1/5] ARM: at91: Add PBL_BREAK support

[Sascha Hauer]

On Tue, 18 Mar 2025 10:43:23 +0300, Alexander Shiyan wrote:
> This adds PBL_BREAK support to the barebox at91 header.
> Since we're here, let's set the header parameters to the defaults
> for the barebox and do some code formatting to improve readability.
> 
> 

Applied, thanks!

[1/5] ARM: at91: Add PBL_BREAK support
      https://git.pengutronix.de/cgit/barebox/commit/?id=3644b3362686 (link may not be stable)
[2/5] ARM: at91: Merge sama5_boot_xload() asm code
      https://git.pengutronix.de/cgit/barebox/commit/?id=1d0cc726f94c (link may not be stable)
[3/5] ARM: at91: Rename xload-mmc.c
      https://git.pengutronix.de/cgit/barebox/commit/?id=5c59dd719417 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>