On 27.09.19 11:00, matthias.bgg@kernel.org wrote:

From: Matthias Brugger mbrugger@suse.com

We move the per SOC define BCM283x_BASE to a global variable. This is a first step to provide a single binary for several bcm283x SoCs.

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

arch/arm/mach-bcm283x/include/mach/base.h | 11 +++++++++++ arch/arm/mach-bcm283x/include/mach/mbox.h | 2 +- arch/arm/mach-bcm283x/include/mach/sdhci.h | 2 +- arch/arm/mach-bcm283x/include/mach/timer.h | 2 +- arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/init.c | 8 ++++++++ arch/arm/mach-bcm283x/mbox.c | 8 ++++++-- arch/arm/mach-bcm283x/reset.c | 7 +++++-- board/raspberrypi/rpi/lowlevel_init.S | 8 ++++++++ include/configs/rpi.h | 7 ++++++- 10 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 arch/arm/mach-bcm283x/include/mach/base.h

diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h new file mode 100644 index 0000000000..c4ae39852f --- /dev/null +++ b/arch/arm/mach-bcm283x/include/mach/base.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*

• • (C) Copyright 2019 Matthias Brugger
• */

+#ifndef _BCM283x_BASE_H_ +#define _BCM283x_BASE_H_

+extern unsigned long rpi_bcm283x_base;

+#endif diff --git a/arch/arm/mach-bcm283x/include/mach/mbox.h b/arch/arm/mach-bcm283x/include/mach/mbox.h index 0b6c2543d5..39cf2cf9d5 100644 --- a/arch/arm/mach-bcm283x/include/mach/mbox.h +++ b/arch/arm/mach-bcm283x/include/mach/mbox.h @@ -37,7 +37,7 @@

/* Raw mailbox HW */

-#define BCM2835_MBOX_PHYSADDR (CONFIG_BCM283x_BASE + 0x0000b880) +#define BCM2835_MBOX_OFFSET 0x0000b880

Why rename and not just redefine as (rpi_bcm283x_base + 0x000...)? Also, you may want to add a BUG_ON(!rpi_bcm283x_base) in the query. There was some fancy way to do that in C. Something like

#define BCM2835_MBOX_PHYSADDR ({ BUG_ON(!rpi_bcm283x_base); rpi_bcm283x_base + 0x0000b880; })

may do the trick.

Also, shouldn't mbox.h just contain the definition for rpi_bcm283x_base?

struct bcm2835_mbox_regs { u32 read; diff --git a/arch/arm/mach-bcm283x/include/mach/sdhci.h b/arch/arm/mach-bcm283x/include/mach/sdhci.h index b443c379d8..16af5787c2 100644 --- a/arch/arm/mach-bcm283x/include/mach/sdhci.h +++ b/arch/arm/mach-bcm283x/include/mach/sdhci.h @@ -6,7 +6,7 @@ #ifndef _BCM2835_SDHCI_H_ #define _BCM2835_SDHCI_H_

-#define BCM2835_SDHCI_BASE (CONFIG_BCM283x_BASE + 0x00300000) +#define BCM2835_SDHCI_OFFSET 0x00300000

int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq);

diff --git a/arch/arm/mach-bcm283x/include/mach/timer.h b/arch/arm/mach-bcm283x/include/mach/timer.h index 014355e759..c789e188be 100644 --- a/arch/arm/mach-bcm283x/include/mach/timer.h +++ b/arch/arm/mach-bcm283x/include/mach/timer.h @@ -6,7 +6,7 @@ #ifndef _BCM2835_TIMER_H #define _BCM2835_TIMER_H

-#define BCM2835_TIMER_PHYSADDR (CONFIG_BCM283x_BASE + 0x00003000) +#define BCM2835_TIMER_OFFSET 0x00003000

#define BCM2835_TIMER_CS_M3 (1 << 3) #define BCM2835_TIMER_CS_M2 (1 << 2) diff --git a/arch/arm/mach-bcm283x/include/mach/wdog.h b/arch/arm/mach-bcm283x/include/mach/wdog.h index 8292b3cf1f..2ca0e8c035 100644 --- a/arch/arm/mach-bcm283x/include/mach/wdog.h +++ b/arch/arm/mach-bcm283x/include/mach/wdog.h @@ -6,7 +6,7 @@ #ifndef _BCM2835_WDOG_H #define _BCM2835_WDOG_H

-#define BCM2835_WDOG_PHYSADDR (CONFIG_BCM283x_BASE + 0x00100000) +#define BCM2835_WDOG_OFFSET 0x00100000

struct bcm2835_wdog_regs { u32 unknown0[7]; diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c index 97414415a6..9dcc96ba2e 100644 --- a/arch/arm/mach-bcm283x/init.c +++ b/arch/arm/mach-bcm283x/init.c @@ -8,6 +8,8 @@

#include <common.h>

+extern unsigned long rpi_bcm283x_base;

Why doesn't this just include the header that you create above?

• int arch_cpu_init(void) { icache_enable();

@@ -15,6 +17,12 @@ int arch_cpu_init(void) return 0; }

+int mach_cpu_init(void) +{

• rpi_bcm283x_base = CONFIG_BCM283x_BASE;
• return 0;

+} #ifdef CONFIG_ARMV7_LPAE void enable_caches(void) { diff --git a/arch/arm/mach-bcm283x/mbox.c b/arch/arm/mach-bcm283x/mbox.c index 3c67f68c17..21a7ce113d 100644 --- a/arch/arm/mach-bcm283x/mbox.c +++ b/arch/arm/mach-bcm283x/mbox.c @@ -10,13 +10,17 @@

#define TIMEOUT 1000 /* ms */

+extern unsigned long rpi_bcm283x_base;

Header?

Alex

• int bcm2835_mbox_call_raw(u32 chan, u32 send, u32 *recv) {

• struct bcm2835_mbox_regs *regs =

• (struct bcm2835_mbox_regs *)BCM2835_MBOX_PHYSADDR;
  

• struct bcm2835_mbox_regs *regs; ulong endtime = get_timer(0) + TIMEOUT; u32 val;

• regs = (struct bcm2835_mbox_regs *)(rpi_bcm283x_base

• 				+ BCM2835_MBOX_OFFSET);
  

• debug("time: %lu timeout: %lu\n", get_timer(0), endtime);

  if (send & BCM2835_CHAN_MASK) {

diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index b3da0c7cd6..aff894b3c6 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -11,6 +11,8 @@ #include <asm/arch/wdog.h> #include <efi_loader.h>

+extern unsigned long rpi_bcm283x_base;

• #define RESET_TIMEOUT 10

  /*

@@ -25,8 +27,7 @@

void hw_watchdog_disable(void) {}

-__efi_runtime_data struct bcm2835_wdog_regs *wdog_regs =

• (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR;

+__efi_runtime_data struct bcm2835_wdog_regs *wdog_regs;

void __efi_runtime reset_cpu(ulong ticks) { @@ -77,6 +78,8 @@ void __efi_runtime EFIAPI efi_reset_system(

efi_status_t efi_reset_system_init(void) {

• wdog_regs = (struct bcm2835_wdog_regs *)(rpi_bcm283x_base +

• 	BCM2835_WDOG_OFFSET);
  

  return efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); }

diff --git a/board/raspberrypi/rpi/lowlevel_init.S b/board/raspberrypi/rpi/lowlevel_init.S index 435eed521f..fcb99ebef7 100644 --- a/board/raspberrypi/rpi/lowlevel_init.S +++ b/board/raspberrypi/rpi/lowlevel_init.S @@ -7,6 +7,14 @@ #include <config.h>

.align 8 +.global rpi_bcm283x_base +rpi_bcm283x_base: +#ifdef CONFIG_ARM64

• .dword 0x0

+#else

• .word 0x0

+#endif

• .global fw_dtb_pointer fw_dtb_pointer: #ifdef CONFIG_ARM64

diff --git a/include/configs/rpi.h b/include/configs/rpi.h index 77d2d5458a..88496ac44d 100644 --- a/include/configs/rpi.h +++ b/include/configs/rpi.h @@ -9,6 +9,10 @@ #include <linux/sizes.h> #include <asm/arch/timer.h>

+#ifndef __ASSEMBLY__ +#include <asm/arch/base.h> +#endif

• #if defined(CONFIG_TARGET_RPI_2) || defined(CONFIG_TARGET_RPI_3_32B) #define CONFIG_SKIP_LOWLEVEL_INIT #endif

@@ -19,7 +23,8 @@ #ifndef CONFIG_ARM64 #define CONFIG_SYS_TIMER_RATE 1000000 #define CONFIG_SYS_TIMER_COUNTER \

• (&((struct bcm2835_timer_regs *)BCM2835_TIMER_PHYSADDR)->clo)

• (&((struct bcm2835_timer_regs *)(rpi_bcm283x_base \

• 			 + BCM2835_TIMER_OFFSET))->clo)
  

  #endif

  /*

On 27.09.19 11: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

arch/arm/mach-bcm283x/Kconfig | 6 ---- arch/arm/mach-bcm283x/init.c | 60 ++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 7 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 9dcc96ba2e..214e1078eb 100644 --- a/arch/arm/mach-bcm283x/init.c +++ b/arch/arm/mach-bcm283x/init.c @@ -7,9 +7,49 @@ */

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

+#define PDATA_BCM2835 0 +#define PDATA_BCM2836 1 +#define PDATA_BCM2837 2 +#define PDATA_BCM2838 3

extern unsigned long rpi_bcm283x_base;

+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_BCM2838] = {

• .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_BCM2838},
• { .compatible = "brcm,bcm2711", .data = PDATA_BCM2838},

Does this match all of the downstream dtbs as well as the upstream ones?

Alex

• { },

+};

• int arch_cpu_init(void) { icache_enable();

@@ -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 27/09/2019 12:14, Alexander Graf wrote:

On 27.09.19 11: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

---

arch/arm/mach-bcm283x/init.c          | 65 +++++++++++++++++++++++++--   board/raspberrypi/rpi/lowlevel_init.S |  6 +++   board/raspberrypi/rpi/rpi.c           | 45 -------------------   3 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c index 214e1078eb..f6c2946922 100644 --- a/arch/arm/mach-bcm283x/init.c +++ b/arch/arm/mach-bcm283x/init.c @@ -8,16 +8,68 @@     #include <common.h>   #include <dm/device.h> +#ifdef CONFIG_ARM64 +#include <asm/armv8/mmu.h> +#endif     #define PDATA_BCM2835    0   #define PDATA_BCM2836    1   #define PDATA_BCM2837    2 -#define PDATA_BCM2838    3 +#define PDATA_BCM2711    3

What is this change?

Wrong patch, I will fix this in v2.

...

extern unsigned long rpi_bcm283x_base;   +#ifdef CONFIG_ARM64 +extern struct mm_region *mem_map;

+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, +    } +}; +#else +struct mm_region { +    /* dummy struct */ +}; +#endif

struct bcm283x_pdata {       unsigned long io_base; +    struct mm_region *m_map;   };     struct bcm283x_pdata pdata_bcm283x[] = { @@ -30,9 +82,11 @@ struct bcm283x_pdata pdata_bcm283x[] = {   #ifdef CONFIG_ARM64       [PDATA_BCM2837] = {           .io_base = 0x3f000000, +        .m_map = bcm283x_mem_map,       }, -    [PDATA_BCM2838] = { +    [PDATA_BCM2711] = {           .io_base = 0xfe000000, +        .m_map = bcm2711_mem_map       },   #endif   }; @@ -45,8 +99,8 @@ 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_BCM2838}, -    { .compatible = "brcm,bcm2711", .data = PDATA_BCM2838}, +    { .compatible = "brcm,bcm2838", .data = PDATA_BCM2711}, +    { .compatible = "brcm,bcm2711", .data = PDATA_BCM2711},       { },   };   @@ -72,6 +126,9 @@ int mach_cpu_init(void)           if (!ret) {               pdat = pdata_bcm283x[of_match->data];               rpi_bcm283x_base = pdat.io_base; +#ifdef CONFIG_ARM64 +            mem_map = pdat.m_map; +#endif               break;           }   diff --git a/board/raspberrypi/rpi/lowlevel_init.S b/board/raspberrypi/rpi/lowlevel_init.S index fcb99ebef7..9786a5a4b3 100644 --- a/board/raspberrypi/rpi/lowlevel_init.S +++ b/board/raspberrypi/rpi/lowlevel_init.S @@ -23,6 +23,12 @@ fw_dtb_pointer:       .word 0x0   #endif   +#ifdef CONFIG_ARM64 +.global mem_map +mem_map: +    .dword 0x0 +#endif

Why does this live in .S?

It took me some time but now I think I understand what happens. rpi_bcm283x_base and mem_map are assigned in mach_cpu_init. My understanding is that for this function holds the same as for board_init_f (from Readme): - BSS is not available, so you cannot use global/static variables, only stack variables and global_data

If we put the declaration of both variables in a source file, it will be put into .bss and we won't be able to boot.

The whole low_level.S lives in text_rest, so if I add __attribute__ ((section (".text"))) to the declaration of the variables in, let's say, init.c I get a compiler warning but U-Boot works as expected.

So my question is now, how to proceed. I could add them in the global_data arch struct. That would work for fine for rpi_bcm283x_base but would need to some bigger changes for mem_map as this is used by armv8 code for all SoCs.

Any idea?

Regards, Matthias

Alex

...

/*    * Routine: save_boot_params (called after reset from start.S)    * Description: save ATAG/FDT address provided by the firmware at boot time diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index 9e0abdda31..cf1666ce5f 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);