On 16.10.19 18:50, Matthias Brugger wrote:
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.
Why not just force put it in ".data"? In fact, it might be as simple as
extern struct mm_region *mem_map = bcm283x_mem_map;
which should already move it right into .data. Then you only need to special the RPi4 (and above).
Alex
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);