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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 49296C433F5 for ; Sat, 22 Jan 2022 14:45:18 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id DC8D182045; Sat, 22 Jan 2022 15:45:15 +0100 (CET) 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 B07F8820FE; Sat, 22 Jan 2022 15:45:14 +0100 (CET) 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 96AD381FF1 for ; Sat, 22 Jan 2022 15:45:11 +0100 (CET) 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 245f5ac0; Sat, 22 Jan 2022 15:45:10 +0100 (CET) Date: Sat, 22 Jan 2022 15:45:10 +0100 (CET) From: Mark Kettenis To: Simon Glass Cc: kettenis@openbsd.org, u-boot@lists.denx.de, jh80.chung@samsung.com, trini@konsulko.com, sven@svenpeter.dev, marcan@marcan.st, bmeng.cn@gmail.com In-Reply-To: (message from Simon Glass on Fri, 21 Jan 2022 18:40:24 -0700) Subject: Re: [PATCH 7/8] nvme: apple: Add driver for Apple NVMe storage controller References: <20220114110438.58452-1-kettenis@openbsd.org> <20220114110438.58452-8-kettenis@openbsd.org> Message-ID: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean > From: Simon Glass > Date: Fri, 21 Jan 2022 18:40:24 -0700 > > Hi Mark, > > On Fri, 14 Jan 2022 at 04:05, Mark Kettenis wrote: > > > > Add a driver for the NVMe storage controller integrated on > > Apple SoCs. This NVMe controller isn't PCI based and deviates > > from the NVMe standard in its implementation of the command > > submission queue and the integration of an NVMMU that needs > > to be managed. This commit tweaks the core NVMe code to > > support the linear command submission queue implemented by > > this controller. But setting up the submission queue and > > managing the NVMMU controller is handled by implementing > > the driver ops that were added in an earlier commit. > > > > Signed-off-by: Mark Kettenis > > Tested-on: firefly-rk3399 > > Tested-by: Mark Kettenis > > --- > > configs/apple_m1_defconfig | 1 + > > drivers/nvme/Kconfig | 11 ++ > > drivers/nvme/Makefile | 1 + > > drivers/nvme/nvme_apple.c | 233 +++++++++++++++++++++++++++++++++++++ > > 4 files changed, 246 insertions(+) > > create mode 100644 drivers/nvme/nvme_apple.c > > Tested on: Macbook Air M1 > Tested-by: Simon Glass > > > > > diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig > > index cb235e4e7d..1528217b17 100644 > > --- a/configs/apple_m1_defconfig > > +++ b/configs/apple_m1_defconfig > > @@ -11,6 +11,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y > > # CONFIG_NET is not set > > # CONFIG_MMC is not set > > CONFIG_DEBUG_UART_ANNOUNCE=y > > +CONFIG_NVME_APPLE=y > > CONFIG_USB_XHCI_HCD=y > > CONFIG_USB_XHCI_DWC3=y > > CONFIG_USB_KEYBOARD=y > > diff --git a/drivers/nvme/Kconfig b/drivers/nvme/Kconfig > > index 78da444c8b..0cb465160b 100644 > > --- a/drivers/nvme/Kconfig > > +++ b/drivers/nvme/Kconfig > > @@ -10,6 +10,17 @@ config NVME > > This option enables support for NVM Express devices. > > It supports basic functions of NVMe (read/write). > > > > +config NVME_APPLE > > + bool "Apple NVMe controller support" > > + select NVME > > + help > > + This option enables support for the NVMe storage > > + controller integrated on Apple SoCs. This controller > > + isn't PCI-based based and deviates from the NVMe > > + standard implementation in its implementation of > > + the command submission queue and the integration > > + of an NVMMU that needs to be managed. > > + > > config NVME_PCI > > bool "NVM Express PCI device support" > > depends on PCI > > diff --git a/drivers/nvme/Makefile b/drivers/nvme/Makefile > > index fad9724e17..fa7b619446 100644 > > --- a/drivers/nvme/Makefile > > +++ b/drivers/nvme/Makefile > > @@ -3,4 +3,5 @@ > > # Copyright (C) 2017, Bin Meng > > > > obj-y += nvme-uclass.o nvme.o nvme_show.o > > +obj-$(CONFIG_NVME_APPLE) += nvme_apple.o > > obj-$(CONFIG_NVME_PCI) += nvme_pci.o > > diff --git a/drivers/nvme/nvme_apple.c b/drivers/nvme/nvme_apple.c > > new file mode 100644 > > index 0000000000..b0dc8492f0 > > --- /dev/null > > +++ b/drivers/nvme/nvme_apple.c > > @@ -0,0 +1,233 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * (C) Copyright 2021 Mark Kettenis > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include "nvme.h" > > +#include > > + > > +#include > > +#include > > asm/arch/ should work It doesn't. For some reason I end up with arch/arm/include/asm/arch -> arch-m1 But this rtkit stuff is expected to be generic and apply to all Apple SoCs, not just the M1. > > > +#include > > + > > +#undef readl_poll_timeout > > +#define readl_poll_timeout readl_poll_sleep_timeout > > Do we need this? No we don't. Fixed. > > + > > +#define REG_CPU_CTRL 0x0044 > > +#define REG_CPU_CTRL_RUN BIT(4) > > + > > +#define ANS_MAX_PEND_CMDS_CTRL 0x01210 > > +#define ANS_MAX_QUEUE_DEPTH 64 > > +#define ANS_BOOT_STATUS 0x01300 > > +#define ANS_BOOT_STATUS_OK 0xde71ce55 > > +#define ANS_MODESEL 0x01304 > > +#define ANS_UNKNOWN_CTRL 0x24008 > > +#define ANS_PRP_NULL_CHECK (1 << 11) > > +#define ANS_LINEAR_SQ_CTRL 0x24908 > > +#define ANS_LINEAR_SQ_CTRL_EN (1 << 0) > > +#define ANS_ASQ_DB 0x2490c > > +#define ANS_IOSQ_DB 0x24910 > > +#define ANS_NVMMU_NUM 0x28100 > > +#define ANS_NVMMU_BASE_ASQ 0x28108 > > +#define ANS_NVMMU_BASE_IOSQ 0x28110 > > +#define ANS_NVMMU_TCB_INVAL 0x28118 > > +#define ANS_NVMMU_TCB_STAT 0x28120 > > + > > +#define ANS_NVMMU_TCB_SIZE 0x4000 > > +#define ANS_NVMMU_TCB_PITCH 0x80 > > + > > +struct ans_nvmmu_tcb { > > + u8 opcode; > > + u8 flags; > > + u8 command_id; > > + u8 pad0; > > + u32 prpl_len; > > + u8 pad1[16]; > > + u64 prp1; > > + u64 prp2; > > Needs comments I put a comment at the top explaining my limited understanding of what the TCBs are used for. > > > +}; > > + > > +#define ANS_NVMMU_TCB_WRITE BIT(0) > > +#define ANS_NVMMU_TCB_READ BIT(1) > > + > > +struct apple_nvme_priv { > > + struct nvme_dev ndev; > > + void *base; > > + void *asc; > > + struct reset_ctl_bulk resets; > > + struct mbox_chan chan; > > + struct ans_nvmmu_tcb *tcbs[NVME_Q_NUM]; > > + u32 __iomem *q_db[NVME_Q_NUM]; > > Needs comments Done, although they give me a captain obvious vibe... > > +}; > > + > > +static int apple_nvme_alloc_queue(struct nvme_queue *nvmeq) > > +{ > > + struct apple_nvme_priv *priv = > > + container_of(nvmeq->dev, struct apple_nvme_priv, ndev); > > + struct nvme_dev *dev = nvmeq->dev; > > + > > + switch (nvmeq->qid) { > > + case NVME_ADMIN_Q: > > + case NVME_IO_Q: > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + priv->tcbs[nvmeq->qid] = (void *)memalign(4096, ANS_NVMMU_TCB_SIZE); > > + memset((void *)priv->tcbs[nvmeq->qid], 0, ANS_NVMMU_TCB_SIZE); > > + > > + switch (nvmeq->qid) { > > + case NVME_ADMIN_Q: > > + priv->q_db[nvmeq->qid] = > > + ((void __iomem *)dev->bar) + ANS_ASQ_DB; > > + nvme_writeq((ulong)priv->tcbs[nvmeq->qid], > > + ((void __iomem *)dev->bar) + ANS_NVMMU_BASE_ASQ); > > + break; > > + case NVME_IO_Q: > > + priv->q_db[nvmeq->qid] = > > + ((void __iomem *)dev->bar) + ANS_IOSQ_DB; > > + nvme_writeq((ulong)priv->tcbs[nvmeq->qid], > > + ((void __iomem *)dev->bar) + ANS_NVMMU_BASE_IOSQ); > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static void apple_nvme_submit_cmd(struct nvme_queue *nvmeq, > > + struct nvme_command *cmd) > > +{ > > + struct apple_nvme_priv *priv = > > + container_of(nvmeq->dev, struct apple_nvme_priv, ndev); > > + struct ans_nvmmu_tcb *tcb; > > + u16 tail = nvmeq->sq_tail; > > + > > + tcb = ((void *)priv->tcbs[nvmeq->qid]) + tail * ANS_NVMMU_TCB_PITCH; > > + memset(tcb, 0, sizeof(*tcb)); > > + tcb->opcode = cmd->common.opcode; > > + tcb->flags = ANS_NVMMU_TCB_WRITE | ANS_NVMMU_TCB_READ; > > + tcb->command_id = tail; > > + tcb->prpl_len = cmd->rw.length; > > + tcb->prp1 = cmd->common.prp1; > > + tcb->prp2 = cmd->common.prp2; > > + > > + writel(tail, priv->q_db[nvmeq->qid]); > > +} > > + > > +static void apple_nvme_complete_cmd(struct nvme_queue *nvmeq, > > + struct nvme_command *cmd) > > +{ > > + struct apple_nvme_priv *priv = > > + container_of(nvmeq->dev, struct apple_nvme_priv, ndev); > > + struct ans_nvmmu_tcb *tcb; > > + u16 tail = nvmeq->sq_tail; > > + > > + tcb = ((void *)priv->tcbs[nvmeq->qid]) + tail * ANS_NVMMU_TCB_PITCH; > > + memset(tcb, 0, sizeof(*tcb)); > > + writel(tail, ((void __iomem *)nvmeq->dev->bar) + ANS_NVMMU_TCB_INVAL); > > + readl(((void __iomem *)nvmeq->dev->bar) + ANS_NVMMU_TCB_STAT); > > + > > + if (++tail == nvmeq->q_depth) > > + tail = 0; > > + nvmeq->sq_tail = tail; > > +} > > + > > +static int apple_nvme_probe(struct udevice *dev) > > +{ > > + struct apple_nvme_priv *priv = dev_get_priv(dev); > > + fdt_addr_t addr; > > + u32 ctrl, stat; > > + int ret; > > + > > + priv->base = dev_read_addr_ptr(dev); > > + if (!priv->base) > > + return -EINVAL; > > + > > + addr = dev_read_addr_index(dev, 1); > > + if (addr == FDT_ADDR_T_NONE) > > + return -EINVAL; > > + priv->asc = map_sysmem(addr, 0); > > + > > + ret = reset_get_bulk(dev, &priv->resets); > > + if (ret < 0) > > + return ret; > > + > > + ret = mbox_get_by_index(dev, 0, &priv->chan); > > Is the mailbox not mentioned in the device tre? There is an "mboxes" property, which is what mbox_get_by_index() uses. But there is only one mbox so it isn't named. > > > + if (ret < 0) > > + return ret; > > + > > + ctrl = readl(priv->asc + REG_CPU_CTRL); > > + writel(ctrl | REG_CPU_CTRL_RUN, priv->asc + REG_CPU_CTRL); > > + > > + ret = apple_rtkit_init(&priv->chan); > > Should probe a driver here See my reply to PATCH 3/8 in this series. > > + if (ret < 0) > > + return ret; > > + > > + ret = readl_poll_timeout(priv->base + ANS_BOOT_STATUS, stat, > > + (stat == ANS_BOOT_STATUS_OK), 100, 500000); > > + if (ret < 0) { > > + printf("%s: NVMe firmware didn't boot\n", __func__); > > + return -ETIMEDOUT; > > + } > > + > > + writel(ANS_LINEAR_SQ_CTRL_EN, priv->base + ANS_LINEAR_SQ_CTRL); > > + writel(((ANS_MAX_QUEUE_DEPTH << 16) | ANS_MAX_QUEUE_DEPTH), > > + priv->base + ANS_MAX_PEND_CMDS_CTRL); > > + > > + writel(readl(priv->base + ANS_UNKNOWN_CTRL) & ~ANS_PRP_NULL_CHECK, > > + priv->base + ANS_UNKNOWN_CTRL); > > + > > + strcpy(priv->ndev.vendor, "Apple"); > > + > > + writel((ANS_NVMMU_TCB_SIZE / ANS_NVMMU_TCB_PITCH) - 1, > > + priv->base + ANS_NVMMU_NUM); > > + writel(0, priv->base + ANS_MODESEL); > > + > > + priv->ndev.bar = priv->base; > > + return nvme_init(dev); > > +} > > + > > +static int apple_nvme_remove(struct udevice *dev) > > +{ > > + struct apple_nvme_priv *priv = dev_get_priv(dev); > > + u32 ctrl; > > + > > + nvme_shutdown(dev); > > + > > + apple_rtkit_shutdown(&priv->chan, APPLE_RTKIT_PWR_STATE_SLEEP); > > + > > + ctrl = readl(priv->asc + REG_CPU_CTRL); > > + writel(ctrl & ~REG_CPU_CTRL_RUN, priv->asc + REG_CPU_CTRL); > > + > > + reset_assert_bulk(&priv->resets); > > + reset_deassert_bulk(&priv->resets); > > + > > + return 0; > > +} > > + > > +static const struct nvme_ops apple_nvme_ops = { > > + .alloc_queue = apple_nvme_alloc_queue, > > + .submit_cmd = apple_nvme_submit_cmd, > > + .complete_cmd = apple_nvme_complete_cmd, > > +}; > > + > > +static const struct udevice_id apple_nvme_ids[] = { > > + { .compatible = "apple,nvme-ans2" }, > > + { /* sentinel */ } > > +}; > > + > > +U_BOOT_DRIVER(apple_nvme) = { > > + .name = "apple_nvme", > > + .id = UCLASS_NVME, > > + .of_match = apple_nvme_ids, > > + .priv_auto = sizeof(struct apple_nvme_priv), > > + .probe = apple_nvme_probe, > > + .remove = apple_nvme_remove, > > + .ops = &apple_nvme_ops, > > + .flags = DM_FLAG_OS_PREPARE, > > +}; > > -- > > 2.34.1 > > > > Regards, > Simon >