Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

nits: please remove the ending .

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

ditto

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

This looks like we should add a new sysreset driver for Apple Arm SoC.

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

Is this filled in by m1n1?

+.global save_boot_params +save_boot_params:

•   adr     x1, fw_dtb_pointer
  

•   str     x0, [x1]
  

•   b       save_boot_params_ret
  

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig new file mode 100644 index 0000000000..a7ae15576b --- /dev/null +++ b/configs/apple_m1_defconfig @@ -0,0 +1,14 @@ +CONFIG_ARM=y +CONFIG_ARCH_APPLE=y +# CONFIG_DISPLAY_CPUINFO is not set +# CONFIG_MMC is not set +# CONFIG_NET is not set +CONFIG_VIDEO_SIMPLE=y +CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y +CONFIG_USB_KEYBOARD=y +CONFIG_USB_STORAGE=y +CONFIG_USE_PREBOOT=y +CONFIG_PREBOOT="usb start" +# CONFIG_GENERATE_SMBIOS_TABLE is not set diff --git a/include/configs/apple.h b/include/configs/apple.h new file mode 100644 index 0000000000..1c246af002 --- /dev/null +++ b/include/configs/apple.h @@ -0,0 +1,38 @@ +#ifndef __CONFIG_H +#define __CONFIG_H

+#include <linux/sizes.h>

+#define CONFIG_SYS_LOAD_ADDR 0x880000000

+#define CONFIG_SYS_SDRAM_BASE 0x880000000

+#define CONFIG_LNX_KRNL_IMG_TEXT_OFFSET_BASE CONFIG_SYS_TEXT_BASE

+/* Environment */ +#define ENV_DEVICE_SETTINGS \

•   "stdin=serial,usbkbd\0" \
  

•   "stdout=serial,vidconsole\0" \
  

•   "stderr=serial,vidconsole\0"
  

+#define ENV_MEM_LAYOUT_SETTINGS \

•   "fdt_addr_r=0x960100000\0" \
  

•   "kernel_addr_r=0x960200000\0"
  

+#if CONFIG_IS_ENABLED(CMD_USB)

•   #define BOOT_TARGET_USB(func) func(USB, usb, 0)
  

+#else

•   #define BOOT_TARGET_USB(func)
  

+#endif

+#define BOOT_TARGET_DEVICES(func) \

•   BOOT_TARGET_USB(func)
  

+#include <config_distro_bootcmd.h>

+#define CONFIG_EXTRA_ENV_SETTINGS \

•   ENV_DEVICE_SETTINGS \
  

•   ENV_MEM_LAYOUT_SETTINGS \
  

•   BOOTENV
  

+#endif

Regards, Bin

From: Bin Meng bmeng.cn@gmail.com Date: Sun, 19 Sep 2021 09:17:07 +0800

Hi Mark,

On Sun, Sep 19, 2021 at 9:04 AM Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

...

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

nits: please remove the ending .

...

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

This is error prone. Someone else updating apple_mem_map may create an incorrect index for us.

I don't see a better way without introducing more complexity.

...

...

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

ditto

...

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

This looks like we should add a new sysreset driver for Apple Arm SoC.

...

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

Is this filled in by m1n1?

Sorry I misread, so this is passed by m1n1 and filled in by U-Boot. I think we should stop using CONFIG_OF_BOARD, and for such case we should use CONFIG_OF_PRIOR_STAGE.

Yes, CONFIG_OF_PRIOR_STAGE would work as well. But Tom was talking about removing that option in favour of CONFIG_OF_BOARD the other day.

...

...

+.global save_boot_params +save_boot_params:

•   adr     x1, fw_dtb_pointer
  

•   str     x0, [x1]
  

•   b       save_boot_params_ret
  

Regards, Bin

On Tue, Sep 21, 2021 at 8:42 PM Tom Rini trini@konsulko.com wrote:

On Sun, Sep 19, 2021 at 10:33:25PM +0200, Mark Kettenis wrote:

...

...

From: Bin Meng bmeng.cn@gmail.com Date: Sun, 19 Sep 2021 09:17:07 +0800

Hi Mark,

On Sun, Sep 19, 2021 at 9:04 AM Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

...

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

nits: please remove the ending .

...

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

This is error prone. Someone else updating apple_mem_map may create an incorrect index for us.

I don't see a better way without introducing more complexity.

...

...

...

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

ditto

...

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

This looks like we should add a new sysreset driver for Apple Arm SoC.

...

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

Is this filled in by m1n1?

Sorry I misread, so this is passed by m1n1 and filled in by U-Boot. I think we should stop using CONFIG_OF_BOARD, and for such case we should use CONFIG_OF_PRIOR_STAGE.

Yes, CONFIG_OF_PRIOR_STAGE would work as well. But Tom was talking about removing that option in favour of CONFIG_OF_BOARD the other day.

Yes. I was even looking for some feedback from you, Bin, on converting some boards from CONFIG_OF_PRIOR_STAGE to CONFIG_OF_BOARD. It seems like CONFIG_OF_PRIOR_STAGE is a subset of CONFIG_OF_BOARD, at the cost of possibly a few bytes.

Ah, I thought we wanted to do the other way around, by removing CONFIG_OF_BOARD, and convert that to CONFIG_OF_PRIORI_STAGE?

Regards, Bin

From: Bin Meng bmeng.cn@gmail.com Date: Tue, 21 Sep 2021 23:53:10 +0800

On Tue, Sep 21, 2021 at 8:42 PM Tom Rini trini@konsulko.com wrote:

...

On Sun, Sep 19, 2021 at 10:33:25PM +0200, Mark Kettenis wrote:

...

...

From: Bin Meng bmeng.cn@gmail.com Date: Sun, 19 Sep 2021 09:17:07 +0800

Hi Mark,

On Sun, Sep 19, 2021 at 9:04 AM Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

...

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

nits: please remove the ending .

...

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

This is error prone. Someone else updating apple_mem_map may create an incorrect index for us.

I don't see a better way without introducing more complexity.

...

...

...

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

ditto

...

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

This looks like we should add a new sysreset driver for Apple Arm SoC.

...

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

Is this filled in by m1n1?

Sorry I misread, so this is passed by m1n1 and filled in by U-Boot. I think we should stop using CONFIG_OF_BOARD, and for such case we should use CONFIG_OF_PRIOR_STAGE.

Yes, CONFIG_OF_PRIOR_STAGE would work as well. But Tom was talking about removing that option in favour of CONFIG_OF_BOARD the other day.

Yes. I was even looking for some feedback from you, Bin, on converting some boards from CONFIG_OF_PRIOR_STAGE to CONFIG_OF_BOARD. It seems like CONFIG_OF_PRIOR_STAGE is a subset of CONFIG_OF_BOARD, at the cost of possibly a few bytes.

Ah, I thought we wanted to do the other way around, by removing CONFIG_OF_BOARD, and convert that to CONFIG_OF_PRIORI_STAGE?

I must say that I see some logic in keeping both, where CONFIG_OF_BOARD indicates that the device tree is somehow stored on the board and U-Boot has to run some code to fetch it, and CONFIG_OF_PRIOR_STAGE is used when the device tree is provided by firmware that runs before U-Boot.

In that case I obviously should use CONFIG_OF_PRIOR_STAGE here.

From: Bin Meng bmeng.cn@gmail.com Date: Sun, 19 Sep 2021 09:04:56 +0800

Hi Bin,

Thanks for taking a look.

Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

...

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

nits: please remove the ending .

Really? The current codebase has both styles... But sure, I can make that change.

...

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

ditto

...

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

This looks like we should add a new sysreset driver for Apple Arm SoC.

Could be. This is actually a watchdog device that is used to reset the machine. So I suppose I should add a watchdog driver and use SYSRESET_WATCHDOG. Slight worry is that this relies on a lot more DM code. Does that mean the reset isn't available during early boot?

If it is acceptable, I would like to add the watchdog device in a future series, once the device tree bindings for the watchdog have been hammered out.

...

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

Is this filled in by m1n1?

...

+.global save_boot_params +save_boot_params:

•   adr     x1, fw_dtb_pointer
  

•   str     x0, [x1]
  

•   b       save_boot_params_ret
  

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig new file mode 100644 index 0000000000..a7ae15576b --- /dev/null +++ b/configs/apple_m1_defconfig @@ -0,0 +1,14 @@ +CONFIG_ARM=y +CONFIG_ARCH_APPLE=y +# CONFIG_DISPLAY_CPUINFO is not set +# CONFIG_MMC is not set +# CONFIG_NET is not set +CONFIG_VIDEO_SIMPLE=y +CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y +CONFIG_USB_KEYBOARD=y +CONFIG_USB_STORAGE=y +CONFIG_USE_PREBOOT=y +CONFIG_PREBOOT="usb start" +# CONFIG_GENERATE_SMBIOS_TABLE is not set diff --git a/include/configs/apple.h b/include/configs/apple.h new file mode 100644 index 0000000000..1c246af002 --- /dev/null +++ b/include/configs/apple.h @@ -0,0 +1,38 @@ +#ifndef __CONFIG_H +#define __CONFIG_H

+#include <linux/sizes.h>

+#define CONFIG_SYS_LOAD_ADDR 0x880000000

+#define CONFIG_SYS_SDRAM_BASE 0x880000000

+#define CONFIG_LNX_KRNL_IMG_TEXT_OFFSET_BASE CONFIG_SYS_TEXT_BASE

+/* Environment */ +#define ENV_DEVICE_SETTINGS \

•   "stdin=serial,usbkbd\0" \
  

•   "stdout=serial,vidconsole\0" \
  

•   "stderr=serial,vidconsole\0"
  

+#define ENV_MEM_LAYOUT_SETTINGS \

•   "fdt_addr_r=0x960100000\0" \
  

•   "kernel_addr_r=0x960200000\0"
  

+#if CONFIG_IS_ENABLED(CMD_USB)

•   #define BOOT_TARGET_USB(func) func(USB, usb, 0)
  

+#else

•   #define BOOT_TARGET_USB(func)
  

+#endif

+#define BOOT_TARGET_DEVICES(func) \

•   BOOT_TARGET_USB(func)
  

+#include <config_distro_bootcmd.h>

+#define CONFIG_EXTRA_ENV_SETTINGS \

•   ENV_DEVICE_SETTINGS \
  

•   ENV_MEM_LAYOUT_SETTINGS \
  

•   BOOTENV
  

+#endif

Regards, Bin

From: Simon Glass sjg@chromium.org Date: Sun, 19 Sep 2021 21:15:57 -0600

Hi Mark,

On Sat, 18 Sept 2021 at 07:55, Mark Kettenis kettenis@openbsd.org wrote:

...

Add support for Apple's M1 SoC that is used in "Apple Silicon" Macs. This builds a basic U-Boot that can be used as a payload for the m1n1 boot loader being developed by the Asahi Linux project.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/Kconfig | 22 ++++ arch/arm/Makefile | 1 + arch/arm/mach-apple/Kconfig | 18 ++++ arch/arm/mach-apple/Makefile | 4 + arch/arm/mach-apple/board.c | 158 ++++++++++++++++++++++++++++ arch/arm/mach-apple/lowlevel_init.S | 16 +++ configs/apple_m1_defconfig | 14 +++ include/configs/apple.h | 38 +++++++ 8 files changed, 271 insertions(+) create mode 100644 arch/arm/mach-apple/Kconfig create mode 100644 arch/arm/mach-apple/Makefile create mode 100644 arch/arm/mach-apple/board.c create mode 100644 arch/arm/mach-apple/lowlevel_init.S create mode 100644 configs/apple_m1_defconfig create mode 100644 include/configs/apple.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b5bd3284cd..7cdea1f615 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -895,6 +895,26 @@ config ARCH_NEXELL select DM select GPIO_EXTRA_HEADER

+config ARCH_APPLE

•   bool "Apple SoCs"
  

•   select ARM64
  

•   select LINUX_KERNEL_IMAGE_HEADER
  

•   select POSITION_INDEPENDENT
  

•   select BLK
  

•   select DM
  

•   select DM_KEYBOARD
  

•   select DM_SERIAL
  

•   select DM_USB
  

•   select DM_VIDEO
  

•   select CMD_USB
  

•   select MISC
  

•   select OF_CONTROL
  

•   select OF_BOARD
  

•   select USB
  

•   imply CMD_DM
  

•   imply CMD_GPT
  

•   imply DISTRO_DEFAULTS
  

Suggest sorting these

As in sort all the selects alphabetically and sort all the implies alphabetically seperately?

Does my use of impy here even make sense?

This whole Kconfig stuff is a bit alien to me and I must say that it isn't obvious what "best-practice" is in this area...

...

config ARCH_OWL bool "Actions Semi OWL SoCs" select DM @@ -1932,6 +1952,8 @@ config ISW_ENTRY_ADDR image headers. endif

+source "arch/arm/mach-apple/Kconfig"

source "arch/arm/mach-aspeed/Kconfig"

source "arch/arm/mach-at91/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index c68e598a67..44178c204b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -51,6 +51,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)

# Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name. +machine-$(CONFIG_ARCH_APPLE) += apple machine-$(CONFIG_ARCH_ASPEED) += aspeed machine-$(CONFIG_ARCH_AT91) += at91 machine-$(CONFIG_ARCH_BCM283X) += bcm283x diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig new file mode 100644 index 0000000000..66cab91b2a --- /dev/null +++ b/arch/arm/mach-apple/Kconfig @@ -0,0 +1,18 @@ +if ARCH_APPLE

+config SYS_TEXT_BASE

•   default 0x00000000
  

+config SYS_CONFIG_NAME

•   default "apple"
  

+config SYS_SOC

•   default "m1"
  

+config SYS_MALLOC_LEN

•   default 0x4000000
  

+config SYS_MALLOC_F_LEN

•   default 0x4000
  

+endif diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile new file mode 100644 index 0000000000..e74a8c9df1 --- /dev/null +++ b/arch/arm/mach-apple/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += board.o +obj-y += lowlevel_init.o diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c new file mode 100644 index 0000000000..0c8b35292e --- /dev/null +++ b/arch/arm/mach-apple/board.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <efi_loader.h>

+#include <asm/armv8/mmu.h> +#include <asm/global_data.h> +#include <asm/io.h> +#include <asm/system.h>

+DECLARE_GLOBAL_DATA_PTR;

+static struct mm_region apple_mem_map[] = {

•   {
  

•           /* I/O */
  

•           .virt = 0x200000000,
  

•           .phys = 0x200000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x500000000,
  

•           .phys = 0x500000000,
  

•           .size = 2UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* I/O */
  

•           .virt = 0x680000000,
  

•           .phys = 0x680000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
  

•                    PTE_BLOCK_NON_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6a0000000,
  

•           .phys = 0x6a0000000,
  

•           .size = SZ_512M,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* PCIE */
  

•           .virt = 0x6c0000000,
  

•           .phys = 0x6c0000000,
  

•           .size = SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
  

•                    PTE_BLOCK_INNER_SHARE |
  

•                    PTE_BLOCK_PXN | PTE_BLOCK_UXN
  

•   }, {
  

•           /* RAM */
  

•           .virt = 0x800000000,
  

•           .phys = 0x800000000,
  

•           .size = 8UL * SZ_1G,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           /* Empty entry for framebuffer */
  

•           0,
  

•   }, {
  

•           /* List terminator */
  

•           0,
  

•   }
  

+};

+struct mm_region *mem_map = apple_mem_map;

+int board_init(void) +{

•   return 0;
  

+}

+int dram_init(void) +{

•   int index, node, ret;
  

•   fdt_addr_t base;
  

•   fdt_size_t size;
  

•   ret = fdtdec_setup_mem_size_base();
  

•   if (ret)
  

•           return ret;
  

•   /* Update RAM mapping. */
  

Do you want the period at the end of that?

Given that Bin brought up the same thing, I gather that not having a full stop at the end of single-line comments is preferred?

...

•   index = ARRAY_SIZE(apple_mem_map) - 3;
  

•   apple_mem_map[index].virt = gd->ram_base;
  

•   apple_mem_map[index].phys = gd->ram_base;
  

•   apple_mem_map[index].size = gd->ram_size;
  

•   node = fdt_path_offset(gd->fdt_blob, "/chosen/framebuffer");
  

Can you use the ofnode interface here and below?

I can try...

...

•   if (node < 0)
  

•           return 0;
  

•   base = fdtdec_get_addr_size(gd->fdt_blob, node, "reg", &size);
  

•   if (base == FDT_ADDR_T_NONE)
  

•           return 0;
  

•   /* Add framebuffer mapping. */
  

•   index = ARRAY_SIZE(apple_mem_map) - 2;
  

•   apple_mem_map[index].virt = base;
  

•   apple_mem_map[index].phys = base;
  

•   apple_mem_map[index].size = size;
  

•   apple_mem_map[index].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
  

•           PTE_BLOCK_INNER_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN;
  

•   return 0;
  

+}

+int dram_init_banksize(void) +{

•   return fdtdec_setup_memory_banksize();
  

+}

+#define APPLE_WDT_BASE 0x23d2b0000ULL

should be in DT I think, with a driver

...

+#define APPLE_WDT_SYS_CTL_ENABLE BIT(2)

+typedef struct apple_wdt {

Needs comments

...

•   u32     reserved0[3];
  

•   u32     chip_ctl;
  

•   u32     sys_tmr;
  

•   u32     sys_cmp;
  

•   u32     reserved1;
  

•   u32     sys_ctl;
  

+} apple_wdt_t;

+void reset_cpu(void)

You should add a sysreset driver, as Bin says.

...

+{

•   apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
  

•   writel(0, &wdt->sys_cmp);
  

•   writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
  

•   while(1)
  

•           wfi();
  

+}

+extern long fw_dtb_pointer;

comment

...

+void *board_fdt_blob_setup(void) +{

•   return (void *)fw_dtb_pointer;
  

+}

+ulong board_get_usable_ram_top(ulong total_size) +{

•   /*
  

•    * Top part of RAM is used by firmware for things like the
  

•    * framebuffer.  This gives us plenty of room to play with.
  

•    */
  

•   return 0x980000000;
  

+} diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S new file mode 100644 index 0000000000..0f5313163e --- /dev/null +++ b/arch/arm/mach-apple/lowlevel_init.S @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2021 Mark Kettenis kettenis@openbsd.org
• */

+.align 8 +.global fw_dtb_pointer +fw_dtb_pointer:

•   .quad   0
  

+.global save_boot_params +save_boot_params:

•   adr     x1, fw_dtb_pointer
  

could use a comment as to where this is set up. Previous-stage firmware, I suppose?

Yes. I'm basically making U-Boot look like a Linux kernel, and m1n1 passes the DT in x1 just like the Linux kernel expects.

I suspect using CONFIG_OF_PRIOR_STAGE would make it more obvious what is happening here. But (a) I didn't know that existed and (b) we discussed removing that option in the near future.

...

•   str     x0, [x1]
  

•   b       save_boot_params_ret
  

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig new file mode 100644 index 0000000000..a7ae15576b --- /dev/null +++ b/configs/apple_m1_defconfig @@ -0,0 +1,14 @@ +CONFIG_ARM=y +CONFIG_ARCH_APPLE=y +# CONFIG_DISPLAY_CPUINFO is not set +# CONFIG_MMC is not set +# CONFIG_NET is not set +CONFIG_VIDEO_SIMPLE=y +CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y +CONFIG_USB_KEYBOARD=y +CONFIG_USB_STORAGE=y +CONFIG_USE_PREBOOT=y +CONFIG_PREBOOT="usb start" +# CONFIG_GENERATE_SMBIOS_TABLE is not set diff --git a/include/configs/apple.h b/include/configs/apple.h new file mode 100644 index 0000000000..1c246af002 --- /dev/null +++ b/include/configs/apple.h @@ -0,0 +1,38 @@ +#ifndef __CONFIG_H +#define __CONFIG_H

+#include <linux/sizes.h>

+#define CONFIG_SYS_LOAD_ADDR 0x880000000

+#define CONFIG_SYS_SDRAM_BASE 0x880000000

+#define CONFIG_LNX_KRNL_IMG_TEXT_OFFSET_BASE CONFIG_SYS_TEXT_BASE

+/* Environment */ +#define ENV_DEVICE_SETTINGS \

•   "stdin=serial,usbkbd\0" \
  

•   "stdout=serial,vidconsole\0" \
  

•   "stderr=serial,vidconsole\0"
  

+#define ENV_MEM_LAYOUT_SETTINGS \

•   "fdt_addr_r=0x960100000\0" \
  

•   "kernel_addr_r=0x960200000\0"
  

+#if CONFIG_IS_ENABLED(CMD_USB)

•   #define BOOT_TARGET_USB(func) func(USB, usb, 0)
  

+#else

•   #define BOOT_TARGET_USB(func)
  

+#endif

+#define BOOT_TARGET_DEVICES(func) \

•   BOOT_TARGET_USB(func)
  

+#include <config_distro_bootcmd.h>

+#define CONFIG_EXTRA_ENV_SETTINGS \

•   ENV_DEVICE_SETTINGS \
  

•   ENV_MEM_LAYOUT_SETTINGS \
  

•   BOOTENV
  

+#endif

2.33.0

Regards, Simon

From: Bin Meng bmeng.cn@gmail.com Date: Sun, 19 Sep 2021 09:11:06 +0800

Hi Mark,

On Sat, Sep 18, 2021 at 9:55 PM Mark Kettenis kettenis@openbsd.org wrote:

...

Apple M1 SoCs include an S5L UART which is a variant of the S5P UART. Add support for this variant and enable it by default on Apple SoCs.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/include/asm/arch-m1/clk.h | 11 ++++++++ arch/arm/include/asm/arch-m1/uart.h | 41 +++++++++++++++++++++++++++++ arch/arm/mach-apple/board.c | 5 ++++ drivers/serial/Kconfig | 2 +- drivers/serial/serial_s5p.c | 22 ++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 arch/arm/include/asm/arch-m1/clk.h create mode 100644 arch/arm/include/asm/arch-m1/uart.h

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

• • Copyright (C) 2021 Mark Kettenis kettenis@openbsd.org
• */

+#ifndef __ASM_ARM_ARCH_CLK_H_ +#define __ASM_ARM_ARCH_CLK_H_

+unsigned long get_uart_clk(int dev_index);

+#endif diff --git a/arch/arm/include/asm/arch-m1/uart.h b/arch/arm/include/asm/arch-m1/uart.h new file mode 100644 index 0000000000..d2a17a221e --- /dev/null +++ b/arch/arm/include/asm/arch-m1/uart.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2009 Samsung Electronics
• • Minkyu Kang mk7.kang@samsung.com
• • Heungjun Kim riverful.kim@samsung.com
• */

+#ifndef __ASM_ARCH_UART_H_ +#define __ASM_ARCH_UART_H_

+#ifndef __ASSEMBLY__ +/* baudrate rest value */ +union br_rest {

•   unsigned short  slot;           /* udivslot */
  

•   unsigned char   value;          /* ufracval */
  

+};

+struct s5p_uart {

•   unsigned int    ulcon;
  

•   unsigned int    ucon;
  

•   unsigned int    ufcon;
  

•   unsigned int    umcon;
  

•   unsigned int    utrstat;
  

•   unsigned int    uerstat;
  

•   unsigned int    ufstat;
  

•   unsigned int    umstat;
  

•   unsigned int    utxh;
  

•   unsigned int    urxh;
  

•   unsigned int    ubrdiv;
  

•   union br_rest   rest;
  

•   unsigned char   res3[0x3fd0];
  

+};

This does not look correct. This should be declared in the s5p UART driver header instead of SoC's

Well, this is the status quo for this driver. There are several variations of the hardware with slightly different register layouts and the header file trick is how that is handled.

...

+static inline int s5p_uart_divslot(void) +{

•   return 0;
  

+}

+#endif /* __ASSEMBLY__ */

+#endif diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index 0c8b35292e..8bc5c2f69e 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -156,3 +156,8 @@ ulong board_get_usable_ram_top(ulong total_size) */ return 0x980000000; }

+unsigned long get_uart_clk(int dev_index) +{

•   return 24000000;
  

+} diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 93348c0929..c3ac773929 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -719,7 +719,7 @@ config ROCKCHIP_SERIAL

config S5P_SERIAL bool "Support for Samsung S5P UART"

•   depends on ARCH_EXYNOS || ARCH_S5PC1XX
  

•   depends on ARCH_APPLE || ARCH_EXYNOS || ARCH_S5PC1XX
    default y
    help
      Select this to enable Samsung S5P UART support.
  

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 6d09952a5d..eb770d9b62 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -21,12 +21,21 @@

DECLARE_GLOBAL_DATA_PTR;

+#ifdef CONFIG_ARCH_APPLE +#define RX_FIFO_COUNT_SHIFT 0 +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT) +#define RX_FIFO_FULL (1 << 8) +#define TX_FIFO_COUNT_SHIFT 4 +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT) +#define TX_FIFO_FULL (1 << 9)

So different bit positions for RX/TX FIFIO register but still it is s5p compatible, strange ...

Yeah, the Apple hardware is based on an older variant of the s5p that had a smaller FIFO. The size of the FIFO was increased somewhere along the way and the bits were moved...

...

+#else #define RX_FIFO_COUNT_SHIFT 0 #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT) #define RX_FIFO_FULL (1 << 8) #define TX_FIFO_COUNT_SHIFT 16 #define TX_FIFO_COUNT_MASK (0xff << TX_FIFO_COUNT_SHIFT) #define TX_FIFO_FULL (1 << 24) +#endif

/* Information about a serial port */ struct s5p_serial_plat { @@ -83,7 +92,11 @@ static void __maybe_unused s5p_serial_baud(struct s5p_uart *uart, uint uclk, if (s5p_uart_divslot()) writew(udivslot[val % 16], &uart->rest.slot); else +#ifdef CONFIG_ARCH_APPLE

•           writel(val % 16, &uart->rest.value);
  

This looks like we should add a property in DT like "reg-width" to control such behavior?

There actually is such a property in the binding called "reg-io-width". So I suppose I should use that.

...

+#else writeb(val % 16, &uart->rest.value); +#endif }

#ifndef CONFIG_SPL_BUILD @@ -148,7 +161,11 @@ static int s5p_serial_getc(struct udevice *dev) return -EAGAIN;

    serial_err_check(uart, 0);

+#ifdef CONFIG_ARCH_APPLE

•   return (int)(readl(&uart->urxh) & 0xff);
  

+#else return (int)(readb(&uart->urxh) & 0xff); +#endif }

static int s5p_serial_putc(struct udevice *dev, const char ch) @@ -159,7 +176,11 @@ static int s5p_serial_putc(struct udevice *dev, const char ch) if (readl(&uart->ufstat) & TX_FIFO_FULL) return -EAGAIN;

+#ifdef CONFIG_ARCH_APPLE

•   writel(ch, &uart->utxh);
  

+#else writeb(ch, &uart->utxh); +#endif serial_err_check(uart, 1);

    return 0;

@@ -201,6 +222,7 @@ static const struct dm_serial_ops s5p_serial_ops = {

static const struct udevice_id s5p_serial_ids[] = { { .compatible = "samsung,exynos4210-uart" },

•   { .compatible = "apple,s5l-uart" },
    { }
  

};

Regards, Bin

Hi Mark,

On Sat, 2 Oct 2021 at 16:16, Mark Kettenis mark.kettenis@xs4all.nl wrote:

...

From: Simon Glass sjg@chromium.org Date: Sun, 19 Sep 2021 21:15:59 -0600

Hi Mark,

On Sat, 18 Sept 2021 at 07:55, Mark Kettenis kettenis@openbsd.org wrote:

...

Apple M1 SoCs include an S5L UART which is a variant of the S5P UART. Add support for this variant and enable it by default on Apple SoCs.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/include/asm/arch-m1/clk.h | 11 ++++++++ arch/arm/include/asm/arch-m1/uart.h | 41 +++++++++++++++++++++++++++++ arch/arm/mach-apple/board.c | 5 ++++ drivers/serial/Kconfig | 2 +- drivers/serial/serial_s5p.c | 22 ++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 arch/arm/include/asm/arch-m1/clk.h create mode 100644 arch/arm/include/asm/arch-m1/uart.h

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

• • Copyright (C) 2021 Mark Kettenis kettenis@openbsd.org
• */

+#ifndef __ASM_ARM_ARCH_CLK_H_ +#define __ASM_ARM_ARCH_CLK_H_

+unsigned long get_uart_clk(int dev_index);

comment? But this should be in a clock driver, unless you are trying to get a debug UART up ASAP with minimal code?

...

+#endif diff --git a/arch/arm/include/asm/arch-m1/uart.h b/arch/arm/include/asm/arch-m1/uart.h new file mode 100644 index 0000000000..d2a17a221e --- /dev/null +++ b/arch/arm/include/asm/arch-m1/uart.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • (C) Copyright 2009 Samsung Electronics
• • Minkyu Kang mk7.kang@samsung.com
• • Heungjun Kim riverful.kim@samsung.com
• */

+#ifndef __ASM_ARCH_UART_H_ +#define __ASM_ARCH_UART_H_

+#ifndef __ASSEMBLY__ +/* baudrate rest value */ +union br_rest {

•   unsigned short  slot;           /* udivslot */
  

•   unsigned char   value;          /* ufracval */
  

+};

+struct s5p_uart {

•   unsigned int    ulcon;
  

•   unsigned int    ucon;
  

•   unsigned int    ufcon;
  

•   unsigned int    umcon;
  

•   unsigned int    utrstat;
  

•   unsigned int    uerstat;
  

•   unsigned int    ufstat;
  

•   unsigned int    umstat;
  

•   unsigned int    utxh;
  

•   unsigned int    urxh;
  

•   unsigned int    ubrdiv;
  

•   union br_rest   rest;
  

•   unsigned char   res3[0x3fd0];
  

+};

+static inline int s5p_uart_divslot(void) +{

•   return 0;
  

+}

+#endif /* __ASSEMBLY__ */

+#endif diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index 0c8b35292e..8bc5c2f69e 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -156,3 +156,8 @@ ulong board_get_usable_ram_top(ulong total_size) */ return 0x980000000; }

+unsigned long get_uart_clk(int dev_index) +{

•   return 24000000;
  

Should add to devicetree for the driver

...

+} diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 93348c0929..c3ac773929 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -719,7 +719,7 @@ config ROCKCHIP_SERIAL

config S5P_SERIAL bool "Support for Samsung S5P UART"

•   depends on ARCH_EXYNOS || ARCH_S5PC1XX
  

•   depends on ARCH_APPLE || ARCH_EXYNOS || ARCH_S5PC1XX
    default y
    help
      Select this to enable Samsung S5P UART support.
  

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 6d09952a5d..eb770d9b62 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -21,12 +21,21 @@

DECLARE_GLOBAL_DATA_PTR;

+#ifdef CONFIG_ARCH_APPLE

This should use the compatible string to decide which variant it is, then the checks happen at runtime. We should never have arch-specific #ifdefs in drivers.

...

+#define RX_FIFO_COUNT_SHIFT 0 +#define RX_FIFO_COUNT_MASK (0xf << RX_FIFO_COUNT_SHIFT) +#define RX_FIFO_FULL (1 << 8) +#define TX_FIFO_COUNT_SHIFT 4 +#define TX_FIFO_COUNT_MASK (0xf << TX_FIFO_COUNT_SHIFT) +#define TX_FIFO_FULL (1 << 9) +#else #define RX_FIFO_COUNT_SHIFT 0 #define RX_FIFO_COUNT_MASK (0xff << RX_FIFO_COUNT_SHIFT) #define RX_FIFO_FULL (1 << 8) #define TX_FIFO_COUNT_SHIFT 16 #define TX_FIFO_COUNT_MASK (0xff << TX_FIFO_COUNT_SHIFT) #define TX_FIFO_FULL (1 << 24) +#endif

/* Information about a serial port */ struct s5p_serial_plat { @@ -83,7 +92,11 @@ static void __maybe_unused s5p_serial_baud(struct s5p_uart *uart, uint uclk, if (s5p_uart_divslot()) writew(udivslot[val % 16], &uart->rest.slot); else +#ifdef CONFIG_ARCH_APPLE

•           writel(val % 16, &uart->rest.value);
  

+#else writeb(val % 16, &uart->rest.value); +#endif }

#ifndef CONFIG_SPL_BUILD @@ -148,7 +161,11 @@ static int s5p_serial_getc(struct udevice *dev) return -EAGAIN;

    serial_err_check(uart, 0);

+#ifdef CONFIG_ARCH_APPLE

•   return (int)(readl(&uart->urxh) & 0xff);
  

+#else return (int)(readb(&uart->urxh) & 0xff); +#endif }

static int s5p_serial_putc(struct udevice *dev, const char ch) @@ -159,7 +176,11 @@ static int s5p_serial_putc(struct udevice *dev, const char ch) if (readl(&uart->ufstat) & TX_FIFO_FULL) return -EAGAIN;

+#ifdef CONFIG_ARCH_APPLE

•   writel(ch, &uart->utxh);
  

+#else writeb(ch, &uart->utxh); +#endif serial_err_check(uart, 1);

    return 0;

@@ -201,6 +222,7 @@ static const struct dm_serial_ops s5p_serial_ops = {

static const struct udevice_id s5p_serial_ids[] = { { .compatible = "samsung,exynos4210-uart" },

•   { .compatible = "apple,s5l-uart" },
    { }
  

};

-- 2.33.0

Can you add debug UART support? Or does this already work?

I don't think it does work. But yes, I want to add it since it will help a lot with debugging. That makes fixing some of the issues raised by you and bing more problematic since the debug UART support code:

• Needs to use 32-bit access on Apple M1, and using the "reg-io-width" property there isn't possible.

• Relies on the FIFO control bits which are in a different position on the Apple M1.

So I am starting to wonder whether it makes sense to simply copy serial_s5p.c to serial_s5l.c and modify that one to do the right thing on the apple SoC.

Alternatively I could introduce Kconfig stuff to switch between S5L and S5P support.

Well, firstly you can use a compatible string as mentioned above which solves the normal case. You just need a flag in plat which indicates that it should use word access for some registers. The ns16550 shows how to call dev_get_driver_data(). You will need to add members to s5p_serial_plat to select the tx/tx FIFO values since these need tobe variables now.

BTW s5p_serial_baud() should have a param to indicate which access method to use.

For the debug UART I suggest just putting an IS_ENABLED(CONFIG_ARCH_APPLE) in there. You could add a Kconfig for the Apple UART (even though it uses the same file) which might be a little nicer, but it is only the debug UART, after all...

Finally, this driver is not perfect as it has CLK_EXYNOS only for some boards. Ideally all boards would move to use the clock driver.

Regards, Simon

Hi Mark,

On Sat, 18 Sept 2021 at 07:55, Mark Kettenis kettenis@openbsd.org wrote:

The DART is an IOMMU that is used on Apple's M1 SoC. This driver supports the DART in bypass mode as well as in a mode where it creates a 1:1 mapping of a subset of RAM as not all DARTs support bypass mode. The USB3 ports integrated on the SoC use a DART that supports bypass mode. The 1:1 mapping will be used in the future to support other devices such as the PCIe host bridge of the M1 SoC.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

drivers/misc/Kconfig | 7 ++ drivers/misc/Makefile | 1 + drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 drivers/misc/apple_dart.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 997b713221..d70b060e74 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -38,6 +38,13 @@ config ALTERA_SYSID Select this to enable a sysid for Altera devices. Please find details on the "Embedded Peripherals IP User Guide" of Altera.

+config APPLE_DART

•   bool "Apple DART support"
  

•   depends on MISC && ARCH_APPLE
  

•   default y
  

•   help
  

•     Enable support for the DART on Apple SoCs.
  

Should have at least 3 lines. E.g. what does DART stand for, what does it do and what does the driver support?

config ATSHA204A bool "Support for Atmel ATSHA204A module" depends on MISC diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index b64cd2a4de..f666cd392d 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -29,6 +29,7 @@ endif endif obj-$(CONFIG_ALI152X) += ali512x.o obj-$(CONFIG_ALTERA_SYSID) += altera_sysid.o +obj-$(CONFIG_APPLE_DART) += apple_dart.o obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o obj-$(CONFIG_DS4510) += ds4510.o diff --git a/drivers/misc/apple_dart.c b/drivers/misc/apple_dart.c new file mode 100644 index 0000000000..f619a624d0 --- /dev/null +++ b/drivers/misc/apple_dart.c @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <common.h> +#include <clk.h> +#include <cpu_func.h> +#include <dm.h> +#include <mapmem.h> +#include <asm/io.h>

+#define DART_PARAMS2 0x0004 +#define DART_PARAMS2_BYPASS_SUPPORT BIT(0) +#define DART_TLB_OP 0x0020 +#define DART_TLB_OP_OPMASK (0xfff << 20) +#define DART_TLB_OP_FLUSH (0x001 << 20) +#define DART_TLB_OP_BUSY BIT(2) +#define DART_TLB_OP_SIDMASK 0x0034 +#define DART_ERROR_STATUS 0x0040 +#define DART_TCR(sid) (0x0100 + 4 * (sid)) +#define DART_TCR_TRANSLATE_ENABLE BIT(7) +#define DART_TCR_BYPASS_DART BIT(8) +#define DART_TCR_BYPASS_DAPF BIT(12) +#define DART_TTBR(sid, idx) (0x0200 + 16 * (sid) + 4 * (idx)) +#define DART_TTBR_VALID BIT(31) +#define DART_TTBR_SHIFT 12

+struct apple_dart_priv {

How about s/apple_dart/dart/ ?

It makes the code easier to read.

•   struct clk_bulk clks;
  

•   void *base;
  

+};

+dma_addr_t apple_dart_bus_start; +phys_addr_t apple_dart_phys_start; +phys_size_t apple_dart_size = SZ_512M;

Try to avoid variables in drivers. Can these go in a priv struct?

+static void apple_dart_flush_tlb(struct apple_dart_priv *priv)

comments on these functions

+{

•   u32 status;
  

•   writel(0xffffffff, priv->base + DART_TLB_OP_SIDMASK);
  

•   writel(DART_TLB_OP_FLUSH, priv->base + DART_TLB_OP);
  

•   for (;;) {
  

•           status = readl(priv->base + DART_TLB_OP);
  

•           if ((status & DART_TLB_OP_OPMASK) == 0)
  

•                   break;
  

•           if ((status & DART_TLB_OP_BUSY) == 0)
  

•                   break;
  

•   }
  

+}

+static int apple_dart_clk_init(struct udevice *dev,

•                          struct apple_dart_priv *priv)
  

+{

•   int ret;
  

•   ret = clk_get_bulk(dev, &priv->clks);
  

•   if (ret == -ENOSYS || ret == -ENOENT)
  

Does -ENOSYS not indicate an error? If it doesn't, I think a comment would help here.

•           return 0;
  

•   if (ret)
  

•           return ret;
  

•   ret = clk_enable_bulk(&priv->clks);
  

•   if (ret) {
  

•           clk_release_bulk(&priv->clks);
  

•           return ret;
  

•   }
  

•   return 0;
  

+}

+static int apple_dart_bind(struct udevice *dev) +{

•   void *base;
  

•   int sid, i;
  

•   base = dev_read_addr_ptr(dev);
  

•   if (!base)
  

•           return -EINVAL;
  

•   u32 params2 = readl(base + DART_PARAMS2);
  

•   if (params2 & DART_PARAMS2_BYPASS_SUPPORT) {
  

•           for (sid = 0; sid < 16; sid++) {
  

•                   writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF,
  

•                          base + DART_TCR(sid));
  

•                   for (i = 0; i < 4; i++)
  

•                           writel(0, base + DART_TTBR(sid, i));
  

•           }
  

•   }
  

Not allowed hardware access in bind(). Can this more to probe() ?

•   return 0;
  

+}

+static int apple_dart_probe(struct udevice *dev) +{

•   struct apple_dart_priv *priv = dev_get_priv(dev);
  

•   phys_addr_t phys;
  

•   u64 *l1, *l2;
  

•   int sid, i, j;
  

•   int ret;
  

•   apple_dart_phys_start = gd->ram_top - apple_dart_size;
  

•   priv->base = dev_read_addr_ptr(dev);
  

•   if (!priv->base)
  

•           return -EINVAL;
  

•   ret = apple_dart_clk_init(dev, priv);
  

•   if (ret)
  

•           return ret;
  

•   l1 = memalign(SZ_64K, SZ_64K);
  

•   memset(l1, 0, SZ_64K);
  

•   i = 0;
  

•   phys = apple_dart_phys_start;
  

•   while (phys < apple_dart_phys_start + apple_dart_size) {
  

•           l2 = memalign(SZ_16K, SZ_16K);