2017-07-25 21:03:13

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 00/16] Switchtec NTB Support

Changes since v2:

- Reordered the ntb_test link patch per Allen
- Removed an extra call to switchtec_ntb_init_mw
- Fixed a typo in the switchtec.txt documentation.

--

Changes since v1:

- Rebased onto latest ntb-next branch (with v4.13-rc1)
- Reworked ntb_mw_count() function so that it can be called all the
time (per discussion with Allen)
- Various spelling and formatting cleanups from Bjorn
- Added request_module() call such that the NTB module is automatically
loaded when appropriate hardware exists.

--

Changes since the rfc:

- Rebased on ntb-next
- Switched ntb_part_op to use sleep instead of delay
- Dropped a number of useless dbg __func__ prints
- Went back to the dynamic instead of the static class
- Swapped the notifier block for a simple callback
- Modified the new ntb api so that a couple functions with pidx
now must be called after link up. Per our discussion on the list.

--

This patchset implements Non-Transparent Bridge (NTB) support for the
Microsemi Switchtec series of switches. We're looking for some
review from the community at this point but hope to get it upstreamed
for v4.14.

Switchtec NTB support is configured over the same function and bar
as the management endpoint. Thus, the new driver hooks into the
management driver which we had merged in v4.12. We use the class
interface API to register an NTB device for every switchtec device
which supports NTB (not all do).

The Switchtec hardware supports doorbells, memory windows and messages.
Seeing there is no native scratchpad support, 128 spads are emulated
through the use of a pre-setup memory window. The switch has 64
doorbells which are shared between the two partitions and a
configurable set of memory windows. While the hardware supports more
than 2 partitions, this driver only supports the first two seeing
the current NTB API only supports two hosts.

The driver has been tested with ntb_netdev and fully passes the
ntb_test script.

This patchset is based off of ntb-next and can be found in this
git repo:

https://github.com/sbates130272/linux-p2pmem.git switchtec_ntb_v3

*** BLURB HERE ***

Logan Gunthorpe (16):
switchtec: move structure definitions into a common header
switchtec: export class symbol for use in upper layer driver
switchtec: add NTB hardware register definitions
switchtec: add link event notifier callback
ntb: ntb_test: ensure the link is up before trying to configure the
mws
ntb: ensure ntb_mw_get_align() is only called when the link is up
ntb: add check and comment for link up to mw_count() and
mw_get_align()
switchtec_ntb: introduce initial NTB driver
switchtec_ntb: initialize hardware for memory windows
switchtec_ntb: initialize hardware for doorbells and messages
switchtec_ntb: add skeleton NTB driver
switchtec_ntb: add link management
switchtec_ntb: implement doorbell registers
switchtec_ntb: implement scratchpad registers
switchtec_ntb: add memory window support
switchtec_ntb: update switchtec documentation with notes for NTB

Documentation/switchtec.txt | 12 +
MAINTAINERS | 2 +
drivers/ntb/hw/Kconfig | 1 +
drivers/ntb/hw/Makefile | 1 +
drivers/ntb/hw/mscc/Kconfig | 9 +
drivers/ntb/hw/mscc/Makefile | 1 +
drivers/ntb/hw/mscc/switchtec_ntb.c | 1211 +++++++++++++++++++++++++++++++
drivers/ntb/ntb_transport.c | 20 +-
drivers/ntb/test/ntb_perf.c | 18 +-
drivers/ntb/test/ntb_tool.c | 6 +-
drivers/pci/switch/switchtec.c | 316 ++------
include/linux/ntb.h | 11 +-
include/linux/switchtec.h | 373 ++++++++++
tools/testing/selftests/ntb/ntb_test.sh | 4 +
14 files changed, 1702 insertions(+), 283 deletions(-)
create mode 100644 drivers/ntb/hw/mscc/Kconfig
create mode 100644 drivers/ntb/hw/mscc/Makefile
create mode 100644 drivers/ntb/hw/mscc/switchtec_ntb.c
create mode 100644 include/linux/switchtec.h

--
2.11.0


2017-07-25 20:58:21

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 16/16] switchtec_ntb: update switchtec documentation with notes for NTB

The switchtec_ntb driver has a couple requirements on the switchec's
hardware configuration so we add these notes to the documentation.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
Documentation/switchtec.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/Documentation/switchtec.txt b/Documentation/switchtec.txt
index a0a9c7b3d4d5..f788264921ff 100644
--- a/Documentation/switchtec.txt
+++ b/Documentation/switchtec.txt
@@ -78,3 +78,15 @@ The following IOCTLs are also supported by the device:
between PCI Function Framework number (used by the event system)
and Switchtec Logic Port ID and Partition number (which is more
user friendly).
+
+
+Non-Transparent Bridge (NTB) Driver
+===================================
+
+An NTB driver is provided for the switchtec hardware in switchtec_ntb.
+Currently, it only supports switches configured with exactly 2
+partitions. It also requires the following configuration settings:
+
+* Both partitions must be able to access each other's GAS spaces.
+ Thus, the bits in the GAS Access Vector under Management Settings
+ must be set to support this.
--
2.11.0

2017-07-25 20:59:14

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 12/16] switchtec_ntb: add link management

switchtec_ntb checks for a link by looking at the shared memory
window. If the magic number is correct and the otherside indicates
their link is enabled then we take the link to be up.

Whenever we change our local link status we send a msg to the
otherside to check whether it's up and change their status.

The current status is maintained in a flag so ntb_is_link_up
can return quickly.

We utilize Switchtec's link status notifier to also check link changes
when the switch notices a port changes state.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 130 +++++++++++++++++++++++++++++++++++-
1 file changed, 129 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index f87c0eef59ef..34d69fa8612d 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -64,6 +64,7 @@ static inline void _iowrite64(u64 val, void __iomem *mmio)

struct shared_mw {
u32 magic;
+ u32 link_sta;
u32 partition_id;
u64 mw_sizes[MAX_MWS];
};
@@ -104,8 +105,17 @@ struct switchtec_ntb {
int peer_nr_direct_mw;
int peer_nr_lut_mw;
int peer_direct_mw_to_bar[MAX_DIRECT_MW];
+
+ bool link_is_up;
+ enum ntb_speed link_speed;
+ enum ntb_width link_width;
};

+static struct switchtec_ntb *ntb_sndev(struct ntb_dev *ntb)
+{
+ return container_of(ntb, struct switchtec_ntb, ntb);
+}
+
static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
struct ntb_ctrl_regs __iomem *ctl,
u32 op, int wait_status)
@@ -163,6 +173,17 @@ static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
return -EIO;
}

+static int switchtec_ntb_send_msg(struct switchtec_ntb *sndev, int idx,
+ u32 val)
+{
+ if (idx < 0 || idx >= ARRAY_SIZE(sndev->mmio_self_dbmsg->omsg))
+ return -EINVAL;
+
+ iowrite32(val, &sndev->mmio_self_dbmsg->omsg[idx].msg);
+
+ return 0;
+}
+
static int switchtec_ntb_mw_count(struct ntb_dev *ntb, int pidx)
{
return 0;
@@ -194,22 +215,124 @@ static int switchtec_ntb_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
return 0;
}

+static void switchtec_ntb_part_link_speed(struct switchtec_ntb *sndev,
+ int partition,
+ enum ntb_speed *speed,
+ enum ntb_width *width)
+{
+ struct switchtec_dev *stdev = sndev->stdev;
+
+ u32 pff = ioread32(&stdev->mmio_part_cfg[partition].vep_pff_inst_id);
+ u32 linksta = ioread32(&stdev->mmio_pff_csr[pff].pci_cap_region[13]);
+
+ if (speed)
+ *speed = (linksta >> 16) & 0xF;
+
+ if (width)
+ *width = (linksta >> 20) & 0x3F;
+}
+
+static void switchtec_ntb_set_link_speed(struct switchtec_ntb *sndev)
+{
+ enum ntb_speed self_speed, peer_speed;
+ enum ntb_width self_width, peer_width;
+
+ if (!sndev->link_is_up) {
+ sndev->link_speed = NTB_SPEED_NONE;
+ sndev->link_width = NTB_WIDTH_NONE;
+ return;
+ }
+
+ switchtec_ntb_part_link_speed(sndev, sndev->self_partition,
+ &self_speed, &self_width);
+ switchtec_ntb_part_link_speed(sndev, sndev->peer_partition,
+ &peer_speed, &peer_width);
+
+ sndev->link_speed = min(self_speed, peer_speed);
+ sndev->link_width = min(self_width, peer_width);
+}
+
+enum {
+ LINK_MESSAGE = 0,
+ MSG_LINK_UP = 1,
+ MSG_LINK_DOWN = 2,
+ MSG_CHECK_LINK = 3,
+};
+
+static void switchtec_ntb_check_link(struct switchtec_ntb *sndev)
+{
+ int link_sta;
+ int old = sndev->link_is_up;
+
+ link_sta = sndev->self_shared->link_sta;
+ if (link_sta) {
+ u64 peer = ioread64(&sndev->peer_shared->magic);
+
+ if ((peer & 0xFFFFFFFF) == SWITCHTEC_NTB_MAGIC)
+ link_sta = peer >> 32;
+ else
+ link_sta = 0;
+ }
+
+ sndev->link_is_up = link_sta;
+ switchtec_ntb_set_link_speed(sndev);
+
+ if (link_sta != old) {
+ switchtec_ntb_send_msg(sndev, LINK_MESSAGE, MSG_CHECK_LINK);
+ ntb_link_event(&sndev->ntb);
+ dev_info(&sndev->stdev->dev, "ntb link %s",
+ link_sta ? "up" : "down");
+ }
+}
+
+static void switchtec_ntb_link_notification(struct switchtec_dev *stdev)
+{
+ struct switchtec_ntb *sndev = stdev->sndev;
+
+ switchtec_ntb_check_link(sndev);
+}
+
static u64 switchtec_ntb_link_is_up(struct ntb_dev *ntb,
enum ntb_speed *speed,
enum ntb_width *width)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (speed)
+ *speed = sndev->link_speed;
+ if (width)
+ *width = sndev->link_width;
+
+ return sndev->link_is_up;
}

static int switchtec_ntb_link_enable(struct ntb_dev *ntb,
enum ntb_speed max_speed,
enum ntb_width max_width)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ dev_dbg(&sndev->stdev->dev, "enabling link");
+
+ sndev->self_shared->link_sta = 1;
+ switchtec_ntb_send_msg(sndev, LINK_MESSAGE, MSG_LINK_UP);
+
+ switchtec_ntb_check_link(sndev);
+
return 0;
}

static int switchtec_ntb_link_disable(struct ntb_dev *ntb)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ dev_dbg(&sndev->stdev->dev, "disabling link");
+
+ sndev->self_shared->link_sta = 0;
+ switchtec_ntb_send_msg(sndev, LINK_MESSAGE, MSG_LINK_UP);
+
+ switchtec_ntb_check_link(sndev);
+
return 0;
}

@@ -572,6 +695,9 @@ static irqreturn_t switchtec_ntb_message_isr(int irq, void *dev)
dev_dbg(&sndev->stdev->dev, "message: %d %08x\n", i,
(u32)msg);
iowrite8(1, &sndev->mmio_self_dbmsg->imsg[i].status);
+
+ if (i == LINK_MESSAGE)
+ switchtec_ntb_check_link(sndev);
}
}

@@ -674,6 +800,7 @@ static int switchtec_ntb_add(struct device *dev,
goto deinit_and_exit;

stdev->sndev = sndev;
+ stdev->link_notifier = switchtec_ntb_link_notification;
dev_info(dev, "NTB device registered");

return 0;
@@ -697,6 +824,7 @@ void switchtec_ntb_remove(struct device *dev,
if (!sndev)
return;

+ stdev->link_notifier = NULL;
stdev->sndev = NULL;
ntb_unregister_device(&sndev->ntb);
switchtec_ntb_deinit_db_msg_irq(sndev);
--
2.11.0

2017-07-25 20:59:59

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 13/16] switchtec_ntb: implement doorbell registers

Pretty straightforward implementation of doorbell registers.
The shift and mask were setup in an earlier patch and this just hooks
up the appropriate portion of the IDB register as the local doorbells
and the opposite portion of ODB as the peer doorbells. The DB mask is
protected by a spinlock to avoid concurrent read-modify-write accesses.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 89 +++++++++++++++++++++++++++++++++++--
1 file changed, 85 insertions(+), 4 deletions(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 34d69fa8612d..3037ca730998 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -98,6 +98,9 @@ struct switchtec_ntb {
int db_shift;
int db_peer_shift;

+ /* synchronize rmw access of db_mask and hw reg */
+ spinlock_t db_mask_lock;
+
int nr_direct_mw;
int nr_lut_mw;
int direct_mw_to_bar[MAX_DIRECT_MW];
@@ -338,41 +341,115 @@ static int switchtec_ntb_link_disable(struct ntb_dev *ntb)

static u64 switchtec_ntb_db_valid_mask(struct ntb_dev *ntb)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ return sndev->db_valid_mask;
}

static int switchtec_ntb_db_vector_count(struct ntb_dev *ntb)
{
- return 0;
+ return 1;
}

static u64 switchtec_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (db_vector < 0 || db_vector > 1)
+ return 0;
+
+ return sndev->db_valid_mask;
}

static u64 switchtec_ntb_db_read(struct ntb_dev *ntb)
{
- return 0;
+ u64 ret;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ ret = ioread64(&sndev->mmio_self_dbmsg->idb) >> sndev->db_shift;
+
+ return ret & sndev->db_valid_mask;
}

static int switchtec_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ iowrite64(db_bits << sndev->db_shift, &sndev->mmio_self_dbmsg->idb);
+
return 0;
}

static int switchtec_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
{
+ unsigned long irqflags;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (db_bits & ~sndev->db_valid_mask)
+ return -EINVAL;
+
+ spin_lock_irqsave(&sndev->db_mask_lock, irqflags);
+ {
+ sndev->db_mask |= db_bits << sndev->db_shift;
+ iowrite64(~sndev->db_mask, &sndev->mmio_self_dbmsg->idb_mask);
+ }
+ spin_unlock_irqrestore(&sndev->db_mask_lock, irqflags);
+
return 0;
}

static int switchtec_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
{
+ unsigned long irqflags;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (db_bits & ~sndev->db_valid_mask)
+ return -EINVAL;
+
+ spin_lock_irqsave(&sndev->db_mask_lock, irqflags);
+ {
+ sndev->db_mask &= ~(db_bits << sndev->db_shift);
+ iowrite64(~sndev->db_mask, &sndev->mmio_self_dbmsg->idb_mask);
+ }
+ spin_unlock_irqrestore(&sndev->db_mask_lock, irqflags);
+
+ return 0;
+}
+
+static u64 switchtec_ntb_db_read_mask(struct ntb_dev *ntb)
+{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ return (sndev->db_mask >> sndev->db_shift) & sndev->db_valid_mask;
+}
+
+static int switchtec_ntb_peer_db_addr(struct ntb_dev *ntb,
+ phys_addr_t *db_addr,
+ resource_size_t *db_size)
+{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+ unsigned long offset;
+
+ offset = (unsigned long)sndev->mmio_self_dbmsg->odb -
+ (unsigned long)sndev->stdev->mmio;
+
+ offset += sndev->db_shift / 8;
+
+ if (db_addr)
+ *db_addr = pci_resource_start(ntb->pdev, 0) + offset;
+ if (db_size)
+ *db_size = sizeof(u32);
+
return 0;
}

static int switchtec_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ iowrite64(db_bits << sndev->db_peer_shift,
+ &sndev->mmio_self_dbmsg->odb);
+
return 0;
}

@@ -413,6 +490,8 @@ static const struct ntb_dev_ops switchtec_ntb_ops = {
.db_clear = switchtec_ntb_db_clear,
.db_set_mask = switchtec_ntb_db_set_mask,
.db_clear_mask = switchtec_ntb_db_clear_mask,
+ .db_read_mask = switchtec_ntb_db_read_mask,
+ .peer_db_addr = switchtec_ntb_peer_db_addr,
.peer_db_set = switchtec_ntb_peer_db_set,
.spad_count = switchtec_ntb_spad_count,
.spad_read = switchtec_ntb_spad_read,
@@ -680,6 +759,8 @@ static irqreturn_t switchtec_ntb_doorbell_isr(int irq, void *dev)

dev_dbg(&sndev->stdev->dev, "doorbell\n");

+ ntb_db_event(&sndev->ntb, 0);
+
return IRQ_HANDLED;
}

--
2.11.0

2017-07-25 21:00:15

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 03/16] switchtec: add NTB hardware register definitions

There are two additional regions: ctrl and dbmsg. The first is
for generic NTB control and memory windows. The second is for doorbells
and message registers. This patch also adds a number of related
constants for using these registers.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
include/linux/switchtec.h | 84 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)

diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 18e388101855..9c2be7631257 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -156,6 +156,12 @@ struct flash_info_regs {
struct partition_info vendor[8];
};

+enum {
+ SWITCHTEC_NTB_REG_INFO_OFFSET = 0x0000,
+ SWITCHTEC_NTB_REG_CTRL_OFFSET = 0x4000,
+ SWITCHTEC_NTB_REG_DBMSG_OFFSET = 0x64000,
+};
+
struct ntb_info_regs {
u8 partition_count;
u8 partition_id;
@@ -191,6 +197,84 @@ struct part_cfg_regs {
} __packed;

enum {
+ NTB_CTRL_PART_OP_LOCK = 0x1,
+ NTB_CTRL_PART_OP_CFG = 0x2,
+ NTB_CTRL_PART_OP_RESET = 0x3,
+
+ NTB_CTRL_PART_STATUS_NORMAL = 0x1,
+ NTB_CTRL_PART_STATUS_LOCKED = 0x2,
+ NTB_CTRL_PART_STATUS_LOCKING = 0x3,
+ NTB_CTRL_PART_STATUS_CONFIGURING = 0x4,
+ NTB_CTRL_PART_STATUS_RESETTING = 0x5,
+
+ NTB_CTRL_BAR_VALID = 1 << 0,
+ NTB_CTRL_BAR_DIR_WIN_EN = 1 << 4,
+ NTB_CTRL_BAR_LUT_WIN_EN = 1 << 5,
+
+ NTB_CTRL_REQ_ID_EN = 1 << 0,
+
+ NTB_CTRL_LUT_EN = 1 << 0,
+
+ NTB_PART_CTRL_ID_PROT_DIS = 1 << 0,
+};
+
+struct ntb_ctrl_regs {
+ u32 partition_status;
+ u32 partition_op;
+ u32 partition_ctrl;
+ u32 bar_setup;
+ u32 bar_error;
+ u16 lut_table_entries;
+ u16 lut_table_offset;
+ u32 lut_error;
+ u16 req_id_table_size;
+ u16 req_id_table_offset;
+ u32 req_id_error;
+ u32 reserved1[7];
+ struct {
+ u32 ctl;
+ u32 win_size;
+ u64 xlate_addr;
+ } bar_entry[6];
+ u32 reserved2[216];
+ u32 req_id_table[256];
+ u32 reserved3[512];
+ u64 lut_entry[512];
+} __packed;
+
+#define NTB_DBMSG_IMSG_STATUS BIT_ULL(32)
+#define NTB_DBMSG_IMSG_MASK BIT_ULL(40)
+
+struct ntb_dbmsg_regs {
+ u32 reserved1[1024];
+ u64 odb;
+ u64 odb_mask;
+ u64 idb;
+ u64 idb_mask;
+ u8 idb_vec_map[64];
+ u32 msg_map;
+ u32 reserved2;
+ struct {
+ u32 msg;
+ u32 status;
+ } omsg[4];
+
+ struct {
+ u32 msg;
+ u8 status;
+ u8 mask;
+ u8 src;
+ u8 reserved;
+ } imsg[4];
+
+ u8 reserved3[3928];
+ u8 msix_table[1024];
+ u8 reserved4[3072];
+ u8 pba[24];
+ u8 reserved5[4072];
+} __packed;
+
+enum {
SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
--
2.11.0

2017-07-25 20:58:11

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 09/16] switchtec_ntb: initialize hardware for memory windows

Add the code to initialize the memory windows in the hardware.
This includes setting up the requester ID table, and figuring out
which bar corresponds to which memory window. (Seeing the switch
can be configured with any number of bars.)

Also, seeing the device doesn't have hardware for scratchpads or
determining the link status, we create a shared memory window that has
these features. A magic number with a version copmonent will be used
to determine if the otherside's driver is actually up.

The shared memory window also informs the other side of the
size and count of the local memory windows.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 355 ++++++++++++++++++++++++++++++++++++
1 file changed, 355 insertions(+)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 253efba72275..1596757413f5 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -15,37 +15,391 @@

#include <linux/switchtec.h>
#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>

MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
MODULE_VERSION("0.1");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Microsemi Corporation");

+static bool use_lut_mws;
+module_param(use_lut_mws, bool, 0644);
+MODULE_PARM_DESC(use_lut_mws,
+ "Enable the use of the LUT based memory windows");
+
+#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 SWITCHTEC_NTB_MAGIC 0x45CC0001
+#define MAX_MWS 128
+
+struct shared_mw {
+ u32 magic;
+ u32 partition_id;
+ u64 mw_sizes[MAX_MWS];
+};
+
+#define MAX_DIRECT_MW ARRAY_SIZE(((struct ntb_ctrl_regs *)(0))->bar_entry)
+#define LUT_SIZE SZ_64K
+
struct switchtec_ntb {
struct switchtec_dev *stdev;
+
+ int self_partition;
+ int peer_partition;
+
+ struct ntb_info_regs __iomem *mmio_ntb;
+ struct ntb_ctrl_regs __iomem *mmio_ctrl;
+ struct ntb_dbmsg_regs __iomem *mmio_dbmsg;
+ struct ntb_ctrl_regs __iomem *mmio_self_ctrl;
+ struct ntb_ctrl_regs __iomem *mmio_peer_ctrl;
+ struct ntb_dbmsg_regs __iomem *mmio_self_dbmsg;
+
+ struct shared_mw *self_shared;
+ struct shared_mw __iomem *peer_shared;
+ dma_addr_t self_shared_dma;
+
+ int nr_direct_mw;
+ int nr_lut_mw;
+ int direct_mw_to_bar[MAX_DIRECT_MW];
+
+ int peer_nr_direct_mw;
+ int peer_nr_lut_mw;
+ int peer_direct_mw_to_bar[MAX_DIRECT_MW];
};

+static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
+ struct ntb_ctrl_regs __iomem *ctl,
+ u32 op, int wait_status)
+{
+ static const char * const op_text[] = {
+ [NTB_CTRL_PART_OP_LOCK] = "lock",
+ [NTB_CTRL_PART_OP_CFG] = "configure",
+ [NTB_CTRL_PART_OP_RESET] = "reset",
+ };
+
+ int i;
+ u32 ps;
+ int status;
+
+ switch (op) {
+ case NTB_CTRL_PART_OP_LOCK:
+ status = NTB_CTRL_PART_STATUS_LOCKING;
+ break;
+ case NTB_CTRL_PART_OP_CFG:
+ status = NTB_CTRL_PART_STATUS_CONFIGURING;
+ break;
+ case NTB_CTRL_PART_OP_RESET:
+ status = NTB_CTRL_PART_STATUS_RESETTING;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ iowrite32(op, &ctl->partition_op);
+
+ for (i = 0; i < 1000; i++) {
+ if (msleep_interruptible(50) != 0) {
+ iowrite32(NTB_CTRL_PART_OP_RESET, &ctl->partition_op);
+ return -EINTR;
+ }
+
+ ps = ioread32(&ctl->partition_status) & 0xFFFF;
+
+ if (ps != status)
+ break;
+ }
+
+ if (ps == wait_status)
+ return 0;
+
+ if (ps == status) {
+ dev_err(&sndev->stdev->dev,
+ "Timed out while peforming %s (%d). (%08x)",
+ op_text[op], op,
+ ioread32(&ctl->partition_status));
+
+ return -ETIMEDOUT;
+ }
+
+ return -EIO;
+}
+
+static void switchtec_ntb_init_sndev(struct switchtec_ntb *sndev)
+{
+ u64 part_map;
+
+ sndev->self_partition = sndev->stdev->partition;
+
+ sndev->mmio_ntb = sndev->stdev->mmio_ntb;
+ part_map = ioread64(&sndev->mmio_ntb->ep_map);
+ part_map &= ~(1 << sndev->self_partition);
+ sndev->peer_partition = ffs(part_map) - 1;
+
+ dev_dbg(&sndev->stdev->dev, "Partition ID %d of %d (%llx)",
+ sndev->self_partition, sndev->stdev->partition_count,
+ part_map);
+
+ sndev->mmio_ctrl = (void * __iomem)sndev->mmio_ntb +
+ SWITCHTEC_NTB_REG_CTRL_OFFSET;
+ sndev->mmio_dbmsg = (void * __iomem)sndev->mmio_ntb +
+ SWITCHTEC_NTB_REG_DBMSG_OFFSET;
+
+ sndev->mmio_self_ctrl = &sndev->mmio_ctrl[sndev->self_partition];
+ sndev->mmio_peer_ctrl = &sndev->mmio_ctrl[sndev->peer_partition];
+ sndev->mmio_self_dbmsg = &sndev->mmio_dbmsg[sndev->self_partition];
+}
+
+static int map_bars(int *map, struct ntb_ctrl_regs __iomem *ctrl)
+{
+ int i;
+ int cnt = 0;
+
+ for (i = 0; i < ARRAY_SIZE(ctrl->bar_entry); i++) {
+ u32 r = ioread32(&ctrl->bar_entry[i].ctl);
+
+ if (r & NTB_CTRL_BAR_VALID)
+ map[cnt++] = i;
+ }
+
+ return cnt;
+}
+
+static void switchtec_ntb_init_mw(struct switchtec_ntb *sndev)
+{
+ sndev->nr_direct_mw = map_bars(sndev->direct_mw_to_bar,
+ sndev->mmio_self_ctrl);
+
+ sndev->nr_lut_mw = ioread16(&sndev->mmio_self_ctrl->lut_table_entries);
+ sndev->nr_lut_mw = rounddown_pow_of_two(sndev->nr_lut_mw);
+
+ dev_dbg(&sndev->stdev->dev, "MWs: %d direct, %d lut",
+ sndev->nr_direct_mw, sndev->nr_lut_mw);
+
+ sndev->peer_nr_direct_mw = map_bars(sndev->peer_direct_mw_to_bar,
+ sndev->mmio_peer_ctrl);
+
+ sndev->peer_nr_lut_mw =
+ ioread16(&sndev->mmio_peer_ctrl->lut_table_entries);
+ sndev->peer_nr_lut_mw = rounddown_pow_of_two(sndev->peer_nr_lut_mw);
+
+ dev_dbg(&sndev->stdev->dev, "Peer MWs: %d direct, %d lut",
+ sndev->peer_nr_direct_mw, sndev->peer_nr_lut_mw);
+
+}
+
+static int switchtec_ntb_init_req_id_table(struct switchtec_ntb *sndev)
+{
+ int rc = 0;
+ u16 req_id = ioread16(&sndev->mmio_ntb->requester_id);
+ u32 error;
+
+ if (ioread32(&sndev->mmio_self_ctrl->req_id_table_size) < 2) {
+ dev_err(&sndev->stdev->dev,
+ "Not enough requester IDs available.");
+ return -EFAULT;
+ }
+
+ rc = switchtec_ntb_part_op(sndev, sndev->mmio_self_ctrl,
+ NTB_CTRL_PART_OP_LOCK,
+ NTB_CTRL_PART_STATUS_LOCKED);
+ if (rc)
+ return rc;
+
+ iowrite32(NTB_PART_CTRL_ID_PROT_DIS,
+ &sndev->mmio_self_ctrl->partition_ctrl);
+
+ // Root Complex Requester ID
+ iowrite32(0 << 16 | NTB_CTRL_REQ_ID_EN,
+ &sndev->mmio_self_ctrl->req_id_table[0]);
+
+ // Host Bridge Requester ID
+ iowrite32(req_id << 16 | NTB_CTRL_REQ_ID_EN,
+ &sndev->mmio_self_ctrl->req_id_table[1]);
+
+ rc = switchtec_ntb_part_op(sndev, sndev->mmio_self_ctrl,
+ NTB_CTRL_PART_OP_CFG,
+ NTB_CTRL_PART_STATUS_NORMAL);
+
+ if (rc == -EIO) {
+ error = ioread32(&sndev->mmio_self_ctrl->req_id_error);
+ dev_err(&sndev->stdev->dev,
+ "Error setting up the requester ID table: %08x",
+ error);
+ }
+
+ return rc;
+}
+
+static void switchtec_ntb_init_shared(struct switchtec_ntb *sndev)
+{
+ int i;
+
+ memset(sndev->self_shared, 0, LUT_SIZE);
+ sndev->self_shared->magic = SWITCHTEC_NTB_MAGIC;
+ sndev->self_shared->partition_id = sndev->stdev->partition;
+
+ for (i = 0; i < sndev->nr_direct_mw; i++) {
+ int bar = sndev->direct_mw_to_bar[i];
+ resource_size_t sz = pci_resource_len(sndev->stdev->pdev, bar);
+
+ if (i == 0)
+ sz = min_t(resource_size_t, sz,
+ LUT_SIZE * sndev->nr_lut_mw);
+
+ sndev->self_shared->mw_sizes[i] = sz;
+ }
+
+ for (i = 0; i < sndev->nr_lut_mw; i++) {
+ int idx = sndev->nr_direct_mw + i;
+
+ sndev->self_shared->mw_sizes[idx] = LUT_SIZE;
+ }
+}
+
+static int switchtec_ntb_init_shared_mw(struct switchtec_ntb *sndev)
+{
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+ int bar = sndev->direct_mw_to_bar[0];
+ u32 ctl_val;
+ int rc;
+
+ sndev->self_shared = dma_zalloc_coherent(&sndev->stdev->pdev->dev,
+ LUT_SIZE,
+ &sndev->self_shared_dma,
+ GFP_KERNEL);
+ if (!sndev->self_shared) {
+ dev_err(&sndev->stdev->dev,
+ "unable to allocate memory for shared mw");
+ return -ENOMEM;
+ }
+
+ switchtec_ntb_init_shared(sndev);
+
+ rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_LOCK,
+ NTB_CTRL_PART_STATUS_LOCKED);
+ if (rc)
+ goto unalloc_and_exit;
+
+ ctl_val = ioread32(&ctl->bar_entry[bar].ctl);
+ ctl_val |= NTB_CTRL_BAR_LUT_WIN_EN;
+ ctl_val &= 0xFF;
+ ctl_val |= ilog2(LUT_SIZE) << 8;
+ ctl_val |= (sndev->nr_lut_mw - 1) << 14;
+ iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
+
+ iowrite64((NTB_CTRL_LUT_EN | (sndev->self_partition << 1) |
+ sndev->self_shared_dma),
+ &ctl->lut_entry[0]);
+
+ rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_CFG,
+ NTB_CTRL_PART_STATUS_NORMAL);
+ if (rc) {
+ u32 bar_error, lut_error;
+
+ bar_error = ioread32(&ctl->bar_error);
+ lut_error = ioread32(&ctl->lut_error);
+ dev_err(&sndev->stdev->dev,
+ "Error setting up shared MW: %08x / %08x",
+ bar_error, lut_error);
+ goto unalloc_and_exit;
+ }
+
+ sndev->peer_shared = pci_iomap(sndev->stdev->pdev, bar, LUT_SIZE);
+ if (!sndev->peer_shared) {
+ rc = -ENOMEM;
+ goto unalloc_and_exit;
+ }
+
+ dev_dbg(&sndev->stdev->dev, "Shared MW Ready");
+ return 0;
+
+unalloc_and_exit:
+ dma_free_coherent(&sndev->stdev->pdev->dev, LUT_SIZE,
+ sndev->self_shared, sndev->self_shared_dma);
+
+ return rc;
+}
+
+static void switchtec_ntb_deinit_shared_mw(struct switchtec_ntb *sndev)
+{
+ if (sndev->peer_shared)
+ pci_iounmap(sndev->stdev->pdev, sndev->peer_shared);
+
+ if (sndev->self_shared)
+ dma_free_coherent(&sndev->stdev->pdev->dev, LUT_SIZE,
+ sndev->self_shared,
+ sndev->self_shared_dma);
+}
+
static int switchtec_ntb_add(struct device *dev,
struct class_interface *class_intf)
{
struct switchtec_dev *stdev = to_stdev(dev);
struct switchtec_ntb *sndev;
+ int rc;

stdev->sndev = NULL;

if (stdev->pdev->class != MICROSEMI_NTB_CLASSCODE)
return -ENODEV;

+ if (stdev->partition_count != 2)
+ dev_warn(dev, "ntb driver only supports 2 partitions");
+
sndev = kzalloc_node(sizeof(*sndev), GFP_KERNEL, dev_to_node(dev));
if (!sndev)
return -ENOMEM;

sndev->stdev = stdev;

+ switchtec_ntb_init_sndev(sndev);
+ switchtec_ntb_init_mw(sndev);
+
+ rc = switchtec_ntb_init_req_id_table(sndev);
+ if (rc)
+ goto free_and_exit;
+
+ rc = switchtec_ntb_init_shared_mw(sndev);
+ if (rc)
+ goto free_and_exit;
+
stdev->sndev = sndev;
dev_info(dev, "NTB device registered");

return 0;
+
+free_and_exit:
+ kfree(sndev);
+ dev_err(dev, "failed to register ntb device: %d", rc);
+ return rc;
}

void switchtec_ntb_remove(struct device *dev,
@@ -58,6 +412,7 @@ void switchtec_ntb_remove(struct device *dev,
return;

stdev->sndev = NULL;
+ switchtec_ntb_deinit_shared_mw(sndev);
kfree(sndev);
dev_info(dev, "ntb device unregistered");
}
--
2.11.0

2017-07-25 21:00:38

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 01/16] switchtec: move structure definitions into a common header

Create the switchtec.h header in include/linux with hardware defines
and the switchtec_dev structure. Both moved directly from switchtec.c.
This is a prep patch for creating an NTB driver for Switchtec.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
MAINTAINERS | 1 +
drivers/pci/switch/switchtec.c | 260 +-------------------------------------
include/linux/switchtec.h | 279 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 281 insertions(+), 259 deletions(-)
create mode 100644 include/linux/switchtec.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 205d3977ac46..4ff2ad7c1c7b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10146,6 +10146,7 @@ F: Documentation/switchtec.txt
F: Documentation/ABI/testing/sysfs-class-switchtec
F: drivers/pci/switch/switchtec*
F: include/uapi/linux/switchtec_ioctl.h
+F: include/linux/switchtec.h

PCI DRIVER FOR NVIDIA TEGRA
M: Thierry Reding <[email protected]>
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index af81b2dec42e..5b75d3008ff8 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -13,6 +13,7 @@
*
*/

+#include <linux/switchtec.h>
#include <linux/switchtec_ioctl.h>

#include <linux/interrupt.h>
@@ -20,8 +21,6 @@
#include <linux/fs.h>
#include <linux/uaccess.h>
#include <linux/poll.h>
-#include <linux/pci.h>
-#include <linux/cdev.h>
#include <linux/wait.h>

MODULE_DESCRIPTION("Microsemi Switchtec(tm) PCIe Management Driver");
@@ -37,263 +36,6 @@ static dev_t switchtec_devt;
static struct class *switchtec_class;
static DEFINE_IDA(switchtec_minor_ida);

-#define MICROSEMI_VENDOR_ID 0x11f8
-#define MICROSEMI_NTB_CLASSCODE 0x068000
-#define MICROSEMI_MGMT_CLASSCODE 0x058000
-
-#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
-#define SWITCHTEC_MAX_PFF_CSR 48
-
-#define SWITCHTEC_EVENT_OCCURRED BIT(0)
-#define SWITCHTEC_EVENT_CLEAR BIT(0)
-#define SWITCHTEC_EVENT_EN_LOG BIT(1)
-#define SWITCHTEC_EVENT_EN_CLI BIT(2)
-#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
-#define SWITCHTEC_EVENT_FATAL BIT(4)
-
-enum {
- SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
- SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
- SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
- SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
- SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
- SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
- SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
- SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
-};
-
-struct mrpc_regs {
- u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
- u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
- u32 cmd;
- u32 status;
- u32 ret_value;
-} __packed;
-
-enum mrpc_status {
- SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
- SWITCHTEC_MRPC_STATUS_DONE = 2,
- SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
- SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
-};
-
-struct sw_event_regs {
- u64 event_report_ctrl;
- u64 reserved1;
- u64 part_event_bitmap;
- u64 reserved2;
- u32 global_summary;
- u32 reserved3[3];
- u32 stack_error_event_hdr;
- u32 stack_error_event_data;
- u32 reserved4[4];
- u32 ppu_error_event_hdr;
- u32 ppu_error_event_data;
- u32 reserved5[4];
- u32 isp_error_event_hdr;
- u32 isp_error_event_data;
- u32 reserved6[4];
- u32 sys_reset_event_hdr;
- u32 reserved7[5];
- u32 fw_exception_hdr;
- u32 reserved8[5];
- u32 fw_nmi_hdr;
- u32 reserved9[5];
- u32 fw_non_fatal_hdr;
- u32 reserved10[5];
- u32 fw_fatal_hdr;
- u32 reserved11[5];
- u32 twi_mrpc_comp_hdr;
- u32 twi_mrpc_comp_data;
- u32 reserved12[4];
- u32 twi_mrpc_comp_async_hdr;
- u32 twi_mrpc_comp_async_data;
- u32 reserved13[4];
- u32 cli_mrpc_comp_hdr;
- u32 cli_mrpc_comp_data;
- u32 reserved14[4];
- u32 cli_mrpc_comp_async_hdr;
- u32 cli_mrpc_comp_async_data;
- u32 reserved15[4];
- u32 gpio_interrupt_hdr;
- u32 gpio_interrupt_data;
- u32 reserved16[4];
-} __packed;
-
-enum {
- SWITCHTEC_CFG0_RUNNING = 0x04,
- SWITCHTEC_CFG1_RUNNING = 0x05,
- SWITCHTEC_IMG0_RUNNING = 0x03,
- SWITCHTEC_IMG1_RUNNING = 0x07,
-};
-
-struct sys_info_regs {
- u32 device_id;
- u32 device_version;
- u32 firmware_version;
- u32 reserved1;
- u32 vendor_table_revision;
- u32 table_format_version;
- u32 partition_id;
- u32 cfg_file_fmt_version;
- u16 cfg_running;
- u16 img_running;
- u32 reserved2[57];
- char vendor_id[8];
- char product_id[16];
- char product_revision[4];
- char component_vendor[8];
- u16 component_id;
- u8 component_revision;
-} __packed;
-
-struct flash_info_regs {
- u32 flash_part_map_upd_idx;
-
- struct active_partition_info {
- u32 address;
- u32 build_version;
- u32 build_string;
- } active_img;
-
- struct active_partition_info active_cfg;
- struct active_partition_info inactive_img;
- struct active_partition_info inactive_cfg;
-
- u32 flash_length;
-
- struct partition_info {
- u32 address;
- u32 length;
- } cfg0;
-
- struct partition_info cfg1;
- struct partition_info img0;
- struct partition_info img1;
- struct partition_info nvlog;
- struct partition_info vendor[8];
-};
-
-struct ntb_info_regs {
- u8 partition_count;
- u8 partition_id;
- u16 reserved1;
- u64 ep_map;
- u16 requester_id;
-} __packed;
-
-struct part_cfg_regs {
- u32 status;
- u32 state;
- u32 port_cnt;
- u32 usp_port_mode;
- u32 usp_pff_inst_id;
- u32 vep_pff_inst_id;
- u32 dsp_pff_inst_id[47];
- u32 reserved1[11];
- u16 vep_vector_number;
- u16 usp_vector_number;
- u32 port_event_bitmap;
- u32 reserved2[3];
- u32 part_event_summary;
- u32 reserved3[3];
- u32 part_reset_hdr;
- u32 part_reset_data[5];
- u32 mrpc_comp_hdr;
- u32 mrpc_comp_data[5];
- u32 mrpc_comp_async_hdr;
- u32 mrpc_comp_async_data[5];
- u32 dyn_binding_hdr;
- u32 dyn_binding_data[5];
- u32 reserved4[159];
-} __packed;
-
-enum {
- SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
- SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
- SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
- SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
-};
-
-struct pff_csr_regs {
- u16 vendor_id;
- u16 device_id;
- u32 pci_cfg_header[15];
- u32 pci_cap_region[48];
- u32 pcie_cap_region[448];
- u32 indirect_gas_window[128];
- u32 indirect_gas_window_off;
- u32 reserved[127];
- u32 pff_event_summary;
- u32 reserved2[3];
- u32 aer_in_p2p_hdr;
- u32 aer_in_p2p_data[5];
- u32 aer_in_vep_hdr;
- u32 aer_in_vep_data[5];
- u32 dpc_hdr;
- u32 dpc_data[5];
- u32 cts_hdr;
- u32 cts_data[5];
- u32 reserved3[6];
- u32 hotplug_hdr;
- u32 hotplug_data[5];
- u32 ier_hdr;
- u32 ier_data[5];
- u32 threshold_hdr;
- u32 threshold_data[5];
- u32 power_mgmt_hdr;
- u32 power_mgmt_data[5];
- u32 tlp_throttling_hdr;
- u32 tlp_throttling_data[5];
- u32 force_speed_hdr;
- u32 force_speed_data[5];
- u32 credit_timeout_hdr;
- u32 credit_timeout_data[5];
- u32 link_state_hdr;
- u32 link_state_data[5];
- u32 reserved4[174];
-} __packed;
-
-struct switchtec_dev {
- struct pci_dev *pdev;
- struct device dev;
- struct cdev cdev;
-
- int partition;
- int partition_count;
- int pff_csr_count;
- char pff_local[SWITCHTEC_MAX_PFF_CSR];
-
- void __iomem *mmio;
- struct mrpc_regs __iomem *mmio_mrpc;
- struct sw_event_regs __iomem *mmio_sw_event;
- struct sys_info_regs __iomem *mmio_sys_info;
- struct flash_info_regs __iomem *mmio_flash_info;
- struct ntb_info_regs __iomem *mmio_ntb;
- struct part_cfg_regs __iomem *mmio_part_cfg;
- struct part_cfg_regs __iomem *mmio_part_cfg_all;
- struct pff_csr_regs __iomem *mmio_pff_csr;
-
- /*
- * The mrpc mutex must be held when accessing the other
- * mrpc_ fields, alive flag and stuser->state field
- */
- struct mutex mrpc_mutex;
- struct list_head mrpc_queue;
- int mrpc_busy;
- struct work_struct mrpc_work;
- struct delayed_work mrpc_timeout;
- bool alive;
-
- wait_queue_head_t event_wq;
- atomic_t event_cnt;
-};
-
-static struct switchtec_dev *to_stdev(struct device *dev)
-{
- return container_of(dev, struct switchtec_dev, dev);
-}
-
enum mrpc_state {
MRPC_IDLE = 0,
MRPC_QUEUED,
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
new file mode 100644
index 000000000000..1cbd0e63b0ab
--- /dev/null
+++ b/include/linux/switchtec.h
@@ -0,0 +1,279 @@
+/*
+ * Microsemi Switchtec PCIe Driver
+ * Copyright (c) 2017, Microsemi Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifndef _SWITCHTEC_H
+#define _SWITCHTEC_H
+
+#include <linux/pci.h>
+#include <linux/cdev.h>
+
+#define MICROSEMI_VENDOR_ID 0x11f8
+#define MICROSEMI_NTB_CLASSCODE 0x068000
+#define MICROSEMI_MGMT_CLASSCODE 0x058000
+
+#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
+#define SWITCHTEC_MAX_PFF_CSR 48
+
+#define SWITCHTEC_EVENT_OCCURRED BIT(0)
+#define SWITCHTEC_EVENT_CLEAR BIT(0)
+#define SWITCHTEC_EVENT_EN_LOG BIT(1)
+#define SWITCHTEC_EVENT_EN_CLI BIT(2)
+#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
+#define SWITCHTEC_EVENT_FATAL BIT(4)
+
+enum {
+ SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
+ SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
+ SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
+ SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
+ SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
+ SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
+ SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
+ SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
+};
+
+struct mrpc_regs {
+ u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
+ u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
+ u32 cmd;
+ u32 status;
+ u32 ret_value;
+} __packed;
+
+enum mrpc_status {
+ SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
+ SWITCHTEC_MRPC_STATUS_DONE = 2,
+ SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
+ SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
+};
+
+struct sw_event_regs {
+ u64 event_report_ctrl;
+ u64 reserved1;
+ u64 part_event_bitmap;
+ u64 reserved2;
+ u32 global_summary;
+ u32 reserved3[3];
+ u32 stack_error_event_hdr;
+ u32 stack_error_event_data;
+ u32 reserved4[4];
+ u32 ppu_error_event_hdr;
+ u32 ppu_error_event_data;
+ u32 reserved5[4];
+ u32 isp_error_event_hdr;
+ u32 isp_error_event_data;
+ u32 reserved6[4];
+ u32 sys_reset_event_hdr;
+ u32 reserved7[5];
+ u32 fw_exception_hdr;
+ u32 reserved8[5];
+ u32 fw_nmi_hdr;
+ u32 reserved9[5];
+ u32 fw_non_fatal_hdr;
+ u32 reserved10[5];
+ u32 fw_fatal_hdr;
+ u32 reserved11[5];
+ u32 twi_mrpc_comp_hdr;
+ u32 twi_mrpc_comp_data;
+ u32 reserved12[4];
+ u32 twi_mrpc_comp_async_hdr;
+ u32 twi_mrpc_comp_async_data;
+ u32 reserved13[4];
+ u32 cli_mrpc_comp_hdr;
+ u32 cli_mrpc_comp_data;
+ u32 reserved14[4];
+ u32 cli_mrpc_comp_async_hdr;
+ u32 cli_mrpc_comp_async_data;
+ u32 reserved15[4];
+ u32 gpio_interrupt_hdr;
+ u32 gpio_interrupt_data;
+ u32 reserved16[4];
+} __packed;
+
+enum {
+ SWITCHTEC_CFG0_RUNNING = 0x04,
+ SWITCHTEC_CFG1_RUNNING = 0x05,
+ SWITCHTEC_IMG0_RUNNING = 0x03,
+ SWITCHTEC_IMG1_RUNNING = 0x07,
+};
+
+struct sys_info_regs {
+ u32 device_id;
+ u32 device_version;
+ u32 firmware_version;
+ u32 reserved1;
+ u32 vendor_table_revision;
+ u32 table_format_version;
+ u32 partition_id;
+ u32 cfg_file_fmt_version;
+ u16 cfg_running;
+ u16 img_running;
+ u32 reserved2[57];
+ char vendor_id[8];
+ char product_id[16];
+ char product_revision[4];
+ char component_vendor[8];
+ u16 component_id;
+ u8 component_revision;
+} __packed;
+
+struct flash_info_regs {
+ u32 flash_part_map_upd_idx;
+
+ struct active_partition_info {
+ u32 address;
+ u32 build_version;
+ u32 build_string;
+ } active_img;
+
+ struct active_partition_info active_cfg;
+ struct active_partition_info inactive_img;
+ struct active_partition_info inactive_cfg;
+
+ u32 flash_length;
+
+ struct partition_info {
+ u32 address;
+ u32 length;
+ } cfg0;
+
+ struct partition_info cfg1;
+ struct partition_info img0;
+ struct partition_info img1;
+ struct partition_info nvlog;
+ struct partition_info vendor[8];
+};
+
+struct ntb_info_regs {
+ u8 partition_count;
+ u8 partition_id;
+ u16 reserved1;
+ u64 ep_map;
+ u16 requester_id;
+} __packed;
+
+struct part_cfg_regs {
+ u32 status;
+ u32 state;
+ u32 port_cnt;
+ u32 usp_port_mode;
+ u32 usp_pff_inst_id;
+ u32 vep_pff_inst_id;
+ u32 dsp_pff_inst_id[47];
+ u32 reserved1[11];
+ u16 vep_vector_number;
+ u16 usp_vector_number;
+ u32 port_event_bitmap;
+ u32 reserved2[3];
+ u32 part_event_summary;
+ u32 reserved3[3];
+ u32 part_reset_hdr;
+ u32 part_reset_data[5];
+ u32 mrpc_comp_hdr;
+ u32 mrpc_comp_data[5];
+ u32 mrpc_comp_async_hdr;
+ u32 mrpc_comp_async_data[5];
+ u32 dyn_binding_hdr;
+ u32 dyn_binding_data[5];
+ u32 reserved4[159];
+} __packed;
+
+enum {
+ SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
+ SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
+ SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
+ SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
+};
+
+struct pff_csr_regs {
+ u16 vendor_id;
+ u16 device_id;
+ u32 pci_cfg_header[15];
+ u32 pci_cap_region[48];
+ u32 pcie_cap_region[448];
+ u32 indirect_gas_window[128];
+ u32 indirect_gas_window_off;
+ u32 reserved[127];
+ u32 pff_event_summary;
+ u32 reserved2[3];
+ u32 aer_in_p2p_hdr;
+ u32 aer_in_p2p_data[5];
+ u32 aer_in_vep_hdr;
+ u32 aer_in_vep_data[5];
+ u32 dpc_hdr;
+ u32 dpc_data[5];
+ u32 cts_hdr;
+ u32 cts_data[5];
+ u32 reserved3[6];
+ u32 hotplug_hdr;
+ u32 hotplug_data[5];
+ u32 ier_hdr;
+ u32 ier_data[5];
+ u32 threshold_hdr;
+ u32 threshold_data[5];
+ u32 power_mgmt_hdr;
+ u32 power_mgmt_data[5];
+ u32 tlp_throttling_hdr;
+ u32 tlp_throttling_data[5];
+ u32 force_speed_hdr;
+ u32 force_speed_data[5];
+ u32 credit_timeout_hdr;
+ u32 credit_timeout_data[5];
+ u32 link_state_hdr;
+ u32 link_state_data[5];
+ u32 reserved4[174];
+} __packed;
+
+struct switchtec_dev {
+ struct pci_dev *pdev;
+ struct device dev;
+ struct cdev cdev;
+
+ int partition;
+ int partition_count;
+ int pff_csr_count;
+ char pff_local[SWITCHTEC_MAX_PFF_CSR];
+
+ void __iomem *mmio;
+ struct mrpc_regs __iomem *mmio_mrpc;
+ struct sw_event_regs __iomem *mmio_sw_event;
+ struct sys_info_regs __iomem *mmio_sys_info;
+ struct flash_info_regs __iomem *mmio_flash_info;
+ struct ntb_info_regs __iomem *mmio_ntb;
+ struct part_cfg_regs __iomem *mmio_part_cfg;
+ struct part_cfg_regs __iomem *mmio_part_cfg_all;
+ struct pff_csr_regs __iomem *mmio_pff_csr;
+
+ /*
+ * The mrpc mutex must be held when accessing the other
+ * mrpc_ fields, alive flag and stuser->state field
+ */
+ struct mutex mrpc_mutex;
+ struct list_head mrpc_queue;
+ int mrpc_busy;
+ struct work_struct mrpc_work;
+ struct delayed_work mrpc_timeout;
+ bool alive;
+
+ wait_queue_head_t event_wq;
+ atomic_t event_cnt;
+};
+
+static inline struct switchtec_dev *to_stdev(struct device *dev)
+{
+ return container_of(dev, struct switchtec_dev, dev);
+}
+
+#endif
--
2.11.0

2017-07-25 21:01:02

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 04/16] switchtec: add link event notifier callback

In order for the Switchtec NTB code to handle link change events we
create a notifier callback in the switchtec code which gets called
whenever an appropriate event interrupt occurs.

In order to preserve userspace's ability to follow these events,
we compare the event count with a stored copy from last time we
checked.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/switch/switchtec.c | 51 ++++++++++++++++++++++++++++++++++++++++++
include/linux/switchtec.h | 4 ++++
2 files changed, 55 insertions(+)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 39c9218d733f..807569e62bee 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -978,6 +978,49 @@ static const struct file_operations switchtec_fops = {
.compat_ioctl = switchtec_dev_ioctl,
};

+static void link_event_work(struct work_struct *work)
+{
+ struct switchtec_dev *stdev;
+
+ stdev = container_of(work, struct switchtec_dev, link_event_work);
+
+ if (stdev->link_notifier)
+ stdev->link_notifier(stdev);
+}
+
+static void check_link_state_events(struct switchtec_dev *stdev)
+{
+ int idx;
+ u32 reg;
+ int count;
+ int occurred = 0;
+
+ for (idx = 0; idx < stdev->pff_csr_count; idx++) {
+ reg = ioread32(&stdev->mmio_pff_csr[idx].link_state_hdr);
+ dev_dbg(&stdev->dev, "link_state: %d->%08x\n", idx, reg);
+ count = (reg >> 5) & 0xFF;
+
+ if (count != stdev->link_event_count[idx]) {
+ occurred = 1;
+ stdev->link_event_count[idx] = count;
+ }
+ }
+
+ if (occurred)
+ schedule_work(&stdev->link_event_work);
+}
+
+static void enable_link_state_events(struct switchtec_dev *stdev)
+{
+ int idx;
+
+ for (idx = 0; idx < stdev->pff_csr_count; idx++) {
+ iowrite32(SWITCHTEC_EVENT_CLEAR |
+ SWITCHTEC_EVENT_EN_IRQ,
+ &stdev->mmio_pff_csr[idx].link_state_hdr);
+ }
+}
+
static void stdev_release(struct device *dev)
{
struct switchtec_dev *stdev = to_stdev(dev);
@@ -1030,6 +1073,7 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)
stdev->mrpc_busy = 0;
INIT_WORK(&stdev->mrpc_work, mrpc_event_work);
INIT_DELAYED_WORK(&stdev->mrpc_timeout, mrpc_timeout_work);
+ INIT_WORK(&stdev->link_event_work, link_event_work);
init_waitqueue_head(&stdev->event_wq);
atomic_set(&stdev->event_cnt, 0);

@@ -1073,6 +1117,9 @@ static int mask_event(struct switchtec_dev *stdev, int eid, int idx)
if (!(hdr & SWITCHTEC_EVENT_OCCURRED && hdr & SWITCHTEC_EVENT_EN_IRQ))
return 0;

+ if (eid == SWITCHTEC_IOCTL_EVENT_LINK_STATE)
+ return 0;
+
dev_dbg(&stdev->dev, "%s: %d %d %x\n", __func__, eid, idx, hdr);
hdr &= ~(SWITCHTEC_EVENT_EN_IRQ | SWITCHTEC_EVENT_OCCURRED);
iowrite32(hdr, hdr_reg);
@@ -1092,6 +1139,7 @@ static int mask_all_events(struct switchtec_dev *stdev, int eid)
for (idx = 0; idx < stdev->pff_csr_count; idx++) {
if (!stdev->pff_local[idx])
continue;
+
count += mask_event(stdev, eid, idx);
}
} else {
@@ -1116,6 +1164,8 @@ static irqreturn_t switchtec_event_isr(int irq, void *dev)
iowrite32(reg, &stdev->mmio_part_cfg->mrpc_comp_hdr);
}

+ check_link_state_events(stdev);
+
for (eid = 0; eid < SWITCHTEC_IOCTL_MAX_EVENTS; eid++)
event_count += mask_all_events(stdev, eid);

@@ -1242,6 +1292,7 @@ static int switchtec_pci_probe(struct pci_dev *pdev,
iowrite32(SWITCHTEC_EVENT_CLEAR |
SWITCHTEC_EVENT_EN_IRQ,
&stdev->mmio_part_cfg->mrpc_comp_hdr);
+ enable_link_state_events(stdev);

rc = cdev_device_add(&stdev->cdev, &stdev->dev);
if (rc)
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 9c2be7631257..d8159944f013 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -353,6 +353,10 @@ struct switchtec_dev {

wait_queue_head_t event_wq;
atomic_t event_cnt;
+
+ struct work_struct link_event_work;
+ void (*link_notifier)(struct switchtec_dev *stdev);
+ u8 link_event_count[SWITCHTEC_MAX_PFF_CSR];
};

static inline struct switchtec_dev *to_stdev(struct device *dev)
--
2.11.0

2017-07-25 21:01:01

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 11/16] switchtec_ntb: add skeleton NTB driver

Add a skeleton NTB driver which will be filled out in subsequent patches.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 148 +++++++++++++++++++++++++++++++++++-
include/linux/ntb.h | 3 +
2 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 877337902363..f87c0eef59ef 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -18,6 +18,7 @@
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/interrupt.h>
+#include <linux/ntb.h>

MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
MODULE_VERSION("0.1");
@@ -71,6 +72,7 @@ struct shared_mw {
#define LUT_SIZE SZ_64K

struct switchtec_ntb {
+ struct ntb_dev ntb;
struct switchtec_dev *stdev;

int self_partition;
@@ -161,10 +163,148 @@ static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
return -EIO;
}

+static int switchtec_ntb_mw_count(struct ntb_dev *ntb, int pidx)
+{
+ return 0;
+}
+
+static int switchtec_ntb_mw_get_align(struct ntb_dev *ntb, int pidx,
+ int widx, resource_size_t *addr_align,
+ resource_size_t *size_align,
+ resource_size_t *size_max)
+{
+ return 0;
+}
+
+static int switchtec_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
+ dma_addr_t addr, resource_size_t size)
+{
+ return 0;
+}
+
+static int switchtec_ntb_peer_mw_count(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static int switchtec_ntb_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
+ phys_addr_t *base,
+ resource_size_t *size)
+{
+ return 0;
+}
+
+static u64 switchtec_ntb_link_is_up(struct ntb_dev *ntb,
+ enum ntb_speed *speed,
+ enum ntb_width *width)
+{
+ return 0;
+}
+
+static int switchtec_ntb_link_enable(struct ntb_dev *ntb,
+ enum ntb_speed max_speed,
+ enum ntb_width max_width)
+{
+ return 0;
+}
+
+static int switchtec_ntb_link_disable(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static u64 switchtec_ntb_db_valid_mask(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static int switchtec_ntb_db_vector_count(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static u64 switchtec_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
+{
+ return 0;
+}
+
+static u64 switchtec_ntb_db_read(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static int switchtec_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
+{
+ return 0;
+}
+
+static int switchtec_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+ return 0;
+}
+
+static int switchtec_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+ return 0;
+}
+
+static int switchtec_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
+{
+ return 0;
+}
+
+static int switchtec_ntb_spad_count(struct ntb_dev *ntb)
+{
+ return 0;
+}
+
+static u32 switchtec_ntb_spad_read(struct ntb_dev *ntb, int idx)
+{
+ return 0;
+}
+
+static int switchtec_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val)
+{
+ return 0;
+}
+
+static int switchtec_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
+ int sidx, u32 val)
+{
+ return 0;
+}
+
+static const struct ntb_dev_ops switchtec_ntb_ops = {
+ .mw_count = switchtec_ntb_mw_count,
+ .mw_get_align = switchtec_ntb_mw_get_align,
+ .mw_set_trans = switchtec_ntb_mw_set_trans,
+ .peer_mw_count = switchtec_ntb_peer_mw_count,
+ .peer_mw_get_addr = switchtec_ntb_peer_mw_get_addr,
+ .link_is_up = switchtec_ntb_link_is_up,
+ .link_enable = switchtec_ntb_link_enable,
+ .link_disable = switchtec_ntb_link_disable,
+ .db_valid_mask = switchtec_ntb_db_valid_mask,
+ .db_vector_count = switchtec_ntb_db_vector_count,
+ .db_vector_mask = switchtec_ntb_db_vector_mask,
+ .db_read = switchtec_ntb_db_read,
+ .db_clear = switchtec_ntb_db_clear,
+ .db_set_mask = switchtec_ntb_db_set_mask,
+ .db_clear_mask = switchtec_ntb_db_clear_mask,
+ .peer_db_set = switchtec_ntb_peer_db_set,
+ .spad_count = switchtec_ntb_spad_count,
+ .spad_read = switchtec_ntb_spad_read,
+ .spad_write = switchtec_ntb_spad_write,
+ .peer_spad_write = switchtec_ntb_peer_spad_write,
+};
+
static void switchtec_ntb_init_sndev(struct switchtec_ntb *sndev)
{
u64 part_map;

+ sndev->ntb.pdev = sndev->stdev->pdev;
+ sndev->ntb.topo = NTB_TOPO_SWITCH;
+ sndev->ntb.ops = &switchtec_ntb_ops;
+
sndev->self_partition = sndev->stdev->partition;

sndev->mmio_ntb = sndev->stdev->mmio_ntb;
@@ -512,7 +652,6 @@ static int switchtec_ntb_add(struct device *dev,
return -ENOMEM;

sndev->stdev = stdev;
-
switchtec_ntb_init_sndev(sndev);
switchtec_ntb_init_mw(sndev);
switchtec_ntb_init_db(sndev);
@@ -530,11 +669,17 @@ static int switchtec_ntb_add(struct device *dev,
if (rc)
goto deinit_shared_and_exit;

+ rc = ntb_register_device(&sndev->ntb);
+ if (rc)
+ goto deinit_and_exit;
+
stdev->sndev = sndev;
dev_info(dev, "NTB device registered");

return 0;

+deinit_and_exit:
+ switchtec_ntb_deinit_db_msg_irq(sndev);
deinit_shared_and_exit:
switchtec_ntb_deinit_shared_mw(sndev);
free_and_exit:
@@ -553,6 +698,7 @@ void switchtec_ntb_remove(struct device *dev,
return;

stdev->sndev = NULL;
+ ntb_unregister_device(&sndev->ntb);
switchtec_ntb_deinit_db_msg_irq(sndev);
switchtec_ntb_deinit_shared_mw(sndev);
kfree(sndev);
diff --git a/include/linux/ntb.h b/include/linux/ntb.h
index 47f2966cfd7f..c308964777eb 100644
--- a/include/linux/ntb.h
+++ b/include/linux/ntb.h
@@ -70,6 +70,7 @@ struct pci_dev;
* @NTB_TOPO_SEC: On secondary side of remote ntb.
* @NTB_TOPO_B2B_USD: On primary side of local ntb upstream of remote ntb.
* @NTB_TOPO_B2B_DSD: On primary side of local ntb downstream of remote ntb.
+ * @NTB_TOPO_SWITCH: Connected via a switch which supports ntb.
*/
enum ntb_topo {
NTB_TOPO_NONE = -1,
@@ -77,6 +78,7 @@ enum ntb_topo {
NTB_TOPO_SEC,
NTB_TOPO_B2B_USD,
NTB_TOPO_B2B_DSD,
+ NTB_TOPO_SWITCH,
};

static inline int ntb_topo_is_b2b(enum ntb_topo topo)
@@ -97,6 +99,7 @@ static inline char *ntb_topo_string(enum ntb_topo topo)
case NTB_TOPO_SEC: return "NTB_TOPO_SEC";
case NTB_TOPO_B2B_USD: return "NTB_TOPO_B2B_USD";
case NTB_TOPO_B2B_DSD: return "NTB_TOPO_B2B_DSD";
+ case NTB_TOPO_SWITCH: return "NTB_TOPO_SWITCH";
}
return "NTB_TOPO_INVALID";
}
--
2.11.0

2017-07-25 21:00:58

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 10/16] switchtec_ntb: initialize hardware for doorbells and messages

Set up some hardware registers and creates interrupt service routines
for the doorbells and messages.

There are 64 doorbells in the switch that are shared between all
partitions. The upper 4 doorbells are also shared with the messages
and are there for not used. Thus, this provides 28 doorbells for each
partition.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 142 ++++++++++++++++++++++++++++++++++++
1 file changed, 142 insertions(+)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 1596757413f5..877337902363 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/kthread.h>
+#include <linux/interrupt.h>

MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
MODULE_VERSION("0.1");
@@ -75,6 +76,9 @@ struct switchtec_ntb {
int self_partition;
int peer_partition;

+ int doorbell_irq;
+ int message_irq;
+
struct ntb_info_regs __iomem *mmio_ntb;
struct ntb_ctrl_regs __iomem *mmio_ctrl;
struct ntb_dbmsg_regs __iomem *mmio_dbmsg;
@@ -86,6 +90,11 @@ struct switchtec_ntb {
struct shared_mw __iomem *peer_shared;
dma_addr_t self_shared_dma;

+ u64 db_mask;
+ u64 db_valid_mask;
+ int db_shift;
+ int db_peer_shift;
+
int nr_direct_mw;
int nr_lut_mw;
int direct_mw_to_bar[MAX_DIRECT_MW];
@@ -215,6 +224,49 @@ static void switchtec_ntb_init_mw(struct switchtec_ntb *sndev)

}

+/*
+ * There are 64 doorbells in the switch hardware but this is
+ * shared among all partitions. So we must split them in half
+ * (32 for each partition). However, the message interrupts are
+ * also shared with the top 4 doorbells so we just limit this to
+ * 28 doorbells per partition
+ */
+static void switchtec_ntb_init_db(struct switchtec_ntb *sndev)
+{
+ sndev->db_valid_mask = 0x0FFFFFFF;
+
+ if (sndev->self_partition < sndev->peer_partition) {
+ sndev->db_shift = 0;
+ sndev->db_peer_shift = 32;
+ } else {
+ sndev->db_shift = 32;
+ sndev->db_peer_shift = 0;
+ }
+
+ sndev->db_mask = 0x0FFFFFFFFFFFFFFFULL;
+ iowrite64(~sndev->db_mask, &sndev->mmio_self_dbmsg->idb_mask);
+ iowrite64(sndev->db_valid_mask << sndev->db_peer_shift,
+ &sndev->mmio_self_dbmsg->odb_mask);
+}
+
+static void switchtec_ntb_init_msgs(struct switchtec_ntb *sndev)
+{
+ int i;
+ u32 msg_map = 0;
+
+ for (i = 0; i < ARRAY_SIZE(sndev->mmio_self_dbmsg->imsg); i++) {
+ int m = i | sndev->peer_partition << 2;
+
+ msg_map |= m << i * 8;
+ }
+
+ iowrite32(msg_map, &sndev->mmio_self_dbmsg->msg_map);
+
+ for (i = 0; i < ARRAY_SIZE(sndev->mmio_self_dbmsg->imsg); i++)
+ iowrite64(NTB_DBMSG_IMSG_STATUS | NTB_DBMSG_IMSG_MASK,
+ &sndev->mmio_self_dbmsg->imsg[i]);
+}
+
static int switchtec_ntb_init_req_id_table(struct switchtec_ntb *sndev)
{
int rc = 0;
@@ -359,6 +411,87 @@ static void switchtec_ntb_deinit_shared_mw(struct switchtec_ntb *sndev)
sndev->self_shared_dma);
}

+static irqreturn_t switchtec_ntb_doorbell_isr(int irq, void *dev)
+{
+ struct switchtec_ntb *sndev = dev;
+
+ dev_dbg(&sndev->stdev->dev, "doorbell\n");
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t switchtec_ntb_message_isr(int irq, void *dev)
+{
+ int i;
+ struct switchtec_ntb *sndev = dev;
+
+ for (i = 0; i < ARRAY_SIZE(sndev->mmio_self_dbmsg->imsg); i++) {
+ u64 msg = ioread64(&sndev->mmio_self_dbmsg->imsg[i]);
+
+ if (msg & NTB_DBMSG_IMSG_STATUS) {
+ dev_dbg(&sndev->stdev->dev, "message: %d %08x\n", i,
+ (u32)msg);
+ iowrite8(1, &sndev->mmio_self_dbmsg->imsg[i].status);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int switchtec_ntb_init_db_msg_irq(struct switchtec_ntb *sndev)
+{
+ int i;
+ int rc;
+ int doorbell_irq = 0;
+ int message_irq = 0;
+ int event_irq;
+ int idb_vecs = sizeof(sndev->mmio_self_dbmsg->idb_vec_map);
+
+ event_irq = ioread32(&sndev->stdev->mmio_part_cfg->vep_vector_number);
+
+ while (doorbell_irq == event_irq)
+ doorbell_irq++;
+ while (message_irq == doorbell_irq ||
+ message_irq == event_irq)
+ message_irq++;
+
+ dev_dbg(&sndev->stdev->dev, "irqs - event: %d, db: %d, msgs: %d",
+ event_irq, doorbell_irq, message_irq);
+
+ for (i = 0; i < idb_vecs - 4; i++)
+ iowrite8(doorbell_irq,
+ &sndev->mmio_self_dbmsg->idb_vec_map[i]);
+
+ for (; i < idb_vecs; i++)
+ iowrite8(message_irq,
+ &sndev->mmio_self_dbmsg->idb_vec_map[i]);
+
+ sndev->doorbell_irq = pci_irq_vector(sndev->stdev->pdev, doorbell_irq);
+ sndev->message_irq = pci_irq_vector(sndev->stdev->pdev, message_irq);
+
+ rc = request_irq(sndev->doorbell_irq,
+ switchtec_ntb_doorbell_isr, 0,
+ "switchtec_ntb_doorbell", sndev);
+ if (rc)
+ return rc;
+
+ rc = request_irq(sndev->message_irq,
+ switchtec_ntb_message_isr, 0,
+ "switchtec_ntb_message", sndev);
+ if (rc) {
+ free_irq(sndev->doorbell_irq, sndev);
+ return rc;
+ }
+
+ return 0;
+}
+
+static void switchtec_ntb_deinit_db_msg_irq(struct switchtec_ntb *sndev)
+{
+ free_irq(sndev->doorbell_irq, sndev);
+ free_irq(sndev->message_irq, sndev);
+}
+
static int switchtec_ntb_add(struct device *dev,
struct class_interface *class_intf)
{
@@ -382,6 +515,8 @@ static int switchtec_ntb_add(struct device *dev,

switchtec_ntb_init_sndev(sndev);
switchtec_ntb_init_mw(sndev);
+ switchtec_ntb_init_db(sndev);
+ switchtec_ntb_init_msgs(sndev);

rc = switchtec_ntb_init_req_id_table(sndev);
if (rc)
@@ -391,11 +526,17 @@ static int switchtec_ntb_add(struct device *dev,
if (rc)
goto free_and_exit;

+ rc = switchtec_ntb_init_db_msg_irq(sndev);
+ if (rc)
+ goto deinit_shared_and_exit;
+
stdev->sndev = sndev;
dev_info(dev, "NTB device registered");

return 0;

+deinit_shared_and_exit:
+ switchtec_ntb_deinit_shared_mw(sndev);
free_and_exit:
kfree(sndev);
dev_err(dev, "failed to register ntb device: %d", rc);
@@ -412,6 +553,7 @@ void switchtec_ntb_remove(struct device *dev,
return;

stdev->sndev = NULL;
+ switchtec_ntb_deinit_db_msg_irq(sndev);
switchtec_ntb_deinit_shared_mw(sndev);
kfree(sndev);
dev_info(dev, "ntb device unregistered");
--
2.11.0

2017-07-25 20:58:09

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 05/16] ntb: ntb_test: ensure the link is up before trying to configure the mws

After the link tests, there is a race on one side of the test for
the link coming up. It's possible, in some cases, for the test script
to write to the 'peer_trans' files before the link has come up.

To fix this, we simply use the link event file to ensure both sides
see the link as up before continuning.

Signed-off-by: Logan Gunthorpe <[email protected]>
---
tools/testing/selftests/ntb/ntb_test.sh | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
index 1c12b5855e4f..5fc7ad359e21 100755
--- a/tools/testing/selftests/ntb/ntb_test.sh
+++ b/tools/testing/selftests/ntb/ntb_test.sh
@@ -333,6 +333,10 @@ function ntb_tool_tests()
link_test $LOCAL_TOOL $REMOTE_TOOL
link_test $REMOTE_TOOL $LOCAL_TOOL

+ #Ensure the link is up on both sides before continuing
+ write_file Y $LOCAL_TOOL/link_event
+ write_file Y $REMOTE_TOOL/link_event
+
for PEER_TRANS in $(ls $LOCAL_TOOL/peer_trans*); do
PT=$(basename $PEER_TRANS)
write_file $MW_SIZE $LOCAL_TOOL/$PT
--
2.11.0

2017-07-25 21:01:54

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 15/16] switchtec_ntb: add memory window support

The Switchtec hardware has two types of memory windows: LUTs and Direct.
The first area in each BAR is for LUT windows and the remaining area is
for the direct region. The total number of LUT entries is set by a
configuration setting in hardware and they all must be the same
size. (This is fixed by switchtec_ntb to be 64K.)

switchtec_ntb enables the LUTs only for the first BAR and enables the
highest power of two possible. Seeing the LUTs are at the beginning of
the BAR, the direct memory window's alignment is affected. Therefore,
the maximum direct memory window size can not be greater than the number
of LUTs times 64K. The direct window in other BARs will not have this
restriction as the LUTs will not be enabled there. LUTs will only be
exposed through the NTB API if the use_lut_mw parameter is set.

Seeing the Switchtec hardware, by default, configures BARs to be 4G a
module parameter is given to limit the size of the advertised memory
windows. Higher layers tend to allocate the maximum BAR size and this
has a tendency to fail when they try to allocate 4GB of contiguous
memory.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 213 +++++++++++++++++++++++++++++++++++-
1 file changed, 210 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index ca403ad2f7ac..c96b723b5621 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -25,6 +25,11 @@ MODULE_VERSION("0.1");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Microsemi Corporation");

+static ulong max_mw_size = SZ_2M;
+module_param(max_mw_size, ulong, 0644);
+MODULE_PARM_DESC(max_mw_size,
+ "Max memory window size reported to the upper layer");
+
static bool use_lut_mws;
module_param(use_lut_mws, bool, 0644);
MODULE_PARM_DESC(use_lut_mws,
@@ -190,7 +195,27 @@ static int switchtec_ntb_send_msg(struct switchtec_ntb *sndev, int idx,

static int switchtec_ntb_mw_count(struct ntb_dev *ntb, int pidx)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+ int nr_direct_mw = sndev->peer_nr_direct_mw;
+ int nr_lut_mw = sndev->peer_nr_lut_mw - 1;
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ if (!use_lut_mws)
+ nr_lut_mw = 0;
+
+ return nr_direct_mw + nr_lut_mw;
+}
+
+static int lut_index(struct switchtec_ntb *sndev, int mw_idx)
+{
+ return mw_idx - sndev->nr_direct_mw + 1;
+}
+
+static int peer_lut_index(struct switchtec_ntb *sndev, int mw_idx)
+{
+ return mw_idx - sndev->peer_nr_direct_mw + 1;
}

static int switchtec_ntb_mw_get_align(struct ntb_dev *ntb, int pidx,
@@ -198,17 +223,192 @@ static int switchtec_ntb_mw_get_align(struct ntb_dev *ntb, int pidx,
resource_size_t *size_align,
resource_size_t *size_max)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+ int lut;
+ resource_size_t size;
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ lut = widx >= sndev->peer_nr_direct_mw;
+ size = ioread64(&sndev->peer_shared->mw_sizes[widx]);
+
+ if (size == 0)
+ return -EINVAL;
+
+ if (addr_align)
+ *addr_align = lut ? size : SZ_4K;
+
+ if (size_align)
+ *size_align = lut ? size : SZ_4K;
+
+ if (size_max)
+ *size_max = size;
+
return 0;
}

+static void switchtec_ntb_mw_clr_direct(struct switchtec_ntb *sndev, int idx)
+{
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+ int bar = sndev->peer_direct_mw_to_bar[idx];
+ u32 ctl_val;
+
+ ctl_val = ioread32(&ctl->bar_entry[bar].ctl);
+ ctl_val &= ~NTB_CTRL_BAR_DIR_WIN_EN;
+ iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
+ iowrite32(0, &ctl->bar_entry[bar].win_size);
+ iowrite64(sndev->self_partition, &ctl->bar_entry[bar].xlate_addr);
+}
+
+static void switchtec_ntb_mw_clr_lut(struct switchtec_ntb *sndev, int idx)
+{
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+
+ iowrite64(0, &ctl->lut_entry[peer_lut_index(sndev, idx)]);
+}
+
+static void switchtec_ntb_mw_set_direct(struct switchtec_ntb *sndev, int idx,
+ dma_addr_t addr, resource_size_t size)
+{
+ int xlate_pos = ilog2(size);
+ int bar = sndev->peer_direct_mw_to_bar[idx];
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+ u32 ctl_val;
+
+ ctl_val = ioread32(&ctl->bar_entry[bar].ctl);
+ ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
+
+ iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
+ iowrite32(xlate_pos | size, &ctl->bar_entry[bar].win_size);
+ iowrite64(sndev->self_partition | addr,
+ &ctl->bar_entry[bar].xlate_addr);
+}
+
+static void switchtec_ntb_mw_set_lut(struct switchtec_ntb *sndev, int idx,
+ dma_addr_t addr, resource_size_t size)
+{
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+
+ iowrite64((NTB_CTRL_LUT_EN | (sndev->self_partition << 1) | addr),
+ &ctl->lut_entry[peer_lut_index(sndev, idx)]);
+}
+
static int switchtec_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx,
dma_addr_t addr, resource_size_t size)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+ struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
+ int xlate_pos = ilog2(size);
+ int nr_direct_mw = sndev->peer_nr_direct_mw;
+ int rc;
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ dev_dbg(&sndev->stdev->dev, "MW %d: part %d addr %pad size %pap",
+ widx, pidx, &addr, &size);
+
+ if (widx >= switchtec_ntb_mw_count(ntb, pidx))
+ return -EINVAL;
+
+ if (xlate_pos < 12)
+ return -EINVAL;
+
+ rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_LOCK,
+ NTB_CTRL_PART_STATUS_LOCKED);
+ if (rc)
+ return rc;
+
+ if (addr == 0 || size == 0) {
+ if (widx < nr_direct_mw)
+ switchtec_ntb_mw_clr_direct(sndev, widx);
+ else
+ switchtec_ntb_mw_clr_lut(sndev, widx);
+ } else {
+ if (widx < nr_direct_mw)
+ switchtec_ntb_mw_set_direct(sndev, widx, addr, size);
+ else
+ switchtec_ntb_mw_set_lut(sndev, widx, addr, size);
+ }
+
+ rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_CFG,
+ NTB_CTRL_PART_STATUS_NORMAL);
+
+ if (rc == -EIO) {
+ dev_err(&sndev->stdev->dev,
+ "Hardware reported an error configuring mw %d: %08x",
+ widx, ioread32(&ctl->bar_error));
+
+ if (widx < nr_direct_mw)
+ switchtec_ntb_mw_clr_direct(sndev, widx);
+ else
+ switchtec_ntb_mw_clr_lut(sndev, widx);
+
+ switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_CFG,
+ NTB_CTRL_PART_STATUS_NORMAL);
+ }
+
+ return rc;
}

static int switchtec_ntb_peer_mw_count(struct ntb_dev *ntb)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ return sndev->nr_direct_mw + (use_lut_mws ? sndev->nr_lut_mw - 1 : 0);
+}
+
+static int switchtec_ntb_direct_get_addr(struct switchtec_ntb *sndev,
+ int idx, phys_addr_t *base,
+ resource_size_t *size)
+{
+ int bar = sndev->direct_mw_to_bar[idx];
+ size_t offset = 0;
+
+ if (bar < 0)
+ return -EINVAL;
+
+ if (idx == 0) {
+ /*
+ * This is the direct BAR shared with the LUTs
+ * which means the actual window will be offset
+ * by the size of all the LUT entries.
+ */
+
+ offset = LUT_SIZE * sndev->nr_lut_mw;
+ }
+
+ if (base)
+ *base = pci_resource_start(sndev->ntb.pdev, bar) + offset;
+
+ if (size) {
+ *size = pci_resource_len(sndev->ntb.pdev, bar) - offset;
+ if (offset && *size > offset)
+ *size = offset;
+
+ if (*size > max_mw_size)
+ *size = max_mw_size;
+ }
+
+ return 0;
+}
+
+static int switchtec_ntb_lut_get_addr(struct switchtec_ntb *sndev,
+ int idx, phys_addr_t *base,
+ resource_size_t *size)
+{
+ int bar = sndev->direct_mw_to_bar[0];
+ int offset;
+
+ offset = LUT_SIZE * lut_index(sndev, idx);
+
+ if (base)
+ *base = pci_resource_start(sndev->ntb.pdev, bar) + offset;
+
+ if (size)
+ *size = LUT_SIZE;
+
return 0;
}

@@ -216,7 +416,14 @@ static int switchtec_ntb_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
phys_addr_t *base,
resource_size_t *size)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (idx < sndev->nr_direct_mw)
+ return switchtec_ntb_direct_get_addr(sndev, idx, base, size);
+ else if (idx < switchtec_ntb_peer_mw_count(ntb))
+ return switchtec_ntb_lut_get_addr(sndev, idx, base, size);
+ else
+ return -EINVAL;
}

static void switchtec_ntb_part_link_speed(struct switchtec_ntb *sndev,
--
2.11.0

2017-07-25 21:02:12

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 07/16] ntb: add check and comment for link up to mw_count() and mw_get_align()

Adds a comment and a check to ntb_mw_get_align() so that it always fails
if the function is called before the link is up.

Also adds a comment to ntb_mw_count() to note that it may return 0 if
it is called before the link is up.

This is to prevent accidental mis-use in clients that are testing
on hardware that this doesn't matter for.

Signed-off-by: Logan Gunthorpe <[email protected]>
---
include/linux/ntb.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/ntb.h b/include/linux/ntb.h
index 609e232c00da..47f2966cfd7f 100644
--- a/include/linux/ntb.h
+++ b/include/linux/ntb.h
@@ -730,7 +730,8 @@ static inline int ntb_link_disable(struct ntb_dev *ntb)
* Hardware and topology may support a different number of memory windows.
* Moreover different peer devices can support different number of memory
* windows. Simply speaking this method returns the number of possible inbound
- * memory windows to share with specified peer device.
+ * memory windows to share with specified peer device. Note: this may return
+ * zero if the link is not up yet.
*
* Return: the number of memory windows.
*/
@@ -751,7 +752,7 @@ static inline int ntb_mw_count(struct ntb_dev *ntb, int pidx)
* Get the alignments of an inbound memory window with specified index.
* NULL may be given for any output parameter if the value is not needed.
* The alignment and size parameters may be used for allocation of proper
- * shared memory.
+ * shared memory. Note: this must only be called when the link is up.
*
* Return: Zero on success, otherwise a negative error number.
*/
@@ -760,6 +761,9 @@ static inline int ntb_mw_get_align(struct ntb_dev *ntb, int pidx, int widx,
resource_size_t *size_align,
resource_size_t *size_max)
{
+ if (!(ntb_link_is_up(ntb, NULL, NULL) & (1 << pidx)))
+ return -ENOTCONN;
+
return ntb->ops->mw_get_align(ntb, pidx, widx, addr_align, size_align,
size_max);
}
--
2.11.0

2017-07-25 20:58:06

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 08/16] switchtec_ntb: introduce initial NTB driver

Seeing the Switchtec NTB hardware shares the same endpoint as the
management endpoint we utilize the class_interface API to register
an NTB driver for every Switchtec device in the system that has the
NTB class code.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
MAINTAINERS | 1 +
drivers/ntb/hw/Kconfig | 1 +
drivers/ntb/hw/Makefile | 1 +
drivers/ntb/hw/mscc/Kconfig | 9 +++++
drivers/ntb/hw/mscc/Makefile | 1 +
drivers/ntb/hw/mscc/switchtec_ntb.c | 81 +++++++++++++++++++++++++++++++++++++
drivers/pci/switch/switchtec.c | 3 ++
include/linux/switchtec.h | 4 ++
8 files changed, 101 insertions(+)
create mode 100644 drivers/ntb/hw/mscc/Kconfig
create mode 100644 drivers/ntb/hw/mscc/Makefile
create mode 100644 drivers/ntb/hw/mscc/switchtec_ntb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ff2ad7c1c7b..6e491ce5e876 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10147,6 +10147,7 @@ F: Documentation/ABI/testing/sysfs-class-switchtec
F: drivers/pci/switch/switchtec*
F: include/uapi/linux/switchtec_ioctl.h
F: include/linux/switchtec.h
+F: drivers/ntb/hw/mscc/

PCI DRIVER FOR NVIDIA TEGRA
M: Thierry Reding <[email protected]>
diff --git a/drivers/ntb/hw/Kconfig b/drivers/ntb/hw/Kconfig
index a89243c9fdd3..e51b581fd102 100644
--- a/drivers/ntb/hw/Kconfig
+++ b/drivers/ntb/hw/Kconfig
@@ -1,3 +1,4 @@
source "drivers/ntb/hw/amd/Kconfig"
source "drivers/ntb/hw/idt/Kconfig"
source "drivers/ntb/hw/intel/Kconfig"
+source "drivers/ntb/hw/mscc/Kconfig"
diff --git a/drivers/ntb/hw/Makefile b/drivers/ntb/hw/Makefile
index 87332c3905f0..923c442db750 100644
--- a/drivers/ntb/hw/Makefile
+++ b/drivers/ntb/hw/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_NTB_AMD) += amd/
obj-$(CONFIG_NTB_IDT) += idt/
obj-$(CONFIG_NTB_INTEL) += intel/
+obj-$(CONFIG_NTB_SWITCHTEC) += mscc/
diff --git a/drivers/ntb/hw/mscc/Kconfig b/drivers/ntb/hw/mscc/Kconfig
new file mode 100644
index 000000000000..013ed6716438
--- /dev/null
+++ b/drivers/ntb/hw/mscc/Kconfig
@@ -0,0 +1,9 @@
+config NTB_SWITCHTEC
+ tristate "MicroSemi Switchtec Non-Transparent Bridge Support"
+ select PCI_SW_SWITCHTEC
+ help
+ Enables NTB support for Switchtec PCI switches. This also
+ selects the Switchtec management driver as they share the same
+ hardware interface.
+
+ If unsure, say N.
diff --git a/drivers/ntb/hw/mscc/Makefile b/drivers/ntb/hw/mscc/Makefile
new file mode 100644
index 000000000000..21907b364e19
--- /dev/null
+++ b/drivers/ntb/hw/mscc/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_NTB_SWITCHTEC) += switchtec_ntb.o
diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
new file mode 100644
index 000000000000..253efba72275
--- /dev/null
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -0,0 +1,81 @@
+/*
+ * Microsemi Switchtec(tm) PCIe Management Driver
+ * Copyright (c) 2017, Microsemi Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/switchtec.h>
+#include <linux/module.h>
+
+MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Microsemi Corporation");
+
+struct switchtec_ntb {
+ struct switchtec_dev *stdev;
+};
+
+static int switchtec_ntb_add(struct device *dev,
+ struct class_interface *class_intf)
+{
+ struct switchtec_dev *stdev = to_stdev(dev);
+ struct switchtec_ntb *sndev;
+
+ stdev->sndev = NULL;
+
+ if (stdev->pdev->class != MICROSEMI_NTB_CLASSCODE)
+ return -ENODEV;
+
+ sndev = kzalloc_node(sizeof(*sndev), GFP_KERNEL, dev_to_node(dev));
+ if (!sndev)
+ return -ENOMEM;
+
+ sndev->stdev = stdev;
+
+ stdev->sndev = sndev;
+ dev_info(dev, "NTB device registered");
+
+ return 0;
+}
+
+void switchtec_ntb_remove(struct device *dev,
+ struct class_interface *class_intf)
+{
+ struct switchtec_dev *stdev = to_stdev(dev);
+ struct switchtec_ntb *sndev = stdev->sndev;
+
+ if (!sndev)
+ return;
+
+ stdev->sndev = NULL;
+ kfree(sndev);
+ dev_info(dev, "ntb device unregistered");
+}
+
+static struct class_interface switchtec_interface = {
+ .add_dev = switchtec_ntb_add,
+ .remove_dev = switchtec_ntb_remove,
+};
+
+static int __init switchtec_ntb_init(void)
+{
+ switchtec_interface.class = switchtec_class;
+ return class_interface_register(&switchtec_interface);
+}
+module_init(switchtec_ntb_init);
+
+static void __exit switchtec_ntb_exit(void)
+{
+ class_interface_unregister(&switchtec_interface);
+}
+module_exit(switchtec_ntb_exit);
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 807569e62bee..0666e33c305b 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1275,6 +1275,9 @@ static int switchtec_pci_probe(struct pci_dev *pdev,
struct switchtec_dev *stdev;
int rc;

+ if (pdev->class == MICROSEMI_NTB_CLASSCODE)
+ request_module_nowait("switchtec_ntb");
+
stdev = stdev_create(pdev);
if (IS_ERR(stdev))
return PTR_ERR(stdev);
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index d8159944f013..09d73d0d1aa8 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -320,6 +320,8 @@ struct pff_csr_regs {
u32 reserved4[174];
} __packed;

+struct switchtec_ntb;
+
struct switchtec_dev {
struct pci_dev *pdev;
struct device dev;
@@ -357,6 +359,8 @@ struct switchtec_dev {
struct work_struct link_event_work;
void (*link_notifier)(struct switchtec_dev *stdev);
u8 link_event_count[SWITCHTEC_MAX_PFF_CSR];
+
+ struct switchtec_ntb *sndev;
};

static inline struct switchtec_dev *to_stdev(struct device *dev)
--
2.11.0

2017-07-25 21:02:39

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers

Seeing there is no dedicated hardware for this, we simply add
these as entries in the shared memory window. Thus, we could support
any number of them but 128 seems like enough, for now.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
---
drivers/ntb/hw/mscc/switchtec_ntb.c | 75 ++++++++++++++++++++++++++++++++++++-
1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 3037ca730998..ca403ad2f7ac 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -67,6 +67,7 @@ struct shared_mw {
u32 link_sta;
u32 partition_id;
u64 mw_sizes[MAX_MWS];
+ u32 spad[128];
};

#define MAX_DIRECT_MW ARRAY_SIZE(((struct ntb_ctrl_regs *)(0))->bar_entry)
@@ -455,22 +456,90 @@ static int switchtec_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)

static int switchtec_ntb_spad_count(struct ntb_dev *ntb)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ return ARRAY_SIZE(sndev->self_shared->spad);
}

static u32 switchtec_ntb_spad_read(struct ntb_dev *ntb, int idx)
{
- return 0;
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (idx < 0 || idx >= ARRAY_SIZE(sndev->self_shared->spad))
+ return 0;
+
+ if (!sndev->self_shared)
+ return 0;
+
+ return sndev->self_shared->spad[idx];
}

static int switchtec_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (idx < 0 || idx >= ARRAY_SIZE(sndev->self_shared->spad))
+ return -EINVAL;
+
+ if (!sndev->self_shared)
+ return -EIO;
+
+ sndev->self_shared->spad[idx] = val;
+
return 0;
}

+static u32 switchtec_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx,
+ int sidx)
+{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ if (sidx < 0 || sidx >= ARRAY_SIZE(sndev->peer_shared->spad))
+ return 0;
+
+ if (!sndev->peer_shared)
+ return 0;
+
+ return ioread32(&sndev->peer_shared->spad[sidx]);
+}
+
static int switchtec_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
int sidx, u32 val)
{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ if (sidx < 0 || sidx >= ARRAY_SIZE(sndev->peer_shared->spad))
+ return -EINVAL;
+
+ if (!sndev->peer_shared)
+ return -EIO;
+
+ iowrite32(val, &sndev->peer_shared->spad[sidx]);
+
+ return 0;
+}
+
+static int switchtec_ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx,
+ int sidx, phys_addr_t *spad_addr)
+{
+ struct switchtec_ntb *sndev = ntb_sndev(ntb);
+ unsigned long offset;
+
+ if (pidx != NTB_DEF_PEER_IDX)
+ return -EINVAL;
+
+ offset = (unsigned long)&sndev->peer_shared->spad[sidx] -
+ (unsigned long)sndev->stdev->mmio;
+
+ if (spad_addr)
+ *spad_addr = pci_resource_start(ntb->pdev, 0) + offset;
+
return 0;
}

@@ -496,7 +565,9 @@ static const struct ntb_dev_ops switchtec_ntb_ops = {
.spad_count = switchtec_ntb_spad_count,
.spad_read = switchtec_ntb_spad_read,
.spad_write = switchtec_ntb_spad_write,
+ .peer_spad_read = switchtec_ntb_peer_spad_read,
.peer_spad_write = switchtec_ntb_peer_spad_write,
+ .peer_spad_addr = switchtec_ntb_peer_spad_addr,
};

static void switchtec_ntb_init_sndev(struct switchtec_ntb *sndev)
--
2.11.0

2017-07-25 21:02:56

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called when the link is up

With Switchtec hardware it's impossible to get the alignment parameters
for a peer's memory window until the peer's driver has configured it's
windows. Strictly speaking, the link doesn't have to be up for this,
but the link being up is the only way the client can tell that
the otherside has been configured.

This patch converts ntb_transport and ntb_perf to use this function after
the link goes up. This simplifies these clients slightly because they
no longer have to store the alignment parameters. It also tweaks
ntb_tool so that peer_mw_trans will print zero if it is run before
the link goes up.

Signed-off-by: Logan Gunthorpe <[email protected]>
---
drivers/ntb/ntb_transport.c | 20 ++++++++++----------
drivers/ntb/test/ntb_perf.c | 18 +++++++++---------
drivers/ntb/test/ntb_tool.c | 6 +++---
3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index b29558ddfe95..7ed745aaa213 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -191,8 +191,6 @@ struct ntb_transport_qp {
struct ntb_transport_mw {
phys_addr_t phys_addr;
resource_size_t phys_size;
- resource_size_t xlat_align;
- resource_size_t xlat_align_size;
void __iomem *vbase;
size_t xlat_size;
size_t buff_size;
@@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
struct pci_dev *pdev = nt->ndev->pdev;
size_t xlat_size, buff_size;
+ resource_size_t xlat_align;
+ resource_size_t xlat_align_size;
int rc;

if (!size)
return -EINVAL;

- xlat_size = round_up(size, mw->xlat_align_size);
- buff_size = round_up(size, mw->xlat_align);
+ rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align,
+ &xlat_align_size, NULL);
+ if (rc)
+ return rc;
+
+ xlat_size = round_up(size, xlat_align_size);
+ buff_size = round_up(size, xlat_align);

/* No need to re-setup */
if (mw->xlat_size == xlat_size)
@@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
* is a requirement of the hardware. It is recommended to setup CMA
* for BAR sizes equal or greater than 4MB.
*/
- if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) {
+ if (!IS_ALIGNED(mw->dma_addr, xlat_align)) {
dev_err(&pdev->dev, "DMA memory %pad is not aligned\n",
&mw->dma_addr);
ntb_free_mw(nt, num_mw);
@@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
for (i = 0; i < mw_count; i++) {
mw = &nt->mw_vec[i];

- rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align,
- &mw->xlat_align_size, NULL);
- if (rc)
- goto err1;
-
rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr,
&mw->phys_size);
if (rc)
diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 759f772fa00c..427112cf101a 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)")
struct perf_mw {
phys_addr_t phys_addr;
resource_size_t phys_size;
- resource_size_t xlat_align;
- resource_size_t xlat_align_size;
void __iomem *vbase;
size_t xlat_size;
size_t buf_size;
@@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size)
{
struct perf_mw *mw = &perf->mw;
size_t xlat_size, buf_size;
+ resource_size_t xlat_align;
+ resource_size_t xlat_align_size;
int rc;

if (!size)
return -EINVAL;

- xlat_size = round_up(size, mw->xlat_align_size);
- buf_size = round_up(size, mw->xlat_align);
+ rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align,
+ &xlat_align_size, NULL);
+ if (rc)
+ return rc;
+
+ xlat_size = round_up(size, xlat_align_size);
+ buf_size = round_up(size, xlat_align);

if (mw->xlat_size == xlat_size)
return 0;
@@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf)

mw = &perf->mw;

- rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align,
- &mw->xlat_align_size, NULL);
- if (rc)
- return rc;
-
rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size);
if (rc)
return rc;
diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
index a69815c45ce6..91526a986caa 100644
--- a/drivers/ntb/test/ntb_tool.c
+++ b/drivers/ntb/test/ntb_tool.c
@@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep,

phys_addr_t base;
resource_size_t mw_size;
- resource_size_t align_addr;
- resource_size_t align_size;
- resource_size_t max_size;
+ resource_size_t align_addr = 0;
+ resource_size_t align_size = 0;
+ resource_size_t max_size = 0;

buf_size = min_t(size_t, size, 512);

--
2.11.0

2017-07-25 21:03:30

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v3 02/16] switchtec: export class symbol for use in upper layer driver

We export the class pointer symbol and add an extern define in the
Switchtec header file.

Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Stephen Bates <[email protected]>
Reviewed-by: Kurt Schwemmer <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/switch/switchtec.c | 4 +++-
include/linux/switchtec.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 5b75d3008ff8..39c9218d733f 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -33,9 +33,11 @@ module_param(max_devices, int, 0644);
MODULE_PARM_DESC(max_devices, "max number of switchtec device instances");

static dev_t switchtec_devt;
-static struct class *switchtec_class;
static DEFINE_IDA(switchtec_minor_ida);

+struct class *switchtec_class;
+EXPORT_SYMBOL_GPL(switchtec_class);
+
enum mrpc_state {
MRPC_IDLE = 0,
MRPC_QUEUED,
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 1cbd0e63b0ab..18e388101855 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -276,4 +276,6 @@ static inline struct switchtec_dev *to_stdev(struct device *dev)
return container_of(dev, struct switchtec_dev, dev);
}

+extern struct class *switchtec_class;
+
#endif
--
2.11.0

2017-07-26 13:11:24

by Allen Hubbe

[permalink] [raw]
Subject: RE: [PATCH v3 00/16] Switchtec NTB Support

From: Logan Gunthorpe
> Changes since v2:
>
> - Reordered the ntb_test link patch per Allen
> - Removed an extra call to switchtec_ntb_init_mw
> - Fixed a typo in the switchtec.txt documentation.

Patches 5..16 (also 5 [was 6], and 14, objections notwithstanding):

Acked-by: Allen Hubbe <[email protected]>

> --
>
> Changes since v1:
>
> - Rebased onto latest ntb-next branch (with v4.13-rc1)
> - Reworked ntb_mw_count() function so that it can be called all the
> time (per discussion with Allen)
> - Various spelling and formatting cleanups from Bjorn
> - Added request_module() call such that the NTB module is automatically
> loaded when appropriate hardware exists.
>
> --
>
> Changes since the rfc:
>
> - Rebased on ntb-next
> - Switched ntb_part_op to use sleep instead of delay
> - Dropped a number of useless dbg __func__ prints
> - Went back to the dynamic instead of the static class
> - Swapped the notifier block for a simple callback
> - Modified the new ntb api so that a couple functions with pidx
> now must be called after link up. Per our discussion on the list.
>
> --
>
> This patchset implements Non-Transparent Bridge (NTB) support for the
> Microsemi Switchtec series of switches. We're looking for some
> review from the community at this point but hope to get it upstreamed
> for v4.14.
>
> Switchtec NTB support is configured over the same function and bar
> as the management endpoint. Thus, the new driver hooks into the
> management driver which we had merged in v4.12. We use the class
> interface API to register an NTB device for every switchtec device
> which supports NTB (not all do).
>
> The Switchtec hardware supports doorbells, memory windows and messages.
> Seeing there is no native scratchpad support, 128 spads are emulated
> through the use of a pre-setup memory window. The switch has 64
> doorbells which are shared between the two partitions and a
> configurable set of memory windows. While the hardware supports more
> than 2 partitions, this driver only supports the first two seeing
> the current NTB API only supports two hosts.
>
> The driver has been tested with ntb_netdev and fully passes the
> ntb_test script.
>
> This patchset is based off of ntb-next and can be found in this
> git repo:
>
> https://github.com/sbates130272/linux-p2pmem.git switchtec_ntb_v3
>
> *** BLURB HERE ***
>
> Logan Gunthorpe (16):
> switchtec: move structure definitions into a common header
> switchtec: export class symbol for use in upper layer driver
> switchtec: add NTB hardware register definitions
> switchtec: add link event notifier callback
> ntb: ntb_test: ensure the link is up before trying to configure the
> mws
> ntb: ensure ntb_mw_get_align() is only called when the link is up
> ntb: add check and comment for link up to mw_count() and
> mw_get_align()
> switchtec_ntb: introduce initial NTB driver
> switchtec_ntb: initialize hardware for memory windows
> switchtec_ntb: initialize hardware for doorbells and messages
> switchtec_ntb: add skeleton NTB driver
> switchtec_ntb: add link management
> switchtec_ntb: implement doorbell registers
> switchtec_ntb: implement scratchpad registers
> switchtec_ntb: add memory window support
> switchtec_ntb: update switchtec documentation with notes for NTB
>
> Documentation/switchtec.txt | 12 +
> MAINTAINERS | 2 +
> drivers/ntb/hw/Kconfig | 1 +
> drivers/ntb/hw/Makefile | 1 +
> drivers/ntb/hw/mscc/Kconfig | 9 +
> drivers/ntb/hw/mscc/Makefile | 1 +
> drivers/ntb/hw/mscc/switchtec_ntb.c | 1211 +++++++++++++++++++++++++++++++
> drivers/ntb/ntb_transport.c | 20 +-
> drivers/ntb/test/ntb_perf.c | 18 +-
> drivers/ntb/test/ntb_tool.c | 6 +-
> drivers/pci/switch/switchtec.c | 316 ++------
> include/linux/ntb.h | 11 +-
> include/linux/switchtec.h | 373 ++++++++++
> tools/testing/selftests/ntb/ntb_test.sh | 4 +
> 14 files changed, 1702 insertions(+), 283 deletions(-)
> create mode 100644 drivers/ntb/hw/mscc/Kconfig
> create mode 100644 drivers/ntb/hw/mscc/Makefile
> create mode 100644 drivers/ntb/hw/mscc/switchtec_ntb.c
> create mode 100644 include/linux/switchtec.h
>
> --
> 2.11.0

2017-07-31 20:15:48

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 01/16] switchtec: move structure definitions into a common header

To follow the existing changelog subject pattern in drivers/ntb/hw, I
would change yours like this:

-switchtec: move structure definitions into a common header
+NTB: switchtec: Move structure definitions into a common header

I.e., add "NTB: " prefix and capitalize the first word of the
description. If I were merging this, I would silently change this
myself, but obviously this already has my ack and will go through
Jon's tree.

On Tue, Jul 25, 2017 at 02:57:38PM -0600, Logan Gunthorpe wrote:
> Create the switchtec.h header in include/linux with hardware defines
> and the switchtec_dev structure. Both moved directly from switchtec.c.
> This is a prep patch for creating an NTB driver for Switchtec.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Reviewed-by: Stephen Bates <[email protected]>
> Reviewed-by: Kurt Schwemmer <[email protected]>
> Acked-by: Greg Kroah-Hartman <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/pci/switch/switchtec.c | 260 +-------------------------------------
> include/linux/switchtec.h | 279 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 281 insertions(+), 259 deletions(-)
> create mode 100644 include/linux/switchtec.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 205d3977ac46..4ff2ad7c1c7b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10146,6 +10146,7 @@ F: Documentation/switchtec.txt
> F: Documentation/ABI/testing/sysfs-class-switchtec
> F: drivers/pci/switch/switchtec*
> F: include/uapi/linux/switchtec_ioctl.h
> +F: include/linux/switchtec.h
>
> PCI DRIVER FOR NVIDIA TEGRA
> M: Thierry Reding <[email protected]>
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index af81b2dec42e..5b75d3008ff8 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -13,6 +13,7 @@
> *
> */
>
> +#include <linux/switchtec.h>
> #include <linux/switchtec_ioctl.h>
>
> #include <linux/interrupt.h>
> @@ -20,8 +21,6 @@
> #include <linux/fs.h>
> #include <linux/uaccess.h>
> #include <linux/poll.h>
> -#include <linux/pci.h>
> -#include <linux/cdev.h>
> #include <linux/wait.h>
>
> MODULE_DESCRIPTION("Microsemi Switchtec(tm) PCIe Management Driver");
> @@ -37,263 +36,6 @@ static dev_t switchtec_devt;
> static struct class *switchtec_class;
> static DEFINE_IDA(switchtec_minor_ida);
>
> -#define MICROSEMI_VENDOR_ID 0x11f8
> -#define MICROSEMI_NTB_CLASSCODE 0x068000
> -#define MICROSEMI_MGMT_CLASSCODE 0x058000
> -
> -#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
> -#define SWITCHTEC_MAX_PFF_CSR 48
> -
> -#define SWITCHTEC_EVENT_OCCURRED BIT(0)
> -#define SWITCHTEC_EVENT_CLEAR BIT(0)
> -#define SWITCHTEC_EVENT_EN_LOG BIT(1)
> -#define SWITCHTEC_EVENT_EN_CLI BIT(2)
> -#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
> -#define SWITCHTEC_EVENT_FATAL BIT(4)
> -
> -enum {
> - SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
> - SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
> - SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
> - SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
> - SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
> - SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
> - SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
> - SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
> -};
> -
> -struct mrpc_regs {
> - u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
> - u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
> - u32 cmd;
> - u32 status;
> - u32 ret_value;
> -} __packed;
> -
> -enum mrpc_status {
> - SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
> - SWITCHTEC_MRPC_STATUS_DONE = 2,
> - SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
> - SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
> -};
> -
> -struct sw_event_regs {
> - u64 event_report_ctrl;
> - u64 reserved1;
> - u64 part_event_bitmap;
> - u64 reserved2;
> - u32 global_summary;
> - u32 reserved3[3];
> - u32 stack_error_event_hdr;
> - u32 stack_error_event_data;
> - u32 reserved4[4];
> - u32 ppu_error_event_hdr;
> - u32 ppu_error_event_data;
> - u32 reserved5[4];
> - u32 isp_error_event_hdr;
> - u32 isp_error_event_data;
> - u32 reserved6[4];
> - u32 sys_reset_event_hdr;
> - u32 reserved7[5];
> - u32 fw_exception_hdr;
> - u32 reserved8[5];
> - u32 fw_nmi_hdr;
> - u32 reserved9[5];
> - u32 fw_non_fatal_hdr;
> - u32 reserved10[5];
> - u32 fw_fatal_hdr;
> - u32 reserved11[5];
> - u32 twi_mrpc_comp_hdr;
> - u32 twi_mrpc_comp_data;
> - u32 reserved12[4];
> - u32 twi_mrpc_comp_async_hdr;
> - u32 twi_mrpc_comp_async_data;
> - u32 reserved13[4];
> - u32 cli_mrpc_comp_hdr;
> - u32 cli_mrpc_comp_data;
> - u32 reserved14[4];
> - u32 cli_mrpc_comp_async_hdr;
> - u32 cli_mrpc_comp_async_data;
> - u32 reserved15[4];
> - u32 gpio_interrupt_hdr;
> - u32 gpio_interrupt_data;
> - u32 reserved16[4];
> -} __packed;
> -
> -enum {
> - SWITCHTEC_CFG0_RUNNING = 0x04,
> - SWITCHTEC_CFG1_RUNNING = 0x05,
> - SWITCHTEC_IMG0_RUNNING = 0x03,
> - SWITCHTEC_IMG1_RUNNING = 0x07,
> -};
> -
> -struct sys_info_regs {
> - u32 device_id;
> - u32 device_version;
> - u32 firmware_version;
> - u32 reserved1;
> - u32 vendor_table_revision;
> - u32 table_format_version;
> - u32 partition_id;
> - u32 cfg_file_fmt_version;
> - u16 cfg_running;
> - u16 img_running;
> - u32 reserved2[57];
> - char vendor_id[8];
> - char product_id[16];
> - char product_revision[4];
> - char component_vendor[8];
> - u16 component_id;
> - u8 component_revision;
> -} __packed;
> -
> -struct flash_info_regs {
> - u32 flash_part_map_upd_idx;
> -
> - struct active_partition_info {
> - u32 address;
> - u32 build_version;
> - u32 build_string;
> - } active_img;
> -
> - struct active_partition_info active_cfg;
> - struct active_partition_info inactive_img;
> - struct active_partition_info inactive_cfg;
> -
> - u32 flash_length;
> -
> - struct partition_info {
> - u32 address;
> - u32 length;
> - } cfg0;
> -
> - struct partition_info cfg1;
> - struct partition_info img0;
> - struct partition_info img1;
> - struct partition_info nvlog;
> - struct partition_info vendor[8];
> -};
> -
> -struct ntb_info_regs {
> - u8 partition_count;
> - u8 partition_id;
> - u16 reserved1;
> - u64 ep_map;
> - u16 requester_id;
> -} __packed;
> -
> -struct part_cfg_regs {
> - u32 status;
> - u32 state;
> - u32 port_cnt;
> - u32 usp_port_mode;
> - u32 usp_pff_inst_id;
> - u32 vep_pff_inst_id;
> - u32 dsp_pff_inst_id[47];
> - u32 reserved1[11];
> - u16 vep_vector_number;
> - u16 usp_vector_number;
> - u32 port_event_bitmap;
> - u32 reserved2[3];
> - u32 part_event_summary;
> - u32 reserved3[3];
> - u32 part_reset_hdr;
> - u32 part_reset_data[5];
> - u32 mrpc_comp_hdr;
> - u32 mrpc_comp_data[5];
> - u32 mrpc_comp_async_hdr;
> - u32 mrpc_comp_async_data[5];
> - u32 dyn_binding_hdr;
> - u32 dyn_binding_data[5];
> - u32 reserved4[159];
> -} __packed;
> -
> -enum {
> - SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
> - SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
> - SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
> - SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
> -};
> -
> -struct pff_csr_regs {
> - u16 vendor_id;
> - u16 device_id;
> - u32 pci_cfg_header[15];
> - u32 pci_cap_region[48];
> - u32 pcie_cap_region[448];
> - u32 indirect_gas_window[128];
> - u32 indirect_gas_window_off;
> - u32 reserved[127];
> - u32 pff_event_summary;
> - u32 reserved2[3];
> - u32 aer_in_p2p_hdr;
> - u32 aer_in_p2p_data[5];
> - u32 aer_in_vep_hdr;
> - u32 aer_in_vep_data[5];
> - u32 dpc_hdr;
> - u32 dpc_data[5];
> - u32 cts_hdr;
> - u32 cts_data[5];
> - u32 reserved3[6];
> - u32 hotplug_hdr;
> - u32 hotplug_data[5];
> - u32 ier_hdr;
> - u32 ier_data[5];
> - u32 threshold_hdr;
> - u32 threshold_data[5];
> - u32 power_mgmt_hdr;
> - u32 power_mgmt_data[5];
> - u32 tlp_throttling_hdr;
> - u32 tlp_throttling_data[5];
> - u32 force_speed_hdr;
> - u32 force_speed_data[5];
> - u32 credit_timeout_hdr;
> - u32 credit_timeout_data[5];
> - u32 link_state_hdr;
> - u32 link_state_data[5];
> - u32 reserved4[174];
> -} __packed;
> -
> -struct switchtec_dev {
> - struct pci_dev *pdev;
> - struct device dev;
> - struct cdev cdev;
> -
> - int partition;
> - int partition_count;
> - int pff_csr_count;
> - char pff_local[SWITCHTEC_MAX_PFF_CSR];
> -
> - void __iomem *mmio;
> - struct mrpc_regs __iomem *mmio_mrpc;
> - struct sw_event_regs __iomem *mmio_sw_event;
> - struct sys_info_regs __iomem *mmio_sys_info;
> - struct flash_info_regs __iomem *mmio_flash_info;
> - struct ntb_info_regs __iomem *mmio_ntb;
> - struct part_cfg_regs __iomem *mmio_part_cfg;
> - struct part_cfg_regs __iomem *mmio_part_cfg_all;
> - struct pff_csr_regs __iomem *mmio_pff_csr;
> -
> - /*
> - * The mrpc mutex must be held when accessing the other
> - * mrpc_ fields, alive flag and stuser->state field
> - */
> - struct mutex mrpc_mutex;
> - struct list_head mrpc_queue;
> - int mrpc_busy;
> - struct work_struct mrpc_work;
> - struct delayed_work mrpc_timeout;
> - bool alive;
> -
> - wait_queue_head_t event_wq;
> - atomic_t event_cnt;
> -};
> -
> -static struct switchtec_dev *to_stdev(struct device *dev)
> -{
> - return container_of(dev, struct switchtec_dev, dev);
> -}
> -
> enum mrpc_state {
> MRPC_IDLE = 0,
> MRPC_QUEUED,
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> new file mode 100644
> index 000000000000..1cbd0e63b0ab
> --- /dev/null
> +++ b/include/linux/switchtec.h
> @@ -0,0 +1,279 @@
> +/*
> + * Microsemi Switchtec PCIe Driver
> + * Copyright (c) 2017, Microsemi Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#ifndef _SWITCHTEC_H
> +#define _SWITCHTEC_H
> +
> +#include <linux/pci.h>
> +#include <linux/cdev.h>
> +
> +#define MICROSEMI_VENDOR_ID 0x11f8
> +#define MICROSEMI_NTB_CLASSCODE 0x068000
> +#define MICROSEMI_MGMT_CLASSCODE 0x058000
> +
> +#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
> +#define SWITCHTEC_MAX_PFF_CSR 48
> +
> +#define SWITCHTEC_EVENT_OCCURRED BIT(0)
> +#define SWITCHTEC_EVENT_CLEAR BIT(0)
> +#define SWITCHTEC_EVENT_EN_LOG BIT(1)
> +#define SWITCHTEC_EVENT_EN_CLI BIT(2)
> +#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
> +#define SWITCHTEC_EVENT_FATAL BIT(4)
> +
> +enum {
> + SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
> + SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
> + SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
> + SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
> + SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
> + SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
> + SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
> + SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
> +};
> +
> +struct mrpc_regs {
> + u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
> + u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
> + u32 cmd;
> + u32 status;
> + u32 ret_value;
> +} __packed;
> +
> +enum mrpc_status {
> + SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
> + SWITCHTEC_MRPC_STATUS_DONE = 2,
> + SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
> + SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
> +};
> +
> +struct sw_event_regs {
> + u64 event_report_ctrl;
> + u64 reserved1;
> + u64 part_event_bitmap;
> + u64 reserved2;
> + u32 global_summary;
> + u32 reserved3[3];
> + u32 stack_error_event_hdr;
> + u32 stack_error_event_data;
> + u32 reserved4[4];
> + u32 ppu_error_event_hdr;
> + u32 ppu_error_event_data;
> + u32 reserved5[4];
> + u32 isp_error_event_hdr;
> + u32 isp_error_event_data;
> + u32 reserved6[4];
> + u32 sys_reset_event_hdr;
> + u32 reserved7[5];
> + u32 fw_exception_hdr;
> + u32 reserved8[5];
> + u32 fw_nmi_hdr;
> + u32 reserved9[5];
> + u32 fw_non_fatal_hdr;
> + u32 reserved10[5];
> + u32 fw_fatal_hdr;
> + u32 reserved11[5];
> + u32 twi_mrpc_comp_hdr;
> + u32 twi_mrpc_comp_data;
> + u32 reserved12[4];
> + u32 twi_mrpc_comp_async_hdr;
> + u32 twi_mrpc_comp_async_data;
> + u32 reserved13[4];
> + u32 cli_mrpc_comp_hdr;
> + u32 cli_mrpc_comp_data;
> + u32 reserved14[4];
> + u32 cli_mrpc_comp_async_hdr;
> + u32 cli_mrpc_comp_async_data;
> + u32 reserved15[4];
> + u32 gpio_interrupt_hdr;
> + u32 gpio_interrupt_data;
> + u32 reserved16[4];
> +} __packed;
> +
> +enum {
> + SWITCHTEC_CFG0_RUNNING = 0x04,
> + SWITCHTEC_CFG1_RUNNING = 0x05,
> + SWITCHTEC_IMG0_RUNNING = 0x03,
> + SWITCHTEC_IMG1_RUNNING = 0x07,
> +};
> +
> +struct sys_info_regs {
> + u32 device_id;
> + u32 device_version;
> + u32 firmware_version;
> + u32 reserved1;
> + u32 vendor_table_revision;
> + u32 table_format_version;
> + u32 partition_id;
> + u32 cfg_file_fmt_version;
> + u16 cfg_running;
> + u16 img_running;
> + u32 reserved2[57];
> + char vendor_id[8];
> + char product_id[16];
> + char product_revision[4];
> + char component_vendor[8];
> + u16 component_id;
> + u8 component_revision;
> +} __packed;
> +
> +struct flash_info_regs {
> + u32 flash_part_map_upd_idx;
> +
> + struct active_partition_info {
> + u32 address;
> + u32 build_version;
> + u32 build_string;
> + } active_img;
> +
> + struct active_partition_info active_cfg;
> + struct active_partition_info inactive_img;
> + struct active_partition_info inactive_cfg;
> +
> + u32 flash_length;
> +
> + struct partition_info {
> + u32 address;
> + u32 length;
> + } cfg0;
> +
> + struct partition_info cfg1;
> + struct partition_info img0;
> + struct partition_info img1;
> + struct partition_info nvlog;
> + struct partition_info vendor[8];
> +};
> +
> +struct ntb_info_regs {
> + u8 partition_count;
> + u8 partition_id;
> + u16 reserved1;
> + u64 ep_map;
> + u16 requester_id;
> +} __packed;
> +
> +struct part_cfg_regs {
> + u32 status;
> + u32 state;
> + u32 port_cnt;
> + u32 usp_port_mode;
> + u32 usp_pff_inst_id;
> + u32 vep_pff_inst_id;
> + u32 dsp_pff_inst_id[47];
> + u32 reserved1[11];
> + u16 vep_vector_number;
> + u16 usp_vector_number;
> + u32 port_event_bitmap;
> + u32 reserved2[3];
> + u32 part_event_summary;
> + u32 reserved3[3];
> + u32 part_reset_hdr;
> + u32 part_reset_data[5];
> + u32 mrpc_comp_hdr;
> + u32 mrpc_comp_data[5];
> + u32 mrpc_comp_async_hdr;
> + u32 mrpc_comp_async_data[5];
> + u32 dyn_binding_hdr;
> + u32 dyn_binding_data[5];
> + u32 reserved4[159];
> +} __packed;
> +
> +enum {
> + SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
> + SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
> + SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
> + SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
> +};
> +
> +struct pff_csr_regs {
> + u16 vendor_id;
> + u16 device_id;
> + u32 pci_cfg_header[15];
> + u32 pci_cap_region[48];
> + u32 pcie_cap_region[448];
> + u32 indirect_gas_window[128];
> + u32 indirect_gas_window_off;
> + u32 reserved[127];
> + u32 pff_event_summary;
> + u32 reserved2[3];
> + u32 aer_in_p2p_hdr;
> + u32 aer_in_p2p_data[5];
> + u32 aer_in_vep_hdr;
> + u32 aer_in_vep_data[5];
> + u32 dpc_hdr;
> + u32 dpc_data[5];
> + u32 cts_hdr;
> + u32 cts_data[5];
> + u32 reserved3[6];
> + u32 hotplug_hdr;
> + u32 hotplug_data[5];
> + u32 ier_hdr;
> + u32 ier_data[5];
> + u32 threshold_hdr;
> + u32 threshold_data[5];
> + u32 power_mgmt_hdr;
> + u32 power_mgmt_data[5];
> + u32 tlp_throttling_hdr;
> + u32 tlp_throttling_data[5];
> + u32 force_speed_hdr;
> + u32 force_speed_data[5];
> + u32 credit_timeout_hdr;
> + u32 credit_timeout_data[5];
> + u32 link_state_hdr;
> + u32 link_state_data[5];
> + u32 reserved4[174];
> +} __packed;
> +
> +struct switchtec_dev {
> + struct pci_dev *pdev;
> + struct device dev;
> + struct cdev cdev;
> +
> + int partition;
> + int partition_count;
> + int pff_csr_count;
> + char pff_local[SWITCHTEC_MAX_PFF_CSR];
> +
> + void __iomem *mmio;
> + struct mrpc_regs __iomem *mmio_mrpc;
> + struct sw_event_regs __iomem *mmio_sw_event;
> + struct sys_info_regs __iomem *mmio_sys_info;
> + struct flash_info_regs __iomem *mmio_flash_info;
> + struct ntb_info_regs __iomem *mmio_ntb;
> + struct part_cfg_regs __iomem *mmio_part_cfg;
> + struct part_cfg_regs __iomem *mmio_part_cfg_all;
> + struct pff_csr_regs __iomem *mmio_pff_csr;
> +
> + /*
> + * The mrpc mutex must be held when accessing the other
> + * mrpc_ fields, alive flag and stuser->state field
> + */
> + struct mutex mrpc_mutex;
> + struct list_head mrpc_queue;
> + int mrpc_busy;
> + struct work_struct mrpc_work;
> + struct delayed_work mrpc_timeout;
> + bool alive;
> +
> + wait_queue_head_t event_wq;
> + atomic_t event_cnt;
> +};
> +
> +static inline struct switchtec_dev *to_stdev(struct device *dev)
> +{
> + return container_of(dev, struct switchtec_dev, dev);
> +}
> +
> +#endif
> --
> 2.11.0
>

2017-07-31 20:17:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called when the link is up

On Tue, Jul 25, 2017 at 02:57:43PM -0600, Logan Gunthorpe wrote:
> With Switchtec hardware it's impossible to get the alignment parameters
> for a peer's memory window until the peer's driver has configured it's

s/it's/its/

> windows. Strictly speaking, the link doesn't have to be up for this,
> but the link being up is the only way the client can tell that
> the otherside has been configured.

s/otherside/other side/

> This patch converts ntb_transport and ntb_perf to use this function after
> the link goes up. This simplifies these clients slightly because they
> no longer have to store the alignment parameters. It also tweaks
> ntb_tool so that peer_mw_trans will print zero if it is run before
> the link goes up.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> ---
> drivers/ntb/ntb_transport.c | 20 ++++++++++----------
> drivers/ntb/test/ntb_perf.c | 18 +++++++++---------
> drivers/ntb/test/ntb_tool.c | 6 +++---
> 3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index b29558ddfe95..7ed745aaa213 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -191,8 +191,6 @@ struct ntb_transport_qp {
> struct ntb_transport_mw {
> phys_addr_t phys_addr;
> resource_size_t phys_size;
> - resource_size_t xlat_align;
> - resource_size_t xlat_align_size;
> void __iomem *vbase;
> size_t xlat_size;
> size_t buff_size;
> @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
> struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
> struct pci_dev *pdev = nt->ndev->pdev;
> size_t xlat_size, buff_size;
> + resource_size_t xlat_align;
> + resource_size_t xlat_align_size;
> int rc;
>
> if (!size)
> return -EINVAL;
>
> - xlat_size = round_up(size, mw->xlat_align_size);
> - buff_size = round_up(size, mw->xlat_align);
> + rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align,
> + &xlat_align_size, NULL);
> + if (rc)
> + return rc;
> +
> + xlat_size = round_up(size, xlat_align_size);
> + buff_size = round_up(size, xlat_align);
>
> /* No need to re-setup */
> if (mw->xlat_size == xlat_size)
> @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
> * is a requirement of the hardware. It is recommended to setup CMA
> * for BAR sizes equal or greater than 4MB.
> */
> - if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) {
> + if (!IS_ALIGNED(mw->dma_addr, xlat_align)) {
> dev_err(&pdev->dev, "DMA memory %pad is not aligned\n",
> &mw->dma_addr);
> ntb_free_mw(nt, num_mw);
> @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
> for (i = 0; i < mw_count; i++) {
> mw = &nt->mw_vec[i];
>
> - rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align,
> - &mw->xlat_align_size, NULL);
> - if (rc)
> - goto err1;
> -
> rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr,
> &mw->phys_size);
> if (rc)
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 759f772fa00c..427112cf101a 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)")
> struct perf_mw {
> phys_addr_t phys_addr;
> resource_size_t phys_size;
> - resource_size_t xlat_align;
> - resource_size_t xlat_align_size;
> void __iomem *vbase;
> size_t xlat_size;
> size_t buf_size;
> @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size)
> {
> struct perf_mw *mw = &perf->mw;
> size_t xlat_size, buf_size;
> + resource_size_t xlat_align;
> + resource_size_t xlat_align_size;
> int rc;
>
> if (!size)
> return -EINVAL;
>
> - xlat_size = round_up(size, mw->xlat_align_size);
> - buf_size = round_up(size, mw->xlat_align);
> + rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align,
> + &xlat_align_size, NULL);
> + if (rc)
> + return rc;
> +
> + xlat_size = round_up(size, xlat_align_size);
> + buf_size = round_up(size, xlat_align);
>
> if (mw->xlat_size == xlat_size)
> return 0;
> @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf)
>
> mw = &perf->mw;
>
> - rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align,
> - &mw->xlat_align_size, NULL);
> - if (rc)
> - return rc;
> -
> rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size);
> if (rc)
> return rc;
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index a69815c45ce6..91526a986caa 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep,
>
> phys_addr_t base;
> resource_size_t mw_size;
> - resource_size_t align_addr;
> - resource_size_t align_size;
> - resource_size_t max_size;
> + resource_size_t align_addr = 0;
> + resource_size_t align_size = 0;
> + resource_size_t max_size = 0;
>
> buf_size = min_t(size_t, size, 512);
>
> --
> 2.11.0
>

2017-07-31 20:25:31

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 09/16] switchtec_ntb: initialize hardware for memory windows

On Tue, Jul 25, 2017 at 02:57:46PM -0600, Logan Gunthorpe wrote:
> Add the code to initialize the memory windows in the hardware.
> This includes setting up the requester ID table, and figuring out
> which bar corresponds to which memory window. (Seeing the switch
> can be configured with any number of bars.)

s/bar/BAR/ (twice)

> Also, seeing the device doesn't have hardware for scratchpads or
> determining the link status, we create a shared memory window that has
> these features. A magic number with a version copmonent will be used

s/copmonent/component/

> to determine if the otherside's driver is actually up.

s/otherside/other side/

> The shared memory window also informs the other side of the
> size and count of the local memory windows.

2017-07-31 20:26:12

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 10/16] switchtec_ntb: initialize hardware for doorbells and messages

On Tue, Jul 25, 2017 at 02:57:47PM -0600, Logan Gunthorpe wrote:
> Set up some hardware registers and creates interrupt service routines
> for the doorbells and messages.
>
> There are 64 doorbells in the switch that are shared between all
> partitions. The upper 4 doorbells are also shared with the messages
> and are there for not used. Thus, this provides 28 doorbells for each

s/there for/therefore/

> partition.

2017-07-31 20:27:05

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 12/16] switchtec_ntb: add link management

On Tue, Jul 25, 2017 at 02:57:49PM -0600, Logan Gunthorpe wrote:
> switchtec_ntb checks for a link by looking at the shared memory
> window. If the magic number is correct and the otherside indicates
> their link is enabled then we take the link to be up.
>
> Whenever we change our local link status we send a msg to the
> otherside to check whether it's up and change their status.

s/otherside/other side/ (twice)

> The current status is maintained in a flag so ntb_is_link_up
> can return quickly.
>
> We utilize Switchtec's link status notifier to also check link changes
> when the switch notices a port changes state.

2017-07-31 20:51:47

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 01/16] switchtec: move structure definitions into a common header

Hey,

I've never found any consistency in the capitalization and prefixes
patch titles. I used to capitalize them all but I've seen many
developers use lower case. In any case, I've changed them as you requested.

I've also fixed all my spelling mistakes you've pointed out in the
commit messages.

I'll submit a v4 set next week with these changes and any other feedback
I receive in the mean time.

Thanks,

Logan

On 31/07/17 02:15 PM, Bjorn Helgaas wrote:
> To follow the existing changelog subject pattern in drivers/ntb/hw, I
> would change yours like this:
>
> -switchtec: move structure definitions into a common header
> +NTB: switchtec: Move structure definitions into a common header
>
> I.e., add "NTB: " prefix and capitalize the first word of the
> description. If I were merging this, I would silently change this
> myself, but obviously this already has my ack and will go through
> Jon's tree.
>
> On Tue, Jul 25, 2017 at 02:57:38PM -0600, Logan Gunthorpe wrote:
>> Create the switchtec.h header in include/linux with hardware defines
>> and the switchtec_dev structure. Both moved directly from switchtec.c.
>> This is a prep patch for creating an NTB driver for Switchtec.
>>
>> Signed-off-by: Logan Gunthorpe <[email protected]>
>> Reviewed-by: Stephen Bates <[email protected]>
>> Reviewed-by: Kurt Schwemmer <[email protected]>
>> Acked-by: Greg Kroah-Hartman <[email protected]>
>> Acked-by: Bjorn Helgaas <[email protected]>
>> ---
>> MAINTAINERS | 1 +
>> drivers/pci/switch/switchtec.c | 260 +-------------------------------------
>> include/linux/switchtec.h | 279 +++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 281 insertions(+), 259 deletions(-)
>> create mode 100644 include/linux/switchtec.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 205d3977ac46..4ff2ad7c1c7b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10146,6 +10146,7 @@ F: Documentation/switchtec.txt
>> F: Documentation/ABI/testing/sysfs-class-switchtec
>> F: drivers/pci/switch/switchtec*
>> F: include/uapi/linux/switchtec_ioctl.h
>> +F: include/linux/switchtec.h
>>
>> PCI DRIVER FOR NVIDIA TEGRA
>> M: Thierry Reding <[email protected]>
>> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
>> index af81b2dec42e..5b75d3008ff8 100644
>> --- a/drivers/pci/switch/switchtec.c
>> +++ b/drivers/pci/switch/switchtec.c
>> @@ -13,6 +13,7 @@
>> *
>> */
>>
>> +#include <linux/switchtec.h>
>> #include <linux/switchtec_ioctl.h>
>>
>> #include <linux/interrupt.h>
>> @@ -20,8 +21,6 @@
>> #include <linux/fs.h>
>> #include <linux/uaccess.h>
>> #include <linux/poll.h>
>> -#include <linux/pci.h>
>> -#include <linux/cdev.h>
>> #include <linux/wait.h>
>>
>> MODULE_DESCRIPTION("Microsemi Switchtec(tm) PCIe Management Driver");
>> @@ -37,263 +36,6 @@ static dev_t switchtec_devt;
>> static struct class *switchtec_class;
>> static DEFINE_IDA(switchtec_minor_ida);
>>
>> -#define MICROSEMI_VENDOR_ID 0x11f8
>> -#define MICROSEMI_NTB_CLASSCODE 0x068000
>> -#define MICROSEMI_MGMT_CLASSCODE 0x058000
>> -
>> -#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
>> -#define SWITCHTEC_MAX_PFF_CSR 48
>> -
>> -#define SWITCHTEC_EVENT_OCCURRED BIT(0)
>> -#define SWITCHTEC_EVENT_CLEAR BIT(0)
>> -#define SWITCHTEC_EVENT_EN_LOG BIT(1)
>> -#define SWITCHTEC_EVENT_EN_CLI BIT(2)
>> -#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
>> -#define SWITCHTEC_EVENT_FATAL BIT(4)
>> -
>> -enum {
>> - SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
>> - SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
>> - SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
>> - SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
>> - SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
>> - SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
>> - SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
>> - SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
>> -};
>> -
>> -struct mrpc_regs {
>> - u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
>> - u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
>> - u32 cmd;
>> - u32 status;
>> - u32 ret_value;
>> -} __packed;
>> -
>> -enum mrpc_status {
>> - SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
>> - SWITCHTEC_MRPC_STATUS_DONE = 2,
>> - SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
>> - SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
>> -};
>> -
>> -struct sw_event_regs {
>> - u64 event_report_ctrl;
>> - u64 reserved1;
>> - u64 part_event_bitmap;
>> - u64 reserved2;
>> - u32 global_summary;
>> - u32 reserved3[3];
>> - u32 stack_error_event_hdr;
>> - u32 stack_error_event_data;
>> - u32 reserved4[4];
>> - u32 ppu_error_event_hdr;
>> - u32 ppu_error_event_data;
>> - u32 reserved5[4];
>> - u32 isp_error_event_hdr;
>> - u32 isp_error_event_data;
>> - u32 reserved6[4];
>> - u32 sys_reset_event_hdr;
>> - u32 reserved7[5];
>> - u32 fw_exception_hdr;
>> - u32 reserved8[5];
>> - u32 fw_nmi_hdr;
>> - u32 reserved9[5];
>> - u32 fw_non_fatal_hdr;
>> - u32 reserved10[5];
>> - u32 fw_fatal_hdr;
>> - u32 reserved11[5];
>> - u32 twi_mrpc_comp_hdr;
>> - u32 twi_mrpc_comp_data;
>> - u32 reserved12[4];
>> - u32 twi_mrpc_comp_async_hdr;
>> - u32 twi_mrpc_comp_async_data;
>> - u32 reserved13[4];
>> - u32 cli_mrpc_comp_hdr;
>> - u32 cli_mrpc_comp_data;
>> - u32 reserved14[4];
>> - u32 cli_mrpc_comp_async_hdr;
>> - u32 cli_mrpc_comp_async_data;
>> - u32 reserved15[4];
>> - u32 gpio_interrupt_hdr;
>> - u32 gpio_interrupt_data;
>> - u32 reserved16[4];
>> -} __packed;
>> -
>> -enum {
>> - SWITCHTEC_CFG0_RUNNING = 0x04,
>> - SWITCHTEC_CFG1_RUNNING = 0x05,
>> - SWITCHTEC_IMG0_RUNNING = 0x03,
>> - SWITCHTEC_IMG1_RUNNING = 0x07,
>> -};
>> -
>> -struct sys_info_regs {
>> - u32 device_id;
>> - u32 device_version;
>> - u32 firmware_version;
>> - u32 reserved1;
>> - u32 vendor_table_revision;
>> - u32 table_format_version;
>> - u32 partition_id;
>> - u32 cfg_file_fmt_version;
>> - u16 cfg_running;
>> - u16 img_running;
>> - u32 reserved2[57];
>> - char vendor_id[8];
>> - char product_id[16];
>> - char product_revision[4];
>> - char component_vendor[8];
>> - u16 component_id;
>> - u8 component_revision;
>> -} __packed;
>> -
>> -struct flash_info_regs {
>> - u32 flash_part_map_upd_idx;
>> -
>> - struct active_partition_info {
>> - u32 address;
>> - u32 build_version;
>> - u32 build_string;
>> - } active_img;
>> -
>> - struct active_partition_info active_cfg;
>> - struct active_partition_info inactive_img;
>> - struct active_partition_info inactive_cfg;
>> -
>> - u32 flash_length;
>> -
>> - struct partition_info {
>> - u32 address;
>> - u32 length;
>> - } cfg0;
>> -
>> - struct partition_info cfg1;
>> - struct partition_info img0;
>> - struct partition_info img1;
>> - struct partition_info nvlog;
>> - struct partition_info vendor[8];
>> -};
>> -
>> -struct ntb_info_regs {
>> - u8 partition_count;
>> - u8 partition_id;
>> - u16 reserved1;
>> - u64 ep_map;
>> - u16 requester_id;
>> -} __packed;
>> -
>> -struct part_cfg_regs {
>> - u32 status;
>> - u32 state;
>> - u32 port_cnt;
>> - u32 usp_port_mode;
>> - u32 usp_pff_inst_id;
>> - u32 vep_pff_inst_id;
>> - u32 dsp_pff_inst_id[47];
>> - u32 reserved1[11];
>> - u16 vep_vector_number;
>> - u16 usp_vector_number;
>> - u32 port_event_bitmap;
>> - u32 reserved2[3];
>> - u32 part_event_summary;
>> - u32 reserved3[3];
>> - u32 part_reset_hdr;
>> - u32 part_reset_data[5];
>> - u32 mrpc_comp_hdr;
>> - u32 mrpc_comp_data[5];
>> - u32 mrpc_comp_async_hdr;
>> - u32 mrpc_comp_async_data[5];
>> - u32 dyn_binding_hdr;
>> - u32 dyn_binding_data[5];
>> - u32 reserved4[159];
>> -} __packed;
>> -
>> -enum {
>> - SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
>> - SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
>> - SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
>> - SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
>> -};
>> -
>> -struct pff_csr_regs {
>> - u16 vendor_id;
>> - u16 device_id;
>> - u32 pci_cfg_header[15];
>> - u32 pci_cap_region[48];
>> - u32 pcie_cap_region[448];
>> - u32 indirect_gas_window[128];
>> - u32 indirect_gas_window_off;
>> - u32 reserved[127];
>> - u32 pff_event_summary;
>> - u32 reserved2[3];
>> - u32 aer_in_p2p_hdr;
>> - u32 aer_in_p2p_data[5];
>> - u32 aer_in_vep_hdr;
>> - u32 aer_in_vep_data[5];
>> - u32 dpc_hdr;
>> - u32 dpc_data[5];
>> - u32 cts_hdr;
>> - u32 cts_data[5];
>> - u32 reserved3[6];
>> - u32 hotplug_hdr;
>> - u32 hotplug_data[5];
>> - u32 ier_hdr;
>> - u32 ier_data[5];
>> - u32 threshold_hdr;
>> - u32 threshold_data[5];
>> - u32 power_mgmt_hdr;
>> - u32 power_mgmt_data[5];
>> - u32 tlp_throttling_hdr;
>> - u32 tlp_throttling_data[5];
>> - u32 force_speed_hdr;
>> - u32 force_speed_data[5];
>> - u32 credit_timeout_hdr;
>> - u32 credit_timeout_data[5];
>> - u32 link_state_hdr;
>> - u32 link_state_data[5];
>> - u32 reserved4[174];
>> -} __packed;
>> -
>> -struct switchtec_dev {
>> - struct pci_dev *pdev;
>> - struct device dev;
>> - struct cdev cdev;
>> -
>> - int partition;
>> - int partition_count;
>> - int pff_csr_count;
>> - char pff_local[SWITCHTEC_MAX_PFF_CSR];
>> -
>> - void __iomem *mmio;
>> - struct mrpc_regs __iomem *mmio_mrpc;
>> - struct sw_event_regs __iomem *mmio_sw_event;
>> - struct sys_info_regs __iomem *mmio_sys_info;
>> - struct flash_info_regs __iomem *mmio_flash_info;
>> - struct ntb_info_regs __iomem *mmio_ntb;
>> - struct part_cfg_regs __iomem *mmio_part_cfg;
>> - struct part_cfg_regs __iomem *mmio_part_cfg_all;
>> - struct pff_csr_regs __iomem *mmio_pff_csr;
>> -
>> - /*
>> - * The mrpc mutex must be held when accessing the other
>> - * mrpc_ fields, alive flag and stuser->state field
>> - */
>> - struct mutex mrpc_mutex;
>> - struct list_head mrpc_queue;
>> - int mrpc_busy;
>> - struct work_struct mrpc_work;
>> - struct delayed_work mrpc_timeout;
>> - bool alive;
>> -
>> - wait_queue_head_t event_wq;
>> - atomic_t event_cnt;
>> -};
>> -
>> -static struct switchtec_dev *to_stdev(struct device *dev)
>> -{
>> - return container_of(dev, struct switchtec_dev, dev);
>> -}
>> -
>> enum mrpc_state {
>> MRPC_IDLE = 0,
>> MRPC_QUEUED,
>> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
>> new file mode 100644
>> index 000000000000..1cbd0e63b0ab
>> --- /dev/null
>> +++ b/include/linux/switchtec.h
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Microsemi Switchtec PCIe Driver
>> + * Copyright (c) 2017, Microsemi Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + */
>> +
>> +#ifndef _SWITCHTEC_H
>> +#define _SWITCHTEC_H
>> +
>> +#include <linux/pci.h>
>> +#include <linux/cdev.h>
>> +
>> +#define MICROSEMI_VENDOR_ID 0x11f8
>> +#define MICROSEMI_NTB_CLASSCODE 0x068000
>> +#define MICROSEMI_MGMT_CLASSCODE 0x058000
>> +
>> +#define SWITCHTEC_MRPC_PAYLOAD_SIZE 1024
>> +#define SWITCHTEC_MAX_PFF_CSR 48
>> +
>> +#define SWITCHTEC_EVENT_OCCURRED BIT(0)
>> +#define SWITCHTEC_EVENT_CLEAR BIT(0)
>> +#define SWITCHTEC_EVENT_EN_LOG BIT(1)
>> +#define SWITCHTEC_EVENT_EN_CLI BIT(2)
>> +#define SWITCHTEC_EVENT_EN_IRQ BIT(3)
>> +#define SWITCHTEC_EVENT_FATAL BIT(4)
>> +
>> +enum {
>> + SWITCHTEC_GAS_MRPC_OFFSET = 0x0000,
>> + SWITCHTEC_GAS_TOP_CFG_OFFSET = 0x1000,
>> + SWITCHTEC_GAS_SW_EVENT_OFFSET = 0x1800,
>> + SWITCHTEC_GAS_SYS_INFO_OFFSET = 0x2000,
>> + SWITCHTEC_GAS_FLASH_INFO_OFFSET = 0x2200,
>> + SWITCHTEC_GAS_PART_CFG_OFFSET = 0x4000,
>> + SWITCHTEC_GAS_NTB_OFFSET = 0x10000,
>> + SWITCHTEC_GAS_PFF_CSR_OFFSET = 0x134000,
>> +};
>> +
>> +struct mrpc_regs {
>> + u8 input_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
>> + u8 output_data[SWITCHTEC_MRPC_PAYLOAD_SIZE];
>> + u32 cmd;
>> + u32 status;
>> + u32 ret_value;
>> +} __packed;
>> +
>> +enum mrpc_status {
>> + SWITCHTEC_MRPC_STATUS_INPROGRESS = 1,
>> + SWITCHTEC_MRPC_STATUS_DONE = 2,
>> + SWITCHTEC_MRPC_STATUS_ERROR = 0xFF,
>> + SWITCHTEC_MRPC_STATUS_INTERRUPTED = 0x100,
>> +};
>> +
>> +struct sw_event_regs {
>> + u64 event_report_ctrl;
>> + u64 reserved1;
>> + u64 part_event_bitmap;
>> + u64 reserved2;
>> + u32 global_summary;
>> + u32 reserved3[3];
>> + u32 stack_error_event_hdr;
>> + u32 stack_error_event_data;
>> + u32 reserved4[4];
>> + u32 ppu_error_event_hdr;
>> + u32 ppu_error_event_data;
>> + u32 reserved5[4];
>> + u32 isp_error_event_hdr;
>> + u32 isp_error_event_data;
>> + u32 reserved6[4];
>> + u32 sys_reset_event_hdr;
>> + u32 reserved7[5];
>> + u32 fw_exception_hdr;
>> + u32 reserved8[5];
>> + u32 fw_nmi_hdr;
>> + u32 reserved9[5];
>> + u32 fw_non_fatal_hdr;
>> + u32 reserved10[5];
>> + u32 fw_fatal_hdr;
>> + u32 reserved11[5];
>> + u32 twi_mrpc_comp_hdr;
>> + u32 twi_mrpc_comp_data;
>> + u32 reserved12[4];
>> + u32 twi_mrpc_comp_async_hdr;
>> + u32 twi_mrpc_comp_async_data;
>> + u32 reserved13[4];
>> + u32 cli_mrpc_comp_hdr;
>> + u32 cli_mrpc_comp_data;
>> + u32 reserved14[4];
>> + u32 cli_mrpc_comp_async_hdr;
>> + u32 cli_mrpc_comp_async_data;
>> + u32 reserved15[4];
>> + u32 gpio_interrupt_hdr;
>> + u32 gpio_interrupt_data;
>> + u32 reserved16[4];
>> +} __packed;
>> +
>> +enum {
>> + SWITCHTEC_CFG0_RUNNING = 0x04,
>> + SWITCHTEC_CFG1_RUNNING = 0x05,
>> + SWITCHTEC_IMG0_RUNNING = 0x03,
>> + SWITCHTEC_IMG1_RUNNING = 0x07,
>> +};
>> +
>> +struct sys_info_regs {
>> + u32 device_id;
>> + u32 device_version;
>> + u32 firmware_version;
>> + u32 reserved1;
>> + u32 vendor_table_revision;
>> + u32 table_format_version;
>> + u32 partition_id;
>> + u32 cfg_file_fmt_version;
>> + u16 cfg_running;
>> + u16 img_running;
>> + u32 reserved2[57];
>> + char vendor_id[8];
>> + char product_id[16];
>> + char product_revision[4];
>> + char component_vendor[8];
>> + u16 component_id;
>> + u8 component_revision;
>> +} __packed;
>> +
>> +struct flash_info_regs {
>> + u32 flash_part_map_upd_idx;
>> +
>> + struct active_partition_info {
>> + u32 address;
>> + u32 build_version;
>> + u32 build_string;
>> + } active_img;
>> +
>> + struct active_partition_info active_cfg;
>> + struct active_partition_info inactive_img;
>> + struct active_partition_info inactive_cfg;
>> +
>> + u32 flash_length;
>> +
>> + struct partition_info {
>> + u32 address;
>> + u32 length;
>> + } cfg0;
>> +
>> + struct partition_info cfg1;
>> + struct partition_info img0;
>> + struct partition_info img1;
>> + struct partition_info nvlog;
>> + struct partition_info vendor[8];
>> +};
>> +
>> +struct ntb_info_regs {
>> + u8 partition_count;
>> + u8 partition_id;
>> + u16 reserved1;
>> + u64 ep_map;
>> + u16 requester_id;
>> +} __packed;
>> +
>> +struct part_cfg_regs {
>> + u32 status;
>> + u32 state;
>> + u32 port_cnt;
>> + u32 usp_port_mode;
>> + u32 usp_pff_inst_id;
>> + u32 vep_pff_inst_id;
>> + u32 dsp_pff_inst_id[47];
>> + u32 reserved1[11];
>> + u16 vep_vector_number;
>> + u16 usp_vector_number;
>> + u32 port_event_bitmap;
>> + u32 reserved2[3];
>> + u32 part_event_summary;
>> + u32 reserved3[3];
>> + u32 part_reset_hdr;
>> + u32 part_reset_data[5];
>> + u32 mrpc_comp_hdr;
>> + u32 mrpc_comp_data[5];
>> + u32 mrpc_comp_async_hdr;
>> + u32 mrpc_comp_async_data[5];
>> + u32 dyn_binding_hdr;
>> + u32 dyn_binding_data[5];
>> + u32 reserved4[159];
>> +} __packed;
>> +
>> +enum {
>> + SWITCHTEC_PART_CFG_EVENT_RESET = 1 << 0,
>> + SWITCHTEC_PART_CFG_EVENT_MRPC_CMP = 1 << 1,
>> + SWITCHTEC_PART_CFG_EVENT_MRPC_ASYNC_CMP = 1 << 2,
>> + SWITCHTEC_PART_CFG_EVENT_DYN_PART_CMP = 1 << 3,
>> +};
>> +
>> +struct pff_csr_regs {
>> + u16 vendor_id;
>> + u16 device_id;
>> + u32 pci_cfg_header[15];
>> + u32 pci_cap_region[48];
>> + u32 pcie_cap_region[448];
>> + u32 indirect_gas_window[128];
>> + u32 indirect_gas_window_off;
>> + u32 reserved[127];
>> + u32 pff_event_summary;
>> + u32 reserved2[3];
>> + u32 aer_in_p2p_hdr;
>> + u32 aer_in_p2p_data[5];
>> + u32 aer_in_vep_hdr;
>> + u32 aer_in_vep_data[5];
>> + u32 dpc_hdr;
>> + u32 dpc_data[5];
>> + u32 cts_hdr;
>> + u32 cts_data[5];
>> + u32 reserved3[6];
>> + u32 hotplug_hdr;
>> + u32 hotplug_data[5];
>> + u32 ier_hdr;
>> + u32 ier_data[5];
>> + u32 threshold_hdr;
>> + u32 threshold_data[5];
>> + u32 power_mgmt_hdr;
>> + u32 power_mgmt_data[5];
>> + u32 tlp_throttling_hdr;
>> + u32 tlp_throttling_data[5];
>> + u32 force_speed_hdr;
>> + u32 force_speed_data[5];
>> + u32 credit_timeout_hdr;
>> + u32 credit_timeout_data[5];
>> + u32 link_state_hdr;
>> + u32 link_state_data[5];
>> + u32 reserved4[174];
>> +} __packed;
>> +
>> +struct switchtec_dev {
>> + struct pci_dev *pdev;
>> + struct device dev;
>> + struct cdev cdev;
>> +
>> + int partition;
>> + int partition_count;
>> + int pff_csr_count;
>> + char pff_local[SWITCHTEC_MAX_PFF_CSR];
>> +
>> + void __iomem *mmio;
>> + struct mrpc_regs __iomem *mmio_mrpc;
>> + struct sw_event_regs __iomem *mmio_sw_event;
>> + struct sys_info_regs __iomem *mmio_sys_info;
>> + struct flash_info_regs __iomem *mmio_flash_info;
>> + struct ntb_info_regs __iomem *mmio_ntb;
>> + struct part_cfg_regs __iomem *mmio_part_cfg;
>> + struct part_cfg_regs __iomem *mmio_part_cfg_all;
>> + struct pff_csr_regs __iomem *mmio_pff_csr;
>> +
>> + /*
>> + * The mrpc mutex must be held when accessing the other
>> + * mrpc_ fields, alive flag and stuser->state field
>> + */
>> + struct mutex mrpc_mutex;
>> + struct list_head mrpc_queue;
>> + int mrpc_busy;
>> + struct work_struct mrpc_work;
>> + struct delayed_work mrpc_timeout;
>> + bool alive;
>> +
>> + wait_queue_head_t event_wq;
>> + atomic_t event_cnt;
>> +};
>> +
>> +static inline struct switchtec_dev *to_stdev(struct device *dev)
>> +{
>> + return container_of(dev, struct switchtec_dev, dev);
>> +}
>> +
>> +#endif
>> --
>> 2.11.0
>>

2017-07-31 22:50:10

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 01/16] switchtec: move structure definitions into a common header

On Mon, Jul 31, 2017 at 02:51:35PM -0600, Logan Gunthorpe wrote:
> Hey,
>
> I've never found any consistency in the capitalization and prefixes
> patch titles. I used to capitalize them all but I've seen many
> developers use lower case. In any case, I've changed them as you requested.

Thanks! Granted, most people don't seem to care, but I'm not
completely alone in this; I try follow Ingo's preferences [1] for
drivers/pci, so there is some consistency there.

Bjorn

[1] http://lkml.kernel.org/r/[email protected]

2017-07-31 23:09:18

by Dave Jiang

[permalink] [raw]
Subject: Re: [PATCH v3 08/16] switchtec_ntb: introduce initial NTB driver



On 07/25/2017 01:57 PM, Logan Gunthorpe wrote:
> Seeing the Switchtec NTB hardware shares the same endpoint as the
> management endpoint we utilize the class_interface API to register
> an NTB driver for every Switchtec device in the system that has the
> NTB class code.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Reviewed-by: Stephen Bates <[email protected]>
> Reviewed-by: Kurt Schwemmer <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/ntb/hw/Kconfig | 1 +
> drivers/ntb/hw/Makefile | 1 +
> drivers/ntb/hw/mscc/Kconfig | 9 +++++
> drivers/ntb/hw/mscc/Makefile | 1 +
> drivers/ntb/hw/mscc/switchtec_ntb.c | 81 +++++++++++++++++++++++++++++++++++++

Minor nit. Conforming to the existing hw drivers naming scheme, maybe
should name switchtec_ntb.c to ntb_hw_switchtec.c?

> drivers/pci/switch/switchtec.c | 3 ++
> include/linux/switchtec.h | 4 ++
> 8 files changed, 101 insertions(+)
> create mode 100644 drivers/ntb/hw/mscc/Kconfig
> create mode 100644 drivers/ntb/hw/mscc/Makefile
> create mode 100644 drivers/ntb/hw/mscc/switchtec_ntb.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4ff2ad7c1c7b..6e491ce5e876 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10147,6 +10147,7 @@ F: Documentation/ABI/testing/sysfs-class-switchtec
> F: drivers/pci/switch/switchtec*
> F: include/uapi/linux/switchtec_ioctl.h
> F: include/linux/switchtec.h
> +F: drivers/ntb/hw/mscc/
>
> PCI DRIVER FOR NVIDIA TEGRA
> M: Thierry Reding <[email protected]>
> diff --git a/drivers/ntb/hw/Kconfig b/drivers/ntb/hw/Kconfig
> index a89243c9fdd3..e51b581fd102 100644
> --- a/drivers/ntb/hw/Kconfig
> +++ b/drivers/ntb/hw/Kconfig
> @@ -1,3 +1,4 @@
> source "drivers/ntb/hw/amd/Kconfig"
> source "drivers/ntb/hw/idt/Kconfig"
> source "drivers/ntb/hw/intel/Kconfig"
> +source "drivers/ntb/hw/mscc/Kconfig"
> diff --git a/drivers/ntb/hw/Makefile b/drivers/ntb/hw/Makefile
> index 87332c3905f0..923c442db750 100644
> --- a/drivers/ntb/hw/Makefile
> +++ b/drivers/ntb/hw/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_NTB_AMD) += amd/
> obj-$(CONFIG_NTB_IDT) += idt/
> obj-$(CONFIG_NTB_INTEL) += intel/
> +obj-$(CONFIG_NTB_SWITCHTEC) += mscc/
> diff --git a/drivers/ntb/hw/mscc/Kconfig b/drivers/ntb/hw/mscc/Kconfig
> new file mode 100644
> index 000000000000..013ed6716438
> --- /dev/null
> +++ b/drivers/ntb/hw/mscc/Kconfig
> @@ -0,0 +1,9 @@
> +config NTB_SWITCHTEC
> + tristate "MicroSemi Switchtec Non-Transparent Bridge Support"
> + select PCI_SW_SWITCHTEC
> + help
> + Enables NTB support for Switchtec PCI switches. This also
> + selects the Switchtec management driver as they share the same
> + hardware interface.
> +
> + If unsure, say N.
> diff --git a/drivers/ntb/hw/mscc/Makefile b/drivers/ntb/hw/mscc/Makefile
> new file mode 100644
> index 000000000000..21907b364e19
> --- /dev/null
> +++ b/drivers/ntb/hw/mscc/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_NTB_SWITCHTEC) += switchtec_ntb.o
> diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
> new file mode 100644
> index 000000000000..253efba72275
> --- /dev/null
> +++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
> @@ -0,0 +1,81 @@
> +/*
> + * Microsemi Switchtec(tm) PCIe Management Driver
> + * Copyright (c) 2017, Microsemi Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include <linux/switchtec.h>
> +#include <linux/module.h>
> +
> +MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
> +MODULE_VERSION("0.1");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Microsemi Corporation");
> +
> +struct switchtec_ntb {
> + struct switchtec_dev *stdev;
> +};
> +
> +static int switchtec_ntb_add(struct device *dev,
> + struct class_interface *class_intf)
> +{
> + struct switchtec_dev *stdev = to_stdev(dev);
> + struct switchtec_ntb *sndev;
> +
> + stdev->sndev = NULL;
> +
> + if (stdev->pdev->class != MICROSEMI_NTB_CLASSCODE)
> + return -ENODEV;
> +
> + sndev = kzalloc_node(sizeof(*sndev), GFP_KERNEL, dev_to_node(dev));
> + if (!sndev)
> + return -ENOMEM;
> +
> + sndev->stdev = stdev;
> +
> + stdev->sndev = sndev;
> + dev_info(dev, "NTB device registered");
> +
> + return 0;
> +}
> +
> +void switchtec_ntb_remove(struct device *dev,
> + struct class_interface *class_intf)
> +{
> + struct switchtec_dev *stdev = to_stdev(dev);
> + struct switchtec_ntb *sndev = stdev->sndev;
> +
> + if (!sndev)
> + return;
> +
> + stdev->sndev = NULL;
> + kfree(sndev);
> + dev_info(dev, "ntb device unregistered");
> +}
> +
> +static struct class_interface switchtec_interface = {
> + .add_dev = switchtec_ntb_add,
> + .remove_dev = switchtec_ntb_remove,
> +};
> +
> +static int __init switchtec_ntb_init(void)
> +{
> + switchtec_interface.class = switchtec_class;
> + return class_interface_register(&switchtec_interface);
> +}
> +module_init(switchtec_ntb_init);
> +
> +static void __exit switchtec_ntb_exit(void)
> +{
> + class_interface_unregister(&switchtec_interface);
> +}
> +module_exit(switchtec_ntb_exit);
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 807569e62bee..0666e33c305b 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -1275,6 +1275,9 @@ static int switchtec_pci_probe(struct pci_dev *pdev,
> struct switchtec_dev *stdev;
> int rc;
>
> + if (pdev->class == MICROSEMI_NTB_CLASSCODE)
> + request_module_nowait("switchtec_ntb");
> +
> stdev = stdev_create(pdev);
> if (IS_ERR(stdev))
> return PTR_ERR(stdev);
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index d8159944f013..09d73d0d1aa8 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -320,6 +320,8 @@ struct pff_csr_regs {
> u32 reserved4[174];
> } __packed;
>
> +struct switchtec_ntb;
> +
> struct switchtec_dev {
> struct pci_dev *pdev;
> struct device dev;
> @@ -357,6 +359,8 @@ struct switchtec_dev {
> struct work_struct link_event_work;
> void (*link_notifier)(struct switchtec_dev *stdev);
> u8 link_event_count[SWITCHTEC_MAX_PFF_CSR];
> +
> + struct switchtec_ntb *sndev;
> };
>
> static inline struct switchtec_dev *to_stdev(struct device *dev)
>

2017-08-01 18:07:36

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 05/16] ntb: ntb_test: ensure the link is up before trying to configure the mws

On Tue, Jul 25, 2017 at 02:57:42PM -0600, Logan Gunthorpe wrote:
> After the link tests, there is a race on one side of the test for
> the link coming up. It's possible, in some cases, for the test script
> to write to the 'peer_trans' files before the link has come up.
>
> To fix this, we simply use the link event file to ensure both sides
> see the link as up before continuning.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>

This looks like a bug fix. Assuming this is the case, I can pull it
out, add a "Fixes" line, and add it to my bug fixes branch. Is this
the case?

Thanks,
Jon

> ---
> tools/testing/selftests/ntb/ntb_test.sh | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
> index 1c12b5855e4f..5fc7ad359e21 100755
> --- a/tools/testing/selftests/ntb/ntb_test.sh
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -333,6 +333,10 @@ function ntb_tool_tests()
> link_test $LOCAL_TOOL $REMOTE_TOOL
> link_test $REMOTE_TOOL $LOCAL_TOOL
>
> + #Ensure the link is up on both sides before continuing
> + write_file Y $LOCAL_TOOL/link_event
> + write_file Y $REMOTE_TOOL/link_event
> +
> for PEER_TRANS in $(ls $LOCAL_TOOL/peer_trans*); do
> PT=$(basename $PEER_TRANS)
> write_file $MW_SIZE $LOCAL_TOOL/$PT
> --
> 2.11.0
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-6-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 18:09:27

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 05/16] ntb: ntb_test: ensure the link is up before trying to configure the mws


On 01/08/17 12:07 PM, Jon Mason wrote:
> On Tue, Jul 25, 2017 at 02:57:42PM -0600, Logan Gunthorpe wrote:
>> After the link tests, there is a race on one side of the test for
>> the link coming up. It's possible, in some cases, for the test script
>> to write to the 'peer_trans' files before the link has come up.
>>
>> To fix this, we simply use the link event file to ensure both sides
>> see the link as up before continuning.
>>
>> Signed-off-by: Logan Gunthorpe <[email protected]>
>
> This looks like a bug fix. Assuming this is the case, I can pull it
> out, add a "Fixes" line, and add it to my bug fixes branch. Is this
> the case?

Sure, yup, if you'd like to do that I'm fine with it. Technically, I
don't think the bug can be triggered until the patches later in the
series are applied.

Thanks,

Logan

2017-08-01 18:11:41

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 07/16] ntb: add check and comment for link up to mw_count() and mw_get_align()

On Tue, Jul 25, 2017 at 02:57:44PM -0600, Logan Gunthorpe wrote:
> Adds a comment and a check to ntb_mw_get_align() so that it always fails
> if the function is called before the link is up.
>
> Also adds a comment to ntb_mw_count() to note that it may return 0 if
> it is called before the link is up.
>
> This is to prevent accidental mis-use in clients that are testing
> on hardware that this doesn't matter for.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>

Similar to the last one, I think this is also a bug fix. I'll pull it
out, etc. :)

Thanks,
Jon

> ---
> include/linux/ntb.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ntb.h b/include/linux/ntb.h
> index 609e232c00da..47f2966cfd7f 100644
> --- a/include/linux/ntb.h
> +++ b/include/linux/ntb.h
> @@ -730,7 +730,8 @@ static inline int ntb_link_disable(struct ntb_dev *ntb)
> * Hardware and topology may support a different number of memory windows.
> * Moreover different peer devices can support different number of memory
> * windows. Simply speaking this method returns the number of possible inbound
> - * memory windows to share with specified peer device.
> + * memory windows to share with specified peer device. Note: this may return
> + * zero if the link is not up yet.
> *
> * Return: the number of memory windows.
> */
> @@ -751,7 +752,7 @@ static inline int ntb_mw_count(struct ntb_dev *ntb, int pidx)
> * Get the alignments of an inbound memory window with specified index.
> * NULL may be given for any output parameter if the value is not needed.
> * The alignment and size parameters may be used for allocation of proper
> - * shared memory.
> + * shared memory. Note: this must only be called when the link is up.
> *
> * Return: Zero on success, otherwise a negative error number.
> */
> @@ -760,6 +761,9 @@ static inline int ntb_mw_get_align(struct ntb_dev *ntb, int pidx, int widx,
> resource_size_t *size_align,
> resource_size_t *size_max)
> {
> + if (!(ntb_link_is_up(ntb, NULL, NULL) & (1 << pidx)))
> + return -ENOTCONN;
> +
> return ntb->ops->mw_get_align(ntb, pidx, widx, addr_align, size_align,
> size_max);
> }
> --
> 2.11.0
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-8-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 18:14:59

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called when the link is up

On Tue, Jul 25, 2017 at 02:57:43PM -0600, Logan Gunthorpe wrote:
> With Switchtec hardware it's impossible to get the alignment parameters
> for a peer's memory window until the peer's driver has configured it's
> windows. Strictly speaking, the link doesn't have to be up for this,
> but the link being up is the only way the client can tell that
> the otherside has been configured.
>
> This patch converts ntb_transport and ntb_perf to use this function after
> the link goes up. This simplifies these clients slightly because they
> no longer have to store the alignment parameters. It also tweaks
> ntb_tool so that peer_mw_trans will print zero if it is run before
> the link goes up.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>

This also looks like a bug fix. However, given that it is a little more
invasive and no one reporting any problems with it, I'm less apt to
pull it into the bug-fixes branch without anyone compilaining of a
bug (lest I break something in -stable).

Thanks,
Jon

> ---
> drivers/ntb/ntb_transport.c | 20 ++++++++++----------
> drivers/ntb/test/ntb_perf.c | 18 +++++++++---------
> drivers/ntb/test/ntb_tool.c | 6 +++---
> 3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index b29558ddfe95..7ed745aaa213 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -191,8 +191,6 @@ struct ntb_transport_qp {
> struct ntb_transport_mw {
> phys_addr_t phys_addr;
> resource_size_t phys_size;
> - resource_size_t xlat_align;
> - resource_size_t xlat_align_size;
> void __iomem *vbase;
> size_t xlat_size;
> size_t buff_size;
> @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
> struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
> struct pci_dev *pdev = nt->ndev->pdev;
> size_t xlat_size, buff_size;
> + resource_size_t xlat_align;
> + resource_size_t xlat_align_size;
> int rc;
>
> if (!size)
> return -EINVAL;
>
> - xlat_size = round_up(size, mw->xlat_align_size);
> - buff_size = round_up(size, mw->xlat_align);
> + rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align,
> + &xlat_align_size, NULL);
> + if (rc)
> + return rc;
> +
> + xlat_size = round_up(size, xlat_align_size);
> + buff_size = round_up(size, xlat_align);
>
> /* No need to re-setup */
> if (mw->xlat_size == xlat_size)
> @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
> * is a requirement of the hardware. It is recommended to setup CMA
> * for BAR sizes equal or greater than 4MB.
> */
> - if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) {
> + if (!IS_ALIGNED(mw->dma_addr, xlat_align)) {
> dev_err(&pdev->dev, "DMA memory %pad is not aligned\n",
> &mw->dma_addr);
> ntb_free_mw(nt, num_mw);
> @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
> for (i = 0; i < mw_count; i++) {
> mw = &nt->mw_vec[i];
>
> - rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align,
> - &mw->xlat_align_size, NULL);
> - if (rc)
> - goto err1;
> -
> rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr,
> &mw->phys_size);
> if (rc)
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 759f772fa00c..427112cf101a 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)")
> struct perf_mw {
> phys_addr_t phys_addr;
> resource_size_t phys_size;
> - resource_size_t xlat_align;
> - resource_size_t xlat_align_size;
> void __iomem *vbase;
> size_t xlat_size;
> size_t buf_size;
> @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size)
> {
> struct perf_mw *mw = &perf->mw;
> size_t xlat_size, buf_size;
> + resource_size_t xlat_align;
> + resource_size_t xlat_align_size;
> int rc;
>
> if (!size)
> return -EINVAL;
>
> - xlat_size = round_up(size, mw->xlat_align_size);
> - buf_size = round_up(size, mw->xlat_align);
> + rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align,
> + &xlat_align_size, NULL);
> + if (rc)
> + return rc;
> +
> + xlat_size = round_up(size, xlat_align_size);
> + buf_size = round_up(size, xlat_align);
>
> if (mw->xlat_size == xlat_size)
> return 0;
> @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf)
>
> mw = &perf->mw;
>
> - rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align,
> - &mw->xlat_align_size, NULL);
> - if (rc)
> - return rc;
> -
> rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size);
> if (rc)
> return rc;
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index a69815c45ce6..91526a986caa 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep,
>
> phys_addr_t base;
> resource_size_t mw_size;
> - resource_size_t align_addr;
> - resource_size_t align_size;
> - resource_size_t max_size;
> + resource_size_t align_addr = 0;
> + resource_size_t align_size = 0;
> + resource_size_t max_size = 0;
>
> buf_size = min_t(size_t, size, 512);
>
> --
> 2.11.0
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-7-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 18:16:27

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 05/16] ntb: ntb_test: ensure the link is up before trying to configure the mws

On Tue, Aug 01, 2017 at 12:09:18PM -0600, Logan Gunthorpe wrote:
>
> On 01/08/17 12:07 PM, Jon Mason wrote:
> > On Tue, Jul 25, 2017 at 02:57:42PM -0600, Logan Gunthorpe wrote:
> >> After the link tests, there is a race on one side of the test for
> >> the link coming up. It's possible, in some cases, for the test script
> >> to write to the 'peer_trans' files before the link has come up.
> >>
> >> To fix this, we simply use the link event file to ensure both sides
> >> see the link as up before continuning.
> >>
> >> Signed-off-by: Logan Gunthorpe <[email protected]>
> >
> > This looks like a bug fix. Assuming this is the case, I can pull it
> > out, add a "Fixes" line, and add it to my bug fixes branch. Is this
> > the case?
>
> Sure, yup, if you'd like to do that I'm fine with it. Technically, I
> don't think the bug can be triggered until the patches later in the
> series are applied.

Given how trivial it is, I think closing the loop here on this would
be a good thing (and one less patch for your v4).

>
> Thanks,
>
> Logan
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/46065aa6-0fe8-59d5-63dc-3beb66b69154%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 18:17:54

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called when the link is up

Hey,

On 01/08/17 12:14 PM, Jon Mason wrote:
> This also looks like a bug fix. However, given that it is a little more
> invasive and no one reporting any problems with it, I'm less apt to
> pull it into the bug-fixes branch without anyone compilaining of a
> bug (lest I break something in -stable).

Yes, 6 and 7 change a semantic of that function. I wouldn't call them
bug fixes. But you can't pull 7 without pulling 6, as 7 would break
everything unless the change in 6 occurs.

Thanks,

Logan

2017-08-01 18:19:28

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 05/16] ntb: ntb_test: ensure the link is up before trying to configure the mws



On 01/08/17 12:16 PM, Jon Mason wrote:
> On Tue, Aug 01, 2017 at 12:09:18PM -0600, Logan Gunthorpe wrote:
>>
>> On 01/08/17 12:07 PM, Jon Mason wrote:
>>> On Tue, Jul 25, 2017 at 02:57:42PM -0600, Logan Gunthorpe wrote:
>>>> After the link tests, there is a race on one side of the test for
>>>> the link coming up. It's possible, in some cases, for the test script
>>>> to write to the 'peer_trans' files before the link has come up.
>>>>
>>>> To fix this, we simply use the link event file to ensure both sides
>>>> see the link as up before continuning.
>>>>
>>>> Signed-off-by: Logan Gunthorpe <[email protected]>
>>>
>>> This looks like a bug fix. Assuming this is the case, I can pull it
>>> out, add a "Fixes" line, and add it to my bug fixes branch. Is this
>>> the case?
>>
>> Sure, yup, if you'd like to do that I'm fine with it. Technically, I
>> don't think the bug can be triggered until the patches later in the
>> series are applied.
>
> Given how trivial it is, I think closing the loop here on this would
> be a good thing (and one less patch for your v4).

Sounds good to me. I'll rebase v4 onto ntb_next again once I see the
patch in. I also have yet to rename the file per Dave's feedback. Once
I've done both those things I'll send a v4.

Thanks,

Logan

2017-08-01 18:28:17

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 06/16] ntb: ensure ntb_mw_get_align() is only called when the link is up

On Tue, Aug 01, 2017 at 12:17:47PM -0600, Logan Gunthorpe wrote:
> Hey,
>
> On 01/08/17 12:14 PM, Jon Mason wrote:
> > This also looks like a bug fix. However, given that it is a little more
> > invasive and no one reporting any problems with it, I'm less apt to
> > pull it into the bug-fixes branch without anyone compilaining of a
> > bug (lest I break something in -stable).
>
> Yes, 6 and 7 change a semantic of that function. I wouldn't call them
> bug fixes. But you can't pull 7 without pulling 6, as 7 would break
> everything unless the change in 6 occurs.

Okay, then I'll hold off on 7 then. It wasn't clear to me, based on
the patch descriptions, that this was fallout from the previous patch.
Thanks for clearing this up for me.

Thanks,
Jon

>
> Thanks,
>
> Logan
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/c4235dc9-90d0-1e68-7c73-c2a5bec83712%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 18:55:13

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 09/16] switchtec_ntb: initialize hardware for memory windows

On Tue, Jul 25, 2017 at 02:57:46PM -0600, Logan Gunthorpe wrote:
> Add the code to initialize the memory windows in the hardware.
> This includes setting up the requester ID table, and figuring out
> which bar corresponds to which memory window. (Seeing the switch
> can be configured with any number of bars.)
>
> Also, seeing the device doesn't have hardware for scratchpads or
> determining the link status, we create a shared memory window that has
> these features. A magic number with a version copmonent will be used
> to determine if the otherside's driver is actually up.

Using shared memory for SPADS is kind of hacky. We should probably
rework the transport to not need SPADs. I'm fine with this being done
after this series, assuming you are willing to do so.

More comments below

>
> The shared memory window also informs the other side of the
> size and count of the local memory windows.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Reviewed-by: Stephen Bates <[email protected]>
> Reviewed-by: Kurt Schwemmer <[email protected]>
> ---
> drivers/ntb/hw/mscc/switchtec_ntb.c | 355 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 355 insertions(+)
>
> diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
> index 253efba72275..1596757413f5 100644
> --- a/drivers/ntb/hw/mscc/switchtec_ntb.c
> +++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
> @@ -15,37 +15,391 @@
>
> #include <linux/switchtec.h>
> #include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/kthread.h>
>
> MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
> MODULE_VERSION("0.1");
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Microsemi Corporation");
>
> +static bool use_lut_mws;
> +module_param(use_lut_mws, bool, 0644);
> +MODULE_PARM_DESC(use_lut_mws,
> + "Enable the use of the LUT based memory windows");
> +
> +#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 SWITCHTEC_NTB_MAGIC 0x45CC0001
> +#define MAX_MWS 128
> +
> +struct shared_mw {
> + u32 magic;
> + u32 partition_id;
> + u64 mw_sizes[MAX_MWS];
> +};
> +
> +#define MAX_DIRECT_MW ARRAY_SIZE(((struct ntb_ctrl_regs *)(0))->bar_entry)
> +#define LUT_SIZE SZ_64K
> +
> struct switchtec_ntb {
> struct switchtec_dev *stdev;
> +
> + int self_partition;
> + int peer_partition;
> +
> + struct ntb_info_regs __iomem *mmio_ntb;
> + struct ntb_ctrl_regs __iomem *mmio_ctrl;
> + struct ntb_dbmsg_regs __iomem *mmio_dbmsg;
> + struct ntb_ctrl_regs __iomem *mmio_self_ctrl;
> + struct ntb_ctrl_regs __iomem *mmio_peer_ctrl;
> + struct ntb_dbmsg_regs __iomem *mmio_self_dbmsg;
> +
> + struct shared_mw *self_shared;
> + struct shared_mw __iomem *peer_shared;
> + dma_addr_t self_shared_dma;
> +
> + int nr_direct_mw;
> + int nr_lut_mw;
> + int direct_mw_to_bar[MAX_DIRECT_MW];
> +
> + int peer_nr_direct_mw;
> + int peer_nr_lut_mw;
> + int peer_direct_mw_to_bar[MAX_DIRECT_MW];
> };
>
> +static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
> + struct ntb_ctrl_regs __iomem *ctl,
> + u32 op, int wait_status)
> +{
> + static const char * const op_text[] = {
> + [NTB_CTRL_PART_OP_LOCK] = "lock",
> + [NTB_CTRL_PART_OP_CFG] = "configure",
> + [NTB_CTRL_PART_OP_RESET] = "reset",
> + };
> +
> + int i;
> + u32 ps;
> + int status;
> +
> + switch (op) {
> + case NTB_CTRL_PART_OP_LOCK:
> + status = NTB_CTRL_PART_STATUS_LOCKING;
> + break;
> + case NTB_CTRL_PART_OP_CFG:
> + status = NTB_CTRL_PART_STATUS_CONFIGURING;
> + break;
> + case NTB_CTRL_PART_OP_RESET:
> + status = NTB_CTRL_PART_STATUS_RESETTING;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + iowrite32(op, &ctl->partition_op);
> +
> + for (i = 0; i < 1000; i++) {
> + if (msleep_interruptible(50) != 0) {
> + iowrite32(NTB_CTRL_PART_OP_RESET, &ctl->partition_op);
> + return -EINTR;
> + }
> +
> + ps = ioread32(&ctl->partition_status) & 0xFFFF;
> +
> + if (ps != status)
> + break;
> + }
> +
> + if (ps == wait_status)
> + return 0;
> +
> + if (ps == status) {
> + dev_err(&sndev->stdev->dev,
> + "Timed out while peforming %s (%d). (%08x)",
> + op_text[op], op,
> + ioread32(&ctl->partition_status));
> +
> + return -ETIMEDOUT;
> + }
> +
> + return -EIO;
> +}
> +
> +static void switchtec_ntb_init_sndev(struct switchtec_ntb *sndev)
> +{
> + u64 part_map;
> +
> + sndev->self_partition = sndev->stdev->partition;
> +
> + sndev->mmio_ntb = sndev->stdev->mmio_ntb;
> + part_map = ioread64(&sndev->mmio_ntb->ep_map);
> + part_map &= ~(1 << sndev->self_partition);
> + sndev->peer_partition = ffs(part_map) - 1;
> +
> + dev_dbg(&sndev->stdev->dev, "Partition ID %d of %d (%llx)",
> + sndev->self_partition, sndev->stdev->partition_count,
> + part_map);
> +
> + sndev->mmio_ctrl = (void * __iomem)sndev->mmio_ntb +
> + SWITCHTEC_NTB_REG_CTRL_OFFSET;
> + sndev->mmio_dbmsg = (void * __iomem)sndev->mmio_ntb +
> + SWITCHTEC_NTB_REG_DBMSG_OFFSET;
> +
> + sndev->mmio_self_ctrl = &sndev->mmio_ctrl[sndev->self_partition];
> + sndev->mmio_peer_ctrl = &sndev->mmio_ctrl[sndev->peer_partition];
> + sndev->mmio_self_dbmsg = &sndev->mmio_dbmsg[sndev->self_partition];
> +}
> +
> +static int map_bars(int *map, struct ntb_ctrl_regs __iomem *ctrl)
> +{
> + int i;
> + int cnt = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(ctrl->bar_entry); i++) {
> + u32 r = ioread32(&ctrl->bar_entry[i].ctl);
> +
> + if (r & NTB_CTRL_BAR_VALID)
> + map[cnt++] = i;
> + }
> +
> + return cnt;
> +}
> +
> +static void switchtec_ntb_init_mw(struct switchtec_ntb *sndev)
> +{
> + sndev->nr_direct_mw = map_bars(sndev->direct_mw_to_bar,
> + sndev->mmio_self_ctrl);
> +
> + sndev->nr_lut_mw = ioread16(&sndev->mmio_self_ctrl->lut_table_entries);
> + sndev->nr_lut_mw = rounddown_pow_of_two(sndev->nr_lut_mw);
> +
> + dev_dbg(&sndev->stdev->dev, "MWs: %d direct, %d lut",
> + sndev->nr_direct_mw, sndev->nr_lut_mw);
> +
> + sndev->peer_nr_direct_mw = map_bars(sndev->peer_direct_mw_to_bar,
> + sndev->mmio_peer_ctrl);
> +
> + sndev->peer_nr_lut_mw =
> + ioread16(&sndev->mmio_peer_ctrl->lut_table_entries);
> + sndev->peer_nr_lut_mw = rounddown_pow_of_two(sndev->peer_nr_lut_mw);
> +
> + dev_dbg(&sndev->stdev->dev, "Peer MWs: %d direct, %d lut",
> + sndev->peer_nr_direct_mw, sndev->peer_nr_lut_mw);
> +
> +}
> +
> +static int switchtec_ntb_init_req_id_table(struct switchtec_ntb *sndev)
> +{
> + int rc = 0;
> + u16 req_id = ioread16(&sndev->mmio_ntb->requester_id);

Nit, please don't do function calls in the variable declaration. If
this doesn't violate Linux coding style, then it should :)

> + u32 error;
> +
> + if (ioread32(&sndev->mmio_self_ctrl->req_id_table_size) < 2) {
> + dev_err(&sndev->stdev->dev,
> + "Not enough requester IDs available.");
> + return -EFAULT;
> + }
> +
> + rc = switchtec_ntb_part_op(sndev, sndev->mmio_self_ctrl,
> + NTB_CTRL_PART_OP_LOCK,
> + NTB_CTRL_PART_STATUS_LOCKED);
> + if (rc)
> + return rc;
> +
> + iowrite32(NTB_PART_CTRL_ID_PROT_DIS,
> + &sndev->mmio_self_ctrl->partition_ctrl);
> +
> + // Root Complex Requester ID

Please use standard /**/ comments (same for below)

> + iowrite32(0 << 16 | NTB_CTRL_REQ_ID_EN,

0? A comment here on 0 might be needed.

> + &sndev->mmio_self_ctrl->req_id_table[0]);
> +
> + // Host Bridge Requester ID
> + iowrite32(req_id << 16 | NTB_CTRL_REQ_ID_EN,
> + &sndev->mmio_self_ctrl->req_id_table[1]);
> +
> + rc = switchtec_ntb_part_op(sndev, sndev->mmio_self_ctrl,
> + NTB_CTRL_PART_OP_CFG,
> + NTB_CTRL_PART_STATUS_NORMAL);
> +

nit, remove this new line

> + if (rc == -EIO) {
> + error = ioread32(&sndev->mmio_self_ctrl->req_id_error);
> + dev_err(&sndev->stdev->dev,
> + "Error setting up the requester ID table: %08x",
> + error);
> + }
> +
> + return rc;
> +}
> +
> +static void switchtec_ntb_init_shared(struct switchtec_ntb *sndev)
> +{
> + int i;
> +
> + memset(sndev->self_shared, 0, LUT_SIZE);
> + sndev->self_shared->magic = SWITCHTEC_NTB_MAGIC;
> + sndev->self_shared->partition_id = sndev->stdev->partition;
> +
> + for (i = 0; i < sndev->nr_direct_mw; i++) {
> + int bar = sndev->direct_mw_to_bar[i];
> + resource_size_t sz = pci_resource_len(sndev->stdev->pdev, bar);
> +
> + if (i == 0)
> + sz = min_t(resource_size_t, sz,
> + LUT_SIZE * sndev->nr_lut_mw);
> +
> + sndev->self_shared->mw_sizes[i] = sz;
> + }
> +
> + for (i = 0; i < sndev->nr_lut_mw; i++) {
> + int idx = sndev->nr_direct_mw + i;
> +
> + sndev->self_shared->mw_sizes[idx] = LUT_SIZE;
> + }
> +}
> +
> +static int switchtec_ntb_init_shared_mw(struct switchtec_ntb *sndev)
> +{
> + struct ntb_ctrl_regs __iomem *ctl = sndev->mmio_peer_ctrl;
> + int bar = sndev->direct_mw_to_bar[0];
> + u32 ctl_val;
> + int rc;
> +
> + sndev->self_shared = dma_zalloc_coherent(&sndev->stdev->pdev->dev,
> + LUT_SIZE,
> + &sndev->self_shared_dma,
> + GFP_KERNEL);
> + if (!sndev->self_shared) {
> + dev_err(&sndev->stdev->dev,
> + "unable to allocate memory for shared mw");
> + return -ENOMEM;
> + }
> +
> + switchtec_ntb_init_shared(sndev);
> +
> + rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_LOCK,
> + NTB_CTRL_PART_STATUS_LOCKED);
> + if (rc)
> + goto unalloc_and_exit;
> +
> + ctl_val = ioread32(&ctl->bar_entry[bar].ctl);
> + ctl_val |= NTB_CTRL_BAR_LUT_WIN_EN;
> + ctl_val &= 0xFF;

This seems odd ordering. Don't you want to clear the unnecessary bits
before adding new ones? If so, maybe flip the 2 lines above.

> + ctl_val |= ilog2(LUT_SIZE) << 8;
> + ctl_val |= (sndev->nr_lut_mw - 1) << 14;
> + iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
> +
> + iowrite64((NTB_CTRL_LUT_EN | (sndev->self_partition << 1) |
> + sndev->self_shared_dma),
> + &ctl->lut_entry[0]);
> +
> + rc = switchtec_ntb_part_op(sndev, ctl, NTB_CTRL_PART_OP_CFG,
> + NTB_CTRL_PART_STATUS_NORMAL);
> + if (rc) {
> + u32 bar_error, lut_error;
> +
> + bar_error = ioread32(&ctl->bar_error);
> + lut_error = ioread32(&ctl->lut_error);
> + dev_err(&sndev->stdev->dev,
> + "Error setting up shared MW: %08x / %08x",
> + bar_error, lut_error);
> + goto unalloc_and_exit;
> + }
> +
> + sndev->peer_shared = pci_iomap(sndev->stdev->pdev, bar, LUT_SIZE);
> + if (!sndev->peer_shared) {
> + rc = -ENOMEM;
> + goto unalloc_and_exit;
> + }
> +
> + dev_dbg(&sndev->stdev->dev, "Shared MW Ready");
> + return 0;
> +
> +unalloc_and_exit:
> + dma_free_coherent(&sndev->stdev->pdev->dev, LUT_SIZE,
> + sndev->self_shared, sndev->self_shared_dma);
> +
> + return rc;
> +}
> +
> +static void switchtec_ntb_deinit_shared_mw(struct switchtec_ntb *sndev)
> +{
> + if (sndev->peer_shared)
> + pci_iounmap(sndev->stdev->pdev, sndev->peer_shared);
> +
> + if (sndev->self_shared)
> + dma_free_coherent(&sndev->stdev->pdev->dev, LUT_SIZE,
> + sndev->self_shared,
> + sndev->self_shared_dma);
> +}
> +
> static int switchtec_ntb_add(struct device *dev,
> struct class_interface *class_intf)
> {
> struct switchtec_dev *stdev = to_stdev(dev);
> struct switchtec_ntb *sndev;
> + int rc;
>
> stdev->sndev = NULL;
>
> if (stdev->pdev->class != MICROSEMI_NTB_CLASSCODE)
> return -ENODEV;
>
> + if (stdev->partition_count != 2)
> + dev_warn(dev, "ntb driver only supports 2 partitions");
> +
> sndev = kzalloc_node(sizeof(*sndev), GFP_KERNEL, dev_to_node(dev));
> if (!sndev)
> return -ENOMEM;
>
> sndev->stdev = stdev;
>
> + switchtec_ntb_init_sndev(sndev);
> + switchtec_ntb_init_mw(sndev);
> +
> + rc = switchtec_ntb_init_req_id_table(sndev);
> + if (rc)
> + goto free_and_exit;
> +
> + rc = switchtec_ntb_init_shared_mw(sndev);
> + if (rc)
> + goto free_and_exit;
> +
> stdev->sndev = sndev;
> dev_info(dev, "NTB device registered");
>
> return 0;
> +
> +free_and_exit:
> + kfree(sndev);
> + dev_err(dev, "failed to register ntb device: %d", rc);
> + return rc;
> }
>
> void switchtec_ntb_remove(struct device *dev,
> @@ -58,6 +412,7 @@ void switchtec_ntb_remove(struct device *dev,
> return;
>
> stdev->sndev = NULL;
> + switchtec_ntb_deinit_shared_mw(sndev);
> kfree(sndev);
> dev_info(dev, "ntb device unregistered");
> }
> --
> 2.11.0
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-10-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 19:00:00

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 13/16] switchtec_ntb: implement doorbell registers

On Tue, Jul 25, 2017 at 02:57:50PM -0600, Logan Gunthorpe wrote:
> Pretty straightforward implementation of doorbell registers.
> The shift and mask were setup in an earlier patch and this just hooks
> up the appropriate portion of the IDB register as the local doorbells
> and the opposite portion of ODB as the peer doorbells. The DB mask is
> protected by a spinlock to avoid concurrent read-modify-write accesses.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Reviewed-by: Stephen Bates <[email protected]>
> Reviewed-by: Kurt Schwemmer <[email protected]>
> ---
> drivers/ntb/hw/mscc/switchtec_ntb.c | 89 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
> index 34d69fa8612d..3037ca730998 100644
> --- a/drivers/ntb/hw/mscc/switchtec_ntb.c
> +++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
> @@ -98,6 +98,9 @@ struct switchtec_ntb {
> int db_shift;
> int db_peer_shift;
>
> + /* synchronize rmw access of db_mask and hw reg */
> + spinlock_t db_mask_lock;
> +
> int nr_direct_mw;
> int nr_lut_mw;
> int direct_mw_to_bar[MAX_DIRECT_MW];
> @@ -338,41 +341,115 @@ static int switchtec_ntb_link_disable(struct ntb_dev *ntb)
>
> static u64 switchtec_ntb_db_valid_mask(struct ntb_dev *ntb)
> {
> - return 0;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + return sndev->db_valid_mask;
> }
>
> static int switchtec_ntb_db_vector_count(struct ntb_dev *ntb)
> {
> - return 0;
> + return 1;
> }
>
> static u64 switchtec_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector)
> {
> - return 0;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (db_vector < 0 || db_vector > 1)
> + return 0;
> +
> + return sndev->db_valid_mask;
> }
>
> static u64 switchtec_ntb_db_read(struct ntb_dev *ntb)
> {
> - return 0;
> + u64 ret;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + ret = ioread64(&sndev->mmio_self_dbmsg->idb) >> sndev->db_shift;
> +
> + return ret & sndev->db_valid_mask;
> }
>
> static int switchtec_ntb_db_clear(struct ntb_dev *ntb, u64 db_bits)
> {
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + iowrite64(db_bits << sndev->db_shift, &sndev->mmio_self_dbmsg->idb);
> +
> return 0;
> }
>
> static int switchtec_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
> {
> + unsigned long irqflags;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (db_bits & ~sndev->db_valid_mask)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&sndev->db_mask_lock, irqflags);
> + {
> + sndev->db_mask |= db_bits << sndev->db_shift;
> + iowrite64(~sndev->db_mask, &sndev->mmio_self_dbmsg->idb_mask);
> + }

unnecessary braces. Same for below

> + spin_unlock_irqrestore(&sndev->db_mask_lock, irqflags);
> +
> return 0;
> }
>
> static int switchtec_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits)
> {
> + unsigned long irqflags;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (db_bits & ~sndev->db_valid_mask)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&sndev->db_mask_lock, irqflags);
> + {
> + sndev->db_mask &= ~(db_bits << sndev->db_shift);
> + iowrite64(~sndev->db_mask, &sndev->mmio_self_dbmsg->idb_mask);
> + }
> + spin_unlock_irqrestore(&sndev->db_mask_lock, irqflags);
> +
> + return 0;
> +}
> +
> +static u64 switchtec_ntb_db_read_mask(struct ntb_dev *ntb)
> +{
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + return (sndev->db_mask >> sndev->db_shift) & sndev->db_valid_mask;
> +}
> +
> +static int switchtec_ntb_peer_db_addr(struct ntb_dev *ntb,
> + phys_addr_t *db_addr,
> + resource_size_t *db_size)
> +{
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> + unsigned long offset;
> +
> + offset = (unsigned long)sndev->mmio_self_dbmsg->odb -
> + (unsigned long)sndev->stdev->mmio;
> +
> + offset += sndev->db_shift / 8;
> +
> + if (db_addr)
> + *db_addr = pci_resource_start(ntb->pdev, 0) + offset;
> + if (db_size)
> + *db_size = sizeof(u32);
> +
> return 0;
> }
>
> static int switchtec_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
> {
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + iowrite64(db_bits << sndev->db_peer_shift,
> + &sndev->mmio_self_dbmsg->odb);
> +
> return 0;
> }
>
> @@ -413,6 +490,8 @@ static const struct ntb_dev_ops switchtec_ntb_ops = {
> .db_clear = switchtec_ntb_db_clear,
> .db_set_mask = switchtec_ntb_db_set_mask,
> .db_clear_mask = switchtec_ntb_db_clear_mask,
> + .db_read_mask = switchtec_ntb_db_read_mask,
> + .peer_db_addr = switchtec_ntb_peer_db_addr,
> .peer_db_set = switchtec_ntb_peer_db_set,
> .spad_count = switchtec_ntb_spad_count,
> .spad_read = switchtec_ntb_spad_read,
> @@ -680,6 +759,8 @@ static irqreturn_t switchtec_ntb_doorbell_isr(int irq, void *dev)
>
> dev_dbg(&sndev->stdev->dev, "doorbell\n");
>
> + ntb_db_event(&sndev->ntb, 0);
> +
> return IRQ_HANDLED;
> }
>
> --
> 2.11.0
>
> --
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-14-logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

2017-08-01 19:10:56

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers

On Tue, Jul 25, 2017 at 02:57:51PM -0600, Logan Gunthorpe wrote:
> Seeing there is no dedicated hardware for this, we simply add
> these as entries in the shared memory window. Thus, we could support
> any number of them but 128 seems like enough, for now.

It would probaly be better if I remarked about the SPADs in the actual
patch about the SPADS :)

The whole point of using the SPADs in the NTB driver was to workaround
the problems establishing a connection between the two sides of the
NTB and where everything lives. So, using a MW to get around the
SPADs is sort of backwards (and slightly funny). I realize you are
trying to use the existing transport with minimal changes to enable
your hardware, and thus this makes logical sense to you. However, if
the SPADs are not really needed, then we should either remove them
from the transport (or use them for something else).

Per my comment in the other patch, I'm amenable to take this series
as-is, assuming you are willing to address this design issue in the
near future. Thoughts?

Thanks,
Jon

>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Reviewed-by: Stephen Bates <[email protected]>
> Reviewed-by: Kurt Schwemmer <[email protected]>
> ---
> drivers/ntb/hw/mscc/switchtec_ntb.c | 75 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
> index 3037ca730998..ca403ad2f7ac 100644
> --- a/drivers/ntb/hw/mscc/switchtec_ntb.c
> +++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
> @@ -67,6 +67,7 @@ struct shared_mw {
> u32 link_sta;
> u32 partition_id;
> u64 mw_sizes[MAX_MWS];
> + u32 spad[128];
> };
>
> #define MAX_DIRECT_MW ARRAY_SIZE(((struct ntb_ctrl_regs *)(0))->bar_entry)
> @@ -455,22 +456,90 @@ static int switchtec_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
>
> static int switchtec_ntb_spad_count(struct ntb_dev *ntb)
> {
> - return 0;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + return ARRAY_SIZE(sndev->self_shared->spad);
> }
>
> static u32 switchtec_ntb_spad_read(struct ntb_dev *ntb, int idx)
> {
> - return 0;
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (idx < 0 || idx >= ARRAY_SIZE(sndev->self_shared->spad))
> + return 0;
> +
> + if (!sndev->self_shared)
> + return 0;
> +
> + return sndev->self_shared->spad[idx];
> }
>
> static int switchtec_ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val)
> {
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (idx < 0 || idx >= ARRAY_SIZE(sndev->self_shared->spad))
> + return -EINVAL;
> +
> + if (!sndev->self_shared)
> + return -EIO;
> +
> + sndev->self_shared->spad[idx] = val;
> +
> return 0;
> }
>
> +static u32 switchtec_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx,
> + int sidx)
> +{
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (pidx != NTB_DEF_PEER_IDX)
> + return -EINVAL;
> +
> + if (sidx < 0 || sidx >= ARRAY_SIZE(sndev->peer_shared->spad))
> + return 0;
> +
> + if (!sndev->peer_shared)
> + return 0;
> +
> + return ioread32(&sndev->peer_shared->spad[sidx]);
> +}
> +
> static int switchtec_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
> int sidx, u32 val)
> {
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> +
> + if (pidx != NTB_DEF_PEER_IDX)
> + return -EINVAL;
> +
> + if (sidx < 0 || sidx >= ARRAY_SIZE(sndev->peer_shared->spad))
> + return -EINVAL;
> +
> + if (!sndev->peer_shared)
> + return -EIO;
> +
> + iowrite32(val, &sndev->peer_shared->spad[sidx]);
> +
> + return 0;
> +}
> +
> +static int switchtec_ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx,
> + int sidx, phys_addr_t *spad_addr)
> +{
> + struct switchtec_ntb *sndev = ntb_sndev(ntb);
> + unsigned long offset;
> +
> + if (pidx != NTB_DEF_PEER_IDX)
> + return -EINVAL;
> +
> + offset = (unsigned long)&sndev->peer_shared->spad[sidx] -
> + (unsigned long)sndev->stdev->mmio;
> +
> + if (spad_addr)
> + *spad_addr = pci_resource_start(ntb->pdev, 0) + offset;
> +
> return 0;
> }
>
> @@ -496,7 +565,9 @@ static const struct ntb_dev_ops switchtec_ntb_ops = {
> .spad_count = switchtec_ntb_spad_count,
> .spad_read = switchtec_ntb_spad_read,
> .spad_write = switchtec_ntb_spad_write,
> + .peer_spad_read = switchtec_ntb_peer_spad_read,
> .peer_spad_write = switchtec_ntb_peer_spad_write,
> + .peer_spad_addr = switchtec_ntb_peer_spad_addr,
> };
>
> static void switchtec_ntb_init_sndev(struct switchtec_ntb *sndev)
> --
> 2.11.0
>

2017-08-01 22:27:05

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers



On 01/08/17 01:10 PM, Jon Mason wrote:
> It would probaly be better if I remarked about the SPADs in the actual
> patch about the SPADS :)
>
> The whole point of using the SPADs in the NTB driver was to workaround
> the problems establishing a connection between the two sides of the
> NTB and where everything lives. So, using a MW to get around the
> SPADs is sort of backwards (and slightly funny). I realize you are
> trying to use the existing transport with minimal changes to enable
> your hardware, and thus this makes logical sense to you. However, if
> the SPADs are not really needed, then we should either remove them
> from the transport (or use them for something else).
>
> Per my comment in the other patch, I'm amenable to take this series
> as-is, assuming you are willing to address this design issue in the
> near future. Thoughts?

Yes, I agree. I'd be willing to help but it seems the clients are
written the way they are for the other drivers, so it's their needs
(which I'm not fully aware of) that have to be considered.

I've also made all the other changes you sent as well as the file rename
Dave requested. Once I see the bug fix patch you were going to pull hit
ntb-next I'll rebase, test and resubmit.

Thanks,

Logan

2017-08-02 13:07:55

by Allen Hubbe

[permalink] [raw]
Subject: RE: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers

From: Logan Gunthorpe
> On 01/08/17 01:10 PM, Jon Mason wrote:
> > It would probaly be better if I remarked about the SPADs in the actual
> > patch about the SPADS :)
> >
> > The whole point of using the SPADs in the NTB driver was to workaround
> > the problems establishing a connection between the two sides of the
> > NTB and where everything lives. So, using a MW to get around the
> > SPADs is sort of backwards (and slightly funny). I realize you are
> > trying to use the existing transport with minimal changes to enable
> > your hardware, and thus this makes logical sense to you. However, if
> > the SPADs are not really needed, then we should either remove them
> > from the transport (or use them for something else).
> >
> > Per my comment in the other patch, I'm amenable to take this series
> > as-is, assuming you are willing to address this design issue in the
> > near future. Thoughts?
>
> Yes, I agree. I'd be willing to help but it seems the clients are
> written the way they are for the other drivers, so it's their needs
> (which I'm not fully aware of) that have to be considered.

The proposed change, removing use of spads from transport, would not affect ntrdma.

> I've also made all the other changes you sent as well as the file rename
> Dave requested. Once I see the bug fix patch you were going to pull hit
> ntb-next I'll rebase, test and resubmit.
>
> Thanks,
>
> Logan

2017-08-02 16:32:27

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers

On Wed, Aug 2, 2017 at 9:06 AM, Allen Hubbe <[email protected]> wrote:
> From: Logan Gunthorpe
>> On 01/08/17 01:10 PM, Jon Mason wrote:
>> > It would probaly be better if I remarked about the SPADs in the actual
>> > patch about the SPADS :)
>> >
>> > The whole point of using the SPADs in the NTB driver was to workaround
>> > the problems establishing a connection between the two sides of the
>> > NTB and where everything lives. So, using a MW to get around the
>> > SPADs is sort of backwards (and slightly funny). I realize you are
>> > trying to use the existing transport with minimal changes to enable
>> > your hardware, and thus this makes logical sense to you. However, if
>> > the SPADs are not really needed, then we should either remove them
>> > from the transport (or use them for something else).
>> >
>> > Per my comment in the other patch, I'm amenable to take this series
>> > as-is, assuming you are willing to address this design issue in the
>> > near future. Thoughts?
>>
>> Yes, I agree. I'd be willing to help but it seems the clients are
>> written the way they are for the other drivers, so it's their needs
>> (which I'm not fully aware of) that have to be considered.
>
> The proposed change, removing use of spads from transport, would not affect ntrdma.

After a long-ish conversation on IRC, the way we want to go forward
would be to provide 2 ways of communicating prior to the MW's being
setup: SPADs and Message Registers. The HW driver would make
available whichever ones are supported by the hardware. The transport
can see which of those are available in the HW driver, select the
appropriate one, and use it to setup the NTB connection, MW, etc.
similar to how the SPADs are being used today. This would allow for
any current clients to work unmodified, and would require minimal
changes to the existing transport layer.

Since this is outside the scope of this series, per my email yesterday
allowing the SPAD workaround, we should start up another thread on the
NTB mailing list and flesh out the details and any benefits/drawbacks.
Then we, as a community, can make the changes necessary to the drivers
and transport to get this working more optimally.

Thanks,
Jon

>> I've also made all the other changes you sent as well as the file rename
>> Dave requested. Once I see the bug fix patch you were going to pull hit
>> ntb-next I'll rebase, test and resubmit.
>>
>> Thanks,
>>
>> Logan
>

2017-08-02 17:03:33

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 14/16] switchtec_ntb: implement scratchpad registers



On 02/08/17 10:32 AM, Jon Mason wrote:
> After a long-ish conversation on IRC, the way we want to go forward
> would be to provide 2 ways of communicating prior to the MW's being
> setup: SPADs and Message Registers. The HW driver would make
> available whichever ones are supported by the hardware. The transport
> can see which of those are available in the HW driver, select the
> appropriate one, and use it to setup the NTB connection, MW, etc.
> similar to how the SPADs are being used today. This would allow for
> any current clients to work unmodified, and would require minimal
> changes to the existing transport layer.

I'm open to this, but I have a couple points:

1) I think there needs to be an ntb library or similar so the client
just has to say "communicate this setup data to the peer X". We don't
need every client replicating the decision to use spads or msgs or whatever.

2) In my experience, message registers are a bit of a pain to send
chunks of data like is being done with spads in the existing clients.
(Some time ago, I originally tried emulating spads with message
registers and it did not work well.) You can only send 32bits at a time
and you have to have a signal indicating the other side is finished with
it before sending another message. I think they are best used just for
signaling infrequent events where if you loose a message it's not an issue.

One thought I had was that maybe we need to just push this decision into
the drivers. ie if there's an ntb api just for communicating setup data,
the driver could then determine the most appropriate way to communicate
it for the hardware (whether via spads, messages or a special memory
window). However, the difficulty with this is that the driver would have
to communicate if it's using spad resources it would otherwise advertise
to the clients.

Though, maybe we don't need spads in the ntb api. Maybe they are the
wrong abstraction. All the existing clients only use them for setup
data, so if we dropped the spad api in favor of a setup data api, the
driver could then just make the decision on how best to communicate it.

Anyway, just my thoughts.

Logan