Re: [PATCH v7 08/10] NTB: ntb_hw_epf: Use the shared NTB control layout
From: Frank Li
Date: Wed Oct 21 2026 - 11:58:29 EST
On Wed, Oct 21, 2026 at 01:22:27PM +0900, Koichiro Den wrote:
> Currently, ntb_hw_epf.c duplicates the endpoint control layout with
> hard-coded register offsets.
>
> Use the shared struct and macros in pci-epf-ntb.h and derive offsets
> with offsetof().
>
> No functional changes.
>
> Suggested-by: Frank Li <Frank.Li@xxxxxxx>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> Changes since v6:
> - Split the host conversion out of the shared-header patch. (Frank)
>
> drivers/ntb/hw/epf/ntb_hw_epf.c | 95 ++++++++++++---------------------
> 1 file changed, 35 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index c47607d4f8a7..ff92c2a61ce7 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -8,41 +8,15 @@
>
> #include <linux/atomic.h>
> #include <linux/delay.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/pci-epf-ntb.h>
> #include <linux/slab.h>
> #include <linux/ntb.h>
>
> -#define NTB_EPF_COMMAND 0x0
> -#define CMD_CONFIGURE_DOORBELL 1
> -#define CMD_TEARDOWN_DOORBELL 2
> -#define CMD_CONFIGURE_MW 3
> -#define CMD_TEARDOWN_MW 4
> -#define CMD_LINK_UP 5
> -#define CMD_LINK_DOWN 6
> -
> -#define NTB_EPF_ARGUMENT 0x4
> -#define MSIX_ENABLE BIT(16)
> -
> -#define NTB_EPF_CMD_STATUS 0x8
> -#define COMMAND_STATUS_OK 1
> -#define COMMAND_STATUS_ERROR 2
> -
> -#define NTB_EPF_LINK_STATUS 0x0A
> -#define LINK_STATUS_UP BIT(0)
> -
> -#define NTB_EPF_TOPOLOGY 0x0C
> -#define NTB_EPF_LOWER_ADDR 0x10
> -#define NTB_EPF_UPPER_ADDR 0x14
> -#define NTB_EPF_LOWER_SIZE 0x18
> -#define NTB_EPF_UPPER_SIZE 0x1C
> -#define NTB_EPF_MW_COUNT 0x20
> -#define NTB_EPF_MW1_OFFSET 0x24
> -#define NTB_EPF_SPAD_OFFSET 0x28
> -#define NTB_EPF_SPAD_COUNT 0x2C
> -#define NTB_EPF_DB_ENTRY_SIZE 0x30
> -#define NTB_EPF_DB_DATA(n) (0x34 + (n) * 4)
> -#define NTB_EPF_DB_OFFSET(n) (0xB4 + (n) * 4)
> +/* Offsets into the control region, see include/linux/pci-epf-ntb.h. */
> +#define NTB_EPF_CTRL(field) offsetof(struct epf_ntb_ctrl, field)
>
> /*
> * Legacy doorbell slot layout when paired with pci-epf-*ntb:
> @@ -132,20 +106,20 @@ static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command,
> u32 status;
>
> mutex_lock(&ndev->cmd_lock);
> - writel(argument, ndev->ctrl_reg + NTB_EPF_ARGUMENT);
> - writel(command, ndev->ctrl_reg + NTB_EPF_COMMAND);
> + writel(argument, ndev->ctrl_reg + NTB_EPF_CTRL(argument));
> + writel(command, ndev->ctrl_reg + NTB_EPF_CTRL(command));
>
> timeout = ktime_add_ms(ktime_get(), NTB_EPF_COMMAND_TIMEOUT);
> while (1) {
> timedout = ktime_after(ktime_get(), timeout);
> - status = readw(ndev->ctrl_reg + NTB_EPF_CMD_STATUS);
> + status = readw(ndev->ctrl_reg + NTB_EPF_CTRL(command_status));
>
> - if (status == COMMAND_STATUS_ERROR) {
> + if (status == EPF_NTB_COMMAND_STATUS_ERROR) {
> ret = -EINVAL;
> break;
> }
>
> - if (status == COMMAND_STATUS_OK)
> + if (status == EPF_NTB_COMMAND_STATUS_OK)
> break;
>
> if (WARN_ON(timedout)) {
> @@ -156,7 +130,7 @@ static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command,
> usleep_range(5, 10);
> }
>
> - writew(0, ndev->ctrl_reg + NTB_EPF_CMD_STATUS);
> + writew(0, ndev->ctrl_reg + NTB_EPF_CTRL(command_status));
> mutex_unlock(&ndev->cmd_lock);
>
> return ret;
> @@ -224,9 +198,9 @@ static u64 ntb_epf_link_is_up(struct ntb_dev *ntb,
> struct ntb_epf_dev *ndev = ntb_ndev(ntb);
> u32 status;
>
> - status = readw(ndev->ctrl_reg + NTB_EPF_LINK_STATUS);
> + status = readw(ndev->ctrl_reg + NTB_EPF_CTRL(link_status));
>
> - return status & LINK_STATUS_UP;
> + return status & EPF_NTB_LINK_STATUS_UP;
> }
>
> static u32 ntb_epf_spad_read(struct ntb_dev *ntb, int idx)
> @@ -240,7 +214,7 @@ static u32 ntb_epf_spad_read(struct ntb_dev *ntb, int idx)
> return 0;
> }
>
> - offset = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
> + offset = readl(ndev->ctrl_reg + NTB_EPF_CTRL(spad_offset));
> offset += (idx << 2);
>
> return readl(ndev->ctrl_reg + offset);
> @@ -258,7 +232,7 @@ static int ntb_epf_spad_write(struct ntb_dev *ntb,
> return -EINVAL;
> }
>
> - offset = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
> + offset = readl(ndev->ctrl_reg + NTB_EPF_CTRL(spad_offset));
> offset += (idx << 2);
> writel(val, ndev->ctrl_reg + offset);
>
> @@ -316,7 +290,7 @@ static int ntb_epf_link_enable(struct ntb_dev *ntb,
> struct device *dev = ndev->dev;
> int ret;
>
> - ret = ntb_epf_send_command(ndev, CMD_LINK_UP, 0);
> + ret = ntb_epf_send_command(ndev, EPF_NTB_COMMAND_LINK_UP, 0);
> if (ret) {
> dev_err(dev, "Fail to enable link\n");
> return ret;
> @@ -331,7 +305,7 @@ static int ntb_epf_link_disable(struct ntb_dev *ntb)
> struct device *dev = ndev->dev;
> int ret;
>
> - ret = ntb_epf_send_command(ndev, CMD_LINK_DOWN, 0);
> + ret = ntb_epf_send_command(ndev, EPF_NTB_COMMAND_LINK_DOWN, 0);
> if (ret) {
> dev_err(dev, "Fail to disable link\n");
> return ret;
> @@ -373,7 +347,7 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
> {
> struct pci_dev *pdev = ndev->ntb.pdev;
> struct device *dev = ndev->dev;
> - u32 argument = MSIX_ENABLE;
> + u32 argument = EPF_NTB_MSIX_ENABLE;
> int irq;
> int ret;
> int i;
> @@ -387,7 +361,7 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
> dev_err(dev, "Failed to get MSI interrupts\n");
> return irq;
> }
> - argument &= ~MSIX_ENABLE;
> + argument &= ~EPF_NTB_MSIX_ENABLE;
> }
>
> ndev->db_count = irq - 1;
> @@ -402,7 +376,7 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
> }
> }
>
> - ret = ntb_epf_send_command(ndev, CMD_CONFIGURE_DOORBELL,
> + ret = ntb_epf_send_command(ndev, EPF_NTB_COMMAND_CONFIGURE_DOORBELL,
> argument | irq);
> if (ret) {
> dev_err(dev, "Failed to configure doorbell\n");
> @@ -494,11 +468,9 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
> return -EINVAL;
> }
>
> - writel(lower_32_bits(addr), ndev->ctrl_reg + NTB_EPF_LOWER_ADDR);
> - writel(upper_32_bits(addr), ndev->ctrl_reg + NTB_EPF_UPPER_ADDR);
> - writel(lower_32_bits(size), ndev->ctrl_reg + NTB_EPF_LOWER_SIZE);
> - writel(upper_32_bits(size), ndev->ctrl_reg + NTB_EPF_UPPER_SIZE);
> - ntb_epf_send_command(ndev, CMD_CONFIGURE_MW, idx);
> + lo_hi_writeq(addr, ndev->ctrl_reg + NTB_EPF_CTRL(addr));
> + lo_hi_writeq(size, ndev->ctrl_reg + NTB_EPF_CTRL(size));
> + ntb_epf_send_command(ndev, EPF_NTB_COMMAND_CONFIGURE_MW, idx);
>
> return 0;
> }
> @@ -509,7 +481,7 @@ static int ntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
> struct device *dev = ndev->dev;
> int ret = 0;
>
> - ntb_epf_send_command(ndev, CMD_TEARDOWN_MW, idx);
> + ntb_epf_send_command(ndev, EPF_NTB_COMMAND_TEARDOWN_MW, idx);
> if (ret)
> dev_err(dev, "Failed to teardown memory window\n");
>
> @@ -524,7 +496,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
> int bar;
>
> if (idx == 0)
> - offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
> + offset = readl(ndev->ctrl_reg + NTB_EPF_CTRL(mw1_offset));
>
> bar = ntb_epf_mw_to_bar(ndev, idx);
> if (bar < 0)
> @@ -562,10 +534,12 @@ static int ntb_epf_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
> return -EINVAL;
> }
>
> - db_entry_size = readl(ndev->ctrl_reg + NTB_EPF_DB_ENTRY_SIZE);
> + db_entry_size = readl(ndev->ctrl_reg + NTB_EPF_CTRL(db_entry_size));
>
> - db_data = readl(ndev->ctrl_reg + NTB_EPF_DB_DATA(interrupt_num));
> - db_offset = readl(ndev->ctrl_reg + NTB_EPF_DB_OFFSET(interrupt_num));
> + db_data = readl(ndev->ctrl_reg +
> + NTB_EPF_CTRL(db_data[interrupt_num]));
> + db_offset = readl(ndev->ctrl_reg +
> + NTB_EPF_CTRL(db_offset[interrupt_num]));
> writel(db_data, ndev->db_reg + (db_entry_size * interrupt_num) +
> db_offset);
>
> @@ -631,7 +605,7 @@ static int ntb_epf_init_dev(struct ntb_epf_dev *ndev)
> struct device *dev = ndev->dev;
> int ret;
>
> - ndev->mw_count = readl(ndev->ctrl_reg + NTB_EPF_MW_COUNT);
> + ndev->mw_count = readl(ndev->ctrl_reg + NTB_EPF_CTRL(num_mws));
> if (ndev->mw_count > NTB_EPF_MAX_MW_COUNT) {
> dev_err(dev, "Unsupported MW count: %u\n", ndev->mw_count);
> return -EINVAL;
> @@ -650,7 +624,7 @@ static int ntb_epf_init_dev(struct ntb_epf_dev *ndev)
> * doorbell layout, hence -1.
> */
> ndev->db_valid_mask = BIT_ULL(ndev->db_count - 1) - 1;
> - ndev->spad_count = readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
> + ndev->spad_count = readl(ndev->ctrl_reg + NTB_EPF_CTRL(spad_count));
>
> return 0;
> }
> @@ -702,8 +676,8 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
> goto err_pci_regions;
> }
> } else {
> - spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
> - spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
> + spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_CTRL(spad_count));
> + spad_off = readl(ndev->ctrl_reg + NTB_EPF_CTRL(spad_offset));
> ndev->peer_spad_reg = ndev->ctrl_reg + spad_off + spad_sz;
> }
>
> @@ -743,7 +717,8 @@ static void ntb_epf_cleanup_isr(struct ntb_epf_dev *ndev)
> struct pci_dev *pdev = ndev->ntb.pdev;
> int i;
>
> - ntb_epf_send_command(ndev, CMD_TEARDOWN_DOORBELL, ndev->db_count + 1);
> + ntb_epf_send_command(ndev, EPF_NTB_COMMAND_TEARDOWN_DOORBELL,
> + ndev->db_count + 1);
>
> for (i = 0; i < ndev->db_count + 1; i++)
> free_irq(pci_irq_vector(pdev, i), &ndev->irq_ctx[i]);
> --
> 2.54.0
>