Re: [PATCH 7/8] nvme: apple: Add driver for Apple NVMe storage controller

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: Simon Glass <sjg@chromium.org>
> Date: Sat, 22 Jan 2022 10:17:08 -0700
> 
> Hi Mark,
> 
> On Sat, 22 Jan 2022 at 07:45, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Simon Glass <sjg@chromium.org>
> > > Date: Fri, 21 Jan 2022 18:40:24 -0700
> > >
> > > Hi Mark,
> > >
> > > On Fri, 14 Jan 2022 at 04:05, Mark Kettenis <kettenis@openbsd.org> 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 <kettenis@openbsd.org>
> > > > Tested-on: firefly-rk3399
> > > > Tested-by: Mark Kettenis <kettenis@openbsd.org>
> > > > ---
> > > >  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 <sjg@chromium.org>
> > >
> > > >
> > > > 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 <bmeng.cn@gmail.com>
> > > >
> > > >  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 <kettenis@openbsd.org>
> > > > + */
> > > > +
> > > > +#include <common.h>
> > > > +#include <dm.h>
> > > > +#include <mailbox.h>
> > > > +#include <mapmem.h>
> > > > +#include "nvme.h"
> > > > +#include <reset.h>
> > > > +
> > > > +#include <asm/io.h>
> > > > +#include <asm/arch-apple/rtkit.h>
> > >
> > > 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.
> 
> Hmm so you plan to have an arch/m2 as well? Can we not put everything
> in arch/apple then? We don't normally have arch directories for
> different generations unless they are completely different.

No, I think a single arch/arch-apple is better.  That isolates us from
whatever silly marketing names Apple comes up with for their SoCs.
The goal is to have a single U-Boot binary that supports multiple
generations of the SoC.

That means I have to set SYS_SOC to "apple" instead of "m1" and move
the existing uart.h file into arch/arm/include/arch/arch-apple/ isn't
it?

I can add a diff that does this to the series.