RE: [PATCH v2 2/9] NTB: Add indexed ports NTB API
From: Serge Semin > There are some NTB hardware, which can combine more than just two domains > over NTB. For instance, some IDT PCIe-switches can have NTB-functions > activated on more than two-ports. The different domains are distinguished > by ports they are connected to. So the new port-related methods are added to > the NTB API: > ntb_port_number() - return local port > ntb_peer_port_count() - return number of peers local port can connect to > ntb_peer_port_number(pdix) - return port number by it index > ntb_peer_port_idx(port) - return port index by it number > > Current test-drivers aren't changed much. They still support two-ports devices > for the time being while multi-ports hardware drivers aren't added. > The port methods are the same for PRI/SEC drivers. Rather than duplicating the code, multiport could be made optional in the api, and default implementations provided by ntb common code. Some comments below. > Signed-off-by: Serge Semin> > --- > drivers/ntb/hw/amd/ntb_hw_amd.c | 47 > drivers/ntb/hw/amd/ntb_hw_amd.h | 9 + > drivers/ntb/hw/intel/ntb_hw_intel.c | 52 ++- > drivers/ntb/hw/intel/ntb_hw_intel.h | 9 + > drivers/ntb/ntb_transport.c | 6 > drivers/ntb/test/ntb_perf.c | 4 +++ > drivers/ntb/test/ntb_pingpong.c | 6 > drivers/ntb/test/ntb_tool.c | 5 +++ > include/linux/ntb.h | 71 > + > 9 files changed, 208 insertions(+), 1 deletion(-) > > diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c > index 6704327..0b767ef 100644 > --- a/drivers/ntb/hw/amd/ntb_hw_amd.c > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c > @@ -71,6 +71,49 @@ MODULE_AUTHOR("AMD Inc."); > static const struct file_operations amd_ntb_debugfs_info; > static struct dentry *debugfs_dir; > > +static int amd_ntb_port_number(struct ntb_dev *ntb) > +{ > + switch (ntb->topo) { > + case NTB_TOPO_PRI: > + case NTB_TOPO_B2B_USD: > + return NTB_PORT_PRI_USD; > + case NTB_TOPO_SEC: > + case NTB_TOPO_B2B_DSD: > + return NTB_PORT_SEC_DSD; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int amd_ntb_peer_port_count(struct ntb_dev *ntb) > +{ > + return NTB_PEER_CNT; > +} > + > +static int amd_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) > +{ > + int local_port = amd_ntb_port_number(ntb); > + > + if (pidx > NTB_PIDX_MAX) > + return -EINVAL; pidx may be negative. > + > + return (local_port == NTB_PORT_PRI_USD ? > + NTB_PORT_SEC_DSD : NTB_PORT_PRI_USD); local_port may be -EINVAL. It may be simpler to have the same switch statement here as in amd_ntb_port_number(), with the return statements swapped. > +} > + > +static int amd_ntb_peer_port_idx(struct ntb_dev *ntb, int port) > +{ > + int local_port = amd_ntb_port_number(ntb); > + > + if ((local_port == NTB_PORT_PRI_USD && port != NTB_PORT_SEC_DSD) || > + (local_port == NTB_PORT_SEC_DSD && port != NTB_PORT_PRI_USD)) > + return -EINVAL; > + How about: peer_port = amd_ntb_peer_port_number(ntb, 0); if (peer_port == -EINVAL || port != peer_port) return -EINVAL; return 0; > + return 0; > +} > + > static int amd_link_is_up(struct amd_ntb_dev *ndev) > { > if (!ndev->peer_sta) > @@ -431,6 +474,10 @@ static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, > } > > static const struct ntb_dev_ops amd_ntb_ops = { > + .port_number= amd_ntb_port_number, > + .peer_port_count= amd_ntb_peer_port_count, > + .peer_port_number = amd_ntb_peer_port_number, > + .peer_port_idx = amd_ntb_peer_port_idx, > .link_is_up = amd_ntb_link_is_up, > .link_enable= amd_ntb_link_enable, > .link_disable = amd_ntb_link_disable, > diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h > index 2eac3cd..1aeb08f 100644 > --- a/drivers/ntb/hw/amd/ntb_hw_amd.h > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h > @@ -62,6 +62,10 @@ > #define NTB_LNK_STA_SPEED(x) (((x) & NTB_LNK_STA_SPEED_MASK) >> 16) > #define NTB_LNK_STA_WIDTH(x) (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20) > > +/* port related constants */ > +#define NTB_PEER_CNT (1) > +#define NTB_PIDX_MAX (0) Just NTB_PEER_CNT is sufficient. Anything that checks if (pidx > NTB_PIDX_MAX) could check if (pidx >= NTB_PEER_CNT). > + > #ifndef read64 > #ifdef readq > #define read64 readq > @@ -91,6 +95,11 @@ static inline void _write64(u64 val, void __iomem *mmio) > #endif > #endif > > +enum amd_ntb_port { > + NTB_PORT_PRI_USD, > + NTB_PORT_SEC_DSD > +}; This could be part of ntb.h, since it will likely be the same for any of the PRI/SEC variety of NTB devices. Making it a part of the api will
RE: [PATCH v2 2/9] NTB: Add indexed ports NTB API
From: Serge Semin > There are some NTB hardware, which can combine more than just two domains > over NTB. For instance, some IDT PCIe-switches can have NTB-functions > activated on more than two-ports. The different domains are distinguished > by ports they are connected to. So the new port-related methods are added to > the NTB API: > ntb_port_number() - return local port > ntb_peer_port_count() - return number of peers local port can connect to > ntb_peer_port_number(pdix) - return port number by it index > ntb_peer_port_idx(port) - return port index by it number > > Current test-drivers aren't changed much. They still support two-ports devices > for the time being while multi-ports hardware drivers aren't added. > The port methods are the same for PRI/SEC drivers. Rather than duplicating the code, multiport could be made optional in the api, and default implementations provided by ntb common code. Some comments below. > Signed-off-by: Serge Semin > > --- > drivers/ntb/hw/amd/ntb_hw_amd.c | 47 > drivers/ntb/hw/amd/ntb_hw_amd.h | 9 + > drivers/ntb/hw/intel/ntb_hw_intel.c | 52 ++- > drivers/ntb/hw/intel/ntb_hw_intel.h | 9 + > drivers/ntb/ntb_transport.c | 6 > drivers/ntb/test/ntb_perf.c | 4 +++ > drivers/ntb/test/ntb_pingpong.c | 6 > drivers/ntb/test/ntb_tool.c | 5 +++ > include/linux/ntb.h | 71 > + > 9 files changed, 208 insertions(+), 1 deletion(-) > > diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c > index 6704327..0b767ef 100644 > --- a/drivers/ntb/hw/amd/ntb_hw_amd.c > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c > @@ -71,6 +71,49 @@ MODULE_AUTHOR("AMD Inc."); > static const struct file_operations amd_ntb_debugfs_info; > static struct dentry *debugfs_dir; > > +static int amd_ntb_port_number(struct ntb_dev *ntb) > +{ > + switch (ntb->topo) { > + case NTB_TOPO_PRI: > + case NTB_TOPO_B2B_USD: > + return NTB_PORT_PRI_USD; > + case NTB_TOPO_SEC: > + case NTB_TOPO_B2B_DSD: > + return NTB_PORT_SEC_DSD; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int amd_ntb_peer_port_count(struct ntb_dev *ntb) > +{ > + return NTB_PEER_CNT; > +} > + > +static int amd_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) > +{ > + int local_port = amd_ntb_port_number(ntb); > + > + if (pidx > NTB_PIDX_MAX) > + return -EINVAL; pidx may be negative. > + > + return (local_port == NTB_PORT_PRI_USD ? > + NTB_PORT_SEC_DSD : NTB_PORT_PRI_USD); local_port may be -EINVAL. It may be simpler to have the same switch statement here as in amd_ntb_port_number(), with the return statements swapped. > +} > + > +static int amd_ntb_peer_port_idx(struct ntb_dev *ntb, int port) > +{ > + int local_port = amd_ntb_port_number(ntb); > + > + if ((local_port == NTB_PORT_PRI_USD && port != NTB_PORT_SEC_DSD) || > + (local_port == NTB_PORT_SEC_DSD && port != NTB_PORT_PRI_USD)) > + return -EINVAL; > + How about: peer_port = amd_ntb_peer_port_number(ntb, 0); if (peer_port == -EINVAL || port != peer_port) return -EINVAL; return 0; > + return 0; > +} > + > static int amd_link_is_up(struct amd_ntb_dev *ndev) > { > if (!ndev->peer_sta) > @@ -431,6 +474,10 @@ static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, > } > > static const struct ntb_dev_ops amd_ntb_ops = { > + .port_number= amd_ntb_port_number, > + .peer_port_count= amd_ntb_peer_port_count, > + .peer_port_number = amd_ntb_peer_port_number, > + .peer_port_idx = amd_ntb_peer_port_idx, > .link_is_up = amd_ntb_link_is_up, > .link_enable= amd_ntb_link_enable, > .link_disable = amd_ntb_link_disable, > diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h > index 2eac3cd..1aeb08f 100644 > --- a/drivers/ntb/hw/amd/ntb_hw_amd.h > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h > @@ -62,6 +62,10 @@ > #define NTB_LNK_STA_SPEED(x) (((x) & NTB_LNK_STA_SPEED_MASK) >> 16) > #define NTB_LNK_STA_WIDTH(x) (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20) > > +/* port related constants */ > +#define NTB_PEER_CNT (1) > +#define NTB_PIDX_MAX (0) Just NTB_PEER_CNT is sufficient. Anything that checks if (pidx > NTB_PIDX_MAX) could check if (pidx >= NTB_PEER_CNT). > + > #ifndef read64 > #ifdef readq > #define read64 readq > @@ -91,6 +95,11 @@ static inline void _write64(u64 val, void __iomem *mmio) > #endif > #endif > > +enum amd_ntb_port { > + NTB_PORT_PRI_USD, > + NTB_PORT_SEC_DSD > +}; This could be part of ntb.h, since it will likely be the same for any of the PRI/SEC variety of NTB devices. Making it a part of the api will encourage other PRI/SEC drivers
[PATCH v2 2/9] NTB: Add indexed ports NTB API
There are some NTB hardware, which can combine more than just two domains over NTB. For instance, some IDT PCIe-switches can have NTB-functions activated on more than two-ports. The different domains are distinguished by ports they are connected to. So the new port-related methods are added to the NTB API: ntb_port_number() - return local port ntb_peer_port_count() - return number of peers local port can connect to ntb_peer_port_number(pdix) - return port number by it index ntb_peer_port_idx(port) - return port index by it number Current test-drivers aren't changed much. They still support two-ports devices for the time being while multi-ports hardware drivers aren't added. Signed-off-by: Serge Semin--- drivers/ntb/hw/amd/ntb_hw_amd.c | 47 drivers/ntb/hw/amd/ntb_hw_amd.h | 9 + drivers/ntb/hw/intel/ntb_hw_intel.c | 52 ++- drivers/ntb/hw/intel/ntb_hw_intel.h | 9 + drivers/ntb/ntb_transport.c | 6 drivers/ntb/test/ntb_perf.c | 4 +++ drivers/ntb/test/ntb_pingpong.c | 6 drivers/ntb/test/ntb_tool.c | 5 +++ include/linux/ntb.h | 71 + 9 files changed, 208 insertions(+), 1 deletion(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 6704327..0b767ef 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -71,6 +71,49 @@ MODULE_AUTHOR("AMD Inc."); static const struct file_operations amd_ntb_debugfs_info; static struct dentry *debugfs_dir; +static int amd_ntb_port_number(struct ntb_dev *ntb) +{ + switch (ntb->topo) { + case NTB_TOPO_PRI: + case NTB_TOPO_B2B_USD: + return NTB_PORT_PRI_USD; + case NTB_TOPO_SEC: + case NTB_TOPO_B2B_DSD: + return NTB_PORT_SEC_DSD; + default: + break; + } + + return -EINVAL; +} + +static int amd_ntb_peer_port_count(struct ntb_dev *ntb) +{ + return NTB_PEER_CNT; +} + +static int amd_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) +{ + int local_port = amd_ntb_port_number(ntb); + + if (pidx > NTB_PIDX_MAX) + return -EINVAL; + + return (local_port == NTB_PORT_PRI_USD ? + NTB_PORT_SEC_DSD : NTB_PORT_PRI_USD); +} + +static int amd_ntb_peer_port_idx(struct ntb_dev *ntb, int port) +{ + int local_port = amd_ntb_port_number(ntb); + + if ((local_port == NTB_PORT_PRI_USD && port != NTB_PORT_SEC_DSD) || + (local_port == NTB_PORT_SEC_DSD && port != NTB_PORT_PRI_USD)) + return -EINVAL; + + return 0; +} + static int amd_link_is_up(struct amd_ntb_dev *ndev) { if (!ndev->peer_sta) @@ -431,6 +474,10 @@ static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, } static const struct ntb_dev_ops amd_ntb_ops = { + .port_number= amd_ntb_port_number, + .peer_port_count= amd_ntb_peer_port_count, + .peer_port_number = amd_ntb_peer_port_number, + .peer_port_idx = amd_ntb_peer_port_idx, .link_is_up = amd_ntb_link_is_up, .link_enable= amd_ntb_link_enable, .link_disable = amd_ntb_link_disable, diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 2eac3cd..1aeb08f 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -62,6 +62,10 @@ #define NTB_LNK_STA_SPEED(x) (((x) & NTB_LNK_STA_SPEED_MASK) >> 16) #define NTB_LNK_STA_WIDTH(x) (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20) +/* port related constants */ +#define NTB_PEER_CNT (1) +#define NTB_PIDX_MAX (0) + #ifndef read64 #ifdef readq #define read64 readq @@ -91,6 +95,11 @@ static inline void _write64(u64 val, void __iomem *mmio) #endif #endif +enum amd_ntb_port { + NTB_PORT_PRI_USD, + NTB_PORT_SEC_DSD +}; + enum { /* AMD NTB Capability */ AMD_MW_CNT = 3, diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c index 68d9908..7e44dc3 100644 --- a/drivers/ntb/hw/intel/ntb_hw_intel.c +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c @@ -1035,6 +1035,49 @@ static void ndev_deinit_debugfs(struct intel_ntb_dev *ndev) debugfs_remove_recursive(ndev->debugfs_dir); } +static int intel_ntb_port_number(struct ntb_dev *ntb) +{ + switch (ntb->topo) { + case NTB_TOPO_PRI: + case NTB_TOPO_B2B_USD: + return NTB_PORT_PRI_USD; + case NTB_TOPO_SEC: + case NTB_TOPO_B2B_DSD: + return NTB_PORT_SEC_DSD; + default: + break; + } + + return -EINVAL; +} + +static int intel_ntb_peer_port_count(struct ntb_dev *ntb) +{ + return NTB_PEER_CNT; +} + +static int intel_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) +{ + int local_port =
[PATCH v2 2/9] NTB: Add indexed ports NTB API
There are some NTB hardware, which can combine more than just two domains over NTB. For instance, some IDT PCIe-switches can have NTB-functions activated on more than two-ports. The different domains are distinguished by ports they are connected to. So the new port-related methods are added to the NTB API: ntb_port_number() - return local port ntb_peer_port_count() - return number of peers local port can connect to ntb_peer_port_number(pdix) - return port number by it index ntb_peer_port_idx(port) - return port index by it number Current test-drivers aren't changed much. They still support two-ports devices for the time being while multi-ports hardware drivers aren't added. Signed-off-by: Serge Semin --- drivers/ntb/hw/amd/ntb_hw_amd.c | 47 drivers/ntb/hw/amd/ntb_hw_amd.h | 9 + drivers/ntb/hw/intel/ntb_hw_intel.c | 52 ++- drivers/ntb/hw/intel/ntb_hw_intel.h | 9 + drivers/ntb/ntb_transport.c | 6 drivers/ntb/test/ntb_perf.c | 4 +++ drivers/ntb/test/ntb_pingpong.c | 6 drivers/ntb/test/ntb_tool.c | 5 +++ include/linux/ntb.h | 71 + 9 files changed, 208 insertions(+), 1 deletion(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 6704327..0b767ef 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -71,6 +71,49 @@ MODULE_AUTHOR("AMD Inc."); static const struct file_operations amd_ntb_debugfs_info; static struct dentry *debugfs_dir; +static int amd_ntb_port_number(struct ntb_dev *ntb) +{ + switch (ntb->topo) { + case NTB_TOPO_PRI: + case NTB_TOPO_B2B_USD: + return NTB_PORT_PRI_USD; + case NTB_TOPO_SEC: + case NTB_TOPO_B2B_DSD: + return NTB_PORT_SEC_DSD; + default: + break; + } + + return -EINVAL; +} + +static int amd_ntb_peer_port_count(struct ntb_dev *ntb) +{ + return NTB_PEER_CNT; +} + +static int amd_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) +{ + int local_port = amd_ntb_port_number(ntb); + + if (pidx > NTB_PIDX_MAX) + return -EINVAL; + + return (local_port == NTB_PORT_PRI_USD ? + NTB_PORT_SEC_DSD : NTB_PORT_PRI_USD); +} + +static int amd_ntb_peer_port_idx(struct ntb_dev *ntb, int port) +{ + int local_port = amd_ntb_port_number(ntb); + + if ((local_port == NTB_PORT_PRI_USD && port != NTB_PORT_SEC_DSD) || + (local_port == NTB_PORT_SEC_DSD && port != NTB_PORT_PRI_USD)) + return -EINVAL; + + return 0; +} + static int amd_link_is_up(struct amd_ntb_dev *ndev) { if (!ndev->peer_sta) @@ -431,6 +474,10 @@ static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, } static const struct ntb_dev_ops amd_ntb_ops = { + .port_number= amd_ntb_port_number, + .peer_port_count= amd_ntb_peer_port_count, + .peer_port_number = amd_ntb_peer_port_number, + .peer_port_idx = amd_ntb_peer_port_idx, .link_is_up = amd_ntb_link_is_up, .link_enable= amd_ntb_link_enable, .link_disable = amd_ntb_link_disable, diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 2eac3cd..1aeb08f 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -62,6 +62,10 @@ #define NTB_LNK_STA_SPEED(x) (((x) & NTB_LNK_STA_SPEED_MASK) >> 16) #define NTB_LNK_STA_WIDTH(x) (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20) +/* port related constants */ +#define NTB_PEER_CNT (1) +#define NTB_PIDX_MAX (0) + #ifndef read64 #ifdef readq #define read64 readq @@ -91,6 +95,11 @@ static inline void _write64(u64 val, void __iomem *mmio) #endif #endif +enum amd_ntb_port { + NTB_PORT_PRI_USD, + NTB_PORT_SEC_DSD +}; + enum { /* AMD NTB Capability */ AMD_MW_CNT = 3, diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c index 68d9908..7e44dc3 100644 --- a/drivers/ntb/hw/intel/ntb_hw_intel.c +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c @@ -1035,6 +1035,49 @@ static void ndev_deinit_debugfs(struct intel_ntb_dev *ndev) debugfs_remove_recursive(ndev->debugfs_dir); } +static int intel_ntb_port_number(struct ntb_dev *ntb) +{ + switch (ntb->topo) { + case NTB_TOPO_PRI: + case NTB_TOPO_B2B_USD: + return NTB_PORT_PRI_USD; + case NTB_TOPO_SEC: + case NTB_TOPO_B2B_DSD: + return NTB_PORT_SEC_DSD; + default: + break; + } + + return -EINVAL; +} + +static int intel_ntb_peer_port_count(struct ntb_dev *ntb) +{ + return NTB_PEER_CNT; +} + +static int intel_ntb_peer_port_number(struct ntb_dev *ntb, int pidx) +{ + int local_port =