From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15B0BC433F5 for ; Mon, 20 Sep 2021 08:49:54 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 454DD60EB4 for ; Mon, 20 Sep 2021 08:49:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 454DD60EB4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 68FFD80F03; Mon, 20 Sep 2021 10:49:51 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 8A17D81280; Mon, 20 Sep 2021 10:49:49 +0200 (CEST) Received: from sibelius.xs4all.nl (sibelius.xs4all.nl [83.163.83.176]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 4684F80C8A for ; Mon, 20 Sep 2021 10:49:45 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=mark.kettenis@xs4all.nl Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id 0bba6ced; Mon, 20 Sep 2021 10:49:37 +0200 (CEST) Date: Mon, 20 Sep 2021 10:49:37 +0200 (CEST) From: Mark Kettenis To: Simon Glass Cc: kettenis@openbsd.org, u-boot@lists.denx.de, bharat.gooty@broadcom.com, rayagonda.kokatanur@broadcom.com, oliver.graute@kococonnector.com, bin.meng@windriver.com, ycliang@andestech.com, tianrui-wei@outlook.com, stephan@gerhold.net, padmarao.begari@microchip.com, kishon@ti.com, xypron.glpk@gmx.de, michael@walle.cc, masami.hiramatsu@linaro.org, ashe@kivikakk.ee, wasim.khan@nxp.com, michal.simek@xilinx.com, igor.opaniuk@foundries.io, hs@denx.de, ye.li@nxp.com, sr@denx.de, vabhav.sharma@nxp.com, marek.behun@nic.cz, weijie.gao@mediatek.com, takahiro.akashi@linaro.org, andriy.shevchenko@linux.intel.com, p.yadav@ti.com In-Reply-To: (message from Simon Glass on Sun, 19 Sep 2021 21:15:57 -0600) Subject: Re: [PATCH 1/5] arm: apple: Add initial support for Apple's M1 SoC References: <20210918135437.36667-1-kettenis@openbsd.org> <20210918135437.36667-2-kettenis@openbsd.org> Message-ID: <56146ded1ff993a7@bloch.sibelius.xs4all.nl> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean > From: Simon Glass > Date: Sun, 19 Sep 2021 21:15:57 -0600 > > Hi Mark, > > On Sat, 18 Sept 2021 at 07:55, Mark Kettenis 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 > > --- > > 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 > > + */ > > + > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +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 > > + */ > > + > > +.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 > > + > > +#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 > > + > > +#define CONFIG_EXTRA_ENV_SETTINGS \ > > + ENV_DEVICE_SETTINGS \ > > + ENV_MEM_LAYOUT_SETTINGS \ > > + BOOTENV > > + > > +#endif > > -- > > 2.33.0 > > > > Regards, > Simon >