From: Frank Li <Frank.li@oss.nxp.com>
To: Koichiro Den <den@valinux.co.jp>
Cc: "Frank Li" <Frank.Li@kernel.org>,
"Manivannan Sadhasivam" <mani@kernel.org>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Kishon Vijay Abraham I" <kishon@kernel.org>,
"Jon Mason" <jdmason@kudzu.us>,
"Dave Jiang" <dave.jiang@intel.com>,
"Allen Hubbe" <allenbh@gmail.com>,
"Niklas Cassel" <cassel@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Jonathan Corbet" <corbet@lwn.net>,
"Shuah Khan" <skhan@linuxfoundation.org>,
"Randy Dunlap" <rdunlap@infradead.org>,
"Jingoo Han" <jingoohan1@gmail.com>,
"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Jerome Brunet" <jbrunet@baylibre.com>,
linux-pci@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, ntb@lists.linux.dev
Subject: Re: [PATCH v7 08/10] NTB: ntb_hw_epf: Use the shared NTB control layout
Date: Wed, 21 Oct 2026 10:53:51 -0500 [thread overview]
Message-ID: <atjgD2O1CoZm8o-Z@SMW015318> (raw)
In-Reply-To: <20261021042229.2280937-9-den@valinux.co.jp>
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@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 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
>
next prev parent reply other threads:[~2026-10-21 15:54 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-21 4:22 [PATCH v7 00/10] PCI: endpoint: Remote DMA support via vNTB Koichiro Den
2026-10-21 4:22 ` [PATCH v7 01/10] PCI: endpoint: Add DMA auxiliary resource metadata Koichiro Den
2026-10-21 4:27 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 02/10] PCI: dwc: Expose endpoint DMA resources Koichiro Den
2026-10-21 4:38 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 03/10] PCI: endpoint: pci-epf-vntb: Move epf_ntb_is_bar_used() up Koichiro Den
2026-10-21 4:25 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 04/10] PCI: endpoint: Add pci-epf-ntb.h for the shared NTB control layout Koichiro Den
2026-10-21 4:25 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 05/10] PCI: endpoint: pci-epf-vntb: Export endpoint DMA channels Koichiro Den
2026-10-21 4:47 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 06/10] PCI: endpoint: pci-epf-vntb: Allow DMA and MW to share a BAR Koichiro Den
2026-10-21 4:43 ` sashiko-bot
2026-10-21 15:52 ` Frank Li
2026-10-21 16:35 ` Koichiro Den
2026-10-21 19:00 ` Frank Li
2026-10-21 4:22 ` [PATCH v7 07/10] Documentation: PCI: endpoint: Document vNTB DMA export Koichiro Den
2026-10-21 4:25 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 08/10] NTB: ntb_hw_epf: Use the shared NTB control layout Koichiro Den
2026-10-21 4:26 ` sashiko-bot
2026-10-21 15:53 ` Frank Li [this message]
2026-10-21 4:22 ` [PATCH v7 09/10] NTB: ntb_hw_epf: Discover vNTB-embedded DMA Koichiro Den
2026-10-21 4:36 ` sashiko-bot
2026-10-21 4:22 ` [PATCH v7 10/10] NTB: ntb_hw_epf: Support DMA and MW sharing a BAR Koichiro Den
2026-10-21 4:33 ` sashiko-bot
2026-10-21 15:58 ` Frank Li
2026-10-21 16:57 ` Koichiro Den
2026-10-21 18:51 ` Frank Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atjgD2O1CoZm8o-Z@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=allenbh@gmail.com \
--cc=bhelgaas@google.com \
--cc=cassel@kernel.org \
--cc=corbet@lwn.net \
--cc=dave.jiang@intel.com \
--cc=den@valinux.co.jp \
--cc=jbrunet@baylibre.com \
--cc=jdmason@kudzu.us \
--cc=jingoohan1@gmail.com \
--cc=kishon@kernel.org \
--cc=kwilczynski@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=mani@kernel.org \
--cc=ntb@lists.linux.dev \
--cc=rdunlap@infradead.org \
--cc=robh@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox