[RFC] nvme: Split out PCI support

[RFC] nvme: Split out PCI support

[Mark Kettenis]

Apple SoCs have an integrated NVMe controller that isn't connected
over a PCIe bus. In preparation for adding support for this NVMe
controller, split out the PCI support into its own file. This file
is selected through a new CONFIG_NVME_PCI Kconfig option, so do
a wholesale replacement of CONFIG_NVME with CONFIG_NVME_PCI.

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

I only did the s/CONFIG_NVME/CONFIG_NVME_PCI/ change for
firefly-rk3399_defconfig for now. If folks agree this is a reasonable
approach I'll do the wholesale replacement mentioned in the commit
message and integrate this in a series that actually adds support for
the Apple SoC NVMe controller.

 configs/firefly-rk3399_defconfig |  2 +-
 drivers/nvme/Kconfig             | 10 ++++++-
 drivers/nvme/Makefile            |  1 +
 drivers/nvme/nvme.c              | 38 ++-----------------------
 drivers/nvme/nvme.h              |  3 ++
 drivers/nvme/nvme_pci.c          | 49 ++++++++++++++++++++++++++++++++
 6 files changed, 66 insertions(+), 37 deletions(-)
 create mode 100644 drivers/nvme/nvme_pci.c

diff --git a/configs/firefly-rk3399_defconfig b/configs/firefly-rk3399_defconfig
index d576b5c38d..fe1c019f1d 100644
--- a/configs/firefly-rk3399_defconfig
+++ b/configs/firefly-rk3399_defconfig
@@ -41,7 +41,7 @@ CONFIG_SF_DEFAULT_SPEED=20000000
 CONFIG_DM_ETH=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_GMAC_ROCKCHIP=y
-CONFIG_NVME=y
+CONFIG_NVME_PCI=y
 CONFIG_PCI=y
 CONFIG_PMIC_RK8XX=y
 CONFIG_REGULATOR_PWM=y
diff --git a/drivers/nvme/Kconfig b/drivers/nvme/Kconfig
index 1f6d1f5648..78da444c8b 100644
--- a/drivers/nvme/Kconfig
+++ b/drivers/nvme/Kconfig
@@ -4,8 +4,16 @@
 
 config NVME
 	bool "NVM Express device support"
-	depends on BLK && PCI
+	depends on BLK
 	select HAVE_BLOCK_DEVICE
 	help
 	  This option enables support for NVM Express devices.
 	  It supports basic functions of NVMe (read/write).
+
+config NVME_PCI
+	bool "NVM Express PCI device support"
+	depends on PCI
+	select NVME
+	help
+	  This option enables support for NVM Express PCI
+	  devices.
diff --git a/drivers/nvme/Makefile b/drivers/nvme/Makefile
index 64f102b208..fad9724e17 100644
--- a/drivers/nvme/Makefile
+++ b/drivers/nvme/Makefile
@@ -3,3 +3,4 @@
 # Copyright (C) 2017, Bin Meng <bmeng.cn@gmail.com>
 
 obj-y += nvme-uclass.o nvme.o nvme_show.o
+obj-$(CONFIG_NVME_PCI) += nvme_pci.o
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 3c529a2fce..be518ec20b 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -12,7 +12,6 @@
 #include <log.h>
 #include <malloc.h>
 #include <memalign.h>
-#include <pci.h>
 #include <time.h>
 #include <dm/device-internal.h>
 #include <linux/compat.h>
@@ -698,7 +697,6 @@ static int nvme_blk_probe(struct udevice *udev)
 	struct blk_desc *desc = dev_get_uclass_plat(udev);
 	struct nvme_ns *ns = dev_get_priv(udev);
 	u8 flbas;
-	struct pci_child_plat *pplat;
 	struct nvme_id_ns *id;
 
 	id = memalign(ndev->page_size, sizeof(struct nvme_id_ns));
@@ -723,8 +721,7 @@ static int nvme_blk_probe(struct udevice *udev)
 	desc->log2blksz = ns->lba_shift;
 	desc->blksz = 1 << ns->lba_shift;
 	desc->bdev = udev;
-	pplat = dev_get_parent_plat(udev->parent);
-	sprintf(desc->vendor, "0x%.4x", pplat->vendor);
+	memcpy(desc->vendor, ndev->vendor, sizeof(ndev->vendor));
 	memcpy(desc->product, ndev->serial, sizeof(ndev->serial));
 	memcpy(desc->revision, ndev->firmware_rev, sizeof(ndev->firmware_rev));
 
@@ -818,27 +815,13 @@ U_BOOT_DRIVER(nvme_blk) = {
 	.priv_auto	= sizeof(struct nvme_ns),
 };
 
-static int nvme_bind(struct udevice *udev)
+int nvme_init(struct udevice *udev)
 {
-	static int ndev_num;
-	char name[20];
-
-	sprintf(name, "nvme#%d", ndev_num++);
-
-	return device_set_name(udev, name);
-}
-
-static int nvme_probe(struct udevice *udev)
-{
-	int ret;
 	struct nvme_dev *ndev = dev_get_priv(udev);
 	struct nvme_id_ns *id;
-
-	ndev->instance = trailing_strtol(udev->name);
+	int ret;
 
 	INIT_LIST_HEAD(&ndev->namespaces);
-	ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0,
-			PCI_REGION_MEM);
 	if (readl(&ndev->bar->csts) == -1) {
 		ret = -ENODEV;
 		printf("Error: %s: Out of memory!\n", udev->name);
@@ -922,18 +905,3 @@ free_queue:
 free_nvme:
 	return ret;
 }
-
-U_BOOT_DRIVER(nvme) = {
-	.name	= "nvme",
-	.id	= UCLASS_NVME,
-	.bind	= nvme_bind,
-	.probe	= nvme_probe,
-	.priv_auto	= sizeof(struct nvme_dev),
-};
-
-struct pci_device_id nvme_supported[] = {
-	{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, ~0) },
-	{}
-};
-
-U_BOOT_PCI_DEVICE(nvme, nvme_supported);
diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
index c6aae4da5d..8e9ae3c7f6 100644
--- a/drivers/nvme/nvme.h
+++ b/drivers/nvme/nvme.h
@@ -608,6 +608,7 @@ struct nvme_dev {
 	u32 ctrl_config;
 	struct nvme_bar __iomem *bar;
 	struct list_head namespaces;
+	char vendor[8];
 	char serial[20];
 	char model[40];
 	char firmware_rev[8];
@@ -635,4 +636,6 @@ struct nvme_ns {
 	u8 flbas;
 };
 
+int nvme_init(struct udevice *udev);
+
 #endif /* __DRIVER_NVME_H__ */
diff --git a/drivers/nvme/nvme_pci.c b/drivers/nvme/nvme_pci.c
new file mode 100644
index 0000000000..5f60fb884f
--- /dev/null
+++ b/drivers/nvme/nvme_pci.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2017 NXP Semiconductors
+ * Copyright (C) 2017 Bin Meng <bmeng.cn@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <pci.h>
+#include "nvme.h"
+
+static int nvme_bind(struct udevice *udev)
+{
+	static int ndev_num;
+	char name[20];
+
+	sprintf(name, "nvme#%d", ndev_num++);
+
+	return device_set_name(udev, name);
+}
+
+static int nvme_probe(struct udevice *udev)
+{
+	struct nvme_dev *ndev = dev_get_priv(udev);
+	struct pci_child_plat *pplat;
+
+	pplat = dev_get_parent_plat(udev);
+	sprintf(ndev->vendor, "0x%.4x", pplat->vendor);
+
+	ndev->instance = trailing_strtol(udev->name);
+	ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0,
+			PCI_REGION_MEM);
+	return nvme_init(udev);
+}
+
+U_BOOT_DRIVER(nvme) = {
+	.name	= "nvme",
+	.id	= UCLASS_NVME,
+	.bind	= nvme_bind,
+	.probe	= nvme_probe,
+	.priv_auto	= sizeof(struct nvme_dev),
+};
+
+struct pci_device_id nvme_supported[] = {
+	{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, ~0) },
+	{}
+};
+
+U_BOOT_PCI_DEVICE(nvme, nvme_supported);
-- 
2.34.1

Re: [RFC] nvme: Split out PCI support

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

On Fri, Dec 31, 2021 at 09:00:20PM +0100, Mark Kettenis wrote:

> Apple SoCs have an integrated NVMe controller that isn't connected
> over a PCIe bus. In preparation for adding support for this NVMe
> controller, split out the PCI support into its own file. This file
> is selected through a new CONFIG_NVME_PCI Kconfig option, so do
> a wholesale replacement of CONFIG_NVME with CONFIG_NVME_PCI.
> 
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> ---
> 
> I only did the s/CONFIG_NVME/CONFIG_NVME_PCI/ change for
> firefly-rk3399_defconfig for now. If folks agree this is a reasonable
> approach I'll do the wholesale replacement mentioned in the commit
> message and integrate this in a series that actually adds support for
> the Apple SoC NVMe controller.

This seems a reasonable approach.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC] nvme: Split out PCI support

[Mark Kettenis]

> Date: Tue, 11 Jan 2022 11:16:54 -0500
> From: Tom Rini <trini@konsulko.com>
> 
> On Fri, Dec 31, 2021 at 09:00:20PM +0100, Mark Kettenis wrote:
> 
> > Apple SoCs have an integrated NVMe controller that isn't connected
> > over a PCIe bus. In preparation for adding support for this NVMe
> > controller, split out the PCI support into its own file. This file
> > is selected through a new CONFIG_NVME_PCI Kconfig option, so do
> > a wholesale replacement of CONFIG_NVME with CONFIG_NVME_PCI.
> > 
> > Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> > ---
> > 
> > I only did the s/CONFIG_NVME/CONFIG_NVME_PCI/ change for
> > firefly-rk3399_defconfig for now. If folks agree this is a reasonable
> > approach I'll do the wholesale replacement mentioned in the commit
> > message and integrate this in a series that actually adds support for
> > the Apple SoC NVMe controller.
> 
> This seems a reasonable approach.

Thanks Tom!

I sent a full series for adding NVMe support for the Apple Silicon
Macs on friday.  And I just sent a small follow-up series to support
the laptop keyboards.  Together with the power domain support series
that has already been reviewed, that pretty much gives us enough
support for all the machines that Apple released with the original M1
SoC.  It would be great if we could ship 2022.04 with that included.

There is a bit more to do afterwards.  PCI support, needed for the
type-A USB ports on the Mac mini, needs a bit more work still.  But
the type-C ports do work, so this shouldn't be a show-stopper for most
people.  Support for te newer laptops with M1 Pro/Max SoCs also needs
a bit more work still.

Thanks,

Mark