All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2015-12-23 13:42 ` Xiangliang Yu
  (?)
@ 2015-12-23  9:29 ` Christoph Hellwig
  2015-12-23 15:09   ` Allen Hubbe
  -1 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2015-12-23  9:29 UTC (permalink / raw)
  To: Xiangliang Yu
  Cc: jdmason, dave.jiang, Allen.Hubbe, linux-ntb, linux-kernel,
	SPG_Linux_Kernel

Just curious, what do you actually use NTB for?  Seems like we're
adding a giant subsystem without actual consumers, or did I miss
something?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2015-12-23 13:42 ` Xiangliang Yu
  0 siblings, 0 replies; 18+ messages in thread
From: Xiangliang Yu @ 2015-12-23 13:42 UTC (permalink / raw)
  To: jdmason, dave.jiang, Allen.Hubbe, linux-ntb, linux-kernel
  Cc: SPG_Linux_Kernel, Xiangliang Yu

AMD NTB support following main features:
(1) Three memory windows;
(2) Sixteen 32-bit scratch pad;
(3) Two 16-bit doorbell interrupt;
(4) Five event interrupts;
(5) One system can wake up opposite system of NTB;
(6) Flush previous request to the opposite system;
(7) There are reset and PME_TO mechanisms between two systems;

Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
---
 drivers/ntb/hw/amd/Kconfig      |    7 +
 drivers/ntb/hw/amd/Makefile     |    1 +
 drivers/ntb/hw/amd/ntb_hw_amd.c | 1229 +++++++++++++++++++++++++++++++++++++++
 drivers/ntb/hw/amd/ntb_hw_amd.h |  263 +++++++++
 4 files changed, 1500 insertions(+)
 create mode 100644 drivers/ntb/hw/amd/Kconfig
 create mode 100644 drivers/ntb/hw/amd/Makefile
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.c
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.h

diff --git a/drivers/ntb/hw/amd/Kconfig b/drivers/ntb/hw/amd/Kconfig
new file mode 100644
index 0000000..cfe903c
--- /dev/null
+++ b/drivers/ntb/hw/amd/Kconfig
@@ -0,0 +1,7 @@
+config NTB_AMD
+	tristate "AMD Non-Transparent Bridge support"
+	depends on X86_64
+	help
+	 This driver supports AMD NTB on capable Zeppelin hardware.
+
+	 If unsure, say N.
diff --git a/drivers/ntb/hw/amd/Makefile b/drivers/ntb/hw/amd/Makefile
new file mode 100644
index 0000000..ad54da9
--- /dev/null
+++ b/drivers/ntb/hw/amd/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_NTB_AMD) += ntb_hw_amd.o
diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
new file mode 100644
index 0000000..cedac8d
--- /dev/null
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
@@ -0,0 +1,1229 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ *
+ *   GPL LICENSE SUMMARY
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ *
+ *   BSD LICENSE
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copy
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of AMD Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * AMD PCIe NTB Linux driver
+ *
+ * Contact Information:
+ * Xiangliang Yu<Xiangliang.Yu@amd.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+#include <linux/ntb.h>
+
+#include "ntb_hw_amd.h"
+
+#define NTB_NAME	"ntb_hw_amd"
+#define NTB_DESC	"AMD(R) PCI-E Non-Transparent Bridge Driver"
+#define NTB_VER		"1.0"
+
+MODULE_DESCRIPTION(NTB_DESC);
+MODULE_VERSION(NTB_VER);
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("AMD Inc.");
+
+static const struct file_operations amd_ntb_debugfs_info;
+static struct dentry *debugfs_dir;
+
+static const struct ntb_dev_ops amd_ntb_ops = {
+	.mw_count		= amd_ntb_mw_count,
+	.mw_get_range		= amd_ntb_mw_get_range,
+	.mw_set_trans		= amd_ntb_mw_set_trans,
+	.link_is_up		= amd_ntb_link_is_up,
+	.link_enable		= amd_ntb_link_enable,
+	.link_disable		= amd_ntb_link_disable,
+	.db_valid_mask		= amd_ntb_db_valid_mask,
+	.db_vector_count	= amd_ntb_db_vector_count,
+	.db_vector_mask		= amd_ntb_db_vector_mask,
+	.db_read		= amd_ntb_db_read,
+	.db_clear		= amd_ntb_db_clear,
+	.db_set_mask		= amd_ntb_db_set_mask,
+	.db_clear_mask		= amd_ntb_db_clear_mask,
+	.peer_db_addr		= amd_ntb_peer_db_addr,
+	.peer_db_set		= amd_ntb_peer_db_set,
+	.spad_count		= amd_ntb_spad_count,
+	.spad_read		= amd_ntb_spad_read,
+	.spad_write		= amd_ntb_spad_write,
+	.peer_spad_addr		= amd_ntb_peer_spad_addr,
+	.peer_spad_read		= amd_ntb_peer_spad_read,
+	.peer_spad_write	= amd_ntb_peer_spad_write,
+};
+
+static int ndev_mw_to_bar(struct amd_ntb_dev *ndev, int idx)
+{
+	if (idx < 0 || idx > ndev->mw_count)
+		return -EINVAL;
+
+	return 1 << idx;
+}
+
+static int amd_ntb_mw_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->mw_count;
+}
+
+static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
+				phys_addr_t *base,
+				resource_size_t *size,
+				resource_size_t *align,
+				resource_size_t *align_size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	int bar;
+
+	bar = ndev_mw_to_bar(ndev, idx);
+	if (bar < 0)
+		return bar;
+
+	if (base)
+		*base = pci_resource_start(ndev->ntb.pdev, bar);
+
+	if (size)
+		*size = pci_resource_len(ndev->ntb.pdev, bar);
+
+	if (align)
+		*align = 4096;
+
+	if (align_size)
+		*align_size = 1;
+
+	return 0;
+}
+
+static int amd_ntb_mw_set_trans(struct ntb_dev *ntb, int idx,
+				dma_addr_t addr, resource_size_t size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	unsigned long xlat_reg, limit_reg = 0;
+	resource_size_t mw_size;
+	void __iomem *mmio, *peer_mmio;
+	u64 base_addr, limit, reg_val;
+	int bar;
+
+	bar = ndev_mw_to_bar(ndev, idx);
+	if (bar < 0)
+		return bar;
+
+	mw_size = pci_resource_len(ndev->ntb.pdev, bar);
+
+	/* make sure the range fits in the usable mw size */
+	if (size > mw_size)
+		return -EINVAL;
+
+	mmio = ndev->self_mmio;
+	peer_mmio = ndev->peer_mmio;
+
+	base_addr = pci_resource_start(ndev->ntb.pdev, bar);
+	if (bar == 1) {
+		xlat_reg = AMD_BAR1XLAT_OFFSET;
+		limit_reg = AMD_BAR1LMT_OFFSET;
+	} else {
+		xlat_reg = AMD_BAR23XLAT_OFFSET + ((bar - 2) << 3);
+		limit_reg = AMD_BAR23LMT_OFFSET + ((bar - 2) << 3);
+	}
+
+	if (bar != 1) {
+		/* Set the limit if supported */
+		limit = base_addr + size;
+
+		/* set and verify setting the translation address */
+		iowrite64(addr, peer_mmio + xlat_reg);
+		reg_val = ioread64(peer_mmio + xlat_reg);
+		if (reg_val != addr) {
+			iowrite64(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+
+		/* set and verify setting the limit */
+		iowrite64(limit, mmio + limit_reg);
+		reg_val = ioread64(mmio + limit_reg);
+		if (reg_val != limit) {
+			iowrite64(base_addr, mmio + limit_reg);
+			iowrite64(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+	} else {
+		/* split bar addr range must all be 32 bit */
+		if (addr & (~0ull << 32))
+			return -EINVAL;
+		if ((addr + size) & (~0ull << 32))
+			return -EINVAL;
+
+		/* Set the limit if supported */
+		limit = base_addr + size;
+
+		/* set and verify setting the translation address */
+		iowrite32(0, peer_mmio + xlat_reg + 0x4);
+		iowrite32(addr, peer_mmio + xlat_reg);
+		reg_val = ioread32(peer_mmio + xlat_reg);
+		if (reg_val != addr) {
+			iowrite32(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+
+		/* set and verify setting the limit */
+		iowrite32(limit, mmio + limit_reg);
+		reg_val = ioread32(mmio + limit_reg);
+		if (reg_val != limit) {
+			iowrite32(base_addr, mmio + limit_reg);
+			iowrite32(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+static int amd_link_is_up(struct amd_ntb_dev *ndev)
+{
+	/* set link down and clear flag */
+	if (ndev->peer_sta & AMD_PEER_RESET_EVENT) {
+		ndev->peer_sta &= ~AMD_PEER_RESET_EVENT;
+		return 0;
+	} else if (ndev->peer_sta & (AMD_PEER_D3_EVENT |
+				     AMD_PEER_PMETO_EVENT)) {
+		return 0;
+	} else if (ndev->peer_sta & AMD_PEER_D0_EVENT) {
+		ndev->peer_sta = 0;
+		return 0;
+	} else
+		return NTB_LNK_STA_ACTIVE(ndev->cntl_sta);
+}
+
+static int amd_ntb_link_is_up(struct ntb_dev *ntb,
+			      enum ntb_speed *speed,
+			      enum ntb_width *width)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	int ret = 0;
+
+	if (amd_link_is_up(ndev)) {
+		if (speed)
+			*speed = NTB_LNK_STA_SPEED(ndev->lnk_sta);
+		if (width)
+			*width = NTB_LNK_STA_WIDTH(ndev->lnk_sta);
+
+		dev_dbg(ndev_dev(ndev), "link is up.\n");
+
+		ret = 1;
+	} else {
+		if (speed)
+			*speed = NTB_SPEED_NONE;
+		if (width)
+			*width = NTB_WIDTH_NONE;
+
+		dev_dbg(ndev_dev(ndev), "link is down.\n");
+	}
+
+	return ret;
+}
+
+static int amd_ntb_link_enable(struct ntb_dev *ntb,
+			       enum ntb_speed max_speed,
+			       enum ntb_width max_width)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	/* Enable event interrupt */
+	ndev->int_mask &= ~AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	if (ndev->ntb.topo == NTB_TOPO_SEC)
+		return -EINVAL;
+	dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
+
+	ntb_ctl = NTB_READ_REG(mmio, CNTL);
+	ntb_ctl |= (3 << 20);
+	NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
+
+	return 0;
+}
+
+static int amd_ntb_link_disable(struct ntb_dev *ntb)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	/* Disable event interrupt */
+	ndev->int_mask |= AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	if (ndev->ntb.topo == NTB_TOPO_SEC)
+		return -EINVAL;
+	dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
+
+	ntb_ctl = NTB_READ_REG(mmio, CNTL);
+	ntb_ctl &= ~(3 << 16);
+	NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
+
+	return 0;
+}
+
+static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->db_valid_mask;
+}
+
+static int amd_ntb_db_vector_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->db_count;
+}
+
+static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (db_vector < 0 || db_vector > ndev->db_count)
+		return 0;
+
+	return ntb_ndev(ntb)->db_valid_mask & (1 << db_vector);
+}
+
+static u64 amd_ntb_db_read(struct ntb_dev *ntb)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	return (u64)NTB_READ_REG(mmio, DBSTAT);
+}
+
+static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	NTB_WRITE_REG(mmio, (u32)db_bits, DBSTAT);
+
+	return 0;
+}
+
+static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned long flags;
+
+	if (db_bits & ~ndev->db_valid_mask)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ndev->db_mask_lock, flags);
+	ndev->db_mask |= db_bits;
+	NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
+	spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
+
+	return 0;
+}
+
+static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned long flags;
+
+	if (db_bits & ~ndev->db_valid_mask)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ndev->db_mask_lock, flags);
+	ndev->db_mask &= ~db_bits;
+	NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
+	spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
+
+	return 0;
+}
+
+static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
+		phys_addr_t *db_addr,
+		resource_size_t *db_size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (db_addr)
+		*db_addr = (phys_addr_t)(ndev->peer_mmio + AMD_DBREQ_OFFSET);
+	if (db_size)
+		*db_size = sizeof(u32);
+
+	return 0;
+}
+
+static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	NTB_WRITE_REG(mmio, (u32)db_bits, DBREQ);
+
+	return 0;
+}
+
+static int amd_ntb_spad_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->spad_count;
+}
+
+static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return 0;
+
+	return NTB_READ_OFFSET(mmio, SPAD, (idx << 2));
+}
+
+static int amd_ntb_spad_write(struct ntb_dev *ntb,
+				int idx, u32 val)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	NTB_WRITE_OFFSET(mmio, val, SPAD, (idx << 2));
+
+	return 0;
+}
+
+static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
+				    phys_addr_t *spad_addr)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	if (spad_addr)
+		*spad_addr = (phys_addr_t)(ndev->self_mmio + AMD_SPAD_OFFSET +
+				ndev->peer_spad + (idx << 2));
+	return 0;
+}
+
+static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 offset;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	offset = ndev->peer_spad + (idx << 2);
+	return NTB_READ_OFFSET(mmio, SPAD, offset);
+}
+
+static int amd_ntb_peer_spad_write(struct ntb_dev *ntb,
+				     int idx, u32 val)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 offset;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	offset = ndev->peer_spad + (idx << 2);
+	NTB_WRITE_OFFSET(mmio, val, SPAD, offset);
+
+	return 0;
+}
+
+static void amd_ack_SMU(struct amd_ntb_dev *ndev, u32 bit)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	int reg;
+
+	reg = NTB_READ_REG(mmio, SMUACK);
+	reg |= bit;
+	NTB_WRITE_REG(mmio, reg, SMUACK);
+
+	ndev->peer_sta |= bit;
+}
+
+#ifdef CONFIG_PM
+/*
+ * flush the requests to peer side
+ */
+static int amd_flush_peer_requests(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 reg;
+
+	if (!amd_link_is_up(ndev)) {
+		dev_err(ndev_dev(ndev), "link is down.\n");
+		return -EINVAL;
+	}
+
+	reg = NTB_READ_REG(mmio, FLUSHTRIG);
+	reg |= 0x1;
+	NTB_WRITE_REG(mmio, reg, FLUSHTRIG);
+
+	wait_for_completion(&ndev->flush_cmpl);
+
+	reinit_completion(&ndev->flush_cmpl);
+
+	return 0;
+}
+#endif
+
+static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 status;
+
+	status = NTB_READ_REG(mmio, INTSTAT);
+	if (!(status & AMD_EVENT_INTMASK))
+		return;
+
+	dev_dbg(ndev_dev(ndev), "status = 0x%x and vec = %d\n", status, vec);
+
+	status &= AMD_EVENT_INTMASK;
+	switch (status) {
+	case AMD_PEER_FLUSH_EVENT:
+		complete(&ndev->flush_cmpl);
+		break;
+	case AMD_PEER_RESET_EVENT:
+		amd_ack_SMU(ndev, AMD_PEER_RESET_EVENT);
+
+		/* link down first */
+		ntb_link_event(&ndev->ntb);
+		/* polling peer status */
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+
+		break;
+	case AMD_PEER_D3_EVENT:
+	case AMD_PEER_PMETO_EVENT:
+		amd_ack_SMU(ndev, status);
+
+		/* link down */
+		ntb_link_event(&ndev->ntb);
+
+		break;
+	case AMD_PEER_D0_EVENT:
+		mmio = ndev->peer_mmio;
+		status = NTB_READ_REG(mmio, PMESTAT);
+		/* check if this is WAKEUP event */
+		if (status & 0x1)
+			complete(&ndev->wakeup_cmpl);
+
+		amd_ack_SMU(ndev, AMD_PEER_D0_EVENT);
+
+		if (amd_link_is_up(ndev))
+			ntb_link_event(&ndev->ntb);
+		else
+			schedule_delayed_work(&ndev->hb_timer,
+						AMD_LINK_HB_TIMEOUT);
+		break;
+	default:
+		dev_info(ndev_dev(ndev), "event status = 0x%x.\n", status);
+		break;
+	}
+}
+
+static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
+{
+	dev_dbg(ndev_dev(ndev), "vec %d\n", vec);
+
+	if (vec > (ndev->msix_vec_count - 1)) {
+		dev_err(ndev_dev(ndev), "Invalid interrupt.\n");
+		return IRQ_HANDLED;
+	}
+
+	if (vec > 15 || (ndev->msix_vec_count == 1))
+		amd_handle_event(ndev, vec);
+
+	if (vec < 16)
+		ntb_db_event(&ndev->ntb, vec);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ndev_vec_isr(int irq, void *dev)
+{
+	struct amd_ntb_vec *nvec = dev;
+
+	return ndev_interrupt(nvec->ndev, nvec->num);
+}
+
+static irqreturn_t ndev_irq_isr(int irq, void *dev)
+{
+	struct amd_ntb_dev *ndev = dev;
+
+	return ndev_interrupt(ndev, irq - ndev_pdev(ndev)->irq);
+}
+
+static int ndev_init_isr(struct amd_ntb_dev *ndev,
+				int msix_min, int msix_max)
+{
+	struct pci_dev *pdev;
+	int rc, i, msix_count, node;
+
+	pdev = ndev_pdev(ndev);
+
+	node = dev_to_node(&pdev->dev);
+
+	ndev->db_mask = ndev->db_valid_mask;
+
+	/* Try to set up msix irq */
+	ndev->vec = kzalloc_node(msix_max * sizeof(*ndev->vec),
+				 GFP_KERNEL, node);
+	if (!ndev->vec)
+		goto err_msix_vec_alloc;
+
+	ndev->msix = kzalloc_node(msix_max * sizeof(*ndev->msix),
+				  GFP_KERNEL, node);
+	if (!ndev->msix)
+		goto err_msix_alloc;
+
+	for (i = 0; i < msix_max; ++i)
+		ndev->msix[i].entry = i;
+
+	msix_count = pci_enable_msix_range(pdev, ndev->msix,
+					   msix_min, msix_max);
+	if (msix_count < 0)
+		goto err_msix_enable;
+
+	/* Disable MSIX if msix count is less than 16 */
+	if (msix_count < msix_min) {
+		pci_disable_msix(pdev);
+		goto err_msix_enable;
+	}
+
+	for (i = 0; i < msix_count; ++i) {
+		ndev->vec[i].ndev = ndev;
+		ndev->vec[i].num = i;
+		rc = request_irq(ndev->msix[i].vector, ndev_vec_isr, 0,
+				 "ndev_vec_isr", &ndev->vec[i]);
+		if (rc)
+			goto err_msix_request;
+	}
+
+	dev_dbg(ndev_dev(ndev), "Using msix interrupts\n");
+	ndev->db_count = msix_min;
+	ndev->msix_vec_count = msix_max;
+	return 0;
+
+err_msix_request:
+	while (i-- > 0)
+		free_irq(ndev->msix[i].vector, ndev);
+	pci_disable_msix(pdev);
+err_msix_enable:
+	kfree(ndev->msix);
+err_msix_alloc:
+	kfree(ndev->vec);
+err_msix_vec_alloc:
+	ndev->msix = NULL;
+	ndev->vec = NULL;
+
+	/* Try to set up msi irq */
+	rc = pci_enable_msi(pdev);
+	if (rc)
+		goto err_msi_enable;
+
+	rc = request_irq(pdev->irq, ndev_irq_isr, 0,
+			 "ndev_irq_isr", ndev);
+	if (rc)
+		goto err_msi_request;
+
+	dev_dbg(ndev_dev(ndev), "Using msi interrupts\n");
+	ndev->db_count = 1;
+	ndev->msix_vec_count = 1;
+	return 0;
+
+err_msi_request:
+	pci_disable_msi(pdev);
+err_msi_enable:
+
+	/* Try to set up intx irq */
+	pci_intx(pdev, 1);
+
+	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
+			 "ndev_irq_isr", ndev);
+	if (rc)
+		goto err_intx_request;
+
+	dev_dbg(ndev_dev(ndev), "Using intx interrupts\n");
+	ndev->db_count = 1;
+	ndev->msix_vec_count = 1;
+	return 0;
+
+err_intx_request:
+	return rc;
+}
+
+static void ndev_deinit_isr(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev;
+	void __iomem *mmio = ndev->self_mmio;
+	int i;
+
+	pdev = ndev_pdev(ndev);
+
+	/* Mask all doorbell interrupts */
+	ndev->db_mask = ndev->db_valid_mask;
+	NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
+
+	if (ndev->msix) {
+		i = ndev->msix_vec_count;
+		while (i--)
+			free_irq(ndev->msix[i].vector, &ndev->vec[i]);
+		pci_disable_msix(pdev);
+		kfree(ndev->msix);
+		kfree(ndev->vec);
+	} else {
+		free_irq(pdev->irq, ndev);
+		if (pci_dev_msi_enabled(pdev))
+			pci_disable_msi(pdev);
+		else
+			pci_intx(pdev, 0);
+	}
+}
+
+static ssize_t ndev_debugfs_read(struct file *filp, char __user *ubuf,
+				 size_t count, loff_t *offp)
+{
+	struct amd_ntb_dev *ndev;
+	void __iomem *mmio;
+	char *buf;
+	size_t buf_size;
+	ssize_t ret, off;
+	union { u64 v64; u32 v32; u16 v16; } u;
+
+	ndev = filp->private_data;
+	mmio = ndev->self_mmio;
+
+	buf_size = min(count, 0x800ul);
+
+	buf = kmalloc(buf_size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	off = 0;
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "NTB Device Information:\n");
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Connection Topology -\t%s\n",
+			 ntb_topo_string(ndev->ntb.topo));
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "LNK STA -\t\t%#06x\n", ndev->lnk_sta);
+
+	if (!amd_link_is_up(ndev)) {
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Status -\t\tDown\n");
+	} else {
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Status -\t\tUp\n");
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Speed -\t\tPCI-E Gen %u\n",
+				 NTB_LNK_STA_SPEED(ndev->lnk_sta));
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Width -\t\tx%u\n",
+				 NTB_LNK_STA_WIDTH(ndev->lnk_sta));
+	}
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Memory Window Count -\t%u\n", ndev->mw_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Scratchpad Count -\t%u\n", ndev->spad_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Count -\t%u\n", ndev->db_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "MSIX Vector Count -\t%u\n", ndev->msix_vec_count);
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Valid Mask -\t%#llx\n", ndev->db_valid_mask);
+
+	u.v64 = (u64)ioread32(ndev->self_mmio + AMD_DBMASK_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Mask -\t\t%#llx\n", u.v64);
+
+	u.v64 = (u64)NTB_READ_REG(mmio, DBSTAT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Bell -\t\t%#llx\n", u.v64);
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "\nNTB Incoming XLAT:\n");
+
+	u.v32 = NTB_READ_REG(mmio, BAR1XLAT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "XLAT1 -\t\t\t%#06x\n", u.v32);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR23XLAT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "XLAT23 -\t\t%#018llx\n", u.v64);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR45XLAT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "XLAT45 -\t\t%#018llx\n", u.v64);
+
+	u.v32 = NTB_READ_REG(mmio, BAR1LMT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "LMT1 -\t\t\t%#06x\n", u.v32);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR23LMT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "LMT23 -\t\t\t%#018llx\n", u.v64);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR45LMT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "LMT45 -\t\t\t%#018llx\n", u.v64);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, buf, off);
+	kfree(buf);
+	return ret;
+}
+
+static void ndev_init_debugfs(struct amd_ntb_dev *ndev)
+{
+	if (!debugfs_dir) {
+		ndev->debugfs_dir = NULL;
+		ndev->debugfs_info = NULL;
+	} else {
+		ndev->debugfs_dir =
+			debugfs_create_dir(ndev_name(ndev), debugfs_dir);
+		if (!ndev->debugfs_dir)
+			ndev->debugfs_info = NULL;
+		else
+			ndev->debugfs_info =
+				debugfs_create_file("info", S_IRUSR,
+						    ndev->debugfs_dir, ndev,
+						    &amd_ntb_debugfs_info);
+	}
+}
+
+static void ndev_deinit_debugfs(struct amd_ntb_dev *ndev)
+{
+	debugfs_remove_recursive(ndev->debugfs_dir);
+}
+
+static inline void ndev_init_struct(struct amd_ntb_dev *ndev,
+				    struct pci_dev *pdev)
+{
+	ndev->ntb.pdev = pdev;
+	ndev->ntb.topo = NTB_TOPO_NONE;
+	ndev->ntb.ops = &amd_ntb_ops;
+	ndev->int_mask = AMD_EVENT_INTMASK;
+	init_completion(&ndev->flush_cmpl);
+	init_completion(&ndev->wakeup_cmpl);
+	spin_lock_init(&ndev->db_mask_lock);
+}
+
+static int amd_poll_link(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->peer_mmio;
+	u32 reg, stat;
+	int rc;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	reg &= NTB_LIN_STA_ACTIVE_BIT;
+
+	dev_dbg(ndev_dev(ndev), "%s: reg_val = 0x%x.\n", __func__, reg);
+
+	if (reg == ndev->cntl_sta)
+		return 0;
+
+	ndev->cntl_sta = reg;
+
+	rc = pci_read_config_dword(ndev->ntb.pdev,
+			AMD_LINK_STATUS_OFFSET, &stat);
+	if (rc)
+		return 0;
+	ndev->lnk_sta = stat;
+
+	return 1;
+}
+
+static void amd_link_hb(struct work_struct *work)
+{
+	struct amd_ntb_dev *ndev = hb_ndev(work);
+
+	if (amd_poll_link(ndev))
+		ntb_link_event(&ndev->ntb);
+
+	if (!amd_link_is_up(ndev))
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+}
+
+static int amd_init_isr(struct amd_ntb_dev *ndev)
+{
+	return ndev_init_isr(ndev, AMD_DB_CNT, AMD_MSIX_VECTOR_CNT);
+}
+
+static void amd_init_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned int reg = 0;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	if (!(reg & 0x2)) {
+		reg |= 0x2;
+		NTB_WRITE_REG(mmio, reg, SIDEINFO);
+	}
+}
+
+static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned int reg = 0;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	if (reg & 0x2) {
+		reg &= ~(0x2);
+		NTB_WRITE_REG(mmio, reg, SIDEINFO);
+		NTB_READ_REG(mmio, SIDEINFO);
+	}
+}
+
+static int amd_init_ntb(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+
+	ndev->mw_count = AMD_MW_CNT;
+	ndev->spad_count = AMD_SPADS_CNT;
+	ndev->db_count = AMD_DB_CNT;
+
+	switch (ndev->ntb.topo) {
+	case NTB_TOPO_PRI:
+	case NTB_TOPO_SEC:
+		ndev->spad_count >>= 1;
+		if (ndev->ntb.topo == NTB_TOPO_PRI)
+			ndev->peer_spad = (4 << 8);
+		else
+			ndev->peer_spad = 0;
+
+		INIT_DELAYED_WORK(&ndev->hb_timer, amd_link_hb);
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+
+		break;
+	default:
+		dev_err(ndev_dev(ndev), "AMD NTB does not support B2B mode.\n");
+		return -EINVAL;
+	}
+
+	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
+
+	/* Mask event interrupts */
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	return 0;
+}
+
+static inline enum ntb_topo amd_get_topo(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 info;
+
+	info = NTB_READ_REG(mmio, SIDEINFO);
+	if (info & AMD_SIDE_MASK)
+		return NTB_TOPO_SEC;
+	else
+		return NTB_TOPO_PRI;
+}
+
+static int amd_init_dev(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev;
+	int rc = 0;
+
+	pdev = ndev_pdev(ndev);
+
+	ndev->ntb.topo = amd_get_topo(ndev);
+	dev_dbg(ndev_dev(ndev), "AMD NTB topo is %s\n",
+			ntb_topo_string(ndev->ntb.topo));
+
+	rc = amd_init_ntb(ndev);
+	if (rc)
+		return rc;
+
+	rc = amd_init_isr(ndev);
+	if (rc) {
+		dev_err(ndev_dev(ndev), "fail to init isr.\n");
+		return rc;
+	}
+
+	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
+
+	return 0;
+}
+
+static void amd_deinit_dev(struct amd_ntb_dev *ndev)
+{
+	cancel_delayed_work_sync(&ndev->hb_timer);
+
+	ndev_deinit_isr(ndev);
+}
+
+static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
+			    struct pci_dev *pdev)
+{
+	int rc;
+
+	pci_set_drvdata(pdev, ndev);
+
+	rc = pci_enable_device(pdev);
+	if (rc)
+		goto err_pci_enable;
+
+	rc = pci_request_regions(pdev, NTB_NAME);
+	if (rc)
+		goto err_pci_regions;
+
+	pci_set_master(pdev);
+
+	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+	if (rc) {
+		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+		if (rc)
+			goto err_dma_mask;
+		dev_warn(ndev_dev(ndev), "Cannot DMA highmem\n");
+	}
+
+	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+	if (rc) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+		if (rc)
+			goto err_dma_mask;
+		dev_warn(ndev_dev(ndev), "Cannot DMA consistent highmem\n");
+	}
+
+	ndev->self_mmio = pci_iomap(pdev, 0, 0);
+	if (!ndev->self_mmio) {
+		rc = -EIO;
+		goto err_mmio;
+	}
+	ndev->peer_mmio = ndev->self_mmio + AMD_PEER_OFFSET;
+
+	return 0;
+
+err_mmio:
+err_dma_mask:
+	pci_clear_master(pdev);
+err_pci_regions:
+	pci_disable_device(pdev);
+err_pci_enable:
+	pci_set_drvdata(pdev, NULL);
+	return rc;
+}
+
+static void amd_ntb_deinit_pci(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev = ndev_pdev(ndev);
+
+	pci_iounmap(pdev, ndev->self_mmio);
+
+	pci_clear_master(pdev);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+
+static int amd_ntb_pci_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *id)
+{
+	struct amd_ntb_dev *ndev;
+	int rc, node;
+
+	node = dev_to_node(&pdev->dev);
+
+	ndev = kzalloc_node(sizeof(*ndev), GFP_KERNEL, node);
+	if (!ndev) {
+		rc = -ENOMEM;
+		goto err_ndev;
+	}
+
+	ndev_init_struct(ndev, pdev);
+
+	rc = amd_ntb_init_pci(ndev, pdev);
+	if (rc)
+		goto err_init_pci;
+
+	rc = amd_init_dev(ndev);
+	if (rc)
+		goto err_init_dev;
+
+	/* write side info */
+	amd_init_side_info(ndev);
+
+	amd_poll_link(ndev);
+
+	ndev_init_debugfs(ndev);
+
+	rc = ntb_register_device(&ndev->ntb);
+	if (rc)
+		goto err_register;
+
+	dev_info(&pdev->dev, "NTB device registered.\n");
+
+	return 0;
+
+err_register:
+	ndev_deinit_debugfs(ndev);
+	amd_deinit_dev(ndev);
+err_init_dev:
+	amd_ntb_deinit_pci(ndev);
+err_init_pci:
+	kfree(ndev);
+err_ndev:
+	return rc;
+}
+
+static void amd_ntb_pci_remove(struct pci_dev *pdev)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+
+	ntb_unregister_device(&ndev->ntb);
+	ndev_deinit_debugfs(ndev);
+	amd_deinit_side_info(ndev);
+	amd_deinit_dev(ndev);
+	amd_ntb_deinit_pci(ndev);
+	kfree(ndev);
+}
+
+static const struct file_operations amd_ntb_debugfs_info = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = ndev_debugfs_read,
+};
+
+static const struct pci_device_id amd_ntb_pci_tbl[] = {
+	{PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_NTB)},
+	{0}
+};
+MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
+
+#ifdef CONFIG_PM
+static int amd_ntb_pci_device_suspend(struct pci_dev *pdev,
+				pm_message_t mesg)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+	void __iomem *mmio = ndev->self_mmio;
+
+	/* flush pending IO */
+	amd_flush_peer_requests(ndev);
+
+	/* link down */
+	ndev->cntl_sta = 0;
+	ntb_link_event(&ndev->ntb);
+
+	/* disable interrupt */
+	ndev->int_mask |= AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+	NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
+
+	/* save pcie state */
+	pci_save_state(pdev);
+	pci_disable_device(pdev);
+	if (mesg.event & PM_EVENT_SLEEP)
+		pci_set_power_state(pdev, PCI_D3hot);
+
+	return 0;
+}
+
+static int amd_ntb_pci_device_resume(struct pci_dev *pdev)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+	void __iomem *mmio = ndev->self_mmio;
+	int rc;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	rc = pci_enable_device(pdev);
+	if (rc) {
+		dev_err(&pdev->dev, "failed to resume ntb device.\n");
+		return rc;
+	}
+
+	pci_set_master(pdev);
+
+	ndev->int_mask &= ~AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	amd_poll_link(ndev);
+	ntb_link_event(&ndev->ntb);
+
+	return 0;
+}
+#endif
+
+static struct pci_driver amd_ntb_pci_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= amd_ntb_pci_tbl,
+	.probe		= amd_ntb_pci_probe,
+	.remove		= amd_ntb_pci_remove,
+#ifdef CONFIG_PM
+	.suspend	= amd_ntb_pci_device_suspend,
+	.resume		= amd_ntb_pci_device_resume,
+#endif
+};
+
+static int __init amd_ntb_pci_driver_init(void)
+{
+	pr_info("%s %s\n", NTB_DESC, NTB_VER);
+
+	if (debugfs_initialized())
+		debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+
+	return pci_register_driver(&amd_ntb_pci_driver);
+}
+module_init(amd_ntb_pci_driver_init);
+
+static void __exit amd_ntb_pci_driver_exit(void)
+{
+	pci_unregister_driver(&amd_ntb_pci_driver);
+	debugfs_remove_recursive(debugfs_dir);
+}
+module_exit(amd_ntb_pci_driver_exit);
diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h
new file mode 100644
index 0000000..6005040
--- /dev/null
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
@@ -0,0 +1,263 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ *
+ *   GPL LICENSE SUMMARY
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ *
+ *   BSD LICENSE
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copy
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of AMD Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * AMD PCIe NTB Linux driver
+ *
+ * Contact Information:
+ * Xiangliang Yu<Xiangliang.Yu@amd.com>
+ */
+
+#ifndef NTB_HW_AMD_H
+#define NTB_HW_AMD_H
+
+#include <linux/ntb.h>
+#include <linux/pci.h>
+
+#define	PCI_DEVICE_ID_AMD_NTB	0x145B
+#define AMD_LINK_HB_TIMEOUT	msecs_to_jiffies(1000)
+#define AMD_LINK_STATUS_OFFSET	0x68
+#define NTB_LIN_STA_ACTIVE_BIT	0x00000002
+#define NTB_LNK_STA_SPEED_MASK	0x000F0000
+#define NTB_LNK_STA_WIDTH_MASK	0x03F00000
+#define NTB_LNK_STA_ACTIVE(x)	(!!((x) & NTB_LIN_STA_ACTIVE_BIT))
+#define NTB_LNK_STA_SPEED(x)	(((x) & NTB_LNK_STA_SPEED_MASK) >> 16)
+#define NTB_LNK_STA_WIDTH(x)	(((x) & NTB_LNK_STA_WIDTH_MASK) >> 20)
+
+#ifndef ioread64
+#ifdef readq
+#define ioread64 readq
+#else
+#define ioread64 _ioread64
+static inline u64 _ioread64(void __iomem *mmio)
+{
+	u64 low, high;
+
+	low = ioread32(mmio);
+	high = ioread32(mmio + sizeof(u32));
+	return low | (high << 32);
+}
+#endif
+#endif
+
+#ifndef iowrite64
+#ifdef writeq
+#define iowrite64 writeq
+#else
+#define iowrite64 _iowrite64
+static inline void _iowrite64(u64 val, void __iomem *mmio)
+{
+	iowrite32(val, mmio);
+	iowrite32(val >> 32, mmio + sizeof(u32));
+}
+#endif
+#endif
+
+#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ## _OFFSET))
+#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +	\
+						AMD_ ## r ## _OFFSET))
+#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +		\
+						AMD_ ## r ## _OFFSET))
+#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +	\
+						of + AMD_ ## r ## _OFFSET))
+
+#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
+#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
+#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
+#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
+#define hb_ndev(work) container_of(work, struct amd_ntb_dev, hb_timer.work)
+#define ntb_hotplug_ndev(context) (container_of((context),	\
+			struct ntb_acpi_hotplug_context, hp)->ndev)
+
+enum {
+	/* AMD NTB Capability */
+	AMD_MW_CNT		= 3,
+	AMD_DB_CNT		= 16,
+	AMD_MSIX_VECTOR_CNT	= 24,
+	AMD_SPADS_CNT		= 16,
+
+	/*  AMD NTB register offset */
+	AMD_CNTL_OFFSET		= 0x200,
+
+	/* NTB control register bits */
+	PMM_REG_CTL		= BIT(21),
+	SMM_REG_CTL		= BIT(20),
+	SMM_REG_ACC_PATH	= BIT(18),
+	PMM_REG_ACC_PATH	= BIT(17),
+	NTB_CLK_EN		= BIT(16),
+
+	AMD_STA_OFFSET		= 0x204,
+	AMD_PGSLV_OFFSET	= 0x208,
+	AMD_SPAD_MUX_OFFSET	= 0x20C,
+	AMD_SPAD_OFFSET		= 0x210,
+	AMD_RSMU_HCID		= 0x250,
+	AMD_RSMU_SIID		= 0x254,
+	AMD_PSION_OFFSET	= 0x300,
+	AMD_SSION_OFFSET	= 0x330,
+	AMD_MMINDEX_OFFSET	= 0x400,
+	AMD_MMDATA_OFFSET	= 0x404,
+	AMD_SIDEINFO_OFFSET	= 0x408,
+
+	AMD_SIDE_MASK		= BIT(0),
+
+	/* limit register */
+	AMD_ROMBARLMT_OFFSET	= 0x410,
+	AMD_BAR1LMT_OFFSET	= 0x414,
+	AMD_BAR23LMT_OFFSET	= 0x418,
+	AMD_BAR45LMT_OFFSET	= 0x420,
+	/* xlat address */
+	AMD_POMBARXLAT_OFFSET	= 0x428,
+	AMD_BAR1XLAT_OFFSET	= 0x430,
+	AMD_BAR23XLAT_OFFSET	= 0x438,
+	AMD_BAR45XLAT_OFFSET	= 0x440,
+	/* doorbell and interrupt */
+	AMD_DBFM_OFFSET		= 0x450,
+	AMD_DBREQ_OFFSET	= 0x454,
+	AMD_MIRRDBSTAT_OFFSET	= 0x458,
+	AMD_DBMASK_OFFSET	= 0x45C,
+	AMD_DBSTAT_OFFSET	= 0x460,
+	AMD_INTMASK_OFFSET	= 0x470,
+	AMD_INTSTAT_OFFSET	= 0x474,
+
+	/* event type */
+	AMD_PEER_FLUSH_EVENT	= BIT(0),
+	AMD_PEER_RESET_EVENT	= BIT(1),
+	AMD_PEER_D3_EVENT	= BIT(2),
+	AMD_PEER_PMETO_EVENT	= BIT(3),
+	AMD_PEER_D0_EVENT	= BIT(4),
+	AMD_EVENT_INTMASK	= (AMD_PEER_FLUSH_EVENT |
+				AMD_PEER_RESET_EVENT | AMD_PEER_D3_EVENT |
+				AMD_PEER_PMETO_EVENT | AMD_PEER_D0_EVENT),
+
+	AMD_PMESTAT_OFFSET	= 0x480,
+	AMD_PMSGTRIG_OFFSET	= 0x490,
+	AMD_LTRLATENCY_OFFSET	= 0x494,
+	AMD_FLUSHTRIG_OFFSET	= 0x498,
+
+	/* SMU register*/
+	AMD_SMUACK_OFFSET	= 0x4A0,
+	AMD_SINRST_OFFSET	= 0x4A4,
+	AMD_RSPNUM_OFFSET	= 0x4A8,
+	AMD_SMU_SPADMUTEX	= 0x4B0,
+	AMD_SMU_SPADOFFSET	= 0x4B4,
+
+	AMD_PEER_OFFSET		= 0x400,
+};
+
+struct amd_ntb_dev;
+
+struct amd_ntb_vec {
+	struct amd_ntb_dev	*ndev;
+	int			num;
+};
+
+struct amd_ntb_dev {
+	struct ntb_dev ntb;
+
+	u32 ntb_side;
+	u32 lnk_sta;
+	u32 cntl_sta;
+	u32 peer_sta;
+
+	unsigned char mw_count;
+	unsigned char spad_count;
+	unsigned char db_count;
+	unsigned char msix_vec_count;
+
+	u64 db_valid_mask;
+	u64 db_mask;
+	u32 int_mask;
+
+	struct msix_entry *msix;
+	struct amd_ntb_vec *vec;
+
+	spinlock_t db_mask_lock;
+
+	void __iomem *self_mmio;
+	void __iomem *peer_mmio;
+	unsigned long peer_spad;
+
+	struct completion flush_cmpl;
+	struct completion wakeup_cmpl;
+
+	struct delayed_work hb_timer;
+
+	struct dentry *debugfs_dir;
+	struct dentry *debugfs_info;
+};
+
+struct ntb_acpi_hotplug_context {
+	struct acpi_hotplug_context hp;
+	struct amd_ntb_dev *ndev;
+};
+
+static int amd_ntb_mw_count(struct ntb_dev *ntb);
+static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
+			phys_addr_t *base, resource_size_t *size,
+			resource_size_t *align, resource_size_t *algin_size);
+static int amd_ntb_mw_set_trans(struct ntb_dev *ndev, int idx,
+				dma_addr_t addr, resource_size_t size);
+static int amd_ntb_link_is_up(struct ntb_dev *ntb,
+				enum ntb_speed *speed,
+				enum ntb_width *width);
+static int amd_ntb_link_enable(struct ntb_dev *ntb,
+				enum ntb_speed speed,
+				enum ntb_width width);
+static int amd_ntb_link_disable(struct ntb_dev *ntb);
+static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb);
+static int amd_ntb_db_vector_count(struct ntb_dev *ntb);
+static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector);
+static u64 amd_ntb_db_read(struct ntb_dev *ntb);
+static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
+				phys_addr_t *db_addr,
+				resource_size_t *db_size);
+static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_spad_count(struct ntb_dev *ntb);
+static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx);
+static int amd_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val);
+static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
+					phys_addr_t *spad_addr);
+static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx);
+static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int idx, u32 val);
+#endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2015-12-23 13:42 ` Xiangliang Yu
  0 siblings, 0 replies; 18+ messages in thread
From: Xiangliang Yu @ 2015-12-23 13:42 UTC (permalink / raw)
  To: jdmason, dave.jiang, Allen.Hubbe, linux-ntb, linux-kernel
  Cc: SPG_Linux_Kernel, Xiangliang Yu

AMD NTB support following main features:
(1) Three memory windows;
(2) Sixteen 32-bit scratch pad;
(3) Two 16-bit doorbell interrupt;
(4) Five event interrupts;
(5) One system can wake up opposite system of NTB;
(6) Flush previous request to the opposite system;
(7) There are reset and PME_TO mechanisms between two systems;

Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
---
 drivers/ntb/hw/amd/Kconfig      |    7 +
 drivers/ntb/hw/amd/Makefile     |    1 +
 drivers/ntb/hw/amd/ntb_hw_amd.c | 1229 +++++++++++++++++++++++++++++++++++++++
 drivers/ntb/hw/amd/ntb_hw_amd.h |  263 +++++++++
 4 files changed, 1500 insertions(+)
 create mode 100644 drivers/ntb/hw/amd/Kconfig
 create mode 100644 drivers/ntb/hw/amd/Makefile
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.c
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.h

diff --git a/drivers/ntb/hw/amd/Kconfig b/drivers/ntb/hw/amd/Kconfig
new file mode 100644
index 0000000..cfe903c
--- /dev/null
+++ b/drivers/ntb/hw/amd/Kconfig
@@ -0,0 +1,7 @@
+config NTB_AMD
+	tristate "AMD Non-Transparent Bridge support"
+	depends on X86_64
+	help
+	 This driver supports AMD NTB on capable Zeppelin hardware.
+
+	 If unsure, say N.
diff --git a/drivers/ntb/hw/amd/Makefile b/drivers/ntb/hw/amd/Makefile
new file mode 100644
index 0000000..ad54da9
--- /dev/null
+++ b/drivers/ntb/hw/amd/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_NTB_AMD) += ntb_hw_amd.o
diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
new file mode 100644
index 0000000..cedac8d
--- /dev/null
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
@@ -0,0 +1,1229 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ *
+ *   GPL LICENSE SUMMARY
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ *
+ *   BSD LICENSE
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copy
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of AMD Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * AMD PCIe NTB Linux driver
+ *
+ * Contact Information:
+ * Xiangliang Yu<Xiangliang.Yu@amd.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+#include <linux/ntb.h>
+
+#include "ntb_hw_amd.h"
+
+#define NTB_NAME	"ntb_hw_amd"
+#define NTB_DESC	"AMD(R) PCI-E Non-Transparent Bridge Driver"
+#define NTB_VER		"1.0"
+
+MODULE_DESCRIPTION(NTB_DESC);
+MODULE_VERSION(NTB_VER);
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("AMD Inc.");
+
+static const struct file_operations amd_ntb_debugfs_info;
+static struct dentry *debugfs_dir;
+
+static const struct ntb_dev_ops amd_ntb_ops = {
+	.mw_count		= amd_ntb_mw_count,
+	.mw_get_range		= amd_ntb_mw_get_range,
+	.mw_set_trans		= amd_ntb_mw_set_trans,
+	.link_is_up		= amd_ntb_link_is_up,
+	.link_enable		= amd_ntb_link_enable,
+	.link_disable		= amd_ntb_link_disable,
+	.db_valid_mask		= amd_ntb_db_valid_mask,
+	.db_vector_count	= amd_ntb_db_vector_count,
+	.db_vector_mask		= amd_ntb_db_vector_mask,
+	.db_read		= amd_ntb_db_read,
+	.db_clear		= amd_ntb_db_clear,
+	.db_set_mask		= amd_ntb_db_set_mask,
+	.db_clear_mask		= amd_ntb_db_clear_mask,
+	.peer_db_addr		= amd_ntb_peer_db_addr,
+	.peer_db_set		= amd_ntb_peer_db_set,
+	.spad_count		= amd_ntb_spad_count,
+	.spad_read		= amd_ntb_spad_read,
+	.spad_write		= amd_ntb_spad_write,
+	.peer_spad_addr		= amd_ntb_peer_spad_addr,
+	.peer_spad_read		= amd_ntb_peer_spad_read,
+	.peer_spad_write	= amd_ntb_peer_spad_write,
+};
+
+static int ndev_mw_to_bar(struct amd_ntb_dev *ndev, int idx)
+{
+	if (idx < 0 || idx > ndev->mw_count)
+		return -EINVAL;
+
+	return 1 << idx;
+}
+
+static int amd_ntb_mw_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->mw_count;
+}
+
+static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
+				phys_addr_t *base,
+				resource_size_t *size,
+				resource_size_t *align,
+				resource_size_t *align_size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	int bar;
+
+	bar = ndev_mw_to_bar(ndev, idx);
+	if (bar < 0)
+		return bar;
+
+	if (base)
+		*base = pci_resource_start(ndev->ntb.pdev, bar);
+
+	if (size)
+		*size = pci_resource_len(ndev->ntb.pdev, bar);
+
+	if (align)
+		*align = 4096;
+
+	if (align_size)
+		*align_size = 1;
+
+	return 0;
+}
+
+static int amd_ntb_mw_set_trans(struct ntb_dev *ntb, int idx,
+				dma_addr_t addr, resource_size_t size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	unsigned long xlat_reg, limit_reg = 0;
+	resource_size_t mw_size;
+	void __iomem *mmio, *peer_mmio;
+	u64 base_addr, limit, reg_val;
+	int bar;
+
+	bar = ndev_mw_to_bar(ndev, idx);
+	if (bar < 0)
+		return bar;
+
+	mw_size = pci_resource_len(ndev->ntb.pdev, bar);
+
+	/* make sure the range fits in the usable mw size */
+	if (size > mw_size)
+		return -EINVAL;
+
+	mmio = ndev->self_mmio;
+	peer_mmio = ndev->peer_mmio;
+
+	base_addr = pci_resource_start(ndev->ntb.pdev, bar);
+	if (bar == 1) {
+		xlat_reg = AMD_BAR1XLAT_OFFSET;
+		limit_reg = AMD_BAR1LMT_OFFSET;
+	} else {
+		xlat_reg = AMD_BAR23XLAT_OFFSET + ((bar - 2) << 3);
+		limit_reg = AMD_BAR23LMT_OFFSET + ((bar - 2) << 3);
+	}
+
+	if (bar != 1) {
+		/* Set the limit if supported */
+		limit = base_addr + size;
+
+		/* set and verify setting the translation address */
+		iowrite64(addr, peer_mmio + xlat_reg);
+		reg_val = ioread64(peer_mmio + xlat_reg);
+		if (reg_val != addr) {
+			iowrite64(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+
+		/* set and verify setting the limit */
+		iowrite64(limit, mmio + limit_reg);
+		reg_val = ioread64(mmio + limit_reg);
+		if (reg_val != limit) {
+			iowrite64(base_addr, mmio + limit_reg);
+			iowrite64(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+	} else {
+		/* split bar addr range must all be 32 bit */
+		if (addr & (~0ull << 32))
+			return -EINVAL;
+		if ((addr + size) & (~0ull << 32))
+			return -EINVAL;
+
+		/* Set the limit if supported */
+		limit = base_addr + size;
+
+		/* set and verify setting the translation address */
+		iowrite32(0, peer_mmio + xlat_reg + 0x4);
+		iowrite32(addr, peer_mmio + xlat_reg);
+		reg_val = ioread32(peer_mmio + xlat_reg);
+		if (reg_val != addr) {
+			iowrite32(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+
+		/* set and verify setting the limit */
+		iowrite32(limit, mmio + limit_reg);
+		reg_val = ioread32(mmio + limit_reg);
+		if (reg_val != limit) {
+			iowrite32(base_addr, mmio + limit_reg);
+			iowrite32(0, peer_mmio + xlat_reg);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+static int amd_link_is_up(struct amd_ntb_dev *ndev)
+{
+	/* set link down and clear flag */
+	if (ndev->peer_sta & AMD_PEER_RESET_EVENT) {
+		ndev->peer_sta &= ~AMD_PEER_RESET_EVENT;
+		return 0;
+	} else if (ndev->peer_sta & (AMD_PEER_D3_EVENT |
+				     AMD_PEER_PMETO_EVENT)) {
+		return 0;
+	} else if (ndev->peer_sta & AMD_PEER_D0_EVENT) {
+		ndev->peer_sta = 0;
+		return 0;
+	} else
+		return NTB_LNK_STA_ACTIVE(ndev->cntl_sta);
+}
+
+static int amd_ntb_link_is_up(struct ntb_dev *ntb,
+			      enum ntb_speed *speed,
+			      enum ntb_width *width)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	int ret = 0;
+
+	if (amd_link_is_up(ndev)) {
+		if (speed)
+			*speed = NTB_LNK_STA_SPEED(ndev->lnk_sta);
+		if (width)
+			*width = NTB_LNK_STA_WIDTH(ndev->lnk_sta);
+
+		dev_dbg(ndev_dev(ndev), "link is up.\n");
+
+		ret = 1;
+	} else {
+		if (speed)
+			*speed = NTB_SPEED_NONE;
+		if (width)
+			*width = NTB_WIDTH_NONE;
+
+		dev_dbg(ndev_dev(ndev), "link is down.\n");
+	}
+
+	return ret;
+}
+
+static int amd_ntb_link_enable(struct ntb_dev *ntb,
+			       enum ntb_speed max_speed,
+			       enum ntb_width max_width)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	/* Enable event interrupt */
+	ndev->int_mask &= ~AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	if (ndev->ntb.topo == NTB_TOPO_SEC)
+		return -EINVAL;
+	dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
+
+	ntb_ctl = NTB_READ_REG(mmio, CNTL);
+	ntb_ctl |= (3 << 20);
+	NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
+
+	return 0;
+}
+
+static int amd_ntb_link_disable(struct ntb_dev *ntb)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	/* Disable event interrupt */
+	ndev->int_mask |= AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	if (ndev->ntb.topo == NTB_TOPO_SEC)
+		return -EINVAL;
+	dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
+
+	ntb_ctl = NTB_READ_REG(mmio, CNTL);
+	ntb_ctl &= ~(3 << 16);
+	NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
+
+	return 0;
+}
+
+static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->db_valid_mask;
+}
+
+static int amd_ntb_db_vector_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->db_count;
+}
+
+static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (db_vector < 0 || db_vector > ndev->db_count)
+		return 0;
+
+	return ntb_ndev(ntb)->db_valid_mask & (1 << db_vector);
+}
+
+static u64 amd_ntb_db_read(struct ntb_dev *ntb)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	return (u64)NTB_READ_REG(mmio, DBSTAT);
+}
+
+static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	NTB_WRITE_REG(mmio, (u32)db_bits, DBSTAT);
+
+	return 0;
+}
+
+static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned long flags;
+
+	if (db_bits & ~ndev->db_valid_mask)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ndev->db_mask_lock, flags);
+	ndev->db_mask |= db_bits;
+	NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
+	spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
+
+	return 0;
+}
+
+static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned long flags;
+
+	if (db_bits & ~ndev->db_valid_mask)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ndev->db_mask_lock, flags);
+	ndev->db_mask &= ~db_bits;
+	NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
+	spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
+
+	return 0;
+}
+
+static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
+		phys_addr_t *db_addr,
+		resource_size_t *db_size)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (db_addr)
+		*db_addr = (phys_addr_t)(ndev->peer_mmio + AMD_DBREQ_OFFSET);
+	if (db_size)
+		*db_size = sizeof(u32);
+
+	return 0;
+}
+
+static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	NTB_WRITE_REG(mmio, (u32)db_bits, DBREQ);
+
+	return 0;
+}
+
+static int amd_ntb_spad_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->spad_count;
+}
+
+static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return 0;
+
+	return NTB_READ_OFFSET(mmio, SPAD, (idx << 2));
+}
+
+static int amd_ntb_spad_write(struct ntb_dev *ntb,
+				int idx, u32 val)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	NTB_WRITE_OFFSET(mmio, val, SPAD, (idx << 2));
+
+	return 0;
+}
+
+static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
+				    phys_addr_t *spad_addr)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	if (spad_addr)
+		*spad_addr = (phys_addr_t)(ndev->self_mmio + AMD_SPAD_OFFSET +
+				ndev->peer_spad + (idx << 2));
+	return 0;
+}
+
+static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 offset;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	offset = ndev->peer_spad + (idx << 2);
+	return NTB_READ_OFFSET(mmio, SPAD, offset);
+}
+
+static int amd_ntb_peer_spad_write(struct ntb_dev *ntb,
+				     int idx, u32 val)
+{
+	struct amd_ntb_dev *ndev = ntb_ndev(ntb);
+	void __iomem *mmio = ndev->self_mmio;
+	u32 offset;
+
+	if (idx < 0 || idx >= ndev->spad_count)
+		return -EINVAL;
+
+	offset = ndev->peer_spad + (idx << 2);
+	NTB_WRITE_OFFSET(mmio, val, SPAD, offset);
+
+	return 0;
+}
+
+static void amd_ack_SMU(struct amd_ntb_dev *ndev, u32 bit)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	int reg;
+
+	reg = NTB_READ_REG(mmio, SMUACK);
+	reg |= bit;
+	NTB_WRITE_REG(mmio, reg, SMUACK);
+
+	ndev->peer_sta |= bit;
+}
+
+#ifdef CONFIG_PM
+/*
+ * flush the requests to peer side
+ */
+static int amd_flush_peer_requests(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 reg;
+
+	if (!amd_link_is_up(ndev)) {
+		dev_err(ndev_dev(ndev), "link is down.\n");
+		return -EINVAL;
+	}
+
+	reg = NTB_READ_REG(mmio, FLUSHTRIG);
+	reg |= 0x1;
+	NTB_WRITE_REG(mmio, reg, FLUSHTRIG);
+
+	wait_for_completion(&ndev->flush_cmpl);
+
+	reinit_completion(&ndev->flush_cmpl);
+
+	return 0;
+}
+#endif
+
+static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 status;
+
+	status = NTB_READ_REG(mmio, INTSTAT);
+	if (!(status & AMD_EVENT_INTMASK))
+		return;
+
+	dev_dbg(ndev_dev(ndev), "status = 0x%x and vec = %d\n", status, vec);
+
+	status &= AMD_EVENT_INTMASK;
+	switch (status) {
+	case AMD_PEER_FLUSH_EVENT:
+		complete(&ndev->flush_cmpl);
+		break;
+	case AMD_PEER_RESET_EVENT:
+		amd_ack_SMU(ndev, AMD_PEER_RESET_EVENT);
+
+		/* link down first */
+		ntb_link_event(&ndev->ntb);
+		/* polling peer status */
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+
+		break;
+	case AMD_PEER_D3_EVENT:
+	case AMD_PEER_PMETO_EVENT:
+		amd_ack_SMU(ndev, status);
+
+		/* link down */
+		ntb_link_event(&ndev->ntb);
+
+		break;
+	case AMD_PEER_D0_EVENT:
+		mmio = ndev->peer_mmio;
+		status = NTB_READ_REG(mmio, PMESTAT);
+		/* check if this is WAKEUP event */
+		if (status & 0x1)
+			complete(&ndev->wakeup_cmpl);
+
+		amd_ack_SMU(ndev, AMD_PEER_D0_EVENT);
+
+		if (amd_link_is_up(ndev))
+			ntb_link_event(&ndev->ntb);
+		else
+			schedule_delayed_work(&ndev->hb_timer,
+						AMD_LINK_HB_TIMEOUT);
+		break;
+	default:
+		dev_info(ndev_dev(ndev), "event status = 0x%x.\n", status);
+		break;
+	}
+}
+
+static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
+{
+	dev_dbg(ndev_dev(ndev), "vec %d\n", vec);
+
+	if (vec > (ndev->msix_vec_count - 1)) {
+		dev_err(ndev_dev(ndev), "Invalid interrupt.\n");
+		return IRQ_HANDLED;
+	}
+
+	if (vec > 15 || (ndev->msix_vec_count == 1))
+		amd_handle_event(ndev, vec);
+
+	if (vec < 16)
+		ntb_db_event(&ndev->ntb, vec);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ndev_vec_isr(int irq, void *dev)
+{
+	struct amd_ntb_vec *nvec = dev;
+
+	return ndev_interrupt(nvec->ndev, nvec->num);
+}
+
+static irqreturn_t ndev_irq_isr(int irq, void *dev)
+{
+	struct amd_ntb_dev *ndev = dev;
+
+	return ndev_interrupt(ndev, irq - ndev_pdev(ndev)->irq);
+}
+
+static int ndev_init_isr(struct amd_ntb_dev *ndev,
+				int msix_min, int msix_max)
+{
+	struct pci_dev *pdev;
+	int rc, i, msix_count, node;
+
+	pdev = ndev_pdev(ndev);
+
+	node = dev_to_node(&pdev->dev);
+
+	ndev->db_mask = ndev->db_valid_mask;
+
+	/* Try to set up msix irq */
+	ndev->vec = kzalloc_node(msix_max * sizeof(*ndev->vec),
+				 GFP_KERNEL, node);
+	if (!ndev->vec)
+		goto err_msix_vec_alloc;
+
+	ndev->msix = kzalloc_node(msix_max * sizeof(*ndev->msix),
+				  GFP_KERNEL, node);
+	if (!ndev->msix)
+		goto err_msix_alloc;
+
+	for (i = 0; i < msix_max; ++i)
+		ndev->msix[i].entry = i;
+
+	msix_count = pci_enable_msix_range(pdev, ndev->msix,
+					   msix_min, msix_max);
+	if (msix_count < 0)
+		goto err_msix_enable;
+
+	/* Disable MSIX if msix count is less than 16 */
+	if (msix_count < msix_min) {
+		pci_disable_msix(pdev);
+		goto err_msix_enable;
+	}
+
+	for (i = 0; i < msix_count; ++i) {
+		ndev->vec[i].ndev = ndev;
+		ndev->vec[i].num = i;
+		rc = request_irq(ndev->msix[i].vector, ndev_vec_isr, 0,
+				 "ndev_vec_isr", &ndev->vec[i]);
+		if (rc)
+			goto err_msix_request;
+	}
+
+	dev_dbg(ndev_dev(ndev), "Using msix interrupts\n");
+	ndev->db_count = msix_min;
+	ndev->msix_vec_count = msix_max;
+	return 0;
+
+err_msix_request:
+	while (i-- > 0)
+		free_irq(ndev->msix[i].vector, ndev);
+	pci_disable_msix(pdev);
+err_msix_enable:
+	kfree(ndev->msix);
+err_msix_alloc:
+	kfree(ndev->vec);
+err_msix_vec_alloc:
+	ndev->msix = NULL;
+	ndev->vec = NULL;
+
+	/* Try to set up msi irq */
+	rc = pci_enable_msi(pdev);
+	if (rc)
+		goto err_msi_enable;
+
+	rc = request_irq(pdev->irq, ndev_irq_isr, 0,
+			 "ndev_irq_isr", ndev);
+	if (rc)
+		goto err_msi_request;
+
+	dev_dbg(ndev_dev(ndev), "Using msi interrupts\n");
+	ndev->db_count = 1;
+	ndev->msix_vec_count = 1;
+	return 0;
+
+err_msi_request:
+	pci_disable_msi(pdev);
+err_msi_enable:
+
+	/* Try to set up intx irq */
+	pci_intx(pdev, 1);
+
+	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
+			 "ndev_irq_isr", ndev);
+	if (rc)
+		goto err_intx_request;
+
+	dev_dbg(ndev_dev(ndev), "Using intx interrupts\n");
+	ndev->db_count = 1;
+	ndev->msix_vec_count = 1;
+	return 0;
+
+err_intx_request:
+	return rc;
+}
+
+static void ndev_deinit_isr(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev;
+	void __iomem *mmio = ndev->self_mmio;
+	int i;
+
+	pdev = ndev_pdev(ndev);
+
+	/* Mask all doorbell interrupts */
+	ndev->db_mask = ndev->db_valid_mask;
+	NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
+
+	if (ndev->msix) {
+		i = ndev->msix_vec_count;
+		while (i--)
+			free_irq(ndev->msix[i].vector, &ndev->vec[i]);
+		pci_disable_msix(pdev);
+		kfree(ndev->msix);
+		kfree(ndev->vec);
+	} else {
+		free_irq(pdev->irq, ndev);
+		if (pci_dev_msi_enabled(pdev))
+			pci_disable_msi(pdev);
+		else
+			pci_intx(pdev, 0);
+	}
+}
+
+static ssize_t ndev_debugfs_read(struct file *filp, char __user *ubuf,
+				 size_t count, loff_t *offp)
+{
+	struct amd_ntb_dev *ndev;
+	void __iomem *mmio;
+	char *buf;
+	size_t buf_size;
+	ssize_t ret, off;
+	union { u64 v64; u32 v32; u16 v16; } u;
+
+	ndev = filp->private_data;
+	mmio = ndev->self_mmio;
+
+	buf_size = min(count, 0x800ul);
+
+	buf = kmalloc(buf_size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	off = 0;
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "NTB Device Information:\n");
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Connection Topology -\t%s\n",
+			 ntb_topo_string(ndev->ntb.topo));
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "LNK STA -\t\t%#06x\n", ndev->lnk_sta);
+
+	if (!amd_link_is_up(ndev)) {
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Status -\t\tDown\n");
+	} else {
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Status -\t\tUp\n");
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Speed -\t\tPCI-E Gen %u\n",
+				 NTB_LNK_STA_SPEED(ndev->lnk_sta));
+		off += scnprintf(buf + off, buf_size - off,
+				 "Link Width -\t\tx%u\n",
+				 NTB_LNK_STA_WIDTH(ndev->lnk_sta));
+	}
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Memory Window Count -\t%u\n", ndev->mw_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Scratchpad Count -\t%u\n", ndev->spad_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Count -\t%u\n", ndev->db_count);
+	off += scnprintf(buf + off, buf_size - off,
+			 "MSIX Vector Count -\t%u\n", ndev->msix_vec_count);
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Valid Mask -\t%#llx\n", ndev->db_valid_mask);
+
+	u.v64 = (u64)ioread32(ndev->self_mmio + AMD_DBMASK_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Mask -\t\t%#llx\n", u.v64);
+
+	u.v64 = (u64)NTB_READ_REG(mmio, DBSTAT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "Doorbell Bell -\t\t%#llx\n", u.v64);
+
+	off += scnprintf(buf + off, buf_size - off,
+			 "\nNTB Incoming XLAT:\n");
+
+	u.v32 = NTB_READ_REG(mmio, BAR1XLAT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "XLAT1 -\t\t\t%#06x\n", u.v32);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR23XLAT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "XLAT23 -\t\t%#018llx\n", u.v64);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR45XLAT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "XLAT45 -\t\t%#018llx\n", u.v64);
+
+	u.v32 = NTB_READ_REG(mmio, BAR1LMT);
+	off += scnprintf(buf + off, buf_size - off,
+			 "LMT1 -\t\t\t%#06x\n", u.v32);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR23LMT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "LMT23 -\t\t\t%#018llx\n", u.v64);
+
+	u.v64 = ioread64(ndev->self_mmio + AMD_BAR45LMT_OFFSET);
+	off += scnprintf(buf + off, buf_size - off,
+				 "LMT45 -\t\t\t%#018llx\n", u.v64);
+
+	ret = simple_read_from_buffer(ubuf, count, offp, buf, off);
+	kfree(buf);
+	return ret;
+}
+
+static void ndev_init_debugfs(struct amd_ntb_dev *ndev)
+{
+	if (!debugfs_dir) {
+		ndev->debugfs_dir = NULL;
+		ndev->debugfs_info = NULL;
+	} else {
+		ndev->debugfs_dir =
+			debugfs_create_dir(ndev_name(ndev), debugfs_dir);
+		if (!ndev->debugfs_dir)
+			ndev->debugfs_info = NULL;
+		else
+			ndev->debugfs_info =
+				debugfs_create_file("info", S_IRUSR,
+						    ndev->debugfs_dir, ndev,
+						    &amd_ntb_debugfs_info);
+	}
+}
+
+static void ndev_deinit_debugfs(struct amd_ntb_dev *ndev)
+{
+	debugfs_remove_recursive(ndev->debugfs_dir);
+}
+
+static inline void ndev_init_struct(struct amd_ntb_dev *ndev,
+				    struct pci_dev *pdev)
+{
+	ndev->ntb.pdev = pdev;
+	ndev->ntb.topo = NTB_TOPO_NONE;
+	ndev->ntb.ops = &amd_ntb_ops;
+	ndev->int_mask = AMD_EVENT_INTMASK;
+	init_completion(&ndev->flush_cmpl);
+	init_completion(&ndev->wakeup_cmpl);
+	spin_lock_init(&ndev->db_mask_lock);
+}
+
+static int amd_poll_link(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->peer_mmio;
+	u32 reg, stat;
+	int rc;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	reg &= NTB_LIN_STA_ACTIVE_BIT;
+
+	dev_dbg(ndev_dev(ndev), "%s: reg_val = 0x%x.\n", __func__, reg);
+
+	if (reg == ndev->cntl_sta)
+		return 0;
+
+	ndev->cntl_sta = reg;
+
+	rc = pci_read_config_dword(ndev->ntb.pdev,
+			AMD_LINK_STATUS_OFFSET, &stat);
+	if (rc)
+		return 0;
+	ndev->lnk_sta = stat;
+
+	return 1;
+}
+
+static void amd_link_hb(struct work_struct *work)
+{
+	struct amd_ntb_dev *ndev = hb_ndev(work);
+
+	if (amd_poll_link(ndev))
+		ntb_link_event(&ndev->ntb);
+
+	if (!amd_link_is_up(ndev))
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+}
+
+static int amd_init_isr(struct amd_ntb_dev *ndev)
+{
+	return ndev_init_isr(ndev, AMD_DB_CNT, AMD_MSIX_VECTOR_CNT);
+}
+
+static void amd_init_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned int reg = 0;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	if (!(reg & 0x2)) {
+		reg |= 0x2;
+		NTB_WRITE_REG(mmio, reg, SIDEINFO);
+	}
+}
+
+static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	unsigned int reg = 0;
+
+	reg = NTB_READ_REG(mmio, SIDEINFO);
+	if (reg & 0x2) {
+		reg &= ~(0x2);
+		NTB_WRITE_REG(mmio, reg, SIDEINFO);
+		NTB_READ_REG(mmio, SIDEINFO);
+	}
+}
+
+static int amd_init_ntb(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+
+	ndev->mw_count = AMD_MW_CNT;
+	ndev->spad_count = AMD_SPADS_CNT;
+	ndev->db_count = AMD_DB_CNT;
+
+	switch (ndev->ntb.topo) {
+	case NTB_TOPO_PRI:
+	case NTB_TOPO_SEC:
+		ndev->spad_count >>= 1;
+		if (ndev->ntb.topo == NTB_TOPO_PRI)
+			ndev->peer_spad = (4 << 8);
+		else
+			ndev->peer_spad = 0;
+
+		INIT_DELAYED_WORK(&ndev->hb_timer, amd_link_hb);
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+
+		break;
+	default:
+		dev_err(ndev_dev(ndev), "AMD NTB does not support B2B mode.\n");
+		return -EINVAL;
+	}
+
+	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
+
+	/* Mask event interrupts */
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	return 0;
+}
+
+static inline enum ntb_topo amd_get_topo(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 info;
+
+	info = NTB_READ_REG(mmio, SIDEINFO);
+	if (info & AMD_SIDE_MASK)
+		return NTB_TOPO_SEC;
+	else
+		return NTB_TOPO_PRI;
+}
+
+static int amd_init_dev(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev;
+	int rc = 0;
+
+	pdev = ndev_pdev(ndev);
+
+	ndev->ntb.topo = amd_get_topo(ndev);
+	dev_dbg(ndev_dev(ndev), "AMD NTB topo is %s\n",
+			ntb_topo_string(ndev->ntb.topo));
+
+	rc = amd_init_ntb(ndev);
+	if (rc)
+		return rc;
+
+	rc = amd_init_isr(ndev);
+	if (rc) {
+		dev_err(ndev_dev(ndev), "fail to init isr.\n");
+		return rc;
+	}
+
+	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
+
+	return 0;
+}
+
+static void amd_deinit_dev(struct amd_ntb_dev *ndev)
+{
+	cancel_delayed_work_sync(&ndev->hb_timer);
+
+	ndev_deinit_isr(ndev);
+}
+
+static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
+			    struct pci_dev *pdev)
+{
+	int rc;
+
+	pci_set_drvdata(pdev, ndev);
+
+	rc = pci_enable_device(pdev);
+	if (rc)
+		goto err_pci_enable;
+
+	rc = pci_request_regions(pdev, NTB_NAME);
+	if (rc)
+		goto err_pci_regions;
+
+	pci_set_master(pdev);
+
+	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+	if (rc) {
+		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+		if (rc)
+			goto err_dma_mask;
+		dev_warn(ndev_dev(ndev), "Cannot DMA highmem\n");
+	}
+
+	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+	if (rc) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+		if (rc)
+			goto err_dma_mask;
+		dev_warn(ndev_dev(ndev), "Cannot DMA consistent highmem\n");
+	}
+
+	ndev->self_mmio = pci_iomap(pdev, 0, 0);
+	if (!ndev->self_mmio) {
+		rc = -EIO;
+		goto err_mmio;
+	}
+	ndev->peer_mmio = ndev->self_mmio + AMD_PEER_OFFSET;
+
+	return 0;
+
+err_mmio:
+err_dma_mask:
+	pci_clear_master(pdev);
+err_pci_regions:
+	pci_disable_device(pdev);
+err_pci_enable:
+	pci_set_drvdata(pdev, NULL);
+	return rc;
+}
+
+static void amd_ntb_deinit_pci(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev = ndev_pdev(ndev);
+
+	pci_iounmap(pdev, ndev->self_mmio);
+
+	pci_clear_master(pdev);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+
+static int amd_ntb_pci_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *id)
+{
+	struct amd_ntb_dev *ndev;
+	int rc, node;
+
+	node = dev_to_node(&pdev->dev);
+
+	ndev = kzalloc_node(sizeof(*ndev), GFP_KERNEL, node);
+	if (!ndev) {
+		rc = -ENOMEM;
+		goto err_ndev;
+	}
+
+	ndev_init_struct(ndev, pdev);
+
+	rc = amd_ntb_init_pci(ndev, pdev);
+	if (rc)
+		goto err_init_pci;
+
+	rc = amd_init_dev(ndev);
+	if (rc)
+		goto err_init_dev;
+
+	/* write side info */
+	amd_init_side_info(ndev);
+
+	amd_poll_link(ndev);
+
+	ndev_init_debugfs(ndev);
+
+	rc = ntb_register_device(&ndev->ntb);
+	if (rc)
+		goto err_register;
+
+	dev_info(&pdev->dev, "NTB device registered.\n");
+
+	return 0;
+
+err_register:
+	ndev_deinit_debugfs(ndev);
+	amd_deinit_dev(ndev);
+err_init_dev:
+	amd_ntb_deinit_pci(ndev);
+err_init_pci:
+	kfree(ndev);
+err_ndev:
+	return rc;
+}
+
+static void amd_ntb_pci_remove(struct pci_dev *pdev)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+
+	ntb_unregister_device(&ndev->ntb);
+	ndev_deinit_debugfs(ndev);
+	amd_deinit_side_info(ndev);
+	amd_deinit_dev(ndev);
+	amd_ntb_deinit_pci(ndev);
+	kfree(ndev);
+}
+
+static const struct file_operations amd_ntb_debugfs_info = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = ndev_debugfs_read,
+};
+
+static const struct pci_device_id amd_ntb_pci_tbl[] = {
+	{PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_NTB)},
+	{0}
+};
+MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
+
+#ifdef CONFIG_PM
+static int amd_ntb_pci_device_suspend(struct pci_dev *pdev,
+				pm_message_t mesg)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+	void __iomem *mmio = ndev->self_mmio;
+
+	/* flush pending IO */
+	amd_flush_peer_requests(ndev);
+
+	/* link down */
+	ndev->cntl_sta = 0;
+	ntb_link_event(&ndev->ntb);
+
+	/* disable interrupt */
+	ndev->int_mask |= AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+	NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
+
+	/* save pcie state */
+	pci_save_state(pdev);
+	pci_disable_device(pdev);
+	if (mesg.event & PM_EVENT_SLEEP)
+		pci_set_power_state(pdev, PCI_D3hot);
+
+	return 0;
+}
+
+static int amd_ntb_pci_device_resume(struct pci_dev *pdev)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+	void __iomem *mmio = ndev->self_mmio;
+	int rc;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	rc = pci_enable_device(pdev);
+	if (rc) {
+		dev_err(&pdev->dev, "failed to resume ntb device.\n");
+		return rc;
+	}
+
+	pci_set_master(pdev);
+
+	ndev->int_mask &= ~AMD_EVENT_INTMASK;
+	NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
+
+	amd_poll_link(ndev);
+	ntb_link_event(&ndev->ntb);
+
+	return 0;
+}
+#endif
+
+static struct pci_driver amd_ntb_pci_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= amd_ntb_pci_tbl,
+	.probe		= amd_ntb_pci_probe,
+	.remove		= amd_ntb_pci_remove,
+#ifdef CONFIG_PM
+	.suspend	= amd_ntb_pci_device_suspend,
+	.resume		= amd_ntb_pci_device_resume,
+#endif
+};
+
+static int __init amd_ntb_pci_driver_init(void)
+{
+	pr_info("%s %s\n", NTB_DESC, NTB_VER);
+
+	if (debugfs_initialized())
+		debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+
+	return pci_register_driver(&amd_ntb_pci_driver);
+}
+module_init(amd_ntb_pci_driver_init);
+
+static void __exit amd_ntb_pci_driver_exit(void)
+{
+	pci_unregister_driver(&amd_ntb_pci_driver);
+	debugfs_remove_recursive(debugfs_dir);
+}
+module_exit(amd_ntb_pci_driver_exit);
diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h
new file mode 100644
index 0000000..6005040
--- /dev/null
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
@@ -0,0 +1,263 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ *
+ *   GPL LICENSE SUMMARY
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ *
+ *   BSD LICENSE
+ *
+ *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copy
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of AMD Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * AMD PCIe NTB Linux driver
+ *
+ * Contact Information:
+ * Xiangliang Yu<Xiangliang.Yu@amd.com>
+ */
+
+#ifndef NTB_HW_AMD_H
+#define NTB_HW_AMD_H
+
+#include <linux/ntb.h>
+#include <linux/pci.h>
+
+#define	PCI_DEVICE_ID_AMD_NTB	0x145B
+#define AMD_LINK_HB_TIMEOUT	msecs_to_jiffies(1000)
+#define AMD_LINK_STATUS_OFFSET	0x68
+#define NTB_LIN_STA_ACTIVE_BIT	0x00000002
+#define NTB_LNK_STA_SPEED_MASK	0x000F0000
+#define NTB_LNK_STA_WIDTH_MASK	0x03F00000
+#define NTB_LNK_STA_ACTIVE(x)	(!!((x) & NTB_LIN_STA_ACTIVE_BIT))
+#define NTB_LNK_STA_SPEED(x)	(((x) & NTB_LNK_STA_SPEED_MASK) >> 16)
+#define NTB_LNK_STA_WIDTH(x)	(((x) & NTB_LNK_STA_WIDTH_MASK) >> 20)
+
+#ifndef ioread64
+#ifdef readq
+#define ioread64 readq
+#else
+#define ioread64 _ioread64
+static inline u64 _ioread64(void __iomem *mmio)
+{
+	u64 low, high;
+
+	low = ioread32(mmio);
+	high = ioread32(mmio + sizeof(u32));
+	return low | (high << 32);
+}
+#endif
+#endif
+
+#ifndef iowrite64
+#ifdef writeq
+#define iowrite64 writeq
+#else
+#define iowrite64 _iowrite64
+static inline void _iowrite64(u64 val, void __iomem *mmio)
+{
+	iowrite32(val, mmio);
+	iowrite32(val >> 32, mmio + sizeof(u32));
+}
+#endif
+#endif
+
+#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ## _OFFSET))
+#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +	\
+						AMD_ ## r ## _OFFSET))
+#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +		\
+						AMD_ ## r ## _OFFSET))
+#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +	\
+						of + AMD_ ## r ## _OFFSET))
+
+#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
+#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
+#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
+#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
+#define hb_ndev(work) container_of(work, struct amd_ntb_dev, hb_timer.work)
+#define ntb_hotplug_ndev(context) (container_of((context),	\
+			struct ntb_acpi_hotplug_context, hp)->ndev)
+
+enum {
+	/* AMD NTB Capability */
+	AMD_MW_CNT		= 3,
+	AMD_DB_CNT		= 16,
+	AMD_MSIX_VECTOR_CNT	= 24,
+	AMD_SPADS_CNT		= 16,
+
+	/*  AMD NTB register offset */
+	AMD_CNTL_OFFSET		= 0x200,
+
+	/* NTB control register bits */
+	PMM_REG_CTL		= BIT(21),
+	SMM_REG_CTL		= BIT(20),
+	SMM_REG_ACC_PATH	= BIT(18),
+	PMM_REG_ACC_PATH	= BIT(17),
+	NTB_CLK_EN		= BIT(16),
+
+	AMD_STA_OFFSET		= 0x204,
+	AMD_PGSLV_OFFSET	= 0x208,
+	AMD_SPAD_MUX_OFFSET	= 0x20C,
+	AMD_SPAD_OFFSET		= 0x210,
+	AMD_RSMU_HCID		= 0x250,
+	AMD_RSMU_SIID		= 0x254,
+	AMD_PSION_OFFSET	= 0x300,
+	AMD_SSION_OFFSET	= 0x330,
+	AMD_MMINDEX_OFFSET	= 0x400,
+	AMD_MMDATA_OFFSET	= 0x404,
+	AMD_SIDEINFO_OFFSET	= 0x408,
+
+	AMD_SIDE_MASK		= BIT(0),
+
+	/* limit register */
+	AMD_ROMBARLMT_OFFSET	= 0x410,
+	AMD_BAR1LMT_OFFSET	= 0x414,
+	AMD_BAR23LMT_OFFSET	= 0x418,
+	AMD_BAR45LMT_OFFSET	= 0x420,
+	/* xlat address */
+	AMD_POMBARXLAT_OFFSET	= 0x428,
+	AMD_BAR1XLAT_OFFSET	= 0x430,
+	AMD_BAR23XLAT_OFFSET	= 0x438,
+	AMD_BAR45XLAT_OFFSET	= 0x440,
+	/* doorbell and interrupt */
+	AMD_DBFM_OFFSET		= 0x450,
+	AMD_DBREQ_OFFSET	= 0x454,
+	AMD_MIRRDBSTAT_OFFSET	= 0x458,
+	AMD_DBMASK_OFFSET	= 0x45C,
+	AMD_DBSTAT_OFFSET	= 0x460,
+	AMD_INTMASK_OFFSET	= 0x470,
+	AMD_INTSTAT_OFFSET	= 0x474,
+
+	/* event type */
+	AMD_PEER_FLUSH_EVENT	= BIT(0),
+	AMD_PEER_RESET_EVENT	= BIT(1),
+	AMD_PEER_D3_EVENT	= BIT(2),
+	AMD_PEER_PMETO_EVENT	= BIT(3),
+	AMD_PEER_D0_EVENT	= BIT(4),
+	AMD_EVENT_INTMASK	= (AMD_PEER_FLUSH_EVENT |
+				AMD_PEER_RESET_EVENT | AMD_PEER_D3_EVENT |
+				AMD_PEER_PMETO_EVENT | AMD_PEER_D0_EVENT),
+
+	AMD_PMESTAT_OFFSET	= 0x480,
+	AMD_PMSGTRIG_OFFSET	= 0x490,
+	AMD_LTRLATENCY_OFFSET	= 0x494,
+	AMD_FLUSHTRIG_OFFSET	= 0x498,
+
+	/* SMU register*/
+	AMD_SMUACK_OFFSET	= 0x4A0,
+	AMD_SINRST_OFFSET	= 0x4A4,
+	AMD_RSPNUM_OFFSET	= 0x4A8,
+	AMD_SMU_SPADMUTEX	= 0x4B0,
+	AMD_SMU_SPADOFFSET	= 0x4B4,
+
+	AMD_PEER_OFFSET		= 0x400,
+};
+
+struct amd_ntb_dev;
+
+struct amd_ntb_vec {
+	struct amd_ntb_dev	*ndev;
+	int			num;
+};
+
+struct amd_ntb_dev {
+	struct ntb_dev ntb;
+
+	u32 ntb_side;
+	u32 lnk_sta;
+	u32 cntl_sta;
+	u32 peer_sta;
+
+	unsigned char mw_count;
+	unsigned char spad_count;
+	unsigned char db_count;
+	unsigned char msix_vec_count;
+
+	u64 db_valid_mask;
+	u64 db_mask;
+	u32 int_mask;
+
+	struct msix_entry *msix;
+	struct amd_ntb_vec *vec;
+
+	spinlock_t db_mask_lock;
+
+	void __iomem *self_mmio;
+	void __iomem *peer_mmio;
+	unsigned long peer_spad;
+
+	struct completion flush_cmpl;
+	struct completion wakeup_cmpl;
+
+	struct delayed_work hb_timer;
+
+	struct dentry *debugfs_dir;
+	struct dentry *debugfs_info;
+};
+
+struct ntb_acpi_hotplug_context {
+	struct acpi_hotplug_context hp;
+	struct amd_ntb_dev *ndev;
+};
+
+static int amd_ntb_mw_count(struct ntb_dev *ntb);
+static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
+			phys_addr_t *base, resource_size_t *size,
+			resource_size_t *align, resource_size_t *algin_size);
+static int amd_ntb_mw_set_trans(struct ntb_dev *ndev, int idx,
+				dma_addr_t addr, resource_size_t size);
+static int amd_ntb_link_is_up(struct ntb_dev *ntb,
+				enum ntb_speed *speed,
+				enum ntb_width *width);
+static int amd_ntb_link_enable(struct ntb_dev *ntb,
+				enum ntb_speed speed,
+				enum ntb_width width);
+static int amd_ntb_link_disable(struct ntb_dev *ntb);
+static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb);
+static int amd_ntb_db_vector_count(struct ntb_dev *ntb);
+static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector);
+static u64 amd_ntb_db_read(struct ntb_dev *ntb);
+static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
+				phys_addr_t *db_addr,
+				resource_size_t *db_size);
+static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits);
+static int amd_ntb_spad_count(struct ntb_dev *ntb);
+static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx);
+static int amd_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val);
+static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
+					phys_addr_t *spad_addr);
+static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx);
+static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int idx, u32 val);
+#endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2015-12-23  9:29 ` Christoph Hellwig
@ 2015-12-23 15:09   ` Allen Hubbe
  2015-12-23 15:51     ` Raghu
  0 siblings, 1 reply; 18+ messages in thread
From: Allen Hubbe @ 2015-12-23 15:09 UTC (permalink / raw)
  To: Christoph Hellwig, Xiangliang Yu
  Cc: SPG_Linux_Kernel, dave.jiang, jdmason, linux-kernel, linux-ntb

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

On Wed, Dec 23, 2015, 3:29 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> Just curious, what do you actually use NTB for?

There is an ethernet driver for ntb in Linux. I believe that has users,
though Dave of John could confirm.

There is an RDMA driver at github.com/ntrdma, waiting for more than one
user and developer before asking to support it in the kernel.

In general, ntb allows peers to write directly into each other's memory to
transfer data, and it provides some additional mechanisms for coordination.

> Seems like we're
> adding a giant subsystem without actual consumers, or did I miss
> something?

It is no surprise that there are few customers. Even with an ntb chip, it
requires a special hardware configuration to connect the systems with Pcie.
This had been done with original design manufactured systems. More
recently, oem systems are becoming available configured for ntb, so there
may be more customers, and more diversity of applications.

Allen

Please pardon me if this email is not plain text as I am on mobile.

[-- Attachment #2: Type: text/html, Size: 1314 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2015-12-23 15:09   ` Allen Hubbe
@ 2015-12-23 15:51     ` Raghu
  0 siblings, 0 replies; 18+ messages in thread
From: Raghu @ 2015-12-23 15:51 UTC (permalink / raw)
  To: Allen Hubbe
  Cc: Christoph Hellwig, Xiangliang Yu, SPG_Linux_Kernel, dave.jiang,
	jdmason, linux-kernel, linux-ntb

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

Christoph, Allen,

I am currently a serious consumer of the NTB subsystem as well. Both the
ntb_netdev (ethernet) driver, as well as the basic NTB transport framework.
I am fairly certain that there are several others who are yet to come
forward and declare that they are a consumer.


Raghu

On Wed, Dec 23, 2015 at 9:09 AM, Allen Hubbe <allenbh@gmail.com> wrote:

> On Wed, Dec 23, 2015, 3:29 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > Just curious, what do you actually use NTB for?
>
> There is an ethernet driver for ntb in Linux. I believe that has users,
> though Dave of John could confirm.
>
> There is an RDMA driver at github.com/ntrdma, waiting for more than one
> user and developer before asking to support it in the kernel.
>
> In general, ntb allows peers to write directly into each other's memory to
> transfer data, and it provides some additional mechanisms for coordination.
>
> > Seems like we're
> > adding a giant subsystem without actual consumers, or did I miss
> > something?
>
> It is no surprise that there are few customers. Even with an ntb chip, it
> requires a special hardware configuration to connect the systems with Pcie.
> This had been done with original design manufactured systems. More
> recently, oem systems are becoming available configured for ntb, so there
> may be more customers, and more diversity of applications.
>
> Allen
>
> Please pardon me if this email is not plain text as I am on mobile.
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/linux-ntb/CAJ80sat9ocSwGe1UBBHEA0TvDQYCftD1ZVMcyTkAAL_cGwAJTw%40mail.gmail.com
> <https://groups.google.com/d/msgid/linux-ntb/CAJ80sat9ocSwGe1UBBHEA0TvDQYCftD1ZVMcyTkAAL_cGwAJTw%40mail.gmail.com?utm_medium=email&utm_source=footer>
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

[-- Attachment #2: Type: text/html, Size: 3144 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2015-12-23 13:42 ` Xiangliang Yu
  (?)
  (?)
@ 2016-01-06 15:05 ` Jon Mason
  2016-01-06 16:52     ` Hubbe, Allen
                     ` (2 more replies)
  -1 siblings, 3 replies; 18+ messages in thread
From: Jon Mason @ 2016-01-06 15:05 UTC (permalink / raw)
  To: Xiangliang Yu
  Cc: Dave Jiang, Hubbe, Allen, linux-ntb, linux-kernel, SPG_Linux_Kernel

On Wed, Dec 23, 2015 at 8:42 AM, Xiangliang Yu <Xiangliang.Yu@amd.com> wrote:
> AMD NTB support following main features:
> (1) Three memory windows;
> (2) Sixteen 32-bit scratch pad;
> (3) Two 16-bit doorbell interrupt;
> (4) Five event interrupts;
> (5) One system can wake up opposite system of NTB;
> (6) Flush previous request to the opposite system;
> (7) There are reset and PME_TO mechanisms between two systems;
>
> Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
> ---
>  drivers/ntb/hw/amd/Kconfig      |    7 +
>  drivers/ntb/hw/amd/Makefile     |    1 +
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 1229 +++++++++++++++++++++++++++++++++++++++
>  drivers/ntb/hw/amd/ntb_hw_amd.h |  263 +++++++++
>  4 files changed, 1500 insertions(+)
>  create mode 100644 drivers/ntb/hw/amd/Kconfig
>  create mode 100644 drivers/ntb/hw/amd/Makefile
>  create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.c
>  create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.h
>
> diff --git a/drivers/ntb/hw/amd/Kconfig b/drivers/ntb/hw/amd/Kconfig
> new file mode 100644
> index 0000000..cfe903c
> --- /dev/null
> +++ b/drivers/ntb/hw/amd/Kconfig
> @@ -0,0 +1,7 @@
> +config NTB_AMD
> +       tristate "AMD Non-Transparent Bridge support"
> +       depends on X86_64
> +       help
> +        This driver supports AMD NTB on capable Zeppelin hardware.
> +
> +        If unsure, say N.
> diff --git a/drivers/ntb/hw/amd/Makefile b/drivers/ntb/hw/amd/Makefile
> new file mode 100644
> index 0000000..ad54da9
> --- /dev/null
> +++ b/drivers/ntb/hw/amd/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_NTB_AMD) += ntb_hw_amd.o
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> new file mode 100644
> index 0000000..cedac8d
> --- /dev/null
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -0,0 +1,1229 @@
> +/*
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + *
> + *   GPL LICENSE SUMMARY
> + *
> + *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + *
> + *   BSD LICENSE
> + *
> + *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copy
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of AMD Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * AMD PCIe NTB Linux driver
> + *
> + * Contact Information:
> + * Xiangliang Yu<Xiangliang.Yu@amd.com>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/random.h>
> +#include <linux/slab.h>
> +#include <linux/ntb.h>
> +
> +#include "ntb_hw_amd.h"
> +
> +#define NTB_NAME       "ntb_hw_amd"
> +#define NTB_DESC       "AMD(R) PCI-E Non-Transparent Bridge Driver"
> +#define NTB_VER                "1.0"
> +
> +MODULE_DESCRIPTION(NTB_DESC);
> +MODULE_VERSION(NTB_VER);
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("AMD Inc.");
> +
> +static const struct file_operations amd_ntb_debugfs_info;
> +static struct dentry *debugfs_dir;
> +
> +static const struct ntb_dev_ops amd_ntb_ops = {
> +       .mw_count               = amd_ntb_mw_count,
> +       .mw_get_range           = amd_ntb_mw_get_range,
> +       .mw_set_trans           = amd_ntb_mw_set_trans,
> +       .link_is_up             = amd_ntb_link_is_up,
> +       .link_enable            = amd_ntb_link_enable,
> +       .link_disable           = amd_ntb_link_disable,
> +       .db_valid_mask          = amd_ntb_db_valid_mask,
> +       .db_vector_count        = amd_ntb_db_vector_count,
> +       .db_vector_mask         = amd_ntb_db_vector_mask,
> +       .db_read                = amd_ntb_db_read,
> +       .db_clear               = amd_ntb_db_clear,
> +       .db_set_mask            = amd_ntb_db_set_mask,
> +       .db_clear_mask          = amd_ntb_db_clear_mask,
> +       .peer_db_addr           = amd_ntb_peer_db_addr,
> +       .peer_db_set            = amd_ntb_peer_db_set,
> +       .spad_count             = amd_ntb_spad_count,
> +       .spad_read              = amd_ntb_spad_read,
> +       .spad_write             = amd_ntb_spad_write,
> +       .peer_spad_addr         = amd_ntb_peer_spad_addr,
> +       .peer_spad_read         = amd_ntb_peer_spad_read,
> +       .peer_spad_write        = amd_ntb_peer_spad_write,
> +};
> +
> +static int ndev_mw_to_bar(struct amd_ntb_dev *ndev, int idx)
> +{
> +       if (idx < 0 || idx > ndev->mw_count)
> +               return -EINVAL;
> +
> +       return 1 << idx;
> +}
> +
> +static int amd_ntb_mw_count(struct ntb_dev *ntb)
> +{
> +       return ntb_ndev(ntb)->mw_count;
> +}
> +
> +static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
> +                               phys_addr_t *base,
> +                               resource_size_t *size,
> +                               resource_size_t *align,
> +                               resource_size_t *align_size)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       int bar;
> +
> +       bar = ndev_mw_to_bar(ndev, idx);
> +       if (bar < 0)
> +               return bar;
> +
> +       if (base)
> +               *base = pci_resource_start(ndev->ntb.pdev, bar);
> +
> +       if (size)
> +               *size = pci_resource_len(ndev->ntb.pdev, bar);
> +
> +       if (align)
> +               *align = 4096;
> +
> +       if (align_size)
> +               *align_size = 1;
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_mw_set_trans(struct ntb_dev *ntb, int idx,
> +                               dma_addr_t addr, resource_size_t size)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       unsigned long xlat_reg, limit_reg = 0;
> +       resource_size_t mw_size;
> +       void __iomem *mmio, *peer_mmio;
> +       u64 base_addr, limit, reg_val;
> +       int bar;
> +
> +       bar = ndev_mw_to_bar(ndev, idx);
> +       if (bar < 0)
> +               return bar;
> +
> +       mw_size = pci_resource_len(ndev->ntb.pdev, bar);
> +
> +       /* make sure the range fits in the usable mw size */
> +       if (size > mw_size)
> +               return -EINVAL;
> +
> +       mmio = ndev->self_mmio;
> +       peer_mmio = ndev->peer_mmio;
> +
> +       base_addr = pci_resource_start(ndev->ntb.pdev, bar);
> +       if (bar == 1) {
> +               xlat_reg = AMD_BAR1XLAT_OFFSET;
> +               limit_reg = AMD_BAR1LMT_OFFSET;
> +       } else {
> +               xlat_reg = AMD_BAR23XLAT_OFFSET + ((bar - 2) << 3);
> +               limit_reg = AMD_BAR23LMT_OFFSET + ((bar - 2) << 3);
> +       }
> +
> +       if (bar != 1) {
> +               /* Set the limit if supported */
> +               limit = base_addr + size;
> +
> +               /* set and verify setting the translation address */
> +               iowrite64(addr, peer_mmio + xlat_reg);
> +               reg_val = ioread64(peer_mmio + xlat_reg);
> +               if (reg_val != addr) {
> +                       iowrite64(0, peer_mmio + xlat_reg);
> +                       return -EIO;
> +               }
> +
> +               /* set and verify setting the limit */
> +               iowrite64(limit, mmio + limit_reg);
> +               reg_val = ioread64(mmio + limit_reg);
> +               if (reg_val != limit) {
> +                       iowrite64(base_addr, mmio + limit_reg);
> +                       iowrite64(0, peer_mmio + xlat_reg);
> +                       return -EIO;
> +               }
> +       } else {
> +               /* split bar addr range must all be 32 bit */
> +               if (addr & (~0ull << 32))
> +                       return -EINVAL;
> +               if ((addr + size) & (~0ull << 32))
> +                       return -EINVAL;
> +
> +               /* Set the limit if supported */
> +               limit = base_addr + size;
> +
> +               /* set and verify setting the translation address */
> +               iowrite32(0, peer_mmio + xlat_reg + 0x4);
> +               iowrite32(addr, peer_mmio + xlat_reg);
> +               reg_val = ioread32(peer_mmio + xlat_reg);
> +               if (reg_val != addr) {
> +                       iowrite32(0, peer_mmio + xlat_reg);
> +                       return -EIO;
> +               }
> +
> +               /* set and verify setting the limit */
> +               iowrite32(limit, mmio + limit_reg);
> +               reg_val = ioread32(mmio + limit_reg);
> +               if (reg_val != limit) {
> +                       iowrite32(base_addr, mmio + limit_reg);
> +                       iowrite32(0, peer_mmio + xlat_reg);
> +                       return -EIO;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int amd_link_is_up(struct amd_ntb_dev *ndev)
> +{
> +       /* set link down and clear flag */
> +       if (ndev->peer_sta & AMD_PEER_RESET_EVENT) {
> +               ndev->peer_sta &= ~AMD_PEER_RESET_EVENT;
> +               return 0;
> +       } else if (ndev->peer_sta & (AMD_PEER_D3_EVENT |
> +                                    AMD_PEER_PMETO_EVENT)) {
> +               return 0;
> +       } else if (ndev->peer_sta & AMD_PEER_D0_EVENT) {
> +               ndev->peer_sta = 0;
> +               return 0;
> +       } else
> +               return NTB_LNK_STA_ACTIVE(ndev->cntl_sta);
> +}
> +
> +static int amd_ntb_link_is_up(struct ntb_dev *ntb,
> +                             enum ntb_speed *speed,
> +                             enum ntb_width *width)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       int ret = 0;
> +
> +       if (amd_link_is_up(ndev)) {
> +               if (speed)
> +                       *speed = NTB_LNK_STA_SPEED(ndev->lnk_sta);
> +               if (width)
> +                       *width = NTB_LNK_STA_WIDTH(ndev->lnk_sta);
> +
> +               dev_dbg(ndev_dev(ndev), "link is up.\n");
> +
> +               ret = 1;
> +       } else {
> +               if (speed)
> +                       *speed = NTB_SPEED_NONE;
> +               if (width)
> +                       *width = NTB_WIDTH_NONE;
> +
> +               dev_dbg(ndev_dev(ndev), "link is down.\n");
> +       }
> +
> +       return ret;
> +}
> +
> +static int amd_ntb_link_enable(struct ntb_dev *ntb,
> +                              enum ntb_speed max_speed,
> +                              enum ntb_width max_width)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 ntb_ctl;
> +
> +       /* Enable event interrupt */
> +       ndev->int_mask &= ~AMD_EVENT_INTMASK;
> +       NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
> +
> +       if (ndev->ntb.topo == NTB_TOPO_SEC)
> +               return -EINVAL;
> +       dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
> +
> +       ntb_ctl = NTB_READ_REG(mmio, CNTL);
> +       ntb_ctl |= (3 << 20);
> +       NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_link_disable(struct ntb_dev *ntb)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 ntb_ctl;
> +
> +       /* Disable event interrupt */
> +       ndev->int_mask |= AMD_EVENT_INTMASK;
> +       NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
> +
> +       if (ndev->ntb.topo == NTB_TOPO_SEC)
> +               return -EINVAL;
> +       dev_dbg(ndev_dev(ndev), "Enabling Link.\n");
> +
> +       ntb_ctl = NTB_READ_REG(mmio, CNTL);
> +       ntb_ctl &= ~(3 << 16);
> +       NTB_WRITE_REG(mmio, ntb_ctl, CNTL);
> +
> +       return 0;
> +}
> +
> +static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb)
> +{
> +       return ntb_ndev(ntb)->db_valid_mask;
> +}
> +
> +static int amd_ntb_db_vector_count(struct ntb_dev *ntb)
> +{
> +       return ntb_ndev(ntb)->db_count;
> +}
> +
> +static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +
> +       if (db_vector < 0 || db_vector > ndev->db_count)
> +               return 0;
> +
> +       return ntb_ndev(ntb)->db_valid_mask & (1 << db_vector);
> +}
> +
> +static u64 amd_ntb_db_read(struct ntb_dev *ntb)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       return (u64)NTB_READ_REG(mmio, DBSTAT);
> +}
> +
> +static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       NTB_WRITE_REG(mmio, (u32)db_bits, DBSTAT);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       unsigned long flags;
> +
> +       if (db_bits & ~ndev->db_valid_mask)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&ndev->db_mask_lock, flags);
> +       ndev->db_mask |= db_bits;
> +       NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
> +       spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       unsigned long flags;
> +
> +       if (db_bits & ~ndev->db_valid_mask)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&ndev->db_mask_lock, flags);
> +       ndev->db_mask &= ~db_bits;
> +       NTB_WRITE_REG(mmio, ndev->db_mask, DBMASK);
> +       spin_unlock_irqrestore(&ndev->db_mask_lock, flags);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
> +               phys_addr_t *db_addr,
> +               resource_size_t *db_size)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +
> +       if (db_addr)
> +               *db_addr = (phys_addr_t)(ndev->peer_mmio + AMD_DBREQ_OFFSET);
> +       if (db_size)
> +               *db_size = sizeof(u32);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       NTB_WRITE_REG(mmio, (u32)db_bits, DBREQ);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_spad_count(struct ntb_dev *ntb)
> +{
> +       return ntb_ndev(ntb)->spad_count;
> +}
> +
> +static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       if (idx < 0 || idx >= ndev->spad_count)
> +               return 0;
> +
> +       return NTB_READ_OFFSET(mmio, SPAD, (idx << 2));
> +}
> +
> +static int amd_ntb_spad_write(struct ntb_dev *ntb,
> +                               int idx, u32 val)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       if (idx < 0 || idx >= ndev->spad_count)
> +               return -EINVAL;
> +
> +       NTB_WRITE_OFFSET(mmio, val, SPAD, (idx << 2));
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
> +                                   phys_addr_t *spad_addr)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +
> +       if (idx < 0 || idx >= ndev->spad_count)
> +               return -EINVAL;
> +
> +       if (spad_addr)
> +               *spad_addr = (phys_addr_t)(ndev->self_mmio + AMD_SPAD_OFFSET +
> +                               ndev->peer_spad + (idx << 2));
> +       return 0;
> +}
> +
> +static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 offset;
> +
> +       if (idx < 0 || idx >= ndev->spad_count)
> +               return -EINVAL;
> +
> +       offset = ndev->peer_spad + (idx << 2);
> +       return NTB_READ_OFFSET(mmio, SPAD, offset);
> +}
> +
> +static int amd_ntb_peer_spad_write(struct ntb_dev *ntb,
> +                                    int idx, u32 val)
> +{
> +       struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 offset;
> +
> +       if (idx < 0 || idx >= ndev->spad_count)
> +               return -EINVAL;
> +
> +       offset = ndev->peer_spad + (idx << 2);
> +       NTB_WRITE_OFFSET(mmio, val, SPAD, offset);
> +
> +       return 0;
> +}
> +
> +static void amd_ack_SMU(struct amd_ntb_dev *ndev, u32 bit)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       int reg;
> +
> +       reg = NTB_READ_REG(mmio, SMUACK);
> +       reg |= bit;
> +       NTB_WRITE_REG(mmio, reg, SMUACK);
> +
> +       ndev->peer_sta |= bit;
> +}
> +
> +#ifdef CONFIG_PM
> +/*
> + * flush the requests to peer side
> + */
> +static int amd_flush_peer_requests(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 reg;
> +
> +       if (!amd_link_is_up(ndev)) {
> +               dev_err(ndev_dev(ndev), "link is down.\n");
> +               return -EINVAL;
> +       }
> +
> +       reg = NTB_READ_REG(mmio, FLUSHTRIG);
> +       reg |= 0x1;
> +       NTB_WRITE_REG(mmio, reg, FLUSHTRIG);
> +
> +       wait_for_completion(&ndev->flush_cmpl);
> +
> +       reinit_completion(&ndev->flush_cmpl);
> +
> +       return 0;
> +}
> +#endif
> +
> +static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 status;
> +
> +       status = NTB_READ_REG(mmio, INTSTAT);
> +       if (!(status & AMD_EVENT_INTMASK))
> +               return;
> +
> +       dev_dbg(ndev_dev(ndev), "status = 0x%x and vec = %d\n", status, vec);
> +
> +       status &= AMD_EVENT_INTMASK;
> +       switch (status) {
> +       case AMD_PEER_FLUSH_EVENT:
> +               complete(&ndev->flush_cmpl);
> +               break;
> +       case AMD_PEER_RESET_EVENT:
> +               amd_ack_SMU(ndev, AMD_PEER_RESET_EVENT);
> +
> +               /* link down first */
> +               ntb_link_event(&ndev->ntb);
> +               /* polling peer status */
> +               schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
> +
> +               break;
> +       case AMD_PEER_D3_EVENT:
> +       case AMD_PEER_PMETO_EVENT:
> +               amd_ack_SMU(ndev, status);
> +
> +               /* link down */
> +               ntb_link_event(&ndev->ntb);
> +
> +               break;
> +       case AMD_PEER_D0_EVENT:
> +               mmio = ndev->peer_mmio;
> +               status = NTB_READ_REG(mmio, PMESTAT);
> +               /* check if this is WAKEUP event */
> +               if (status & 0x1)
> +                       complete(&ndev->wakeup_cmpl);
> +
> +               amd_ack_SMU(ndev, AMD_PEER_D0_EVENT);
> +
> +               if (amd_link_is_up(ndev))
> +                       ntb_link_event(&ndev->ntb);
> +               else
> +                       schedule_delayed_work(&ndev->hb_timer,
> +                                               AMD_LINK_HB_TIMEOUT);
> +               break;
> +       default:
> +               dev_info(ndev_dev(ndev), "event status = 0x%x.\n", status);
> +               break;
> +       }
> +}
> +
> +static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
> +{
> +       dev_dbg(ndev_dev(ndev), "vec %d\n", vec);
> +
> +       if (vec > (ndev->msix_vec_count - 1)) {
> +               dev_err(ndev_dev(ndev), "Invalid interrupt.\n");
> +               return IRQ_HANDLED;
> +       }
> +
> +       if (vec > 15 || (ndev->msix_vec_count == 1))
> +               amd_handle_event(ndev, vec);
> +
> +       if (vec < 16)
> +               ntb_db_event(&ndev->ntb, vec);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t ndev_vec_isr(int irq, void *dev)
> +{
> +       struct amd_ntb_vec *nvec = dev;
> +
> +       return ndev_interrupt(nvec->ndev, nvec->num);
> +}
> +
> +static irqreturn_t ndev_irq_isr(int irq, void *dev)
> +{
> +       struct amd_ntb_dev *ndev = dev;
> +
> +       return ndev_interrupt(ndev, irq - ndev_pdev(ndev)->irq);
> +}
> +
> +static int ndev_init_isr(struct amd_ntb_dev *ndev,
> +                               int msix_min, int msix_max)
> +{
> +       struct pci_dev *pdev;
> +       int rc, i, msix_count, node;
> +
> +       pdev = ndev_pdev(ndev);
> +
> +       node = dev_to_node(&pdev->dev);
> +
> +       ndev->db_mask = ndev->db_valid_mask;
> +
> +       /* Try to set up msix irq */
> +       ndev->vec = kzalloc_node(msix_max * sizeof(*ndev->vec),
> +                                GFP_KERNEL, node);
> +       if (!ndev->vec)
> +               goto err_msix_vec_alloc;
> +
> +       ndev->msix = kzalloc_node(msix_max * sizeof(*ndev->msix),
> +                                 GFP_KERNEL, node);
> +       if (!ndev->msix)
> +               goto err_msix_alloc;
> +
> +       for (i = 0; i < msix_max; ++i)
> +               ndev->msix[i].entry = i;
> +
> +       msix_count = pci_enable_msix_range(pdev, ndev->msix,
> +                                          msix_min, msix_max);
> +       if (msix_count < 0)
> +               goto err_msix_enable;
> +
> +       /* Disable MSIX if msix count is less than 16 */
> +       if (msix_count < msix_min) {
> +               pci_disable_msix(pdev);
> +               goto err_msix_enable;
> +       }
> +
> +       for (i = 0; i < msix_count; ++i) {
> +               ndev->vec[i].ndev = ndev;
> +               ndev->vec[i].num = i;
> +               rc = request_irq(ndev->msix[i].vector, ndev_vec_isr, 0,
> +                                "ndev_vec_isr", &ndev->vec[i]);
> +               if (rc)
> +                       goto err_msix_request;
> +       }
> +
> +       dev_dbg(ndev_dev(ndev), "Using msix interrupts\n");
> +       ndev->db_count = msix_min;
> +       ndev->msix_vec_count = msix_max;
> +       return 0;
> +
> +err_msix_request:
> +       while (i-- > 0)
> +               free_irq(ndev->msix[i].vector, ndev);
> +       pci_disable_msix(pdev);
> +err_msix_enable:
> +       kfree(ndev->msix);
> +err_msix_alloc:
> +       kfree(ndev->vec);
> +err_msix_vec_alloc:
> +       ndev->msix = NULL;
> +       ndev->vec = NULL;
> +
> +       /* Try to set up msi irq */
> +       rc = pci_enable_msi(pdev);
> +       if (rc)
> +               goto err_msi_enable;
> +
> +       rc = request_irq(pdev->irq, ndev_irq_isr, 0,
> +                        "ndev_irq_isr", ndev);
> +       if (rc)
> +               goto err_msi_request;
> +
> +       dev_dbg(ndev_dev(ndev), "Using msi interrupts\n");
> +       ndev->db_count = 1;
> +       ndev->msix_vec_count = 1;
> +       return 0;
> +
> +err_msi_request:
> +       pci_disable_msi(pdev);
> +err_msi_enable:
> +
> +       /* Try to set up intx irq */
> +       pci_intx(pdev, 1);
> +
> +       rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
> +                        "ndev_irq_isr", ndev);
> +       if (rc)
> +               goto err_intx_request;
> +
> +       dev_dbg(ndev_dev(ndev), "Using intx interrupts\n");
> +       ndev->db_count = 1;
> +       ndev->msix_vec_count = 1;
> +       return 0;
> +
> +err_intx_request:
> +       return rc;
> +}
> +
> +static void ndev_deinit_isr(struct amd_ntb_dev *ndev)
> +{
> +       struct pci_dev *pdev;
> +       void __iomem *mmio = ndev->self_mmio;
> +       int i;
> +
> +       pdev = ndev_pdev(ndev);
> +
> +       /* Mask all doorbell interrupts */
> +       ndev->db_mask = ndev->db_valid_mask;
> +       NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
> +
> +       if (ndev->msix) {
> +               i = ndev->msix_vec_count;
> +               while (i--)
> +                       free_irq(ndev->msix[i].vector, &ndev->vec[i]);
> +               pci_disable_msix(pdev);
> +               kfree(ndev->msix);
> +               kfree(ndev->vec);
> +       } else {
> +               free_irq(pdev->irq, ndev);
> +               if (pci_dev_msi_enabled(pdev))
> +                       pci_disable_msi(pdev);
> +               else
> +                       pci_intx(pdev, 0);
> +       }
> +}
> +
> +static ssize_t ndev_debugfs_read(struct file *filp, char __user *ubuf,
> +                                size_t count, loff_t *offp)
> +{
> +       struct amd_ntb_dev *ndev;
> +       void __iomem *mmio;
> +       char *buf;
> +       size_t buf_size;
> +       ssize_t ret, off;
> +       union { u64 v64; u32 v32; u16 v16; } u;
> +
> +       ndev = filp->private_data;
> +       mmio = ndev->self_mmio;
> +
> +       buf_size = min(count, 0x800ul);
> +
> +       buf = kmalloc(buf_size, GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       off = 0;
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "NTB Device Information:\n");
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Connection Topology -\t%s\n",
> +                        ntb_topo_string(ndev->ntb.topo));
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "LNK STA -\t\t%#06x\n", ndev->lnk_sta);
> +
> +       if (!amd_link_is_up(ndev)) {
> +               off += scnprintf(buf + off, buf_size - off,
> +                                "Link Status -\t\tDown\n");
> +       } else {
> +               off += scnprintf(buf + off, buf_size - off,
> +                                "Link Status -\t\tUp\n");
> +               off += scnprintf(buf + off, buf_size - off,
> +                                "Link Speed -\t\tPCI-E Gen %u\n",
> +                                NTB_LNK_STA_SPEED(ndev->lnk_sta));
> +               off += scnprintf(buf + off, buf_size - off,
> +                                "Link Width -\t\tx%u\n",
> +                                NTB_LNK_STA_WIDTH(ndev->lnk_sta));
> +       }
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Memory Window Count -\t%u\n", ndev->mw_count);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Scratchpad Count -\t%u\n", ndev->spad_count);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Doorbell Count -\t%u\n", ndev->db_count);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "MSIX Vector Count -\t%u\n", ndev->msix_vec_count);
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Doorbell Valid Mask -\t%#llx\n", ndev->db_valid_mask);
> +
> +       u.v64 = (u64)ioread32(ndev->self_mmio + AMD_DBMASK_OFFSET);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Doorbell Mask -\t\t%#llx\n", u.v64);
> +
> +       u.v64 = (u64)NTB_READ_REG(mmio, DBSTAT);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "Doorbell Bell -\t\t%#llx\n", u.v64);
> +
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "\nNTB Incoming XLAT:\n");
> +
> +       u.v32 = NTB_READ_REG(mmio, BAR1XLAT);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "XLAT1 -\t\t\t%#06x\n", u.v32);
> +
> +       u.v64 = ioread64(ndev->self_mmio + AMD_BAR23XLAT_OFFSET);
> +       off += scnprintf(buf + off, buf_size - off,
> +                                "XLAT23 -\t\t%#018llx\n", u.v64);
> +
> +       u.v64 = ioread64(ndev->self_mmio + AMD_BAR45XLAT_OFFSET);
> +       off += scnprintf(buf + off, buf_size - off,
> +                                "XLAT45 -\t\t%#018llx\n", u.v64);
> +
> +       u.v32 = NTB_READ_REG(mmio, BAR1LMT);
> +       off += scnprintf(buf + off, buf_size - off,
> +                        "LMT1 -\t\t\t%#06x\n", u.v32);
> +
> +       u.v64 = ioread64(ndev->self_mmio + AMD_BAR23LMT_OFFSET);
> +       off += scnprintf(buf + off, buf_size - off,
> +                                "LMT23 -\t\t\t%#018llx\n", u.v64);
> +
> +       u.v64 = ioread64(ndev->self_mmio + AMD_BAR45LMT_OFFSET);
> +       off += scnprintf(buf + off, buf_size - off,
> +                                "LMT45 -\t\t\t%#018llx\n", u.v64);
> +
> +       ret = simple_read_from_buffer(ubuf, count, offp, buf, off);
> +       kfree(buf);
> +       return ret;
> +}
> +
> +static void ndev_init_debugfs(struct amd_ntb_dev *ndev)
> +{
> +       if (!debugfs_dir) {
> +               ndev->debugfs_dir = NULL;
> +               ndev->debugfs_info = NULL;
> +       } else {
> +               ndev->debugfs_dir =
> +                       debugfs_create_dir(ndev_name(ndev), debugfs_dir);
> +               if (!ndev->debugfs_dir)
> +                       ndev->debugfs_info = NULL;
> +               else
> +                       ndev->debugfs_info =
> +                               debugfs_create_file("info", S_IRUSR,
> +                                                   ndev->debugfs_dir, ndev,
> +                                                   &amd_ntb_debugfs_info);
> +       }
> +}
> +
> +static void ndev_deinit_debugfs(struct amd_ntb_dev *ndev)
> +{
> +       debugfs_remove_recursive(ndev->debugfs_dir);
> +}
> +
> +static inline void ndev_init_struct(struct amd_ntb_dev *ndev,
> +                                   struct pci_dev *pdev)
> +{
> +       ndev->ntb.pdev = pdev;
> +       ndev->ntb.topo = NTB_TOPO_NONE;
> +       ndev->ntb.ops = &amd_ntb_ops;
> +       ndev->int_mask = AMD_EVENT_INTMASK;
> +       init_completion(&ndev->flush_cmpl);
> +       init_completion(&ndev->wakeup_cmpl);
> +       spin_lock_init(&ndev->db_mask_lock);
> +}
> +
> +static int amd_poll_link(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->peer_mmio;
> +       u32 reg, stat;
> +       int rc;
> +
> +       reg = NTB_READ_REG(mmio, SIDEINFO);
> +       reg &= NTB_LIN_STA_ACTIVE_BIT;
> +
> +       dev_dbg(ndev_dev(ndev), "%s: reg_val = 0x%x.\n", __func__, reg);
> +
> +       if (reg == ndev->cntl_sta)
> +               return 0;
> +
> +       ndev->cntl_sta = reg;
> +
> +       rc = pci_read_config_dword(ndev->ntb.pdev,
> +                       AMD_LINK_STATUS_OFFSET, &stat);
> +       if (rc)
> +               return 0;
> +       ndev->lnk_sta = stat;
> +
> +       return 1;
> +}
> +
> +static void amd_link_hb(struct work_struct *work)
> +{
> +       struct amd_ntb_dev *ndev = hb_ndev(work);
> +
> +       if (amd_poll_link(ndev))
> +               ntb_link_event(&ndev->ntb);
> +
> +       if (!amd_link_is_up(ndev))
> +               schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
> +}
> +
> +static int amd_init_isr(struct amd_ntb_dev *ndev)
> +{
> +       return ndev_init_isr(ndev, AMD_DB_CNT, AMD_MSIX_VECTOR_CNT);
> +}
> +
> +static void amd_init_side_info(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       unsigned int reg = 0;
> +
> +       reg = NTB_READ_REG(mmio, SIDEINFO);
> +       if (!(reg & 0x2)) {
> +               reg |= 0x2;
> +               NTB_WRITE_REG(mmio, reg, SIDEINFO);
> +       }
> +}
> +
> +static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       unsigned int reg = 0;
> +
> +       reg = NTB_READ_REG(mmio, SIDEINFO);
> +       if (reg & 0x2) {
> +               reg &= ~(0x2);
> +               NTB_WRITE_REG(mmio, reg, SIDEINFO);
> +               NTB_READ_REG(mmio, SIDEINFO);
> +       }
> +}
> +
> +static int amd_init_ntb(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       ndev->mw_count = AMD_MW_CNT;
> +       ndev->spad_count = AMD_SPADS_CNT;
> +       ndev->db_count = AMD_DB_CNT;
> +
> +       switch (ndev->ntb.topo) {
> +       case NTB_TOPO_PRI:
> +       case NTB_TOPO_SEC:
> +               ndev->spad_count >>= 1;
> +               if (ndev->ntb.topo == NTB_TOPO_PRI)
> +                       ndev->peer_spad = (4 << 8);
> +               else
> +                       ndev->peer_spad = 0;
> +
> +               INIT_DELAYED_WORK(&ndev->hb_timer, amd_link_hb);
> +               schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
> +
> +               break;
> +       default:
> +               dev_err(ndev_dev(ndev), "AMD NTB does not support B2B mode.\n");
> +               return -EINVAL;
> +       }
> +
> +       ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
> +
> +       /* Mask event interrupts */
> +       NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
> +
> +       return 0;
> +}
> +
> +static inline enum ntb_topo amd_get_topo(struct amd_ntb_dev *ndev)
> +{
> +       void __iomem *mmio = ndev->self_mmio;
> +       u32 info;
> +
> +       info = NTB_READ_REG(mmio, SIDEINFO);
> +       if (info & AMD_SIDE_MASK)
> +               return NTB_TOPO_SEC;
> +       else
> +               return NTB_TOPO_PRI;
> +}
> +
> +static int amd_init_dev(struct amd_ntb_dev *ndev)
> +{
> +       struct pci_dev *pdev;
> +       int rc = 0;
> +
> +       pdev = ndev_pdev(ndev);
> +
> +       ndev->ntb.topo = amd_get_topo(ndev);
> +       dev_dbg(ndev_dev(ndev), "AMD NTB topo is %s\n",
> +                       ntb_topo_string(ndev->ntb.topo));
> +
> +       rc = amd_init_ntb(ndev);
> +       if (rc)
> +               return rc;
> +
> +       rc = amd_init_isr(ndev);
> +       if (rc) {
> +               dev_err(ndev_dev(ndev), "fail to init isr.\n");
> +               return rc;
> +       }
> +
> +       ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
> +
> +       return 0;
> +}
> +
> +static void amd_deinit_dev(struct amd_ntb_dev *ndev)
> +{
> +       cancel_delayed_work_sync(&ndev->hb_timer);
> +
> +       ndev_deinit_isr(ndev);
> +}
> +
> +static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
> +                           struct pci_dev *pdev)
> +{
> +       int rc;
> +
> +       pci_set_drvdata(pdev, ndev);
> +
> +       rc = pci_enable_device(pdev);
> +       if (rc)
> +               goto err_pci_enable;
> +
> +       rc = pci_request_regions(pdev, NTB_NAME);
> +       if (rc)
> +               goto err_pci_regions;
> +
> +       pci_set_master(pdev);
> +
> +       rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> +       if (rc) {
> +               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> +               if (rc)
> +                       goto err_dma_mask;
> +               dev_warn(ndev_dev(ndev), "Cannot DMA highmem\n");
> +       }
> +
> +       rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
> +       if (rc) {
> +               rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
> +               if (rc)
> +                       goto err_dma_mask;
> +               dev_warn(ndev_dev(ndev), "Cannot DMA consistent highmem\n");
> +       }
> +
> +       ndev->self_mmio = pci_iomap(pdev, 0, 0);
> +       if (!ndev->self_mmio) {
> +               rc = -EIO;
> +               goto err_mmio;
> +       }
> +       ndev->peer_mmio = ndev->self_mmio + AMD_PEER_OFFSET;
> +
> +       return 0;
> +
> +err_mmio:
> +err_dma_mask:
> +       pci_clear_master(pdev);
> +err_pci_regions:
> +       pci_disable_device(pdev);
> +err_pci_enable:
> +       pci_set_drvdata(pdev, NULL);
> +       return rc;
> +}
> +
> +static void amd_ntb_deinit_pci(struct amd_ntb_dev *ndev)
> +{
> +       struct pci_dev *pdev = ndev_pdev(ndev);
> +
> +       pci_iounmap(pdev, ndev->self_mmio);
> +
> +       pci_clear_master(pdev);
> +       pci_release_regions(pdev);
> +       pci_disable_device(pdev);
> +       pci_set_drvdata(pdev, NULL);
> +}
> +
> +
> +static int amd_ntb_pci_probe(struct pci_dev *pdev,
> +                            const struct pci_device_id *id)
> +{
> +       struct amd_ntb_dev *ndev;
> +       int rc, node;
> +
> +       node = dev_to_node(&pdev->dev);
> +
> +       ndev = kzalloc_node(sizeof(*ndev), GFP_KERNEL, node);
> +       if (!ndev) {
> +               rc = -ENOMEM;
> +               goto err_ndev;
> +       }
> +
> +       ndev_init_struct(ndev, pdev);
> +
> +       rc = amd_ntb_init_pci(ndev, pdev);
> +       if (rc)
> +               goto err_init_pci;
> +
> +       rc = amd_init_dev(ndev);
> +       if (rc)
> +               goto err_init_dev;
> +
> +       /* write side info */
> +       amd_init_side_info(ndev);
> +
> +       amd_poll_link(ndev);
> +
> +       ndev_init_debugfs(ndev);
> +
> +       rc = ntb_register_device(&ndev->ntb);
> +       if (rc)
> +               goto err_register;
> +
> +       dev_info(&pdev->dev, "NTB device registered.\n");
> +
> +       return 0;
> +
> +err_register:
> +       ndev_deinit_debugfs(ndev);
> +       amd_deinit_dev(ndev);
> +err_init_dev:
> +       amd_ntb_deinit_pci(ndev);
> +err_init_pci:
> +       kfree(ndev);
> +err_ndev:
> +       return rc;
> +}
> +
> +static void amd_ntb_pci_remove(struct pci_dev *pdev)
> +{
> +       struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
> +
> +       ntb_unregister_device(&ndev->ntb);
> +       ndev_deinit_debugfs(ndev);
> +       amd_deinit_side_info(ndev);
> +       amd_deinit_dev(ndev);
> +       amd_ntb_deinit_pci(ndev);
> +       kfree(ndev);
> +}
> +
> +static const struct file_operations amd_ntb_debugfs_info = {
> +       .owner = THIS_MODULE,
> +       .open = simple_open,
> +       .read = ndev_debugfs_read,
> +};
> +
> +static const struct pci_device_id amd_ntb_pci_tbl[] = {
> +       {PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_NTB)},
> +       {0}
> +};
> +MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
> +
> +#ifdef CONFIG_PM
> +static int amd_ntb_pci_device_suspend(struct pci_dev *pdev,
> +                               pm_message_t mesg)
> +{
> +       struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
> +       void __iomem *mmio = ndev->self_mmio;
> +
> +       /* flush pending IO */
> +       amd_flush_peer_requests(ndev);
> +
> +       /* link down */
> +       ndev->cntl_sta = 0;
> +       ntb_link_event(&ndev->ntb);
> +
> +       /* disable interrupt */
> +       ndev->int_mask |= AMD_EVENT_INTMASK;
> +       NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
> +       NTB_WRITE_REG(mmio, ndev->db_valid_mask, DBMASK);
> +
> +       /* save pcie state */
> +       pci_save_state(pdev);
> +       pci_disable_device(pdev);
> +       if (mesg.event & PM_EVENT_SLEEP)
> +               pci_set_power_state(pdev, PCI_D3hot);
> +
> +       return 0;
> +}
> +
> +static int amd_ntb_pci_device_resume(struct pci_dev *pdev)
> +{
> +       struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
> +       void __iomem *mmio = ndev->self_mmio;
> +       int rc;
> +
> +       pci_set_power_state(pdev, PCI_D0);
> +       pci_restore_state(pdev);
> +
> +       rc = pci_enable_device(pdev);
> +       if (rc) {
> +               dev_err(&pdev->dev, "failed to resume ntb device.\n");
> +               return rc;
> +       }
> +
> +       pci_set_master(pdev);
> +
> +       ndev->int_mask &= ~AMD_EVENT_INTMASK;
> +       NTB_WRITE_REG(mmio, ndev->int_mask, INTMASK);
> +
> +       amd_poll_link(ndev);
> +       ntb_link_event(&ndev->ntb);
> +
> +       return 0;
> +}
> +#endif
> +
> +static struct pci_driver amd_ntb_pci_driver = {
> +       .name           = KBUILD_MODNAME,
> +       .id_table       = amd_ntb_pci_tbl,
> +       .probe          = amd_ntb_pci_probe,
> +       .remove         = amd_ntb_pci_remove,
> +#ifdef CONFIG_PM
> +       .suspend        = amd_ntb_pci_device_suspend,
> +       .resume         = amd_ntb_pci_device_resume,
> +#endif
> +};
> +
> +static int __init amd_ntb_pci_driver_init(void)
> +{
> +       pr_info("%s %s\n", NTB_DESC, NTB_VER);
> +
> +       if (debugfs_initialized())
> +               debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
> +
> +       return pci_register_driver(&amd_ntb_pci_driver);
> +}
> +module_init(amd_ntb_pci_driver_init);
> +
> +static void __exit amd_ntb_pci_driver_exit(void)
> +{
> +       pci_unregister_driver(&amd_ntb_pci_driver);
> +       debugfs_remove_recursive(debugfs_dir);
> +}
> +module_exit(amd_ntb_pci_driver_exit);
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h
> new file mode 100644
> index 0000000..6005040
> --- /dev/null
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
> @@ -0,0 +1,263 @@
> +/*
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + *
> + *   GPL LICENSE SUMMARY
> + *
> + *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + *
> + *   BSD LICENSE
> + *
> + *   Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copy
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of AMD Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * AMD PCIe NTB Linux driver
> + *
> + * Contact Information:
> + * Xiangliang Yu<Xiangliang.Yu@amd.com>
> + */
> +
> +#ifndef NTB_HW_AMD_H
> +#define NTB_HW_AMD_H
> +
> +#include <linux/ntb.h>
> +#include <linux/pci.h>
> +
> +#define        PCI_DEVICE_ID_AMD_NTB   0x145B

This looks like a tab and not a space

> +#define AMD_LINK_HB_TIMEOUT    msecs_to_jiffies(1000)
> +#define AMD_LINK_STATUS_OFFSET 0x68
> +#define NTB_LIN_STA_ACTIVE_BIT 0x00000002
> +#define NTB_LNK_STA_SPEED_MASK 0x000F0000
> +#define NTB_LNK_STA_WIDTH_MASK 0x03F00000
> +#define NTB_LNK_STA_ACTIVE(x)  (!!((x) & NTB_LIN_STA_ACTIVE_BIT))
> +#define NTB_LNK_STA_SPEED(x)   (((x) & NTB_LNK_STA_SPEED_MASK) >> 16)
> +#define NTB_LNK_STA_WIDTH(x)   (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20)
> +
> +#ifndef ioread64
> +#ifdef readq
> +#define ioread64 readq
> +#else
> +#define ioread64 _ioread64
> +static inline u64 _ioread64(void __iomem *mmio)
> +{
> +       u64 low, high;
> +
> +       low = ioread32(mmio);
> +       high = ioread32(mmio + sizeof(u32));
> +       return low | (high << 32);
> +}
> +#endif
> +#endif
> +
> +#ifndef iowrite64
> +#ifdef writeq
> +#define iowrite64 writeq
> +#else
> +#define iowrite64 _iowrite64
> +static inline void _iowrite64(u64 val, void __iomem *mmio)
> +{
> +       iowrite32(val, mmio);
> +       iowrite32(val >> 32, mmio + sizeof(u32));
> +}
> +#endif
> +#endif
>
> +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ## _OFFSET))
> +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> +                                               AMD_ ## r ## _OFFSET))
> +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +             \
> +                                               AMD_ ## r ## _OFFSET))
> +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +      \
> +                                               of + AMD_ ## r ## _OFFSET))

Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite directly.

> +
> +#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
> +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
> +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
> +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> +#define hb_ndev(work) container_of(work, struct amd_ntb_dev, hb_timer.work)
> +#define ntb_hotplug_ndev(context) (container_of((context),     \
> +                       struct ntb_acpi_hotplug_context, hp)->ndev)

Seems like these are hiding things too.  Please use them directly (or
at least put them in the C file and not the header file).

> +
> +enum {
> +       /* AMD NTB Capability */
> +       AMD_MW_CNT              = 3,
> +       AMD_DB_CNT              = 16,
> +       AMD_MSIX_VECTOR_CNT     = 24,
> +       AMD_SPADS_CNT           = 16,
> +
> +       /*  AMD NTB register offset */
> +       AMD_CNTL_OFFSET         = 0x200,
> +
> +       /* NTB control register bits */
> +       PMM_REG_CTL             = BIT(21),
> +       SMM_REG_CTL             = BIT(20),
> +       SMM_REG_ACC_PATH        = BIT(18),
> +       PMM_REG_ACC_PATH        = BIT(17),
> +       NTB_CLK_EN              = BIT(16),
> +
> +       AMD_STA_OFFSET          = 0x204,
> +       AMD_PGSLV_OFFSET        = 0x208,
> +       AMD_SPAD_MUX_OFFSET     = 0x20C,
> +       AMD_SPAD_OFFSET         = 0x210,
> +       AMD_RSMU_HCID           = 0x250,
> +       AMD_RSMU_SIID           = 0x254,
> +       AMD_PSION_OFFSET        = 0x300,
> +       AMD_SSION_OFFSET        = 0x330,
> +       AMD_MMINDEX_OFFSET      = 0x400,
> +       AMD_MMDATA_OFFSET       = 0x404,
> +       AMD_SIDEINFO_OFFSET     = 0x408,
> +
> +       AMD_SIDE_MASK           = BIT(0),
> +
> +       /* limit register */
> +       AMD_ROMBARLMT_OFFSET    = 0x410,
> +       AMD_BAR1LMT_OFFSET      = 0x414,
> +       AMD_BAR23LMT_OFFSET     = 0x418,
> +       AMD_BAR45LMT_OFFSET     = 0x420,
> +       /* xlat address */
> +       AMD_POMBARXLAT_OFFSET   = 0x428,
> +       AMD_BAR1XLAT_OFFSET     = 0x430,
> +       AMD_BAR23XLAT_OFFSET    = 0x438,
> +       AMD_BAR45XLAT_OFFSET    = 0x440,
> +       /* doorbell and interrupt */
> +       AMD_DBFM_OFFSET         = 0x450,
> +       AMD_DBREQ_OFFSET        = 0x454,
> +       AMD_MIRRDBSTAT_OFFSET   = 0x458,
> +       AMD_DBMASK_OFFSET       = 0x45C,
> +       AMD_DBSTAT_OFFSET       = 0x460,
> +       AMD_INTMASK_OFFSET      = 0x470,
> +       AMD_INTSTAT_OFFSET      = 0x474,
> +
> +       /* event type */
> +       AMD_PEER_FLUSH_EVENT    = BIT(0),
> +       AMD_PEER_RESET_EVENT    = BIT(1),
> +       AMD_PEER_D3_EVENT       = BIT(2),
> +       AMD_PEER_PMETO_EVENT    = BIT(3),
> +       AMD_PEER_D0_EVENT       = BIT(4),
> +       AMD_EVENT_INTMASK       = (AMD_PEER_FLUSH_EVENT |
> +                               AMD_PEER_RESET_EVENT | AMD_PEER_D3_EVENT |
> +                               AMD_PEER_PMETO_EVENT | AMD_PEER_D0_EVENT),
> +
> +       AMD_PMESTAT_OFFSET      = 0x480,
> +       AMD_PMSGTRIG_OFFSET     = 0x490,
> +       AMD_LTRLATENCY_OFFSET   = 0x494,
> +       AMD_FLUSHTRIG_OFFSET    = 0x498,
> +
> +       /* SMU register*/
> +       AMD_SMUACK_OFFSET       = 0x4A0,
> +       AMD_SINRST_OFFSET       = 0x4A4,
> +       AMD_RSPNUM_OFFSET       = 0x4A8,
> +       AMD_SMU_SPADMUTEX       = 0x4B0,
> +       AMD_SMU_SPADOFFSET      = 0x4B4,
> +
> +       AMD_PEER_OFFSET         = 0x400,
> +};
> +
> +struct amd_ntb_dev;
> +
> +struct amd_ntb_vec {
> +       struct amd_ntb_dev      *ndev;
> +       int                     num;
> +};
> +
> +struct amd_ntb_dev {
> +       struct ntb_dev ntb;
> +
> +       u32 ntb_side;
> +       u32 lnk_sta;
> +       u32 cntl_sta;
> +       u32 peer_sta;
> +
> +       unsigned char mw_count;
> +       unsigned char spad_count;
> +       unsigned char db_count;
> +       unsigned char msix_vec_count;
> +
> +       u64 db_valid_mask;
> +       u64 db_mask;
> +       u32 int_mask;
> +
> +       struct msix_entry *msix;
> +       struct amd_ntb_vec *vec;
> +
> +       spinlock_t db_mask_lock;
> +
> +       void __iomem *self_mmio;
> +       void __iomem *peer_mmio;
> +       unsigned long peer_spad;
> +
> +       struct completion flush_cmpl;
> +       struct completion wakeup_cmpl;
> +
> +       struct delayed_work hb_timer;
> +
> +       struct dentry *debugfs_dir;
> +       struct dentry *debugfs_info;
> +};
> +
> +struct ntb_acpi_hotplug_context {
> +       struct acpi_hotplug_context hp;
> +       struct amd_ntb_dev *ndev;
> +};
> +
> +static int amd_ntb_mw_count(struct ntb_dev *ntb);
> +static int amd_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
> +                       phys_addr_t *base, resource_size_t *size,
> +                       resource_size_t *align, resource_size_t *algin_size);
> +static int amd_ntb_mw_set_trans(struct ntb_dev *ndev, int idx,
> +                               dma_addr_t addr, resource_size_t size);
> +static int amd_ntb_link_is_up(struct ntb_dev *ntb,
> +                               enum ntb_speed *speed,
> +                               enum ntb_width *width);
> +static int amd_ntb_link_enable(struct ntb_dev *ntb,
> +                               enum ntb_speed speed,
> +                               enum ntb_width width);
> +static int amd_ntb_link_disable(struct ntb_dev *ntb);
> +static u64 amd_ntb_db_valid_mask(struct ntb_dev *ntb);
> +static int amd_ntb_db_vector_count(struct ntb_dev *ntb);
> +static u64 amd_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector);
> +static u64 amd_ntb_db_read(struct ntb_dev *ntb);
> +static int amd_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits);
> +static int amd_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits);
> +static int amd_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits);
> +static int amd_ntb_peer_db_addr(struct ntb_dev *ntb,
> +                               phys_addr_t *db_addr,
> +                               resource_size_t *db_size);
> +static int amd_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits);
> +static int amd_ntb_spad_count(struct ntb_dev *ntb);
> +static u32 amd_ntb_spad_read(struct ntb_dev *ntb, int idx);
> +static int amd_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val);
> +static int amd_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
> +                                       phys_addr_t *spad_addr);
> +static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx);
> +static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int idx, u32 val);
> +#endif
> --
> 1.9.1


Minor changes suggested above.  Glad to see more HW developers are
making NTB products.

Thanks,
Jon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-06 15:05 ` Jon Mason
@ 2016-01-06 16:52     ` Hubbe, Allen
  2016-01-07  2:50     ` Yu, Xiangliang
  2016-01-08  3:14   ` Yu, Xiangliang
  2 siblings, 0 replies; 18+ messages in thread
From: Hubbe, Allen @ 2016-01-06 16:52 UTC (permalink / raw)
  To: Jon Mason, Xiangliang Yu
  Cc: Dave Jiang, linux-ntb, linux-kernel, SPG_Linux_Kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1708 bytes --]

From: Jon Mason <jdmason@kudzu.us>:
> On Wed, Dec 23, 2015 at 8:42 AM, Xiangliang Yu <Xiangliang.Yu@amd.com>
> wrote:

> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
> > +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
> > +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> hb_timer.work)
> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
> 
> Seems like these are hiding things too.  Please use them directly (or
> at least put them in the C file and not the header file).

I like these macros for up/down casting.  Putting them close to the structure definition seems appropriate to me, too.  I would rather see them moved to right below the definition of struct amd_ntb_dev, instead of to the c file.  That is my opinion, but Jon can make the final decision on it.

However, these in particular are buggy:

> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> hb_timer.work)

Note: "ntb" will be replaced in all occurrences to the right.  This only works if the name "ntb" is passed as the argument.  If the argument is named "foo", it will either fail at compile time to find the member "foo" in struct amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of the struct.

Rename the macro parameter __ntb.

Allen
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2016-01-06 16:52     ` Hubbe, Allen
  0 siblings, 0 replies; 18+ messages in thread
From: Hubbe, Allen @ 2016-01-06 16:52 UTC (permalink / raw)
  To: Jon Mason, Xiangliang Yu
  Cc: Dave Jiang, linux-ntb, linux-kernel, SPG_Linux_Kernel

From: Jon Mason <jdmason@kudzu.us>:
> On Wed, Dec 23, 2015 at 8:42 AM, Xiangliang Yu <Xiangliang.Yu@amd.com>
> wrote:

> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
> > +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
> > +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> hb_timer.work)
> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
> 
> Seems like these are hiding things too.  Please use them directly (or
> at least put them in the C file and not the header file).

I like these macros for up/down casting.  Putting them close to the structure definition seems appropriate to me, too.  I would rather see them moved to right below the definition of struct amd_ntb_dev, instead of to the c file.  That is my opinion, but Jon can make the final decision on it.

However, these in particular are buggy:

> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> hb_timer.work)

Note: "ntb" will be replaced in all occurrences to the right.  This only works if the name "ntb" is passed as the argument.  If the argument is named "foo", it will either fail at compile time to find the member "foo" in struct amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of the struct.

Rename the macro parameter __ntb.

Allen

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-06 16:52     ` Hubbe, Allen
  (?)
@ 2016-01-06 17:48     ` Jon Mason
  2016-01-07  2:53         ` Yu, Xiangliang
  -1 siblings, 1 reply; 18+ messages in thread
From: Jon Mason @ 2016-01-06 17:48 UTC (permalink / raw)
  To: Hubbe, Allen
  Cc: Xiangliang Yu, Dave Jiang, linux-ntb, linux-kernel, SPG_Linux_Kernel

On Wed, Jan 6, 2016 at 11:52 AM, Hubbe, Allen <Allen.Hubbe@emc.com> wrote:
> From: Jon Mason <jdmason@kudzu.us>:
>> On Wed, Dec 23, 2015 at 8:42 AM, Xiangliang Yu <Xiangliang.Yu@amd.com>
>> wrote:
>
>> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev)
>> > +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
>> > +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
>> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
>> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
>> hb_timer.work)
>> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
>> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
>>
>> Seems like these are hiding things too.  Please use them directly (or
>> at least put them in the C file and not the header file).
>
> I like these macros for up/down casting.  Putting them close to the structure definition seems appropriate to me, too.  I would rather see them moved to right below the definition of struct amd_ntb_dev, instead of to the c file.  That is my opinion, but Jon can make the final decision on it.

My opinion wasn't super strong on these.  If Allen is fine with them,
then good enough for me :)


> However, these in particular are buggy:
>
>> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
>> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
>> hb_timer.work)
>
> Note: "ntb" will be replaced in all occurrences to the right.  This only works if the name "ntb" is passed as the argument.  If the argument is named "foo", it will either fail at compile time to find the member "foo" in struct amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of the struct.
>
> Rename the macro parameter __ntb.

Good call.  Please make the necessary mods Xiangliang.

Thanks,
Jon


> Allen
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/40F65EF2B5E2254199711F58E3ACB84D99EF72E1%40MX104CL02.corp.emc.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-06 15:05 ` Jon Mason
@ 2016-01-07  2:50     ` Yu, Xiangliang
  2016-01-07  2:50     ` Yu, Xiangliang
  2016-01-08  3:14   ` Yu, Xiangliang
  2 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-07  2:50 UTC (permalink / raw)
  To: Jon Mason
  Cc: Dave Jiang, Hubbe, Allen, linux-ntb, linux-kernel, SPG_Linux_Kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2273 bytes --]

> > +
> > +#define        PCI_DEVICE_ID_AMD_NTB   0x145B
> 
> This looks like a tab and not a space

I'll update it.

> 
> > +#define AMD_LINK_HB_TIMEOUT    msecs_to_jiffies(1000)
> > +#define AMD_LINK_STATUS_OFFSET 0x68
> > +#define NTB_LIN_STA_ACTIVE_BIT 0x00000002
> > +#define NTB_LNK_STA_SPEED_MASK 0x000F0000
> > +#define NTB_LNK_STA_WIDTH_MASK 0x03F00000
> > +#define NTB_LNK_STA_ACTIVE(x)  (!!((x) & NTB_LIN_STA_ACTIVE_BIT))
> > +#define NTB_LNK_STA_SPEED(x)   (((x) & NTB_LNK_STA_SPEED_MASK) >>
> 16)
> > +#define NTB_LNK_STA_WIDTH(x)   (((x) &
> NTB_LNK_STA_WIDTH_MASK) >> 20)
> > +
> > +#ifndef ioread64
> > +#ifdef readq
> > +#define ioread64 readq
> > +#else
> > +#define ioread64 _ioread64
> > +static inline u64 _ioread64(void __iomem *mmio)
> > +{
> > +       u64 low, high;
> > +
> > +       low = ioread32(mmio);
> > +       high = ioread32(mmio + sizeof(u32));
> > +       return low | (high << 32);
> > +}
> > +#endif
> > +#endif
> > +
> > +#ifndef iowrite64
> > +#ifdef writeq
> > +#define iowrite64 writeq
> > +#else
> > +#define iowrite64 _iowrite64
> > +static inline void _iowrite64(u64 val, void __iomem *mmio)
> > +{
> > +       iowrite32(val, mmio);
> > +       iowrite32(val >> 32, mmio + sizeof(u32));
> > +}
> > +#endif
> > +#endif
> >

How about put ioread64/iowrite64 macro into ntb.h file? So low level driver can avoid to define these macro.

> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> _OFFSET))
> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> > +                                               AMD_ ## r ## _OFFSET))
> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +             \
> > +                                               AMD_ ## r ## _OFFSET))
> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +      \
> > +                                               of + AMD_ ## r ## _OFFSET))
> 
> Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite directly.

I don't see any wrong to hide ioread/iowrite, and I think the macros can make code readable and easy to maintain. 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2016-01-07  2:50     ` Yu, Xiangliang
  0 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-07  2:50 UTC (permalink / raw)
  To: Jon Mason
  Cc: Dave Jiang, Hubbe, Allen, linux-ntb, linux-kernel, SPG_Linux_Kernel

> > +
> > +#define        PCI_DEVICE_ID_AMD_NTB   0x145B
> 
> This looks like a tab and not a space

I'll update it.

> 
> > +#define AMD_LINK_HB_TIMEOUT    msecs_to_jiffies(1000)
> > +#define AMD_LINK_STATUS_OFFSET 0x68
> > +#define NTB_LIN_STA_ACTIVE_BIT 0x00000002
> > +#define NTB_LNK_STA_SPEED_MASK 0x000F0000
> > +#define NTB_LNK_STA_WIDTH_MASK 0x03F00000
> > +#define NTB_LNK_STA_ACTIVE(x)  (!!((x) & NTB_LIN_STA_ACTIVE_BIT))
> > +#define NTB_LNK_STA_SPEED(x)   (((x) & NTB_LNK_STA_SPEED_MASK) >>
> 16)
> > +#define NTB_LNK_STA_WIDTH(x)   (((x) &
> NTB_LNK_STA_WIDTH_MASK) >> 20)
> > +
> > +#ifndef ioread64
> > +#ifdef readq
> > +#define ioread64 readq
> > +#else
> > +#define ioread64 _ioread64
> > +static inline u64 _ioread64(void __iomem *mmio)
> > +{
> > +       u64 low, high;
> > +
> > +       low = ioread32(mmio);
> > +       high = ioread32(mmio + sizeof(u32));
> > +       return low | (high << 32);
> > +}
> > +#endif
> > +#endif
> > +
> > +#ifndef iowrite64
> > +#ifdef writeq
> > +#define iowrite64 writeq
> > +#else
> > +#define iowrite64 _iowrite64
> > +static inline void _iowrite64(u64 val, void __iomem *mmio)
> > +{
> > +       iowrite32(val, mmio);
> > +       iowrite32(val >> 32, mmio + sizeof(u32));
> > +}
> > +#endif
> > +#endif
> >

How about put ioread64/iowrite64 macro into ntb.h file? So low level driver can avoid to define these macro.

> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> _OFFSET))
> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> > +                                               AMD_ ## r ## _OFFSET))
> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +             \
> > +                                               AMD_ ## r ## _OFFSET))
> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +      \
> > +                                               of + AMD_ ## r ## _OFFSET))
> 
> Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite directly.

I don't see any wrong to hide ioread/iowrite, and I think the macros can make code readable and easy to maintain. 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-06 17:48     ` Jon Mason
@ 2016-01-07  2:53         ` Yu, Xiangliang
  0 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-07  2:53 UTC (permalink / raw)
  To: Jon Mason, Hubbe, Allen
  Cc: Dave Jiang, linux-ntb, linux-kernel, SPG_Linux_Kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1871 bytes --]

> >
> >> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev) #define
> ndev_name(ndev)
> >> > +pci_name(ndev_pdev(ndev)) #define ndev_dev(ndev)
> >> > +(&ndev_pdev(ndev)->dev) #define ntb_ndev(ntb) container_of(ntb,
> >> > +struct amd_ntb_dev, ntb) #define hb_ndev(work) container_of(work,
> >> > +struct amd_ntb_dev,
> >> hb_timer.work)
> >> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
> >> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
> >>
> >> Seems like these are hiding things too.  Please use them directly (or
> >> at least put them in the C file and not the header file).
> >
> > I like these macros for up/down casting.  Putting them close to the
> structure definition seems appropriate to me, too.  I would rather see them
> moved to right below the definition of struct amd_ntb_dev, instead of to the
> c file.  That is my opinion, but Jon can make the final decision on it.
> 
> My opinion wasn't super strong on these.  If Allen is fine with them, then
> good enough for me :)

Agree with Allen's opinion.

> 
> > However, these in particular are buggy:
> >
> >> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> >> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> >> hb_timer.work)
> >
> > Note: "ntb" will be replaced in all occurrences to the right.  This only works
> if the name "ntb" is passed as the argument.  If the argument is named "foo",
> it will either fail at compile time to find the member "foo" in struct
> amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of
> the struct.
> >
> > Rename the macro parameter __ntb.
> 
> Good call.  Please make the necessary mods Xiangliang.

Ok
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2016-01-07  2:53         ` Yu, Xiangliang
  0 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-07  2:53 UTC (permalink / raw)
  To: Jon Mason, Hubbe, Allen
  Cc: Dave Jiang, linux-ntb, linux-kernel, SPG_Linux_Kernel

> >
> >> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev) #define
> ndev_name(ndev)
> >> > +pci_name(ndev_pdev(ndev)) #define ndev_dev(ndev)
> >> > +(&ndev_pdev(ndev)->dev) #define ntb_ndev(ntb) container_of(ntb,
> >> > +struct amd_ntb_dev, ntb) #define hb_ndev(work) container_of(work,
> >> > +struct amd_ntb_dev,
> >> hb_timer.work)
> >> > +#define ntb_hotplug_ndev(context) (container_of((context),     \
> >> > +                       struct ntb_acpi_hotplug_context, hp)->ndev)
> >>
> >> Seems like these are hiding things too.  Please use them directly (or
> >> at least put them in the C file and not the header file).
> >
> > I like these macros for up/down casting.  Putting them close to the
> structure definition seems appropriate to me, too.  I would rather see them
> moved to right below the definition of struct amd_ntb_dev, instead of to the
> c file.  That is my opinion, but Jon can make the final decision on it.
> 
> My opinion wasn't super strong on these.  If Allen is fine with them, then
> good enough for me :)

Agree with Allen's opinion.

> 
> > However, these in particular are buggy:
> >
> >> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb)
> >> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev,
> >> hb_timer.work)
> >
> > Note: "ntb" will be replaced in all occurrences to the right.  This only works
> if the name "ntb" is passed as the argument.  If the argument is named "foo",
> it will either fail at compile time to find the member "foo" in struct
> amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of
> the struct.
> >
> > Rename the macro parameter __ntb.
> 
> Good call.  Please make the necessary mods Xiangliang.

Ok

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-06 15:05 ` Jon Mason
  2016-01-06 16:52     ` Hubbe, Allen
  2016-01-07  2:50     ` Yu, Xiangliang
@ 2016-01-08  3:14   ` Yu, Xiangliang
  2 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-08  3:14 UTC (permalink / raw)
  To: Jon Mason
  Cc: Dave Jiang, Hubbe, Allen, linux-ntb, linux-kernel, SPG_Linux_Kernel

Hi Jon,

I find there are lots of macros to hide ioread/iowrite in kernel, I think it is reasonable. 
What is your concern?

> > > +
> > > +#define        PCI_DEVICE_ID_AMD_NTB   0x145B
> >
> > This looks like a tab and not a space
> 
> I'll update it.
> 
> >
> > > +#define AMD_LINK_HB_TIMEOUT    msecs_to_jiffies(1000)
> > > +#define AMD_LINK_STATUS_OFFSET 0x68 #define
> NTB_LIN_STA_ACTIVE_BIT
> > > +0x00000002 #define NTB_LNK_STA_SPEED_MASK 0x000F0000 #define
> > > +NTB_LNK_STA_WIDTH_MASK 0x03F00000 #define
> NTB_LNK_STA_ACTIVE(x)
> > > +(!!((x) & NTB_LIN_STA_ACTIVE_BIT))
> > > +#define NTB_LNK_STA_SPEED(x)   (((x) &
> NTB_LNK_STA_SPEED_MASK) >>
> > 16)
> > > +#define NTB_LNK_STA_WIDTH(x)   (((x) &
> > NTB_LNK_STA_WIDTH_MASK) >> 20)
> > > +
> > > +#ifndef ioread64
> > > +#ifdef readq
> > > +#define ioread64 readq
> > > +#else
> > > +#define ioread64 _ioread64
> > > +static inline u64 _ioread64(void __iomem *mmio) {
> > > +       u64 low, high;
> > > +
> > > +       low = ioread32(mmio);
> > > +       high = ioread32(mmio + sizeof(u32));
> > > +       return low | (high << 32);
> > > +}
> > > +#endif
> > > +#endif
> > > +
> > > +#ifndef iowrite64
> > > +#ifdef writeq
> > > +#define iowrite64 writeq
> > > +#else
> > > +#define iowrite64 _iowrite64
> > > +static inline void _iowrite64(u64 val, void __iomem *mmio) {
> > > +       iowrite32(val, mmio);
> > > +       iowrite32(val >> 32, mmio + sizeof(u32)); } #endif #endif
> > >
> 
> How about put ioread64/iowrite64 macro into ntb.h file? So low level driver
> can avoid to define these macro.
> 
> > > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> > _OFFSET))
> > > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> > > +                                               AMD_ ## r ## _OFFSET))
> > > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +             \
> > > +                                               AMD_ ## r ## _OFFSET))
> > > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +      \
> > > +                                               of + AMD_ ## r ##
> > > +_OFFSET))
> >
> > Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite
> directly.
> 
> I don't see any wrong to hide ioread/iowrite, and I think the macros can
> make code readable and easy to maintain.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-07  2:50     ` Yu, Xiangliang
  (?)
@ 2016-01-08 15:01     ` Jon Mason
  2016-01-08 15:39         ` Allen Hubbe
  -1 siblings, 1 reply; 18+ messages in thread
From: Jon Mason @ 2016-01-08 15:01 UTC (permalink / raw)
  To: Yu, Xiangliang
  Cc: Dave Jiang, Hubbe, Allen, linux-ntb, linux-kernel, SPG_Linux_Kernel

On Wed, Jan 6, 2016 at 9:50 PM, Yu, Xiangliang <Xiangliang.Yu@amd.com> wrote:
>> > +
>> > +#define        PCI_DEVICE_ID_AMD_NTB   0x145B
>>
>> This looks like a tab and not a space
>
> I'll update it.
>
>>
>> > +#define AMD_LINK_HB_TIMEOUT    msecs_to_jiffies(1000)
>> > +#define AMD_LINK_STATUS_OFFSET 0x68
>> > +#define NTB_LIN_STA_ACTIVE_BIT 0x00000002
>> > +#define NTB_LNK_STA_SPEED_MASK 0x000F0000
>> > +#define NTB_LNK_STA_WIDTH_MASK 0x03F00000
>> > +#define NTB_LNK_STA_ACTIVE(x)  (!!((x) & NTB_LIN_STA_ACTIVE_BIT))
>> > +#define NTB_LNK_STA_SPEED(x)   (((x) & NTB_LNK_STA_SPEED_MASK) >>
>> 16)
>> > +#define NTB_LNK_STA_WIDTH(x)   (((x) &
>> NTB_LNK_STA_WIDTH_MASK) >> 20)
>> > +
>> > +#ifndef ioread64
>> > +#ifdef readq
>> > +#define ioread64 readq
>> > +#else
>> > +#define ioread64 _ioread64
>> > +static inline u64 _ioread64(void __iomem *mmio)
>> > +{
>> > +       u64 low, high;
>> > +
>> > +       low = ioread32(mmio);
>> > +       high = ioread32(mmio + sizeof(u32));
>> > +       return low | (high << 32);
>> > +}
>> > +#endif
>> > +#endif
>> > +
>> > +#ifndef iowrite64
>> > +#ifdef writeq
>> > +#define iowrite64 writeq
>> > +#else
>> > +#define iowrite64 _iowrite64
>> > +static inline void _iowrite64(u64 val, void __iomem *mmio)
>> > +{
>> > +       iowrite32(val, mmio);
>> > +       iowrite32(val >> 32, mmio + sizeof(u32));
>> > +}
>> > +#endif
>> > +#endif
>> >
>
> How about put ioread64/iowrite64 macro into ntb.h file? So low level driver can avoid to define these macro.
>
>> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
>> _OFFSET))
>> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
>> > +                                               AMD_ ## r ## _OFFSET))
>> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +             \
>> > +                                               AMD_ ## r ## _OFFSET))
>> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +      \
>> > +                                               of + AMD_ ## r ## _OFFSET))
>>
>> Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite directly.
>
> I don't see any wrong to hide ioread/iowrite, and I think the macros can make code readable and easy to maintain.

I disagree.  It is an unnecessary layer and can add to confusion.
Please make the change.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-08 15:01     ` Jon Mason
@ 2016-01-08 15:39         ` Allen Hubbe
  0 siblings, 0 replies; 18+ messages in thread
From: Allen Hubbe @ 2016-01-08 15:39 UTC (permalink / raw)
  To: 'Jon Mason', 'Yu, Xiangliang'
  Cc: 'Dave Jiang', linux-ntb, 'linux-kernel',
	'SPG_Linux_Kernel'

From: Jon Mason <jdmason@kudzu.us>
> On Wed, Jan 6, 2016 at 9:50 PM, Yu, Xiangliang <Xiangliang.Yu@amd.com>
> wrote:
> >> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> >> _OFFSET))
> >> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> >> > +                                               AMD_ ## r ##
> _OFFSET))
> >> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +
> \
> >> > +                                               AMD_ ## r ##
> _OFFSET))
> >> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +
> \
> >> > +                                               of + AMD_ ## r ##
> _OFFSET))
> >>
> >> Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite
> directly.
> >
> > I don't see any wrong to hide ioread/iowrite, and I think the macros
> can make code readable and easy to maintain.
> 
> I disagree.  It is an unnecessary layer and can add to confusion.
> Please make the change.

I don't like AMD_##r##_OFFSET in these macros.  It hides the use of a globally named constant like AMD_FOO_OFFSET, since one would read only FOO in the code.  It makes cross referencing difficult, since the reader needs to know FOO is really AMD_FOO_OFFSET.  This would defeat automatic cross referencing like cscope and lxr.

#define AMD_FOO_OFFSET 0xc0ff33
vs
NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo)
// Where is FOO defined?

This macro would have at least been better written without ##; so cross referencing would still work.

NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo)
vs
NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo)
// AMD_FOO_OFFSET is 0xcoff33 (obviously)

But without ##, the macro is just the addition of its parameters.  Change the commas to addition, and the macro to ioread, and you'll see there is no benefit for having this macro any more.

NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo)
vs
ioread32(dev->foo_base + AMD_FOO_OFFSET + offset_in_foo)

I second Jon's opinion.  Please make the change.  This would be better as simply ioread/write in the code.

Allen

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
@ 2016-01-08 15:39         ` Allen Hubbe
  0 siblings, 0 replies; 18+ messages in thread
From: Allen Hubbe @ 2016-01-08 15:39 UTC (permalink / raw)
  To: 'Jon Mason', 'Yu, Xiangliang'
  Cc: 'Dave Jiang', linux-ntb, 'linux-kernel',
	'SPG_Linux_Kernel'

From: Jon Mason <jdmason@kudzu.us>
> On Wed, Jan 6, 2016 at 9:50 PM, Yu, Xiangliang <Xiangliang.Yu@amd.com>
> wrote:
> >> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> >> _OFFSET))
> >> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> >> > +                                               AMD_ ## r ##
> _OFFSET))
> >> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +
> \
> >> > +                                               AMD_ ## r ##
> _OFFSET))
> >> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base +
> \
> >> > +                                               of + AMD_ ## r ##
> _OFFSET))
> >>
> >> Please do not use marcos to hide ioread/iowrite.  Call iorwad/iowrite
> directly.
> >
> > I don't see any wrong to hide ioread/iowrite, and I think the macros
> can make code readable and easy to maintain.
> 
> I disagree.  It is an unnecessary layer and can add to confusion.
> Please make the change.

I don't like AMD_##r##_OFFSET in these macros.  It hides the use of a globally named constant like AMD_FOO_OFFSET, since one would read only FOO in the code.  It makes cross referencing difficult, since the reader needs to know FOO is really AMD_FOO_OFFSET.  This would defeat automatic cross referencing like cscope and lxr.

#define AMD_FOO_OFFSET 0xc0ff33
vs
NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo)
// Where is FOO defined?

This macro would have at least been better written without ##; so cross referencing would still work.

NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo)
vs
NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo)
// AMD_FOO_OFFSET is 0xcoff33 (obviously)

But without ##, the macro is just the addition of its parameters.  Change the commas to addition, and the macro to ioread, and you'll see there is no benefit for having this macro any more.

NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo)
vs
ioread32(dev->foo_base + AMD_FOO_OFFSET + offset_in_foo)

I second Jon's opinion.  Please make the change.  This would be better as simply ioread/write in the code.

Allen


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver
  2016-01-08 15:39         ` Allen Hubbe
  (?)
@ 2016-01-11  7:07         ` Yu, Xiangliang
  -1 siblings, 0 replies; 18+ messages in thread
From: Yu, Xiangliang @ 2016-01-11  7:07 UTC (permalink / raw)
  To: Allen Hubbe, 'Jon Mason'
  Cc: 'Dave Jiang', linux-ntb, 'linux-kernel',
	SPG_Linux_Kernel

> From: Jon Mason <jdmason@kudzu.us>
> > On Wed, Jan 6, 2016 at 9:50 PM, Yu, Xiangliang <Xiangliang.Yu@amd.com>
> > wrote:
> > >> > +#define NTB_READ_REG(base, r) (ioread32(base + AMD_ ## r ##
> > >> _OFFSET))
> > >> > +#define NTB_WRITE_REG(base, val, r) (iowrite32(val, base +     \
> > >> > +                                               AMD_ ## r ##
> > _OFFSET))
> > >> > +#define NTB_READ_OFFSET(base, r, of) (ioread32(base + of +
> > \
> > >> > +                                               AMD_ ## r ##
> > _OFFSET))
> > >> > +#define NTB_WRITE_OFFSET(base, val, r, of) (iowrite32(val, base
> > >> > ++
> > \
> > >> > +                                               of + AMD_ ## r ##
> > _OFFSET))
> > >>
> > >> Please do not use marcos to hide ioread/iowrite.  Call
> > >> iorwad/iowrite
> > directly.
> > >
> > > I don't see any wrong to hide ioread/iowrite, and I think the macros
> > can make code readable and easy to maintain.
> >
> > I disagree.  It is an unnecessary layer and can add to confusion.
> > Please make the change.
> 
> I don't like AMD_##r##_OFFSET in these macros.  It hides the use of a
> globally named constant like AMD_FOO_OFFSET, since one would read only
> FOO in the code.  It makes cross referencing difficult, since the reader needs
> to know FOO is really AMD_FOO_OFFSET.  This would defeat automatic cross
> referencing like cscope and lxr.
> 
> #define AMD_FOO_OFFSET 0xc0ff33
> vs
> NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo) // Where is FOO
> defined?
> 
> This macro would have at least been better written without ##; so cross
> referencing would still work.
> 
> NTB_READ_OFFSET(dev->foo_base, FOO, offset_in_foo) vs
> NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo) //
> AMD_FOO_OFFSET is 0xcoff33 (obviously)
> 
> But without ##, the macro is just the addition of its parameters.  Change the
> commas to addition, and the macro to ioread, and you'll see there is no
> benefit for having this macro any more.
> 
> NTB_READ_OFFSET(dev->foo_base, AMD_FOO_OFFSET, offset_in_foo) vs
> ioread32(dev->foo_base + AMD_FOO_OFFSET + offset_in_foo)
> 
> I second Jon's opinion.  Please make the change.  This would be better as
> simply ioread/write in the code.

The main aim of these macros is to focuses on the name of register (FOO), 
not the whole macros of register. 
and I see there are lots of these style in kernel.
If your guys still can't accept the style, I can change it. 
And I'll change ioread/iowrite to readl/writel because it only access mmio space.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-01-11  7:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23 13:42 [PATCH V2 1/3] NTB: Add AMD PCI-Express NTB driver Xiangliang Yu
2015-12-23 13:42 ` Xiangliang Yu
2015-12-23  9:29 ` Christoph Hellwig
2015-12-23 15:09   ` Allen Hubbe
2015-12-23 15:51     ` Raghu
2016-01-06 15:05 ` Jon Mason
2016-01-06 16:52   ` Hubbe, Allen
2016-01-06 16:52     ` Hubbe, Allen
2016-01-06 17:48     ` Jon Mason
2016-01-07  2:53       ` Yu, Xiangliang
2016-01-07  2:53         ` Yu, Xiangliang
2016-01-07  2:50   ` Yu, Xiangliang
2016-01-07  2:50     ` Yu, Xiangliang
2016-01-08 15:01     ` Jon Mason
2016-01-08 15:39       ` Allen Hubbe
2016-01-08 15:39         ` Allen Hubbe
2016-01-11  7:07         ` Yu, Xiangliang
2016-01-08  3:14   ` Yu, Xiangliang

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.