AMD ntb driver fixes and improvements

PATCH In review Arindam Nath 3 participants 20 messages Feb 5, 2020 – Mar 30, 2020

Subsystems (2)

NTB driver core NTB AMD driver

lore

Share & tools

lore

Arindam Nath

author

2020-02-05 15:54 UTC #0 lore raw

cover AMD ntb driver fixes and improvements

This patch series fixes some bugs in the existing AMD NTB driver, cleans-up code, and modifies the code to handle NTB link-up/down events. The series is based on Linus' tree,

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Arindam Nath (15):
NTB: Fix access to link status and control register
NTB: clear interrupt status register
NTB: Enable link up and down event notification
NTB: define a new function to get link status
NTB: return the side info status from amd_poll_link
NTB: set peer_sta within event handler itself
NTB: remove handling of peer_sta from amd_link_is_up
NTB: handle link down event correctly
NTB: handle link up, D0 and D3 events correctly
NTB: move ntb_ctrl handling to init and deinit
NTB: add helper functions to set and clear sideinfo
NTB: return link up status correctly for PRI and SEC
NTB: remove redundant setting of DB valid mask
NTB: send DB event when driver is loaded or un-loaded
NTB: add pci shutdown handler for AMD NTB

 drivers/ntb/hw/amd/ntb_hw_amd.c | 290 ++++++++++++++++++++++++++------
 drivers/ntb/hw/amd/ntb_hw_amd.h |   8 +-
 2 files changed, 247 insertions(+), 51 deletions(-)

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:54 UTC #1 lore raw

PATCH 1/15 NTB: Fix access to link status and control register

The design of AMD NTB implementation is such that NTB primary acts as an endpoint device and NTB secondary is an endpoint device behind a combination of Switch Upstream and Switch Downstream. Considering that, the link status and control register needs to be accessed differently based on the NTB topology.

So in the case of NTB secondary, we first get the pointer to the Switch Downstream device for the NTB device. Then we get the pointer to the Switch Upstream device. Once we have that, we read the Link Status and Control register to get the correct status of link at the secondary.

In the case of NTB primary, simply reading the Link Status and Control register of the NTB device itself will suffice.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 40 ++++++++++++++++++++++++++++++---
 drivers/ntb/hw/amd/ntb_hw_amd.h |  1 -
 2 files changed, 37 insertions(+), 4 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +37 -3

index e52b300b2f5b..9a60f34a37c2 100644
@@ -842,6 +842,9 @@ static inline void ndev_init_struct(struct amd_ntb_dev *ndev,
 static int amd_poll_link(struct amd_ntb_dev *ndev)
 {
 	void __iomem *mmio = ndev->peer_mmio;
+	struct pci_dev *pdev = NULL;
+	struct pci_dev *pci_swds = NULL;
+	struct pci_dev *pci_swus = NULL;
 	u32 reg, stat;
 	int rc;

@@ -855,10 +858,41 @@ static int amd_poll_link(struct amd_ntb_dev *ndev)
 
 	ndev->cntl_sta = reg;
 
-	rc = pci_read_config_dword(ndev->ntb.pdev,
-				   AMD_LINK_STATUS_OFFSET, &stat);
-	if (rc)
+	if (ndev->ntb.topo == NTB_TOPO_SEC) {
+		/* Locate the pointer to Downstream Switch for this device */
+		pci_swds = pci_upstream_bridge(ndev->ntb.pdev);
+		if (pci_swds) {
+			/*
+			 * Locate the pointer to Upstream Switch for
+			 * the Downstream Switch.
+			 */
+			pci_swus = pci_upstream_bridge(pci_swds);
+			if (pci_swus) {
+				rc = pcie_capability_read_dword(pci_swus,
+								PCI_EXP_LNKCTL,
+								&stat);
+				if (rc)
+					return 0;
+			} else {
+				return 0;
+			}
+		} else {
+			return 0;
+		}
+	} else if (ndev->ntb.topo == NTB_TOPO_PRI) {
+		/*
+		 * For NTB primary, we simply read the Link Status and control
+		 * register of the NTB device itself.
+		 */
+		pdev = ndev->ntb.pdev;
+		rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat);
+		if (rc)
+			return 0;
+	} else {
+		/* Catch all for everything else */
 		return 0;
+	}
+
 	ndev->lnk_sta = stat;
 
 	return 1;

drivers/ntb/hw/amd/ntb_hw_amd.h +0 -1

index 139a307147bc..39e5d18e12ff 100644
@@ -53,7 +53,6 @@
 #include <linux/pci.h>
 
 #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

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:54 UTC #2 lore raw

PATCH 2/15 NTB: clear interrupt status register

In reply to Arindam Nath

The interrupt status register should be cleared
by driver once the particular event is handled.
The patch fixes this.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/ntb/hw/amd/ntb_hw_amd.c +3 -0

index 9a60f34a37c2..150e4db11485 100644
@@ -550,6 +550,9 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 		dev_info(dev, "event status = 0x%x.\n", status);
 		break;
 	}
+
+	/* Clear the interrupt status */
+	writel(status, mmio + AMD_INTSTAT_OFFSET);
 }
 
 static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:54 UTC #3 lore raw

PATCH 3/15 NTB: Enable link up and down event notification

In reply to Arindam Nath

Link-Up and Link-Down events can occur irrespective of whether a data transfer is in progress or not. So we need to enable the interrupt delivery for these events early during driver load.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/ntb/hw/amd/ntb_hw_amd.c +5 -0

index 150e4db11485..111f33ff2bd7 100644
@@ -994,6 +994,7 @@ static enum ntb_topo amd_get_topo(struct amd_ntb_dev *ndev)
 
 static int amd_init_dev(struct amd_ntb_dev *ndev)
 {
+	void __iomem *mmio = ndev->self_mmio;
 	struct pci_dev *pdev;
 	int rc = 0;

@@ -1015,6 +1016,10 @@ static int amd_init_dev(struct amd_ntb_dev *ndev)
 
 	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
 
+	/* Enable Link-Up and Link-Down event interrupts */
+	ndev->int_mask &= ~(AMD_LINK_UP_EVENT | AMD_LINK_DOWN_EVENT);
+	writel(ndev->int_mask, mmio + AMD_INTMASK_OFFSET);
+
 	return 0;
 }

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #4 lore raw

PATCH 4/15 NTB: define a new function to get link status

In reply to Arindam Nath

Since getting the status of link is a logically separate operation, we simply create a new function which will store the link status to be used later.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 93 ++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 43 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +50 ~~-43~~

index 111f33ff2bd7..f50537e0917b 100644
@@ -195,6 +195,54 @@ static int amd_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
 	return 0;
 }
 
+static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev)
+{
+	struct pci_dev *pdev = NULL;
+	struct pci_dev *pci_swds = NULL;
+	struct pci_dev *pci_swus = NULL;
+	u32 stat;
+	int rc;
+
+	if (ndev->ntb.topo == NTB_TOPO_SEC) {
+		/* Locate the pointer to Downstream Switch for this device */
+		pci_swds = pci_upstream_bridge(ndev->ntb.pdev);
+		if (pci_swds) {
+			/*
+			 * Locate the pointer to Upstream Switch for
+			 * the Downstream Switch.
+			 */
+			pci_swus = pci_upstream_bridge(pci_swds);
+			if (pci_swus) {
+				rc = pcie_capability_read_dword(pci_swus,
+								PCI_EXP_LNKCTL,
+								&stat);
+				if (rc)
+					return 0;
+			} else {
+				return 0;
+			}
+		} else {
+			return 0;
+		}
+	} else if (ndev->ntb.topo == NTB_TOPO_PRI) {
+		/*
+		 * For NTB primary, we simply read the Link Status and control
+		 * register of the NTB device itself.
+		 */
+		pdev = ndev->ntb.pdev;
+		rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat);
+		if (rc)
+			return 0;
+	} else {
+		/* Catch all for everything else */
+		return 0;
+	}
+
+	ndev->lnk_sta = stat;
+
+	return 1;
+}
+
 static int amd_link_is_up(struct amd_ntb_dev *ndev)
 {
 	if (!ndev->peer_sta)

@@ -845,11 +893,7 @@ static inline void ndev_init_struct(struct amd_ntb_dev *ndev,
 static int amd_poll_link(struct amd_ntb_dev *ndev)
 {
 	void __iomem *mmio = ndev->peer_mmio;
-	struct pci_dev *pdev = NULL;
-	struct pci_dev *pci_swds = NULL;
-	struct pci_dev *pci_swus = NULL;
-	u32 reg, stat;
-	int rc;
+	u32 reg;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
 	reg &= NTB_LIN_STA_ACTIVE_BIT;

@@ -861,44 +905,7 @@ static int amd_poll_link(struct amd_ntb_dev *ndev)
 
 	ndev->cntl_sta = reg;
 
-	if (ndev->ntb.topo == NTB_TOPO_SEC) {
-		/* Locate the pointer to Downstream Switch for this device */
-		pci_swds = pci_upstream_bridge(ndev->ntb.pdev);
-		if (pci_swds) {
-			/*
-			 * Locate the pointer to Upstream Switch for
-			 * the Downstream Switch.
-			 */
-			pci_swus = pci_upstream_bridge(pci_swds);
-			if (pci_swus) {
-				rc = pcie_capability_read_dword(pci_swus,
-								PCI_EXP_LNKCTL,
-								&stat);
-				if (rc)
-					return 0;
-			} else {
-				return 0;
-			}
-		} else {
-			return 0;
-		}
-	} else if (ndev->ntb.topo == NTB_TOPO_PRI) {
-		/*
-		 * For NTB primary, we simply read the Link Status and control
-		 * register of the NTB device itself.
-		 */
-		pdev = ndev->ntb.pdev;
-		rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat);
-		if (rc)
-			return 0;
-	} else {
-		/* Catch all for everything else */
-		return 0;
-	}
-
-	ndev->lnk_sta = stat;
-
-	return 1;
+	return amd_ntb_get_link_status(ndev);
 }
 
 static void amd_link_hb(struct work_struct *work)

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #5 lore raw

PATCH 5/15 NTB: return the side info status from amd_poll_link

In reply to Arindam Nath

Bit 1 of SIDE_INFO register is an indication that the driver on the other side of link is ready. We set this bit during driver initialization sequence. So rather than having separate macros to return the status, we can simply return the status of this bit from amd_poll_link(). So a return of 1 or 0 from this function will indicate to the caller whether the driver on the other side of link is ready or not, respectively.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 11 +++++------
 drivers/ntb/hw/amd/ntb_hw_amd.h |  2 --
 2 files changed, 5 insertions(+), 8 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +5 -6

index f50537e0917b..84723420d70b 100644
@@ -246,7 +246,7 @@ static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev)
 static int amd_link_is_up(struct amd_ntb_dev *ndev)
 {
 	if (!ndev->peer_sta)
-		return NTB_LNK_STA_ACTIVE(ndev->cntl_sta);
+		return ndev->cntl_sta;
 
 	if (ndev->peer_sta & AMD_LINK_UP_EVENT) {
 		ndev->peer_sta = 0;

@@ -896,16 +896,15 @@ static int amd_poll_link(struct amd_ntb_dev *ndev)
 	u32 reg;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
-	reg &= NTB_LIN_STA_ACTIVE_BIT;
+	reg &= AMD_SIDE_READY;
 
 	dev_dbg(&ndev->ntb.pdev->dev, "%s: reg_val = 0x%x.\n", __func__, reg);
 
-	if (reg == ndev->cntl_sta)
-		return 0;
-
 	ndev->cntl_sta = reg;
 
-	return amd_ntb_get_link_status(ndev);
+	amd_ntb_get_link_status(ndev);
+
+	return ndev->cntl_sta;
 }
 
 static void amd_link_hb(struct work_struct *work)

drivers/ntb/hw/amd/ntb_hw_amd.h +0 -2

index 39e5d18e12ff..156a4a92b803 100644
@@ -53,10 +53,8 @@
 #include <linux/pci.h>
 
 #define AMD_LINK_HB_TIMEOUT	msecs_to_jiffies(1000)
-#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)

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #6 lore raw

PATCH 6/15 NTB: set peer_sta within event handler itself

In reply to Arindam Nath

amd_ack_smu() should only set the corresponding bits into SMUACK register. Setting the bitmask of peer_sta should be done within the event handler. They are two different things, and so should be handled differently and at different places.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +4 -2

index 84723420d70b..b85af150f2c6 100644
@@ -541,8 +541,6 @@ static void amd_ack_smu(struct amd_ntb_dev *ndev, u32 bit)
 	reg = readl(mmio + AMD_SMUACK_OFFSET);
 	reg |= bit;
 	writel(reg, mmio + AMD_SMUACK_OFFSET);
-
-	ndev->peer_sta |= bit;
 }
 
 static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)

@@ -560,9 +558,11 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 	status &= AMD_EVENT_INTMASK;
 	switch (status) {
 	case AMD_PEER_FLUSH_EVENT:
+		ndev->peer_sta |= AMD_PEER_FLUSH_EVENT;
 		dev_info(dev, "Flush is done.\n");
 		break;
 	case AMD_PEER_RESET_EVENT:
+		ndev->peer_sta |= AMD_PEER_RESET_EVENT;
 		amd_ack_smu(ndev, AMD_PEER_RESET_EVENT);
 
 		/* link down first */

@@ -575,6 +575,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 	case AMD_PEER_PMETO_EVENT:
 	case AMD_LINK_UP_EVENT:
 	case AMD_LINK_DOWN_EVENT:
+		ndev->peer_sta |= status;
 		amd_ack_smu(ndev, status);
 
 		/* link down */

@@ -588,6 +589,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 		if (status & 0x1)
 			dev_info(dev, "Wakeup is done.\n");
 
+		ndev->peer_sta |= AMD_PEER_D0_EVENT;
 		amd_ack_smu(ndev, AMD_PEER_D0_EVENT);
 
 		/* start a timer to poll link status */

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #7 lore raw

PATCH 7/15 NTB: remove handling of peer_sta from amd_link_is_up

In reply to Arindam Nath

amd_link_is_up() is a callback to inquire whether the NTB link is up or not. So it should not indulge itself into clearing the bitmasks of peer_sta.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 11 -----------
 1 file changed, 11 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +0 ~~-11~~

index b85af150f2c6..e964442ae2c3 100644
@@ -253,17 +253,6 @@ static int amd_link_is_up(struct amd_ntb_dev *ndev)
 		return 1;
 	}
 
-	/* If peer_sta is reset or D0 event, the ISR has
-	 * started a timer to check link status of hardware.
-	 * So here just clear status bit. And if peer_sta is
-	 * D3 or PME_TO, D0/reset event will be happened when
-	 * system wakeup/poweron, so do nothing here.
-	 */
-	if (ndev->peer_sta & AMD_PEER_RESET_EVENT)
-		ndev->peer_sta &= ~AMD_PEER_RESET_EVENT;
-	else if (ndev->peer_sta & (AMD_PEER_D0_EVENT | AMD_LINK_DOWN_EVENT))
-		ndev->peer_sta = 0;
-
 	return 0;
 }

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #8 lore raw

PATCH 8/15 NTB: handle link down event correctly

In reply to Arindam Nath

Link-Up and Link-Down are mutually exclusive events. So when we receive a Link-Down event, we should also clear the bitmask for Link-Up event in peer_sta.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +6 -3

index e964442ae2c3..d933a1dffdc6 100644
@@ -551,8 +551,12 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 		dev_info(dev, "Flush is done.\n");
 		break;
 	case AMD_PEER_RESET_EVENT:
-		ndev->peer_sta |= AMD_PEER_RESET_EVENT;
-		amd_ack_smu(ndev, AMD_PEER_RESET_EVENT);
+	case AMD_LINK_DOWN_EVENT:
+		ndev->peer_sta |= status;
+		if (status == AMD_LINK_DOWN_EVENT)
+			ndev->peer_sta &= ~AMD_LINK_UP_EVENT;
+
+		amd_ack_smu(ndev, status);
 
 		/* link down first */
 		ntb_link_event(&ndev->ntb);

@@ -563,7 +567,6 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 	case AMD_PEER_D3_EVENT:
 	case AMD_PEER_PMETO_EVENT:
 	case AMD_LINK_UP_EVENT:
-	case AMD_LINK_DOWN_EVENT:
 		ndev->peer_sta |= status;
 		amd_ack_smu(ndev, status);

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #9 lore raw

PATCH 9/15 NTB: handle link up, D0 and D3 events correctly

In reply to Arindam Nath

Just like for Link-Down event, Link-Up and D3 events are also mutually exclusive to Link-Down and D0 events respectively. So we clear the bitmasks in peer_sta depending on event type.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/ntb/hw/amd/ntb_hw_amd.c +6 -0

index d933a1dffdc6..a1c4a21c58c3 100644
@@ -568,6 +568,11 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 	case AMD_PEER_PMETO_EVENT:
 	case AMD_LINK_UP_EVENT:
 		ndev->peer_sta |= status;
+		if (status == AMD_LINK_UP_EVENT)
+			ndev->peer_sta &= ~AMD_LINK_DOWN_EVENT;
+		else if (status == AMD_PEER_D3_EVENT)
+			ndev->peer_sta &= ~AMD_PEER_D0_EVENT;
+
 		amd_ack_smu(ndev, status);
 
 		/* link down */

@@ -582,6 +587,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 			dev_info(dev, "Wakeup is done.\n");
 
 		ndev->peer_sta |= AMD_PEER_D0_EVENT;
+		ndev->peer_sta &= ~AMD_PEER_D3_EVENT;
 		amd_ack_smu(ndev, AMD_PEER_D0_EVENT);
 
 		/* start a timer to poll link status */

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #10 lore raw

PATCH 10/15 NTB: move ntb_ctrl handling to init and deinit

In reply to Arindam Nath

It does not really make sense to enable or disable the bits of NTB_CTRL register only during enable and disable link callbacks. They should be done independent of these callbacks. The correct placement for that is during the amd_init_side_info() and amd_deinit_side_info() functions, which are invoked during probe and remove respectively.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +10 ~~-10~~

index a1c4a21c58c3..621a69a0cff2 100644
@@ -290,7 +290,6 @@ static int amd_ntb_link_enable(struct ntb_dev *ntb,
 {
 	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;

@@ -300,10 +299,6 @@ static int amd_ntb_link_enable(struct ntb_dev *ntb,
 		return -EINVAL;
 	dev_dbg(&ntb->pdev->dev, "Enabling Link.\n");
 
-	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
-	ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL);
-	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
-
 	return 0;
 }

@@ -311,7 +306,6 @@ 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;

@@ -321,10 +315,6 @@ static int amd_ntb_link_disable(struct ntb_dev *ntb)
 		return -EINVAL;
 	dev_dbg(&ntb->pdev->dev, "Enabling Link.\n");
 
-	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
-	ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL);
-	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
-
 	return 0;
 }

@@ -927,18 +917,24 @@ static void amd_init_side_info(struct amd_ntb_dev *ndev)
 {
 	void __iomem *mmio = ndev->self_mmio;
 	unsigned int reg;
+	u32 ntb_ctl;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
 	if (!(reg & AMD_SIDE_READY)) {
 		reg |= AMD_SIDE_READY;
 		writel(reg, mmio + AMD_SIDEINFO_OFFSET);
 	}
+
+	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
+	ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL);
+	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
 }
 
 static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
 {
 	void __iomem *mmio = ndev->self_mmio;
 	unsigned int reg;
+	u32 ntb_ctl;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
 	if (reg & AMD_SIDE_READY) {

@@ -946,6 +942,10 @@ static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
 		writel(reg, mmio + AMD_SIDEINFO_OFFSET);
 		readl(mmio + AMD_SIDEINFO_OFFSET);
 	}
+
+	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
+	ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL);
+	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
 }
 
 static int amd_init_ntb(struct amd_ntb_dev *ndev)

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #11 lore raw

PATCH 11/15 NTB: add helper functions to set and clear sideinfo

In reply to Arindam Nath

We define two new helper functions to set and clear sideinfo registers respectively. These functions take an additional boolean parameter which signifies whether we want to set/clear the sideinfo register of the peer(true) or local host(false).

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 44 +++++++++++++++++++++++++--------
 drivers/ntb/hw/amd/ntb_hw_amd.h |  3 +++
 2 files changed, 37 insertions(+), 10 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +34 ~~-10~~

index 621a69a0cff2..d4029d531466 100644
@@ -913,28 +913,32 @@ 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)
+static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer)
 {
-	void __iomem *mmio = ndev->self_mmio;
+	void __iomem *mmio = NULL;
 	unsigned int reg;
-	u32 ntb_ctl;
+
+	if (peer)
+		mmio = ndev->peer_mmio;
+	else
+		mmio = ndev->self_mmio;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
 	if (!(reg & AMD_SIDE_READY)) {
 		reg |= AMD_SIDE_READY;
 		writel(reg, mmio + AMD_SIDEINFO_OFFSET);
 	}
-
-	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
-	ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL);
-	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
 }
 
-static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
+static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer)
 {
-	void __iomem *mmio = ndev->self_mmio;
+	void __iomem *mmio = NULL;
 	unsigned int reg;
-	u32 ntb_ctl;
+
+	if (peer)
+		mmio = ndev->peer_mmio;
+	else
+		mmio = ndev->self_mmio;
 
 	reg = readl(mmio + AMD_SIDEINFO_OFFSET);
 	if (reg & AMD_SIDE_READY) {

@@ -942,6 +946,26 @@ static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
 		writel(reg, mmio + AMD_SIDEINFO_OFFSET);
 		readl(mmio + AMD_SIDEINFO_OFFSET);
 	}
+}
+
+static void amd_init_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	amd_set_side_info_reg(ndev, false);
+
+	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
+	ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL);
+	writel(ntb_ctl, mmio + AMD_CNTL_OFFSET);
+}
+
+static void amd_deinit_side_info(struct amd_ntb_dev *ndev)
+{
+	void __iomem *mmio = ndev->self_mmio;
+	u32 ntb_ctl;
+
+	amd_clear_side_info_reg(ndev, false);
 
 	ntb_ctl = readl(mmio + AMD_CNTL_OFFSET);
 	ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL);

drivers/ntb/hw/amd/ntb_hw_amd.h +3 -0

index 156a4a92b803..62ffdf35b683 100644
@@ -215,4 +215,7 @@ struct amd_ntb_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)
 
+static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer);
+static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer);
+
 #endif

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #12 lore raw

PATCH 12/15 NTB: return link up status correctly for PRI and SEC

In reply to Arindam Nath

Since NTB connects two physically separate systems, there can be scenarios where one system goes down while the other one remains active. In case of NTB primary, if the NTB secondary goes down, a Link-Down event is received. For the NTB secondary, if the NTB primary goes down, the PCIe hotplug mechanism ensures that the driver on the secondary side is also unloaded.

But there are other scenarios to consider as well, when suppose the physical link remains active, but the driver on primary or secondary side is loaded or un-loaded.

When the driver is loaded, on either side, it sets SIDE_READY bit(bit-1) of SIDE_INFO register. Similarly, when the driver is un-loaded, it resets the same bit.

We consider the NTB link to be up and operational only when the driver on both sides of link are loaded and ready. But we also need to take account of Link Up and Down events which signify the physical link status. So amd_link_is_up() is modified to take care of the above scenarios.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 64 ++++++++++++++++++++++++++++++---
 drivers/ntb/hw/amd/ntb_hw_amd.h |  1 +
 2 files changed, 60 insertions(+), 5 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +59 -5

index d4029d531466..8a9852343de6 100644
@@ -245,12 +245,66 @@ static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev)
 
 static int amd_link_is_up(struct amd_ntb_dev *ndev)
 {
-	if (!ndev->peer_sta)
-		return ndev->cntl_sta;
+	int ret;
+
+	/*
+	 * We consider the link to be up under two conditions:
+	 *
+	 *   - When a link-up event is received. This is indicated by
+	 *     AMD_LINK_UP_EVENT set in peer_sta.
+	 *   - When driver on both sides of the link have been loaded.
+	 *     This is indicated by bit 1 being set in the peer
+	 *     SIDEINFO register.
+	 *
+	 * This function should return 1 when the latter of the above
+	 * two conditions is true.
+	 *
+	 * Now consider the sequence of events - Link-Up event occurs,
+	 * then the peer side driver loads. In this case, we would have
+	 * received LINK_UP event and bit 1 of peer SIDEINFO is also
+	 * set. What happens now if the link goes down? Bit 1 of
+	 * peer SIDEINFO remains set, but LINK_DOWN bit is set in
+	 * peer_sta. So we should return 0 from this function. Not only
+	 * that, we clear bit 1 of peer SIDEINFO to 0, since the peer
+	 * side driver did not even get a chance to clear it before
+	 * the link went down. This can be the case of surprise link
+	 * removal.
+	 *
+	 * LINK_UP event will always occur before the peer side driver
+	 * gets loaded the very first time. So there can be a case when
+	 * the LINK_UP event has occurred, but the peer side driver hasn't
+	 * yet loaded. We return 0 in that case.
+	 *
+	 * There is also a special case when the primary side driver is
+	 * unloaded and then loaded again. Since there is no change in
+	 * the status of NTB secondary in this case, there is no Link-Up
+	 * or Link-Down notification received. We recognize this condition
+	 * with peer_sta being set to 0.
+	 *
+	 * If bit 1 of peer SIDEINFO register is not set, then we
+	 * simply return 0 irrespective of the link up or down status
+	 * set in peer_sta.
+	 */
+	ret = amd_poll_link(ndev);
+	if (ret) {
+		/*
+		 * We need to check the below only for NTB primary. For NTB
+		 * secondary, simply checking the result of PSIDE_INFO
+		 * register will suffice.
+		 */
+		if (ndev->ntb.topo == NTB_TOPO_PRI) {
+			if ((ndev->peer_sta & AMD_LINK_UP_EVENT) ||
+			    (ndev->peer_sta == 0))
+				return ret;
+			else if (ndev->peer_sta & AMD_LINK_DOWN_EVENT) {
+				/* Clear peer sideinfo register */
+				amd_clear_side_info_reg(ndev, true);
 
-	if (ndev->peer_sta & AMD_LINK_UP_EVENT) {
-		ndev->peer_sta = 0;
-		return 1;
+				return 0;
+			}
+		} else { /* NTB_TOPO_SEC */
+			return ret;
+		}
 	}
 
 	return 0;

drivers/ntb/hw/amd/ntb_hw_amd.h +1 -0

index 62ffdf35b683..73959c0b9972 100644
@@ -217,5 +217,6 @@ struct amd_ntb_dev {
 
 static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer);
 static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer);
+static int amd_poll_link(struct amd_ntb_dev *ndev);
 
 #endif

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #13 lore raw

PATCH 13/15 NTB: remove redundant setting of DB valid mask

In reply to Arindam Nath

db_valid_mask is set at two places, once within amd_init_ntb(), and again within amd_init_dev(). Since amd_init_ntb() is actually called from amd_init_dev(), setting db_valid_mask from former does not really make sense. So remove it.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 2 --
 1 file changed, 2 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +0 -2

index 8a9852343de6..04be1482037b 100644
@@ -1056,8 +1056,6 @@ static int amd_init_ntb(struct amd_ntb_dev *ndev)
 		return -EINVAL;
 	}
 
-	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
-
 	/* Mask event interrupts */
 	writel(ndev->int_mask, mmio + AMD_INTMASK_OFFSET);

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #14 lore raw

PATCH 14/15 NTB: send DB event when driver is loaded or un-loaded

In reply to Arindam Nath

When the driver on the local side is loaded, it sets SIDE_READY bit in SIDE_INFO register. Likewise, when it is un-loaded, it clears the bit.

Also just after being loaded, the driver polls for peer SIDE_READY bit to be set. Since that bit is set when the peer side driver has loaded, the polling on local side breaks as soon as this condition is met.

But the situation is different when the driver is un-loaded. Since the polling has already been stopped as mentioned before, if the peer side driver gets un-loaded, the driver on the local side is not notified implicitly.

So, we improvise using existing doorbell mechanism. We reserve the highest order bit of the DB register to send a notification to peer when the driver on local side is un-loaded. This also means that now we are one short of 16 DB events and that is taken care of in the valid DB mask.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 57 +++++++++++++++++++++++++++++++--
 drivers/ntb/hw/amd/ntb_hw_amd.h |  1 +
 2 files changed, 56 insertions(+), 2 deletions(-)

drivers/ntb/hw/amd/ntb_hw_amd.c +55 -2

index 04be1482037b..c6cea0005553 100644
@@ -647,6 +647,36 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
 	writel(status, mmio + AMD_INTSTAT_OFFSET);
 }
 
+static void amd_handle_db_event(struct amd_ntb_dev *ndev, int vec)
+{
+	struct device *dev = &ndev->ntb.pdev->dev;
+	u64 status;
+
+	status = amd_ntb_db_read(&ndev->ntb);
+
+	dev_dbg(dev, "status = 0x%llx and vec = %d\n", status, vec);
+
+	/*
+	 * Since we had reserved highest order bit of DB for signaling peer of
+	 * a special event, this is the only status bit we should be concerned
+	 * here now.
+	 */
+	if (status & BIT(ndev->db_last_bit)) {
+		ntb_db_clear(&ndev->ntb, BIT(ndev->db_last_bit));
+		/* send link down event notification */
+		ntb_link_event(&ndev->ntb);
+
+		/*
+		 * If we are here, that means the peer has signalled a special
+		 * event which notifies that the peer driver has been
+		 * un-loaded for some reason. Since there is a chance that the
+		 * peer will load its driver again sometime, we schedule link
+		 * polling routine.
+		 */
+		schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
+	}
+}
+
 static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
 {
 	dev_dbg(&ndev->ntb.pdev->dev, "vec %d\n", vec);

@@ -654,8 +684,10 @@ static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
 	if (vec > (AMD_DB_CNT - 1) || (ndev->msix_vec_count == 1))
 		amd_handle_event(ndev, vec);
 
-	if (vec < AMD_DB_CNT)
+	if (vec < AMD_DB_CNT) {
+		amd_handle_db_event(ndev, vec);
 		ntb_db_event(&ndev->ntb, vec);
+	}
 
 	return IRQ_HANDLED;
 }

@@ -1096,6 +1128,21 @@ static int amd_init_dev(struct amd_ntb_dev *ndev)
 		return rc;
 	}
 
+	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
+	/*
+	 * We reserve the highest order bit of the DB register which will
+	 * be used to notify peer when the driver on this side is being
+	 * un-loaded.
+	 */
+	ndev->db_last_bit =
+			find_last_bit((unsigned long *)&ndev->db_valid_mask,
+				      hweight64(ndev->db_valid_mask));
+	writew((u16)~BIT(ndev->db_last_bit), mmio + AMD_DBMASK_OFFSET);
+	/*
+	 * Since now there is one less bit to account for, the DB count
+	 * and DB mask should be adjusted accordingly.
+	 */
+	ndev->db_count -= 1;
 	ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1;
 
 	/* Enable Link-Up and Link-Down event interrupts */

@@ -1235,9 +1282,15 @@ static void amd_ntb_pci_remove(struct pci_dev *pdev)
 {
 	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
 
+	/*
+	 * Clear the READY bit in SIDEINFO register before sending DB event
+	 * to the peer. This will make sure that when the peer handles the
+	 * DB event, it correctly reads this bit as being 0.
+	 */
+	amd_deinit_side_info(ndev);
+	ntb_peer_db_set(&ndev->ntb, BIT_ULL(ndev->db_last_bit));
 	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);

drivers/ntb/hw/amd/ntb_hw_amd.h +1 -0

index 73959c0b9972..5f337b1572a0 100644
@@ -193,6 +193,7 @@ struct amd_ntb_dev {
 
 	u64 db_valid_mask;
 	u64 db_mask;
+	u64 db_last_bit;
 	u32 int_mask;
 
 	struct msix_entry *msix;

Signature · 1 line

2.17.1

Arindam Nath

author

2020-02-05 15:55 UTC #15 lore raw

PATCH 15/15 NTB: add pci shutdown handler for AMD NTB

In reply to Arindam Nath

The PCI shutdown handler is invoked in response to system reboot or shutdown. A data transfer might still be in flight when this happens. So the very first action we take here is to send a link down notification, so that any pending data transfer is terminated. Rest of the actions are same as that of PCI remove handler.

Signed-off-by Arindam Nath <[email protected]>

---
 drivers/ntb/hw/amd/ntb_hw_amd.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/ntb/hw/amd/ntb_hw_amd.c +17 -0

index c6cea0005553..9e310e1ad4d0 100644
@@ -1296,6 +1296,22 @@ static void amd_ntb_pci_remove(struct pci_dev *pdev)
 	kfree(ndev);
 }
 
+static void amd_ntb_pci_shutdown(struct pci_dev *pdev)
+{
+	struct amd_ntb_dev *ndev = pci_get_drvdata(pdev);
+
+	/* Send link down notification */
+	ntb_link_event(&ndev->ntb);
+
+	amd_deinit_side_info(ndev);
+	ntb_peer_db_set(&ndev->ntb, BIT_ULL(ndev->db_last_bit));
+	ntb_unregister_device(&ndev->ntb);
+	ndev_deinit_debugfs(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,

@@ -1326,6 +1342,7 @@ static struct pci_driver amd_ntb_pci_driver = {
 	.id_table	= amd_ntb_pci_tbl,
 	.probe		= amd_ntb_pci_probe,
 	.remove		= amd_ntb_pci_remove,
+	.shutdown	= amd_ntb_pci_shutdown,
 };
 
 static int __init amd_ntb_pci_driver_init(void)

Signature · 1 line

2.17.1

Sanjay R Mehta

Reviewed-by on the v1 series

2020-02-07 10:59 UTC #16 lore raw

In reply to Arindam Nath

Quoted 9 lines · Arindam Nath

[CAUTION: External Email]
This patch series fixes some bugs in the existing AMD NTB driver, cleans-up code, and modifies the code to handle NTB link-up/down events. The series is based on Linus' tree,
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Quoted 16 lines

Arindam Nath (15):
NTB: Fix access to link status and control register
NTB: clear interrupt status register
NTB: Enable link up and down event notification
NTB: define a new function to get link status
NTB: return the side info status from amd_poll_link
NTB: set peer_sta within event handler itself
NTB: remove handling of peer_sta from amd_link_is_up
NTB: handle link down event correctly
NTB: handle link up, D0 and D3 events correctly
NTB: move ntb_ctrl handling to init and deinit
NTB: add helper functions to set and clear sideinfo
NTB: return link up status correctly for PRI and SEC
NTB: remove redundant setting of DB valid mask
NTB: send DB event when driver is loaded or un-loaded
NTB: add pci shutdown handler for AMD NTB

The patch series looks good to me. Thanks for the changes.

For all the ntb_hw_amd changes:

Reviewed-by Sanjay R Mehta <[email protected]>

Quoted 8 lines

 drivers/ntb/hw/amd/ntb_hw_amd.c | 290 ++++++++++++++++++++++++++------
 drivers/ntb/hw/amd/ntb_hw_amd.h |   8 +-
 2 files changed, 247 insertions(+), 51 deletions(-)

--
2.17.1

Jon Mason

maintainer

2020-03-13 00:25 UTC #17 lore raw

Quoted 29 lines · Sanjay R Mehta

Quoted 9 lines · Arindam Nath
[CAUTION: External Email]
This patch series fixes some bugs in the existing AMD NTB driver, cleans-up code, and modifies the code to handle NTB link-up/down events. The series is based on Linus' tree,
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Quoted 16 lines
Arindam Nath (15):
NTB: Fix access to link status and control register
NTB: clear interrupt status register
NTB: Enable link up and down event notification
NTB: define a new function to get link status
NTB: return the side info status from amd_poll_link
NTB: set peer_sta within event handler itself
NTB: remove handling of peer_sta from amd_link_is_up
NTB: handle link down event correctly
NTB: handle link up, D0 and D3 events correctly
NTB: move ntb_ctrl handling to init and deinit
NTB: add helper functions to set and clear sideinfo
NTB: return link up status correctly for PRI and SEC
NTB: remove redundant setting of DB valid mask
NTB: send DB event when driver is loaded or un-loaded
NTB: add pci shutdown handler for AMD NTB

Quoted 5 lines

The patch series looks good to me. Thanks for the changes.
For all the ntb_hw_amd changes:
Reviewed-by Sanjay R Mehta <[email protected]>

I had to rework the first patch, since Jiasen's patch was already in my tree for a couple months. The rest applied fine and will be in my git trees on github in a couple of hours (sanity build pending).

Thanks,
Jon

Quoted 10 lines



 drivers/ntb/hw/amd/ntb_hw_amd.c | 290 ++++++++++++++++++++++++++------
 drivers/ntb/hw/amd/ntb_hw_amd.h |   8 +-
 2 files changed, 247 insertions(+), 51 deletions(-)

--
2.17.1

Nath, Arindam

author

2020-03-30 18:43 UTC #18 lore raw

Quoted 43 lines

-----Original Message-----
From: Jon Mason <[email protected]>
Sent: Friday, March 13, 2020 05:55
To: Mehta, Sanju <[email protected]>
Cc: Nath, Arindam <[email protected]>; S-k, Shyam-sundar <Shyam- [email protected]>; Dave Jiang <[email protected]>; Allen Hubbe <[email protected]>; Jiasen Lin <[email protected]>; Mehta, Sanju <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 00/15] AMD ntb driver fixes and improvements
Quoted 31 lines · Sanjay R Mehta
Quoted 9 lines · Arindam Nath
[CAUTION: External Email]
This patch series fixes some bugs in the existing AMD NTB driver, cleans-up code, and modifies the code to handle NTB link-up/down events. The series is based on Linus' tree,
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Quoted 16 lines
Arindam Nath (15):
NTB: Fix access to link status and control register
NTB: clear interrupt status register
NTB: Enable link up and down event notification
NTB: define a new function to get link status
NTB: return the side info status from amd_poll_link
NTB: set peer_sta within event handler itself
NTB: remove handling of peer_sta from amd_link_is_up
NTB: handle link down event correctly
NTB: handle link up, D0 and D3 events correctly
NTB: move ntb_ctrl handling to init and deinit
NTB: add helper functions to set and clear sideinfo
NTB: return link up status correctly for PRI and SEC
NTB: remove redundant setting of DB valid mask
NTB: send DB event when driver is loaded or un-loaded
NTB: add pci shutdown handler for AMD NTB
The patch series looks good to me. Thanks for the changes.

Quoted 8 lines

Quoted 3 lines
For all the ntb_hw_amd changes:
Reviewed-by Sanjay R Mehta <[email protected]>
I had to rework the first patch, since Jiasen's patch was already in my tree for a couple months. The rest applied fine and will be in my git trees on github in a couple of hours (sanity build pending).

Hi Jon,

Just wanted to know whether the changes are in your tree now?

Thanks,
Arindam

Quoted 14 lines

Thanks,
Jon
Quoted 4 lines
Quoted 4 lines
 drivers/ntb/hw/amd/ntb_hw_amd.c | 290
++++++++++++++++++++++++++------
Quoted 6 lines
Quoted 6 lines
 drivers/ntb/hw/amd/ntb_hw_amd.h |   8 +-
 2 files changed, 247 insertions(+), 51 deletions(-)
--
2.17.1

Jon Mason

maintainer

2020-03-30 21:09 UTC #19 lore raw

Quoted 48 lines · Arindam

Quoted 48 lines
-----Original Message-----
From: Jon Mason <[email protected]>
Sent: Friday, March 13, 2020 05:55
To: Mehta, Sanju <[email protected]>
Cc: Nath, Arindam <[email protected]>; S-k, Shyam-sundar <Shyam- [email protected]>; Dave Jiang <[email protected]>; Allen Hubbe <[email protected]>; Jiasen Lin <[email protected]>; Mehta, Sanju <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH 00/15] AMD ntb driver fixes and improvements
Quoted 34 lines · Sanjay R Mehta
Quoted 9 lines · Arindam Nath
[CAUTION: External Email]
This patch series fixes some bugs in the existing AMD NTB driver, cleans-up code, and modifies the code to handle NTB link-up/down events. The series is based on Linus' tree,
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Quoted 16 lines
Arindam Nath (15):
NTB: Fix access to link status and control register
NTB: clear interrupt status register
NTB: Enable link up and down event notification
NTB: define a new function to get link status
NTB: return the side info status from amd_poll_link
NTB: set peer_sta within event handler itself
NTB: remove handling of peer_sta from amd_link_is_up
NTB: handle link down event correctly
NTB: handle link up, D0 and D3 events correctly
NTB: move ntb_ctrl handling to init and deinit
NTB: add helper functions to set and clear sideinfo
NTB: return link up status correctly for PRI and SEC
NTB: remove redundant setting of DB valid mask
NTB: send DB event when driver is loaded or un-loaded
NTB: add pci shutdown handler for AMD NTB
The patch series looks good to me. Thanks for the changes.
For all the ntb_hw_amd changes:
Reviewed-by Sanjay R Mehta <[email protected]>

Quoted 8 lines

Quoted 4 lines
I had to rework the first patch, since Jiasen's patch was already in my tree for a couple months. The rest applied fine and will be in my git trees on github in a couple of hours (sanity build pending).
Hi Jon,
Just wanted to know whether the changes are in your tree now?

You should see them in my ntb branch and I'll be sending out a pull req

Quoted 18 lines

Thanks,
Arindam
Quoted 14 lines
Thanks,
Jon
Quoted 4 lines
Quoted 4 lines
 drivers/ntb/hw/amd/ntb_hw_amd.c | 290
++++++++++++++++++++++++++------
Quoted 6 lines
Quoted 6 lines
 drivers/ntb/hw/amd/ntb_hw_amd.h |   8 +-
 2 files changed, 247 insertions(+), 51 deletions(-)
--
2.17.1