On 12.11.19 13:00, matthias.bgg@kernel.org wrote:

From: Matthias Brugger mbrugger@suse.com

The fw_dtb_pointer was defined in the assembly code, which makes him live in section .text_rest Put that's not necessary, we can push the variable in the .data section.

This will prevent relocation errors like: board/raspberrypi/rpi/rpi.c:317:(.text.board_get_usable_ram_top+0x8): relocation truncated to fit: R_AARCH64_LDST64_ABS_LO12_NC against symbol `fw_dtb_pointer' defined in .text section in board/raspberrypi/rpi/built-in.o

Signed-off-by: Matthias Brugger mbrugger@suse.com

---

Changes in v3:

• fix armv7 build

Changes in v2:

• push fw_dtb_pointer into the .data section

  board/raspberrypi/rpi/lowlevel_init.S | 12 ++---------- board/raspberrypi/rpi/rpi.c | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/board/raspberrypi/rpi/lowlevel_init.S b/board/raspberrypi/rpi/lowlevel_init.S index 435eed521f..8c39b3e12e 100644 --- a/board/raspberrypi/rpi/lowlevel_init.S +++ b/board/raspberrypi/rpi/lowlevel_init.S @@ -6,15 +6,6 @@

#include <config.h>

-.align 8 -.global fw_dtb_pointer -fw_dtb_pointer: -#ifdef CONFIG_ARM64

• .dword 0x0

-#else

• .word 0x0

-#endif

• /*
  • Routine: save_boot_params (called after reset from start.S)
  • Description: save ATAG/FDT address provided by the firmware at boot time

@@ -28,7 +19,8 @@ save_boot_params: adr x8, fw_dtb_pointer str x0, [x8] #else

• str r2, fw_dtb_pointer

• ldr r8, =fw_dtb_pointer

• str r2, [r8] #endif

  /* Returns */

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index 9e0abdda31..0e05d59e1f 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -27,8 +27,8 @@

DECLARE_GLOBAL_DATA_PTR;

-/* From lowlevel_init.S */ -extern unsigned long fw_dtb_pointer; +/* Assigned in lowlevel_init.S */ +unsigned long fw_dtb_pointer = 0x1;

I assume you assign the 0x1 here so that it doesn't land in .bss which may get cleared after you wrote it? If so, please document that in the comment. Also even better yet, document it in a comment and just manually assign the variable to the ".data" section using

__attribute__((section(".data")))

Alex

On 12.11.19 13:00, matthias.bgg@kernel.org wrote:

From: Matthias Brugger mbrugger@suse.com

As part of the effort to create one binary for several bcm83x SoCs we use the SoC compatible to decide which IO base address we use.

Signed-off-by: Matthias Brugger mbrugger@suse.com

Is there any reason we can't just base on the ranges property of the /soc node instead? That way we don't duplicate the base offset all over the place.

Alex

---

Changes in v3: None Changes in v2:

• rename BCM2838 to BCM2711 in the correct patch

• push rpi_bcm283x_base into the .data section

  arch/arm/mach-bcm283x/Kconfig | 6 ---- arch/arm/mach-bcm283x/init.c | 62 +++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig index b08275f598..e8e0ff0eb4 100644 --- a/arch/arm/mach-bcm283x/Kconfig +++ b/arch/arm/mach-bcm283x/Kconfig @@ -202,10 +202,4 @@ config SYS_SOC config SYS_CONFIG_NAME default "rpi"

-config BCM283x_BASE

• hex
• default "0x20000000" if BCM2835
• default "0x3f000000" if BCM2836 || BCM2837
• default "0xfe000000" if BCM2711
• endmenu

diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c index d36017e823..d374fb60ba 100644 --- a/arch/arm/mach-bcm283x/init.c +++ b/arch/arm/mach-bcm283x/init.c @@ -7,8 +7,48 @@ */

#include <common.h> +#include <dm/device.h>

-unsigned long rpi_bcm283x_base; +#define PDATA_BCM2835 0 +#define PDATA_BCM2836 1 +#define PDATA_BCM2837 2 +#define PDATA_BCM2711 3

+unsigned long rpi_bcm283x_base = 0x3f000000;

+struct bcm283x_pdata {

• unsigned long io_base;

+};

+struct bcm283x_pdata pdata_bcm283x[] = {

• [PDATA_BCM2835] = {

• .io_base = 0x20000000,
  

• },
• [PDATA_BCM2836] = {

• .io_base = 0x3f000000,
  

• },

+#ifdef CONFIG_ARM64

• [PDATA_BCM2837] = {

• .io_base = 0x3f000000,
  

• },
• [PDATA_BCM2711] = {

• .io_base = 0xfe000000,
  

• },

+#endif +};

+/*

• • I/O address space varies on different chip versions.
• • We set the base address by inspecting the DTB.
• */

+static const struct udevice_id board_ids[] = {

• { .compatible = "brcm,bcm2835", .data = PDATA_BCM2835},
• { .compatible = "brcm,bcm2836", .data = PDATA_BCM2836},
• { .compatible = "brcm,bcm2837", .data = PDATA_BCM2837},
• { .compatible = "brcm,bcm2838", .data = PDATA_BCM2711},
• { .compatible = "brcm,bcm2711", .data = PDATA_BCM2711},
• { },

+};

int arch_cpu_init(void) { @@ -19,10 +59,28 @@ int arch_cpu_init(void)

int mach_cpu_init(void) {

• rpi_bcm283x_base = CONFIG_BCM283x_BASE;

• const struct udevice_id *of_match = board_ids;

• int ret;

• rpi_bcm283x_base = 0;

• while (of_match->compatible) {

• struct bcm283x_pdata pdat;
  

• ret = fdt_node_check_compatible(gd->fdt_blob, 0,
  

• 				of_match->compatible);
  

• if (!ret) {
  

• 	pdat = pdata_bcm283x[of_match->data];
  

• 	rpi_bcm283x_base = pdat.io_base;
  

• 	break;
  

• }
  

• of_match++;
  

• }

  return 0; }

• #ifdef CONFIG_ARMV7_LPAE void enable_caches(void) {

On 12.11.19 13:00, matthias.bgg@kernel.org wrote:

From: Matthias Brugger mbrugger@suse.com

For bcm283x based on arm64 we also have to change the mm_region. Add assign this in mach_cpu_init() so we can create now one binary for RPi3 and RPi4.

Signed-off-by: Matthias Brugger mbrugger@suse.com

For the future, can you think of a reason why we could not always generate the map at runtime?

Alex

---

Changes in v3: None Changes in v2:

• Move mem_map out of assembly file

• push mem_map into the .data section

• update the members of mem_map inestead of the pointer

  arch/arm/mach-bcm283x/init.c | 74 ++++++++++++++++++++++++++++++++++++ board/raspberrypi/rpi/rpi.c | 45 ---------------------- 2 files changed, 74 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c index d374fb60ba..639a5cf975 100644 --- a/arch/arm/mach-bcm283x/init.c +++ b/arch/arm/mach-bcm283x/init.c @@ -8,6 +8,9 @@

#include <common.h> #include <dm/device.h> +#ifdef CONFIG_ARM64 +#include <asm/armv8/mmu.h> +#endif

#define PDATA_BCM2835 0 #define PDATA_BCM2836 1 @@ -16,23 +19,77 @@

unsigned long rpi_bcm283x_base = 0x3f000000;

+#ifdef CONFIG_ARM64 +static struct mm_region bcm283x_mem_map[] = {

• {

• .virt = 0x00000000UL,
  

• .phys = 0x00000000UL,
  

• .size = 0x3f000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

• 	 PTE_BLOCK_INNER_SHARE
  

• }, {

• .virt = 0x3f000000UL,
  

• .phys = 0x3f000000UL,
  

• .size = 0x01000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

• 	 PTE_BLOCK_NON_SHARE |
  

• 	 PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

• }, {

• /* List terminator */
  

• 0,
  

• }

+};

+static struct mm_region bcm2711_mem_map[] = {

• {

• .virt = 0x00000000UL,
  

• .phys = 0x00000000UL,
  

• .size = 0xfe000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

• 	 PTE_BLOCK_INNER_SHARE
  

• }, {

• .virt = 0xfe000000UL,
  

• .phys = 0xfe000000UL,
  

• .size = 0x01800000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

• 	 PTE_BLOCK_NON_SHARE |
  

• 	 PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

• }, {

• /* List terminator */
  

• 0,
  

• }

+};

+struct mm_region *mem_map = bcm283x_mem_map;

+#else +struct mm_region {

• /* dummy struct */

+}; +#endif

• struct bcm283x_pdata { unsigned long io_base;

• struct mm_region *m_map; };

  struct bcm283x_pdata pdata_bcm283x[] = { [PDATA_BCM2835] = { .io_base = 0x20000000,

• .m_map = NULL,
  

  }, [PDATA_BCM2836] = { .io_base = 0x3f000000,

• .m_map = NULL,
  

  }, #ifdef CONFIG_ARM64 [PDATA_BCM2837] = { .io_base = 0x3f000000,

• .m_map = bcm283x_mem_map,
  

  }, [PDATA_BCM2711] = { .io_base = 0xfe000000,

• .m_map = bcm2711_mem_map
  

  }, #endif };

@@ -57,6 +114,22 @@ int arch_cpu_init(void) return 0; }

+#ifdef CONFIG_ARM64 +static void rpi_updated_mem_map(struct mm_region *pd) +{

• int i;
• for (i = 0; i < 2; i++) {

• mem_map[i].virt = pd[i].virt;
  

• mem_map[i].phys = pd[i].phys;
  

• mem_map[i].size = pd[i].size;
  

• mem_map[i].attrs = pd[i].attrs;
  

• }

+} +#else +static void rpi_updated_mem_map(struct mm_region *pd) {} +#endif

• int mach_cpu_init(void) { const struct udevice_id *of_match = board_ids;

@@ -72,6 +145,7 @@ int mach_cpu_init(void) if (!ret) { pdat = pdata_bcm283x[of_match->data]; rpi_bcm283x_base = pdat.io_base;

• 	rpi_updated_mem_map(pdat.m_map);
  break;
  

  }

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index 0e05d59e1f..248d04bfd2 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -248,51 +248,6 @@ static uint32_t rev_scheme; static uint32_t rev_type; static const struct rpi_model *model;

-#ifdef CONFIG_ARM64 -#ifndef CONFIG_BCM2711 -static struct mm_region bcm283x_mem_map[] = {

• {

• .virt = 0x00000000UL,
  

• .phys = 0x00000000UL,
  

• .size = 0x3f000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

• 	 PTE_BLOCK_INNER_SHARE
  

• }, {

• .virt = 0x3f000000UL,
  

• .phys = 0x3f000000UL,
  

• .size = 0x01000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

• 	 PTE_BLOCK_NON_SHARE |
  

• 	 PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

• }, {

• /* List terminator */
  

• 0,
  

• }

-}; -#else -static struct mm_region bcm283x_mem_map[] = {

• {

• .virt = 0x00000000UL,
  

• .phys = 0x00000000UL,
  

• .size = 0xfe000000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

• 	 PTE_BLOCK_INNER_SHARE
  

• }, {

• .virt = 0xfe000000UL,
  

• .phys = 0xfe000000UL,
  

• .size = 0x01800000UL,
  

• .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

• 	 PTE_BLOCK_NON_SHARE |
  

• 	 PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

• }, {

• /* List terminator */
  

• 0,
  

• }

-}; -#endif -struct mm_region *mem_map = bcm283x_mem_map; -#endif

• int dram_init(void) { ALLOC_CACHE_ALIGN_BUFFER(struct msg_get_arm_mem, msg, 1);