RE: [PATCH v2 5/9] NTB: Alter Scratchpads API to support multi-ports devices

[Allen Hubbe]

From: Serge Semin 
> Even though there is no any real NTB hardware, which would have both more
> than two ports and Scratchpad registers, it is logically correct to have
> Scratchpad API accepting a peer port index as well. Intel/AMD drivers utilize
> Primary and Secondary topology to split Scratchpad between connected root
> devices. Since port-index API introduced, Intel/AMD NTB hadrware drivers can

s/hadrware/hardware/

> use device port to determine which Scratchpad registers actually belong to
> local and peer devices. The same approach can be used if some potential
> hardware in future will be multi-port and have some set of Scratchpads.
> Here are the brief of changes in the API:
>  ntb_spad_count() - return number of Scratchpad per each port
>  ntb_peer_spad_addr(pidx, sidx) - address of Scratchpad register of the
> peer device with pidx-index
>  ntb_peer_spad_read(pidx, sidx) - read specified Scratchpad register of the
> peer with pidx-index
>  ntb_peer_spad_write(pidx, sidx) - write data to Scratchpad register of the
> peer with pidx-index
> 
> Since there is hardware which doesn't support Scratchpad registers, the
> corresponding API methods are now made optional.

The api change looks good.  See the comment to simplify ntb_tool.

> Signed-off-by: Serge Semin 
> 
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 14 +++
>  drivers/ntb/hw/intel/ntb_hw_intel.c | 14 +++
>  drivers/ntb/ntb_transport.c | 17 -
>  drivers/ntb/test/ntb_perf.c |  6 +--
>  drivers/ntb/test/ntb_pingpong.c |  8 +++-
>  drivers/ntb/test/ntb_tool.c | 45 +-
>  include/linux/ntb.h | 76 
> +++--
>  7 files changed, 115 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 74fe9b8..a2596ad 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -476,30 +476,30 @@ static int amd_ntb_spad_write(struct ntb_dev *ntb,
>   return 0;
>  }
> 
> -static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
> +static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx)
>  {
>   struct amd_ntb_dev *ndev = ntb_ndev(ntb);
>   void __iomem *mmio = ndev->self_mmio;
>   u32 offset;
> 
> - if (idx < 0 || idx >= ndev->spad_count)
> + if (sidx < 0 || sidx >= ndev->spad_count)
>   return -EINVAL;
> 
> - offset = ndev->peer_spad + (idx << 2);
> + offset = ndev->peer_spad + (sidx << 2);
>   return readl(mmio + AMD_SPAD_OFFSET + offset);
>  }
> 
> -static int amd_ntb_peer_spad_write(struct ntb_dev *ntb,
> -int idx, u32 val)
> +static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
> +int sidx, u32 val)
>  {
>   struct amd_ntb_dev *ndev = ntb_ndev(ntb);
>   void __iomem *mmio = ndev->self_mmio;
>   u32 offset;
> 
> - if (idx < 0 || idx >= ndev->spad_count)
> + if (sidx < 0 || sidx >= ndev->spad_count)
>   return -EINVAL;
> 
> - offset = ndev->peer_spad + (idx << 2);
> + offset = ndev->peer_spad + (sidx << 2);
>   writel(val, mmio + AMD_SPAD_OFFSET + offset);
> 
>   return 0;
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c 
> b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index 5a57d9e..471b0ba 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -1452,30 +1452,30 @@ static int intel_ntb_spad_write(struct ntb_dev *ntb,
>  ndev->self_reg->spad);
>  }
> 
> -static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
> +static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx, int sidx,
>   phys_addr_t *spad_addr)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_addr(ndev, idx, spad_addr, ndev->peer_addr,
> + return ndev_spad_addr(ndev, sidx, spad_addr, ndev->peer_addr,
> ndev->peer_reg->spad);
>  }
> 
> -static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
> +static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_read(ndev, idx,
> + return ndev_spad_read(ndev, sidx,
> ndev->peer_mmio +
> ndev->peer_reg->spad);
>  }
> 
> -static int intel_ntb_peer_spad_write(struct ntb_dev *ntb,
> -  int idx, u32 val)
> +static int intel_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
> +  int sidx, u32 val)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_write(ndev, idx, val,
> + return ndev_spad_write(ndev, sidx, val,
>  ndev->peer_mmio +
>

RE: [PATCH v2 5/9] NTB: Alter Scratchpads API to support multi-ports devices

[Allen Hubbe]

From: Serge Semin 
> Even though there is no any real NTB hardware, which would have both more
> than two ports and Scratchpad registers, it is logically correct to have
> Scratchpad API accepting a peer port index as well. Intel/AMD drivers utilize
> Primary and Secondary topology to split Scratchpad between connected root
> devices. Since port-index API introduced, Intel/AMD NTB hadrware drivers can

s/hadrware/hardware/

> use device port to determine which Scratchpad registers actually belong to
> local and peer devices. The same approach can be used if some potential
> hardware in future will be multi-port and have some set of Scratchpads.
> Here are the brief of changes in the API:
>  ntb_spad_count() - return number of Scratchpad per each port
>  ntb_peer_spad_addr(pidx, sidx) - address of Scratchpad register of the
> peer device with pidx-index
>  ntb_peer_spad_read(pidx, sidx) - read specified Scratchpad register of the
> peer with pidx-index
>  ntb_peer_spad_write(pidx, sidx) - write data to Scratchpad register of the
> peer with pidx-index
> 
> Since there is hardware which doesn't support Scratchpad registers, the
> corresponding API methods are now made optional.

The api change looks good.  See the comment to simplify ntb_tool.

> Signed-off-by: Serge Semin 
> 
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 14 +++
>  drivers/ntb/hw/intel/ntb_hw_intel.c | 14 +++
>  drivers/ntb/ntb_transport.c | 17 -
>  drivers/ntb/test/ntb_perf.c |  6 +--
>  drivers/ntb/test/ntb_pingpong.c |  8 +++-
>  drivers/ntb/test/ntb_tool.c | 45 +-
>  include/linux/ntb.h | 76 
> +++--
>  7 files changed, 115 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 74fe9b8..a2596ad 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -476,30 +476,30 @@ static int amd_ntb_spad_write(struct ntb_dev *ntb,
>   return 0;
>  }
> 
> -static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
> +static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx)
>  {
>   struct amd_ntb_dev *ndev = ntb_ndev(ntb);
>   void __iomem *mmio = ndev->self_mmio;
>   u32 offset;
> 
> - if (idx < 0 || idx >= ndev->spad_count)
> + if (sidx < 0 || sidx >= ndev->spad_count)
>   return -EINVAL;
> 
> - offset = ndev->peer_spad + (idx << 2);
> + offset = ndev->peer_spad + (sidx << 2);
>   return readl(mmio + AMD_SPAD_OFFSET + offset);
>  }
> 
> -static int amd_ntb_peer_spad_write(struct ntb_dev *ntb,
> -int idx, u32 val)
> +static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
> +int sidx, u32 val)
>  {
>   struct amd_ntb_dev *ndev = ntb_ndev(ntb);
>   void __iomem *mmio = ndev->self_mmio;
>   u32 offset;
> 
> - if (idx < 0 || idx >= ndev->spad_count)
> + if (sidx < 0 || sidx >= ndev->spad_count)
>   return -EINVAL;
> 
> - offset = ndev->peer_spad + (idx << 2);
> + offset = ndev->peer_spad + (sidx << 2);
>   writel(val, mmio + AMD_SPAD_OFFSET + offset);
> 
>   return 0;
> diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c 
> b/drivers/ntb/hw/intel/ntb_hw_intel.c
> index 5a57d9e..471b0ba 100644
> --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> @@ -1452,30 +1452,30 @@ static int intel_ntb_spad_write(struct ntb_dev *ntb,
>  ndev->self_reg->spad);
>  }
> 
> -static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx,
> +static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx, int sidx,
>   phys_addr_t *spad_addr)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_addr(ndev, idx, spad_addr, ndev->peer_addr,
> + return ndev_spad_addr(ndev, sidx, spad_addr, ndev->peer_addr,
> ndev->peer_reg->spad);
>  }
> 
> -static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int idx)
> +static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_read(ndev, idx,
> + return ndev_spad_read(ndev, sidx,
> ndev->peer_mmio +
> ndev->peer_reg->spad);
>  }
> 
> -static int intel_ntb_peer_spad_write(struct ntb_dev *ntb,
> -  int idx, u32 val)
> +static int intel_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx,
> +  int sidx, u32 val)
>  {
>   struct intel_ntb_dev *ndev = ntb_ndev(ntb);
> 
> - return ndev_spad_write(ndev, idx, val,
> + return ndev_spad_write(ndev, sidx, val,
>  ndev->peer_mmio +
>