Hi Kishon,

Subject: Re: [PATCH v7 15/18] NTB: Add support for EPF PCI-Express Non-
Transparent Bridge

Hi Sherry,

On 09/11/20 3:07 pm, Sherry Sun wrote:

quoted

Hi Kishon,

quoted

Subject: [PATCH v7 15/18] NTB: Add support for EPF PCI-Express Non-
Transparent Bridge

From: Kishon Vijay Abraham I <redacted>

Add support for EPF PCI-Express Non-Transparent Bridge (NTB) device.
This driver is platform independent and could be used by any platform
which have multiple PCIe endpoint instances configured using the pci-epf-

ntb driver.

quoted

quoted

The driver connnects to the standard NTB sub-system interface. The
EPF NTB device has configurable number of memory windows (Max 4),
configurable number of doorbell (Max 32), and configurable number of
scratch-pad registers.

Signed-off-by: Kishon Vijay Abraham I <redacted>
---
 drivers/ntb/hw/Kconfig          |   1 +
 drivers/ntb/hw/Makefile         |   1 +
 drivers/ntb/hw/epf/Kconfig      |   6 +
 drivers/ntb/hw/epf/Makefile     |   1 +
 drivers/ntb/hw/epf/ntb_hw_epf.c | 755
++++++++++++++++++++++++++++++++
 5 files changed, 764 insertions(+)
 create mode 100644 drivers/ntb/hw/epf/Kconfig  create mode 100644
drivers/ntb/hw/epf/Makefile  create mode 100644
drivers/ntb/hw/epf/ntb_hw_epf.c

diff --git a/drivers/ntb/hw/Kconfig b/drivers/ntb/hw/Kconfig index

e77c587060ff..c325be526b80 100644

--- a/drivers/ntb/hw/Kconfig
+++ b/drivers/ntb/hw/Kconfig

@@ -2,4 +2,5 @@
 source "drivers/ntb/hw/amd/Kconfig"
 source "drivers/ntb/hw/idt/Kconfig"
 source "drivers/ntb/hw/intel/Kconfig"
+source "drivers/ntb/hw/epf/Kconfig"
 source "drivers/ntb/hw/mscc/Kconfig"

diff --git a/drivers/ntb/hw/Makefile b/drivers/ntb/hw/Makefile index

4714d6238845..223ca592b5f9 100644

--- a/drivers/ntb/hw/Makefile
+++ b/drivers/ntb/hw/Makefile

@@ -2,4 +2,5 @@
 obj-$(CONFIG_NTB_AMD)	+= amd/
 obj-$(CONFIG_NTB_IDT)	+= idt/
 obj-$(CONFIG_NTB_INTEL)	+= intel/
+obj-$(CONFIG_NTB_EPF)	+= epf/
 obj-$(CONFIG_NTB_SWITCHTEC) += mscc/ diff --git

a/drivers/ntb/hw/epf/Kconfig b/drivers/ntb/hw/epf/Kconfig new file
mode 100644 index 000000000000..6197d1aab344

--- /dev/null
+++ b/drivers/ntb/hw/epf/Kconfig

@@ -0,0 +1,6 @@
+config NTB_EPF
+	tristate "Generic EPF Non-Transparent Bridge support"
+	depends on m
+	help
+	  This driver supports EPF NTB on configurable endpoint.
+	  If unsure, say N.

diff --git a/drivers/ntb/hw/epf/Makefile

b/drivers/ntb/hw/epf/Makefile new file mode 100644 index
000000000000..2f560a422bc6

--- /dev/null
+++ b/drivers/ntb/hw/epf/Makefile

@@ -0,0 +1 @@
+obj-$(CONFIG_NTB_EPF) += ntb_hw_epf.o

diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c

b/drivers/ntb/hw/epf/ntb_hw_epf.c new file mode 100644 index
000000000000..0a144987851a

--- /dev/null
+++ b/drivers/ntb/hw/epf/ntb_hw_epf.c

@@ -0,0 +1,755 @@

......

quoted

+static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
+			    struct pci_dev *pdev)
+{
+	struct device *dev = ndev->dev;
+	int ret;
+
+	pci_set_drvdata(pdev, ndev);
+
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(dev, "Cannot enable PCI device\n");
+		goto err_pci_enable;
+	}
+
+	ret = pci_request_regions(pdev, "ntb");
+	if (ret) {
+		dev_err(dev, "Cannot obtain PCI resources\n");
+		goto err_pci_regions;
+	}
+
+	pci_set_master(pdev);
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret) {
+		ret = dma_set_mask_and_coherent(dev,
DMA_BIT_MASK(32));
+		if (ret) {
+			dev_err(dev, "Cannot set DMA mask\n");
+			goto err_dma_mask;
+		}
+		dev_warn(&pdev->dev, "Cannot DMA highmem\n");
+	}
+
+	ndev->ctrl_reg = pci_iomap(pdev, 0, 0);

The second parameter of pci_iomap should be ndev->ctrl_reg_bar instead

of the hardcode value 0, right?

quoted

quoted

+	if (!ndev->ctrl_reg) {
+		ret = -EIO;
+		goto err_dma_mask;
+	}
+
+	ndev->peer_spad_reg = pci_iomap(pdev, 1, 0);

pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);

quoted

+	if (!ndev->peer_spad_reg) {
+		ret = -EIO;
+		goto err_dma_mask;
+	}
+
+	ndev->db_reg = pci_iomap(pdev, 2, 0);

pci_iomap(pdev, ndev->db_reg_bar, 0);

Good catch. Will fix it and send. Thank you for reviewing.

You're welcome.
By the way, since I've studied VOP(virtio over pcie) before, and only recently learned
about NTB related code. I have some questions about NTB.

For NTB, in order to make two(or more) different systems to communicate with each other, 
seems at least three boards are required(two host boards and one board with multiple EP
instances as bridge), right?
But for VOP, only two boards are needed(one board as host and one board as card) to realize the
communication between the two systems, so my question is what are the advantages of using NTB? 
Because I think the architecture of NTB seems more complicated. Many thanks!

Best regards
Sherry

Regards,
Kishon