All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: Simon Glass <sjg@chromium.org>
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
Subject: Re: [PATCH 1/5] arm: apple: Add initial support for Apple's M1 SoC
Date: Mon, 20 Sep 2021 10:49:37 +0200 (CEST)	[thread overview]
Message-ID: <56146ded1ff993a7@bloch.sibelius.xs4all.nl> (raw)
In-Reply-To: <CAPnjgZ0Tt_n3T=VsDO=q8m3=uW==gqoRWRzXu+aX4AX2Beq0MA@mail.gmail.com> (message from Simon Glass on Sun, 19 Sep 2021 21:15:57 -0600)

> 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
> 

  reply	other threads:[~2021-09-20  8:49 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-18 13:54 [PATCH 0/5] Apple M1 Support Mark Kettenis
2021-09-18 13:54 ` [PATCH 1/5] arm: apple: Add initial support for Apple's M1 SoC Mark Kettenis
2021-09-19  1:04   ` Bin Meng
2021-09-19  1:17     ` Bin Meng
2021-09-19 20:33       ` Mark Kettenis
2021-09-21 12:42         ` Tom Rini
2021-09-21 15:53           ` Bin Meng
2021-09-21 16:04             ` Tom Rini
2021-09-21 16:08             ` Mark Kettenis
2021-09-25 13:27               ` Simon Glass
2021-09-19 20:05     ` Mark Kettenis
2021-09-20  3:15   ` Simon Glass
2021-09-20  8:49     ` Mark Kettenis [this message]
2021-09-21  1:11       ` Simon Glass
2021-09-18 13:54 ` [PATCH 2/5] serial: s5p: Add Apple M1 support Mark Kettenis
2021-09-19  1:11   ` Bin Meng
2021-09-19 20:30     ` Mark Kettenis
2021-09-20  3:15   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-10-02 22:15     ` Mark Kettenis
2021-10-03  2:01       ` Simon Glass
2021-09-18 13:54 ` [PATCH 3/5] misc: Add Apple DART driver Mark Kettenis
2021-09-20  3:16   ` Simon Glass
2021-09-20  8:33     ` Mark Kettenis
2021-09-21  1:11       ` Simon Glass
2021-09-25 13:27         ` Simon Glass
2021-09-26 20:53         ` Mark Kettenis
2021-09-27 20:14           ` Simon Glass
2021-09-18 13:54 ` [PATCH 4/5] arm: dts: apple: Add preliminary device trees Mark Kettenis
2021-09-20  3:16   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-09-18 13:54 ` [PATCH 5/5] doc: board: apple: Add Apple M1 documentation Mark Kettenis
2021-09-19  1:22   ` Bin Meng
2021-09-20  3:16   ` Simon Glass
2021-09-25 13:27     ` Simon Glass
2021-09-20  8:45   ` Igor Opaniuk
2021-09-25  1:20 ` [PATCH 0/5] Apple M1 Support Simon Glass
2021-09-25  8:11   ` Mark Kettenis
2021-09-25 13:27     ` Simon Glass
2021-09-25 13:52       ` Mark Kettenis
2021-09-25 14:42         ` Simon Glass
2021-09-25 16:45           ` Mark Kettenis
2021-09-26 15:53             ` Simon Glass
2021-09-28  3:46               ` Simon Glass
2021-09-28  7:36                 ` Mark Kettenis
2021-09-28 12:07                   ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56146ded1ff993a7@bloch.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ashe@kivikakk.ee \
    --cc=bharat.gooty@broadcom.com \
    --cc=bin.meng@windriver.com \
    --cc=hs@denx.de \
    --cc=igor.opaniuk@foundries.io \
    --cc=kettenis@openbsd.org \
    --cc=kishon@ti.com \
    --cc=marek.behun@nic.cz \
    --cc=masami.hiramatsu@linaro.org \
    --cc=michael@walle.cc \
    --cc=michal.simek@xilinx.com \
    --cc=oliver.graute@kococonnector.com \
    --cc=p.yadav@ti.com \
    --cc=padmarao.begari@microchip.com \
    --cc=rayagonda.kokatanur@broadcom.com \
    --cc=sjg@chromium.org \
    --cc=sr@denx.de \
    --cc=stephan@gerhold.net \
    --cc=takahiro.akashi@linaro.org \
    --cc=tianrui-wei@outlook.com \
    --cc=u-boot@lists.denx.de \
    --cc=vabhav.sharma@nxp.com \
    --cc=wasim.khan@nxp.com \
    --cc=weijie.gao@mediatek.com \
    --cc=xypron.glpk@gmx.de \
    --cc=ycliang@andestech.com \
    --cc=ye.li@nxp.com \
    --subject='Re: [PATCH 1/5] arm: apple: Add initial support for Apple'\''s M1 SoC' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.