Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1667249 > unrolled thread

[RFC PATCH 00/13] Switchtec NTB Support

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:40 +0200
    [RFC PATCH 09/13] switchtec_ntb: add link management Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:40 +0200
    [RFC PATCH 04/13] switchtec: add link event notifier block Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:40 +0200
      Re: [RFC PATCH 04/13] switchtec: add link event notifier block Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-17 07:20 +0200
        Re: [RFC PATCH 04/13] switchtec: add link event notifier block Logan Gunthorpe <logang@deltatee.com> - 2017-06-17 18:30 +0200
    [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:50 +0200
      Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory  windows Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-17 07:20 +0200
        Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory  windows Logan Gunthorpe <logang@deltatee.com> - 2017-06-17 18:40 +0200
          Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory  windows Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-18 02:40 +0200
    [RFC PATCH 13/13] switchtec_ntb: update switchtec documentation with notes for ntb Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:50 +0200
    [RFC PATCH 02/13] switchtec: export class symbol for use in upper layer driver Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:50 +0200
      Re: [RFC PATCH 02/13] switchtec: export class symbol for use in  upper layer driver Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-17 07:20 +0200
        Re: [RFC PATCH 02/13] switchtec: export class symbol for use in upper  layer driver Logan Gunthorpe <logang@deltatee.com> - 2017-06-17 18:20 +0200
    [RFC PATCH 08/13] switchtec_ntb: add skeleton ntb driver Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:50 +0200
    [RFC PATCH 05/13] switchtec_ntb: introduce initial ntb driver Logan Gunthorpe <logang@deltatee.com> - 2017-06-15 22:50 +0200
    RE: [RFC PATCH 00/13] Switchtec NTB Support "Allen Hubbe" <Allen.Hubbe@dell.com> - 2017-06-16 16:00 +0200
      Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-16 16:20 +0200
        RE: [RFC PATCH 00/13] Switchtec NTB Support "Allen Hubbe" <Allen.Hubbe@dell.com> - 2017-06-16 17:40 +0200
          Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-16 18:50 +0200
            Re: [RFC PATCH 00/13] Switchtec NTB Support Serge Semin <fancer.lancer@gmail.com> - 2017-06-16 19:40 +0200
            RE: [RFC PATCH 00/13] Switchtec NTB Support "Allen Hubbe" <Allen.Hubbe@dell.com> - 2017-06-16 20:10 +0200
              Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-16 21:20 +0200
        Re: [RFC PATCH 00/13] Switchtec NTB Support Serge Semin <fancer.lancer@gmail.com> - 2017-06-16 18:40 +0200
          Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-16 19:10 +0200
            Re: [RFC PATCH 00/13] Switchtec NTB Support Serge Semin <fancer.lancer@gmail.com> - 2017-06-16 20:40 +0200
              Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-16 21:40 +0200
                Re: [RFC PATCH 00/13] Switchtec NTB Support Serge Semin <fancer.lancer@gmail.com> - 2017-06-16 22:30 +0200
                  Re: [RFC PATCH 00/13] Switchtec NTB Support 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> - 2017-06-17 07:20 +0200
                    Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-17 18:20 +0200
                    Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-17 18:20 +0200
                    Re: [RFC PATCH 00/13] Switchtec NTB Support Jon Mason <jdmason@kudzu.us> - 2017-06-19 21:20 +0200
    Re: [RFC PATCH 00/13] Switchtec NTB Support Jon Mason <jdmason@kudzu.us> - 2017-06-19 22:10 +0200
      Re: [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-06-19 22:30 +0200
        Re: [RFC PATCH 00/13] Switchtec NTB Support Jon Mason <jdmason@kudzu.us> - 2017-06-19 23:10 +0200

Page 1 of 2  [1] 2  Next page →

#1667249 — [RFC PATCH 00/13] Switchtec NTB Support

Hi,

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 v4.12-rc5 and can be found in this
git repo:

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

Thanks,

Logan


Logan Gunthorpe (13):
  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 block
  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 | 1144 +++++++++++++++++++++++++++++++++++
 drivers/pci/switch/switchtec.c      |  319 ++--------
 include/linux/ntb.h                 |    3 +
 include/linux/switchtec.h           |  365 +++++++++++
 10 files changed, 1601 insertions(+), 256 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

[toc] | [next] | [standalone]

#1667251 — [RFC PATCH 09/13] 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 <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 drivers/ntb/hw/mscc/switchtec_ntb.c | 160 +++++++++++++++++++++++++++++++++++-
 1 file changed, 159 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index dadbf3c38d0e..19fa8fb0f15c 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -57,6 +57,7 @@ static inline void _iowrite64(u64 val, void __iomem *mmio)
 
 struct shared_mw {
 	u32 magic;
+	u32 link_sta;
 	u32 partition_id;
 };
 
@@ -92,8 +93,18 @@ struct switchtec_ntb {
 	int nr_direct_mw;
 	int nr_lut_mw;
 	int direct_mw_to_bar[MAX_DIRECT_MW];
+
+	bool link_is_up;
+	enum ntb_speed link_speed;
+	enum ntb_width link_width;
+	struct notifier_block link_notifier;
 };
 
+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)
@@ -147,6 +158,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)
 {
 	return 0;
@@ -167,22 +189,148 @@ static int switchtec_ntb_mw_set_trans(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_event(struct switchtec_ntb *sndev, int msg)
+{
+	switchtec_ntb_check_link(sndev);
+}
+
+static int switchtec_ntb_link_notification(struct notifier_block *nb,
+					   unsigned long event, void *data)
+{
+	struct switchtec_ntb *sndev = container_of(nb, struct switchtec_ntb,
+						   link_notifier);
+
+	dev_dbg(&sndev->stdev->dev, "%s", __func__);
+	switchtec_ntb_check_link(sndev);
+
+	return NOTIFY_OK;
+}
+
+static int switchtec_ntb_init_link_notifier(struct switchtec_ntb *sndev)
+{
+	sndev->link_notifier.notifier_call = switchtec_ntb_link_notification;
+
+	return blocking_notifier_chain_register(&sndev->stdev->link_notifier,
+						&sndev->link_notifier);
+}
+
+static void switchtec_ntb_deinit_link_notifier(struct switchtec_ntb *sndev)
+{
+	blocking_notifier_chain_unregister(&sndev->stdev->link_notifier,
+					   &sndev->link_notifier);
+}
+
 static int 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;
 }
 
@@ -499,6 +647,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_link_event(sndev, msg);
 		}
 	}
 
@@ -596,6 +747,10 @@ static int switchtec_ntb_add(struct device *dev,
 	if (rc)
 		goto deinit_shared_and_exit;
 
+	rc = switchtec_ntb_init_link_notifier(sndev);
+	if (rc)
+		goto denit_db_msg_and_exit;
+
 	rc = ntb_register_device(&sndev->ntb);
 	if (rc)
 		goto deinit_and_exit;
@@ -606,6 +761,8 @@ static int switchtec_ntb_add(struct device *dev,
 	return 0;
 
 deinit_and_exit:
+	switchtec_ntb_deinit_link_notifier(sndev);
+denit_db_msg_and_exit:
 	switchtec_ntb_deinit_db_msg_irq(sndev);
 deinit_shared_and_exit:
 	switchtec_ntb_deinit_shared_mw(sndev);
@@ -626,6 +783,7 @@ void switchtec_ntb_remove(struct device *dev,
 
 	stdev->sndev = NULL;
 	ntb_unregister_device(&sndev->ntb);
+	switchtec_ntb_deinit_link_notifier(sndev);
 	switchtec_ntb_deinit_db_msg_irq(sndev);
 	switchtec_ntb_deinit_shared_mw(sndev);
 	kfree(sndev);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1667252 — [RFC PATCH 04/13] switchtec: add link event notifier block

In order for the switchtec NTB code to handle link change events we
create a notifier block 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 <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 drivers/pci/switch/switchtec.c | 53 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/switchtec.h      |  5 ++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index e9bf17b1934e..63e305b24fb9 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -972,6 +972,50 @@ 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);
+
+	dev_dbg(&stdev->dev, "%s\n", __func__);
+
+	blocking_notifier_call_chain(&stdev->link_notifier, 0, 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, "%s: %d->%08x\n", __func__, 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);
@@ -1024,6 +1068,8 @@ 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);
+	BLOCKING_INIT_NOTIFIER_HEAD(&stdev->link_notifier);
 	init_waitqueue_head(&stdev->event_wq);
 	atomic_set(&stdev->event_cnt, 0);
 
@@ -1067,6 +1113,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);
@@ -1086,6 +1135,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 {
@@ -1110,6 +1160,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);
 
@@ -1236,6 +1288,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 3c6b964885e9..8d66a0659cef 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -18,6 +18,7 @@
 
 #include <linux/pci.h>
 #include <linux/cdev.h>
+#include <linux/notifier.h>
 
 #define MICROSEMI_VENDOR_ID         0x11f8
 #define MICROSEMI_NTB_CLASSCODE     0x068000
@@ -344,6 +345,10 @@ struct switchtec_dev {
 
 	wait_queue_head_t event_wq;
 	atomic_t event_cnt;
+
+	struct work_struct link_event_work;
+	struct blocking_notifier_head link_notifier;
+	u8 link_event_count[SWITCHTEC_MAX_PFF_CSR];
 };
 
 static inline struct switchtec_dev *to_stdev(struct device *dev)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1668177 — Re: [RFC PATCH 04/13] switchtec: add link event notifier block

On Thu, Jun 15, 2017 at 02:37:20PM -0600, Logan Gunthorpe wrote:
> In order for the switchtec NTB code to handle link change events we
> create a notifier block 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 <logang@deltatee.com>
> Reviewed-by: Stephen Bates <sbates@raithlin.com>
> Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> ---
>  drivers/pci/switch/switchtec.c | 53 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/switchtec.h      |  5 ++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index e9bf17b1934e..63e305b24fb9 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -972,6 +972,50 @@ 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);
> +
> +	dev_dbg(&stdev->dev, "%s\n", __func__);

You do know about ftrace, right?  It's good to drop debugging code like
this for "final" versions.

> +
> +	blocking_notifier_call_chain(&stdev->link_notifier, 0, stdev);
> +}

Do you really need a notifier call chain?  How many different things are
going to "hook up" to this?  I ask as they tend to get really messy over
time while direct callbacks are easier to handle and manage.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1668362 — Re: [RFC PATCH 04/13] switchtec: add link event notifier block

On 16/06/17 11:14 PM, Greg Kroah-Hartman wrote:
> You do know about ftrace, right?  It's good to drop debugging code like
> this for "final" versions.

I've never actually used it but maybe I should give it a try. I'll
remove these debug lines.

>> +
>> +	blocking_notifier_call_chain(&stdev->link_notifier, 0, stdev);
>> +}
> 
> Do you really need a notifier call chain?  How many different things are
> going to "hook up" to this?  I ask as they tend to get really messy over
> time while direct callbacks are easier to handle and manage.

Ok, understood. I only expect the one callback at this time so I'll
change it to a single function pointer.

Logan

[toc] | [prev] | [next] | [standalone]

#1667260 — [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows

This commit adds 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.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 drivers/ntb/hw/mscc/switchtec_ntb.c | 296 ++++++++++++++++++++++++++++++++++++
 1 file changed, 296 insertions(+)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 1f094216aa1c..756307d1a8a3 100644
--- a/drivers/ntb/hw/mscc/switchtec_ntb.c
+++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
@@ -15,37 +15,332 @@
 
 #include <linux/switchtec.h>
 #include <linux/module.h>
+#include <linux/delay.h>
 
 MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
 MODULE_VERSION("0.1");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Microsemi Corporation");
 
+#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
+
+struct shared_mw {
+	u32 magic;
+	u32 partition_id;
+};
+
+#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];
 };
 
+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++) {
+		mdelay(50);
+		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 void switchtec_ntb_init_mw(struct switchtec_ntb *sndev)
+{
+	int i;
+
+	sndev->nr_direct_mw = 0;
+	for (i = 0; i < ARRAY_SIZE(sndev->mmio_self_ctrl->bar_entry); i++) {
+		u32 r = ioread32(&sndev->mmio_self_ctrl->bar_entry[i].ctl);
+
+		if (r & NTB_CTRL_BAR_VALID)
+			sndev->direct_mw_to_bar[sndev->nr_direct_mw++] = i;
+	}
+
+	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);
+}
+
+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 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;
+	}
+
+	memset(sndev->self_shared, 0, LUT_SIZE);
+	sndev->self_shared->magic = SWITCHTEC_NTB_MAGIC;
+	sndev->self_shared->partition_id = sndev->stdev->partition;
+
+	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 +353,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

[toc] | [prev] | [next] | [standalone]

#1668174 — Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows

On Thu, Jun 15, 2017 at 02:37:22PM -0600, Logan Gunthorpe wrote:
> This commit adds 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.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Stephen Bates <sbates@raithlin.com>
> Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> ---
>  drivers/ntb/hw/mscc/switchtec_ntb.c | 296 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 296 insertions(+)
> 
> diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
> index 1f094216aa1c..756307d1a8a3 100644
> --- a/drivers/ntb/hw/mscc/switchtec_ntb.c
> +++ b/drivers/ntb/hw/mscc/switchtec_ntb.c
> @@ -15,37 +15,332 @@
>  
>  #include <linux/switchtec.h>
>  #include <linux/module.h>
> +#include <linux/delay.h>
>  
>  MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
>  MODULE_VERSION("0.1");
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Microsemi Corporation");
>  
> +#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

Really?  Don't we have ioread64 in generic code for all arches?  If not,
that should be fixed, don't hide this in a random driver please.  Or
just restrict your driver to only building on those arches that does
provide this api.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1668366 — Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows

On 16/06/17 11:16 PM, Greg Kroah-Hartman wrote:
>> +#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
> 
> Really?  Don't we have ioread64 in generic code for all arches?  If not,
> that should be fixed, don't hide this in a random driver please.  Or
> just restrict your driver to only building on those arches that does
> provide this api.

Yes, I know these are _very_ ugly. Unfortunately, the other ntb drivers
each have the exact same thing. ioread64 is not provided universally at
this time and I did spend a bit of time digging and things are a bit
messy so I wasn't at all sure of the correct solution.

For starters, ioread64 is only defined on 64 bit machines. They are
surrounded by ifdef CONFIG_64BIT and it's not clear to me if the above
wrapper (around two ioread32s) would be acceptable universally.

Second, the x86_64 version doesn't even compile. This is because the
arch doesn't provide any ioread64 implementation anywhere and the macro
in io.h isn't used because CONFIG_GENERIC_IOMAP is defined.

The only arch where I _think_ ioread64 would work is powerpc or any that
define CONFIG_64BIT and not CONFIG_GENERIC_IOMAP.

If you have any guidance on this I'd be happy to try and make a patch or
two for it.

Thanks,

Logan

[toc] | [prev] | [next] | [standalone]

#1668496 — Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows

On Sat, Jun 17, 2017 at 10:39:35AM -0600, Logan Gunthorpe wrote:
> 
> 
> On 16/06/17 11:16 PM, Greg Kroah-Hartman wrote:
> >> +#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
> > 
> > Really?  Don't we have ioread64 in generic code for all arches?  If not,
> > that should be fixed, don't hide this in a random driver please.  Or
> > just restrict your driver to only building on those arches that does
> > provide this api.
> 
> Yes, I know these are _very_ ugly. Unfortunately, the other ntb drivers
> each have the exact same thing. ioread64 is not provided universally at
> this time and I did spend a bit of time digging and things are a bit
> messy so I wasn't at all sure of the correct solution.

That implies that this needs to be fixed, no driver should ever have to
define this themselves.  The fact that they all do it at the same time
is a huge hint that it's wrong.

> For starters, ioread64 is only defined on 64 bit machines. They are
> surrounded by ifdef CONFIG_64BIT and it's not clear to me if the above
> wrapper (around two ioread32s) would be acceptable universally.
> 
> Second, the x86_64 version doesn't even compile. This is because the
> arch doesn't provide any ioread64 implementation anywhere and the macro
> in io.h isn't used because CONFIG_GENERIC_IOMAP is defined.
> 
> The only arch where I _think_ ioread64 would work is powerpc or any that
> define CONFIG_64BIT and not CONFIG_GENERIC_IOMAP.
> 
> If you have any guidance on this I'd be happy to try and make a patch or
> two for it.

I suggest coming up with something simple like what you have here,
adding it to the arch-generic code and posting it to the linux-arch
mailing list and seeing if anyone screams :)

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1667262 — [RFC PATCH 13/13] 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 <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 Documentation/switchtec.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/switchtec.txt b/Documentation/switchtec.txt
index a0a9c7b3d4d5..978a346e1fb3 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 switchec_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

[toc] | [prev] | [next] | [standalone]

#1667264 — [RFC PATCH 02/13] switchtec: export class symbol for use in upper layer driver

We switch to class_register/unregister and a declared class which
is exported for use in the switchtec_ntb driver.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 drivers/pci/switch/switchtec.c | 21 +++++++++++----------
 include/linux/switchtec.h      |  2 ++
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index c4369ba7bbc1..e9bf17b1934e 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -21,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");
@@ -35,9 +33,14 @@ 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 = {
+	.owner = THIS_MODULE,
+	.name = "switchtec",
+};
+EXPORT_SYMBOL(switchtec_class);
+
 enum mrpc_state {
 	MRPC_IDLE = 0,
 	MRPC_QUEUED,
@@ -1026,7 +1029,7 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)
 
 	dev = &stdev->dev;
 	device_initialize(dev);
-	dev->class = switchtec_class;
+	dev->class = &switchtec_class;
 	dev->parent = &pdev->dev;
 	dev->groups = switchtec_device_groups;
 	dev->release = stdev_release;
@@ -1313,11 +1316,9 @@ static int __init switchtec_init(void)
 	if (rc)
 		return rc;
 
-	switchtec_class = class_create(THIS_MODULE, "switchtec");
-	if (IS_ERR(switchtec_class)) {
-		rc = PTR_ERR(switchtec_class);
+	rc = class_register(&switchtec_class);
+	if (rc)
 		goto err_create_class;
-	}
 
 	rc = pci_register_driver(&switchtec_pci_driver);
 	if (rc)
@@ -1328,7 +1329,7 @@ static int __init switchtec_init(void)
 	return 0;
 
 err_pci_register:
-	class_destroy(switchtec_class);
+	class_unregister(&switchtec_class);
 
 err_create_class:
 	unregister_chrdev_region(switchtec_devt, max_devices);
@@ -1340,7 +1341,7 @@ module_init(switchtec_init);
 static void __exit switchtec_exit(void)
 {
 	pci_unregister_driver(&switchtec_pci_driver);
-	class_destroy(switchtec_class);
+	class_unregister(&switchtec_class);
 	unregister_chrdev_region(switchtec_devt, max_devices);
 	ida_destroy(&switchtec_minor_ida);
 
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 508cda78a430..3b87618fc42f 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -267,4 +267,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

[toc] | [prev] | [next] | [standalone]

#1668175 — Re: [RFC PATCH 02/13] switchtec: export class symbol for use in upper layer driver

On Thu, Jun 15, 2017 at 02:37:18PM -0600, Logan Gunthorpe wrote:
> We switch to class_register/unregister and a declared class which
> is exported for use in the switchtec_ntb driver.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Stephen Bates <sbates@raithlin.com>
> Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> ---
>  drivers/pci/switch/switchtec.c | 21 +++++++++++----------
>  include/linux/switchtec.h      |  2 ++
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index c4369ba7bbc1..e9bf17b1934e 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -21,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");
> @@ -35,9 +33,14 @@ 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 = {
> +	.owner = THIS_MODULE,
> +	.name = "switchtec",
> +};
> +EXPORT_SYMBOL(switchtec_class);

EXPORT_SYMBOL_GPL()?

And do you really have to move from a dynamic class to a static one?  I
know it will work just the same, but I hate seeing static structures
that have reference counts on them :)

thanks,

greg k-h

> +
>  enum mrpc_state {
>  	MRPC_IDLE = 0,
>  	MRPC_QUEUED,
> @@ -1026,7 +1029,7 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)
>  
>  	dev = &stdev->dev;
>  	device_initialize(dev);
> -	dev->class = switchtec_class;
> +	dev->class = &switchtec_class;
>  	dev->parent = &pdev->dev;
>  	dev->groups = switchtec_device_groups;
>  	dev->release = stdev_release;
> @@ -1313,11 +1316,9 @@ static int __init switchtec_init(void)
>  	if (rc)
>  		return rc;
>  
> -	switchtec_class = class_create(THIS_MODULE, "switchtec");
> -	if (IS_ERR(switchtec_class)) {
> -		rc = PTR_ERR(switchtec_class);
> +	rc = class_register(&switchtec_class);
> +	if (rc)
>  		goto err_create_class;
> -	}
>  
>  	rc = pci_register_driver(&switchtec_pci_driver);
>  	if (rc)
> @@ -1328,7 +1329,7 @@ static int __init switchtec_init(void)
>  	return 0;
>  
>  err_pci_register:
> -	class_destroy(switchtec_class);
> +	class_unregister(&switchtec_class);
>  
>  err_create_class:
>  	unregister_chrdev_region(switchtec_devt, max_devices);
> @@ -1340,7 +1341,7 @@ module_init(switchtec_init);
>  static void __exit switchtec_exit(void)
>  {
>  	pci_unregister_driver(&switchtec_pci_driver);
> -	class_destroy(switchtec_class);
> +	class_unregister(&switchtec_class);
>  	unregister_chrdev_region(switchtec_devt, max_devices);
>  	ida_destroy(&switchtec_minor_ida);
>  
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index 508cda78a430..3b87618fc42f 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -267,4 +267,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

[toc] | [prev] | [next] | [standalone]

#1668361 — Re: [RFC PATCH 02/13] switchtec: export class symbol for use in upper layer driver

On 16/06/17 11:11 PM, Greg Kroah-Hartman wrote:
> EXPORT_SYMBOL_GPL()?
> 
> And do you really have to move from a dynamic class to a static one?  I
> know it will work just the same, but I hate seeing static structures
> that have reference counts on them :)

I'll change both for v1. I didn't really need a static one. I just saw
an example of some other user of class_interface which and it seemed
appropriate.

Logan

[toc] | [prev] | [next] | [standalone]

#1667265 — [RFC PATCH 08/13] switchtec_ntb: add skeleton ntb driver

This patch simply adds a skeleton NTB driver which will be filled
out in subsequent patches.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 drivers/ntb/hw/mscc/switchtec_ntb.c | 134 +++++++++++++++++++++++++++++++++++-
 include/linux/ntb.h                 |   3 +
 2 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/hw/mscc/switchtec_ntb.c b/drivers/ntb/hw/mscc/switchtec_ntb.c
index 6c9b4e337cdf..dadbf3c38d0e 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/interrupt.h>
+#include <linux/ntb.h>
 
 MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
 MODULE_VERSION("0.1");
@@ -63,6 +64,7 @@ struct shared_mw {
 #define LUT_SIZE SZ_64K
 
 struct switchtec_ntb {
+	struct ntb_dev ntb;
 	struct switchtec_dev *stdev;
 
 	int self_partition;
@@ -145,10 +147,134 @@ static int switchtec_ntb_part_op(struct switchtec_ntb *sndev,
 	return -EIO;
 }
 
+static int switchtec_ntb_mw_count(struct ntb_dev *ntb)
+{
+	return 0;
+}
+
+static int switchtec_ntb_mw_get_range(struct ntb_dev *ntb, int idx,
+				      phys_addr_t *base,
+				      resource_size_t *size,
+				      resource_size_t *align,
+				      resource_size_t *align_size)
+{
+	return 0;
+}
+
+static int switchtec_ntb_mw_set_trans(struct ntb_dev *ntb, int idx,
+				      dma_addr_t addr, resource_size_t size)
+{
+	return 0;
+}
+
+static int 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 idx, u32 val)
+{
+	return 0;
+}
+
+static const struct ntb_dev_ops switchtec_ntb_ops = {
+	.mw_count		= switchtec_ntb_mw_count,
+	.mw_get_range		= switchtec_ntb_mw_get_range,
+	.mw_set_trans		= switchtec_ntb_mw_set_trans,
+	.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;
@@ -453,7 +579,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);
@@ -471,11 +596,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:
@@ -494,6 +625,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 de87ceac110e..ae351f0bd4ae 100644
--- a/include/linux/ntb.h
+++ b/include/linux/ntb.h
@@ -68,6 +68,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,
@@ -75,6 +76,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)
@@ -95,6 +97,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

[toc] | [prev] | [next] | [standalone]

#1667268 — [RFC PATCH 05/13] 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 <logang@deltatee.com>
Reviewed-by: Stephen Bates <sbates@raithlin.com>
Reviewed-by: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
---
 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 +++++++++++++++++++++++++++++++++++++
 include/linux/switchtec.h           |  4 ++
 7 files changed, 98 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 6cee5e253ec3..fbd88be1be03 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9842,6 +9842,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 <thierry.reding@gmail.com>
diff --git a/drivers/ntb/hw/Kconfig b/drivers/ntb/hw/Kconfig
index 7116472b4625..1d1360db87e1 100644
--- a/drivers/ntb/hw/Kconfig
+++ b/drivers/ntb/hw/Kconfig
@@ -1,2 +1,3 @@
 source "drivers/ntb/hw/amd/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 532e0859b4a1..690c00274174 100644
--- a/drivers/ntb/hw/Makefile
+++ b/drivers/ntb/hw/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_NTB_AMD)	+= amd/
 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..1f094216aa1c
--- /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  = {
+	.class = &switchtec_class,
+	.add_dev = switchtec_ntb_add,
+	.remove_dev = switchtec_ntb_remove,
+};
+
+static int __init switchtec_ntb_init(void)
+{
+	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/include/linux/switchtec.h b/include/linux/switchtec.h
index 8d66a0659cef..0a7d0ed77507 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -312,6 +312,8 @@ struct pff_csr_regs {
 	u32 reserved4[174];
 } __packed;
 
+struct switchtec_ntb;
+
 struct switchtec_dev {
 	struct pci_dev *pdev;
 	struct device dev;
@@ -349,6 +351,8 @@ struct switchtec_dev {
 	struct work_struct link_event_work;
 	struct blocking_notifier_head link_notifier;
 	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

[toc] | [prev] | [next] | [standalone]

#1667762

From: Logan Gunthorpe
> Hi,
> 
> 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.

See what is staged in https://github.com/jonmason/ntb.git ntb-next, with the addition of multi-peer support by Serge.  It would be good at this stage to understand whether the api changes there would also support the Switchtec driver, and what if anything must change, or be planned to change, to support the Switchtec driver.

Thanks for providing the patch set for the Switchtec driver.  My first impression is that it is a good patch set.  Only to be included, it needs to be reconciled with the api changes in ntb-next.  I will follow up with a more detailed review of patches in this series, but sending this now as I don't want to delay your review of ntb-next.

> 
> The driver has been tested with ntb_netdev and fully passes the
> ntb_test script.
> 
> This patchset is based off of v4.12-rc5 and can be found in this
> git repo:
> 
> https://github.com/sbates130272/linux-p2pmem.git switchtec_ntb
> 
> Thanks,
> 
> Logan
> 
> 
> Logan Gunthorpe (13):
>   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 block
>   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 | 1144 +++++++++++++++++++++++++++++++++++
>  drivers/pci/switch/switchtec.c      |  319 ++--------
>  include/linux/ntb.h                 |    3 +
>  include/linux/switchtec.h           |  365 +++++++++++
>  10 files changed, 1601 insertions(+), 256 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

[toc] | [prev] | [next] | [standalone]

#1667784

On 16/06/17 07:53 AM, Allen Hubbe wrote:
> See what is staged in https://github.com/jonmason/ntb.git ntb-next, with the addition of multi-peer support by Serge.  It would be good at this stage to understand whether the api changes there would also support the Switchtec driver, and what if anything must change, or be planned to change, to support the Switchtec driver.

Ah, yes I had seen that patchset some time ago but I wasn't aware of
it's status or that it was queued up in ntb-next. I think it will be no
problem to reconcile with the switchtec driver and I'll rebase onto
ntb-next for the next posting of the patch set. However, I *may* save
full multi-host switchtec support for a follow up submission. My initial
impression is the new API will support the switchtec hardware well.

Thanks,

Logan

[toc] | [prev] | [next] | [standalone]

#1667832

From: Logan Gunthorpe
> On 16/06/17 07:53 AM, Allen Hubbe wrote:
> > See what is staged in https://github.com/jonmason/ntb.git ntb-next, with the addition of multi-peer
> support by Serge.  It would be good at this stage to understand whether the api changes there would
> also support the Switchtec driver, and what if anything must change, or be planned to change, to
> support the Switchtec driver.
> 
> Ah, yes I had seen that patchset some time ago but I wasn't aware of
> it's status or that it was queued up in ntb-next. I think it will be no
> problem to reconcile with the switchtec driver and I'll rebase onto
> ntb-next for the next posting of the patch set. However, I *may* save
> full multi-host switchtec support for a follow up submission. My initial
> impression is the new API will support the switchtec hardware well.

Alright!

In code review, I really only have found minor nits.  Overall, the driver looks good.

In switchtec_ntb_part_op, there is a delay of up to 50s (1000 * 50ms).  This looks like a thread context, so it could involve the scheduler for the delay instead of spinning for up to 50s before bailing.

There are a few instances like this:
> +	dev_dbg(&stdev->dev, "%s\n", __func__);

Where the printing of __func__ could be controlled by dyndbg=+pf.  The debug message could be more useful.

In switchtec_ntb_db_set_mask and friends, an in-memory copy of the mask bits is protected by a spinlock.  Elsewhere, you noted that the db bits are shared between all ports, so the db bitset is chopped up to be shared between the ports.  Is the db mask also shared, and how is the spinlock sufficient for synchronizing access to the mask bits between multiple ports?

The IDT switch also does not have hardware scratchpads.  Could the code you wrote for emulated scratchpads be made into shared library code for ntb drivers?  Also, some ntb clients may not need scratchpad support.  If it is not natively supported by a driver, can the emulated scratchpad support be an optional feature?

> 
> Thanks,
> 
> Logan

[toc] | [prev] | [next] | [standalone]

#1667893

On 16/06/17 09:34 AM, Allen Hubbe wrote:
> In code review, I really only have found minor nits.  Overall, the driver looks good.

Great, thanks for such a quick review!

> In switchtec_ntb_part_op, there is a delay of up to 50s (1000 * 50ms).  This looks like a thread context, so it could involve the scheduler for the delay instead of spinning for up to 50s before bailing.

Good point. If I were to change this to msleep_interruptible would that
be acceptable?

> There are a few instances like this:
>> +	dev_dbg(&stdev->dev, "%s\n", __func__);

> Where the printing of __func__ could be controlled by dyndbg=+pf.  The debug message could be more useful.

Ok, I'll change that.

> In switchtec_ntb_db_set_mask and friends, an in-memory copy of the mask bits is protected by a spinlock.  Elsewhere, you noted that the db bits are shared between all ports, so the db bitset is chopped up to be shared between the ports.  Is the db mask also shared, and how is the spinlock sufficient for synchronizing access to the mask bits between multiple ports?

Well, there are 64 doorbells that are shared between ports but each port
has it's own in and out registers for the doorbells. So triggering
doorbell one on one port's ODB actually triggers it on every ports IDB.
So these are shared only in the sense that each port needs to know which
dbs it cares about. Seeing each port has their own registers they don't
have to worry about synchronization.

The mask is only protected by a spin lock seeing multiple callers of
db_set_mask and db_clr_mask on the same port may step on each others
toes. So if two processes try to mask different bits they both must get
masked in the end and therefore some kind of synchronization must be
involved.

> The IDT switch also does not have hardware scratchpads.  Could the code you wrote for emulated scratchpads be made into shared library code for ntb drivers?  Also, some ntb clients may not need scratchpad support.  If it is not natively supported by a driver, can the emulated scratchpad support be an optional feature?

Hmm, interesting idea. A few pieces could possibly be made common but it
depends mostly on hardware having the resources to make use of it.
Switchtec has extra LUT memory windows that made this possible. Unless
you object I'm inclined to leave it as is and I'd be happy to work with
the IDT folks to create a common solution in the future.

Logan

[toc] | [prev] | [next] | [standalone]

#1667916

On Fri, Jun 16, 2017 at 10:47:21AM -0600, Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> 
> On 16/06/17 09:34 AM, Allen Hubbe wrote:
> > In code review, I really only have found minor nits.  Overall, the driver looks good.
> 
> Great, thanks for such a quick review!
> 
> > In switchtec_ntb_part_op, there is a delay of up to 50s (1000 * 50ms).  This looks like a thread context, so it could involve the scheduler for the delay instead of spinning for up to 50s before bailing.
> 
> Good point. If I were to change this to msleep_interruptible would that
> be acceptable?
> 
> > There are a few instances like this:
> >> +	dev_dbg(&stdev->dev, "%s\n", __func__);
> 
> > Where the printing of __func__ could be controlled by dyndbg=+pf.  The debug message could be more useful.
> 
> Ok, I'll change that.
> 
> > In switchtec_ntb_db_set_mask and friends, an in-memory copy of the mask bits is protected by a spinlock.  Elsewhere, you noted that the db bits are shared between all ports, so the db bitset is chopped up to be shared between the ports.  Is the db mask also shared, and how is the spinlock sufficient for synchronizing access to the mask bits between multiple ports?
> 
> Well, there are 64 doorbells that are shared between ports but each port
> has it's own in and out registers for the doorbells. So triggering
> doorbell one on one port's ODB actually triggers it on every ports IDB.
> So these are shared only in the sense that each port needs to know which
> dbs it cares about. Seeing each port has their own registers they don't
> have to worry about synchronization.
> 

It's exactly the way the IDT hardware works. There is Global doorbell
registers, directly connected to doorbell registers of each port, unless
doorbell routing isn't configured differently by global switch configuration.
Each port has got it's own in and out doorbell registers as well as the
doorbell mask preventing the corresponding doorbell bit from generating
interrupt.

> 
> The mask is only protected by a spin lock seeing multiple callers of
> db_set_mask and db_clr_mask on the same port may step on each others
> toes. So if two processes try to mask different bits they both must get
> masked in the end and therefore some kind of synchronization must be
> involved.
> 
> > The IDT switch also does not have hardware scratchpads.  Could the code you wrote for emulated scratchpads be made into shared library code for ntb drivers?  Also, some ntb clients may not need scratchpad support.  If it is not natively supported by a driver, can the emulated scratchpad support be an optional feature?
> 
> Hmm, interesting idea. A few pieces could possibly be made common but it
> depends mostly on hardware having the resources to make use of it.
> Switchtec has extra LUT memory windows that made this possible. Unless
> you object I'm inclined to leave it as is and I'd be happy to work with
> the IDT folks to create a common solution in the future.
> 
> Logan

Alas there are no IDT folks here. It's only me for now, who was responsible
for NTB API alteration (together with much of help from others NTB guys) and
IDT NTB driver development fitting that API.

Regards,
-Sergey

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web