[PATCH v19 00/10] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[PATCH v19 00/10] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Frank Li]

┌────────────┐   ┌───────────────────────────────────┐   ┌────────────────┐
│            │   │                                   │   │                │
│            │   │ PCI Endpoint                      │   │ PCI Host       │
│            │   │                                   │   │                │
│            │◄──┤ 1.platform_msi_domain_alloc_irqs()│   │                │
│            │   │                                   │   │                │
│ MSI        ├──►│ 2.write_msi_msg()                 ├──►├─BAR<n>         │
│ Controller │   │   update doorbell register address│   │                │
│            │   │   for BAR                         │   │                │
│            │   │                                   │   │ 3. Write BAR<n>│
│            │◄──┼───────────────────────────────────┼───┤                │
│            │   │                                   │   │                │
│            ├──►│ 4.Irq Handle                      │   │                │
│            │   │                                   │   │                │
│            │   │                                   │   │                │
└────────────┘   └───────────────────────────────────┘   └────────────────┘

This patches based on old https://lore.kernel.org/imx/20221124055036.1630573-1-Frank.Li@nxp.com/

Original patch only target to vntb driver. But actually it is common
method.

This patches add new API to pci-epf-core, so any EP driver can use it.

Previous v2 discussion here.
https://lore.kernel.org/imx/20230911220920.1817033-1-Frank.Li@nxp.com/

Changes in v19:
- irq part already in v6.16-rc1, only missed pcie/dts part
- rebase to v6.16-rc1
- update commit message for patch IMMUTABLE check.
- Link to v18: https://lore.kernel.org/r/20250414-ep-msi-v18-0-f69b49917464@nxp.com

Changes in v18:
- pci-ep.yaml: sort property order, fix maxvalue to 0x7ffff for msi-map-mask and
iommu-map-mask
- Link to v17: https://lore.kernel.org/r/20250407-ep-msi-v17-0-633ab45a31d0@nxp.com

Changes in v17:
- move document part to pci-ep.yaml
- Link to v16: https://lore.kernel.org/r/20250404-ep-msi-v16-0-d4919d68c0d0@nxp.com

Changes in v16:
- remove arm64: dts: imx95-19x19-evk: Add PCIe1 endpoint function overlay file
because there are better patches, which under review.
- Add document for pcie-ep msi-map usage
- other change to see each patch's change log
About IMMUTABLE (No change for this part, tglx provide feedback)
> - This IMMUTABLE thing serves no purpose, because you don't randomly
>   plug this end-point block on any MSI controller. They come as part
>   of an SoC.

"Yes and no. The problem is that the EP implementation is meant to be a
generic library and while GIC-ITS guarantees immutability of the
address/data pair after setup, there are architectures (x86, loongson,
riscv) where the base MSI controller does not and immutability is only
achieved when interrupt remapping is enabled. The latter can be disabled
at boot-time and then the EP implementation becomes a lottery across
affinity changes.

That was my concern about this library implementation and that's why I
asked for a mechanism to ensure that the underlying irqdomain provides a
immutable address/data pair.

So it does not matter for GIC-ITS, but in the larger picture it matters.

Thanks,

        tglx
"

So it does not matter for GIC-ITS, but in the larger picture it matters.

- Link to v15: https://lore.kernel.org/r/20250211-ep-msi-v15-0-bcacc1f2b1a9@nxp.com

Changes in v15:
- rebase to v6.14-rc1
- fix build issue find by kernel test robot
- Link to v14: https://lore.kernel.org/r/20250207-ep-msi-v14-0-9671b136f2b8@nxp.com

Changes in v14:
Marc Zyngier raised concerns about adding DOMAIN_BUS_DEVICE_PCI_EP_MSI. As
a result, the approach has been reverted to the v9 method. However, there
are several improvements:

MSI now supports msi-map in addition to msi-parent.
  - The struct device: id is used as the endpoint function (EPF) device
identity to map to the stream ID (sideband information).
  - The EPC device tree source (DTS) utilizes msi-map to provide such
information.
  - The EPF device's of_node is set to the EPC controller’s node. This
approach is commonly used for multi-function device (MFD) platform child
devices, allowing them to inherit properties from the MFD device’s DTS,
such as reset-cells and gpio-cells. This method is well-suited for the
current case, as the EPF is inherently created/binded to the EPC and
should inherit the EPC’s DTS node properties.

Additionally:

Since the basic IMX95 LUT support has already been merged into the
mainline, a DTS and driver increment patch is added to complete the
solution. The patch is rebased onto the latest linux-next tree and
aligned with the new pcitest framework.

- Link to v13: https://lore.kernel.org/r/20241218-ep-msi-v13-0-646e2192dc24@nxp.com

Changes in v13:
- Change to use DOMAIN_BUS_PCI_DEVICE_EP_MSI
- Change request id as  func | vfunc << 3
- Remove IRQ_DOMAIN_MSI_IMMUTABLE

Thomas Gleixner:

I hope capture all your points in review comments. If missed, let me know.

- Link to v12: https://lore.kernel.org/r/20241211-ep-msi-v12-0-33d4532fa520@nxp.com

Changes in v12:
- Change to use IRQ_DOMAIN_MSI_IMMUTABLE and add help function
irq_domain_msi_is_immuatble().
- split PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check to 3 patches
- Link to v11: https://lore.kernel.org/r/20241209-ep-msi-v11-0-7434fa8397bd@nxp.com

Changes in v11:
- Change to use MSI_FLAG_MSG_IMMUTABLE
- Link to v10: https://lore.kernel.org/r/20241204-ep-msi-v10-0-87c378dbcd6d@nxp.com

Changes in v10:

Thomas Gleixner:
	There are big change in pci-ep-msi.c. I am sure if go on the
corrent path. The key improvement is remove only 1 function devices's
limitation.

	I use new patch for imutable check, which relative additional
feature compared to base enablement patch.

- Remove patch Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
- Add new patch irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info
- Remove only support 1 endpoint function limiation.
- Create one MSI domain for each endpoint function devices.
- Use "msi-map" in pci ep controler node, instead of of msi-parent. first
argument is
	(func_no << 8 | vfunc_no)

- Link to v9: https://lore.kernel.org/r/20241203-ep-msi-v9-0-a60dbc3f15dd@nxp.com

Changes in v9
- Add patch platform-msi: Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
- Remove patch PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering
- Remove API pci_epf_align_inbound_addr_lo_hi
- Move doorbell_alloc in to doorbell_enable function.
- Link to v8: https://lore.kernel.org/r/20241116-ep-msi-v8-0-6f1f68ffd1bb@nxp.com

Changes in v8:
- update helper function name to pci_epf_align_inbound_addr()
- Link to v7: https://lore.kernel.org/r/20241114-ep-msi-v7-0-d4ac7aafbd2c@nxp.com

Changes in v7:
- Add helper function pci_epf_align_addr();
- Link to v6: https://lore.kernel.org/r/20241112-ep-msi-v6-0-45f9722e3c2a@nxp.com

Changes in v6:
- change doorbell_addr to doorbell_offset
- use round_down()
- add Niklas's test by tag
- rebase to pci/endpoint
- Link to v5: https://lore.kernel.org/r/20241108-ep-msi-v5-0-a14951c0d007@nxp.com

Changes in v5:
- Move request_irq to epf test function driver for more flexiable user case
- Add fixed size bar handler
- Some minor improvememtn to see each patches's changelog.
- Link to v4: https://lore.kernel.org/r/20241031-ep-msi-v4-0-717da2d99b28@nxp.com

Changes in v4:
- Remove patch genirq/msi: Add cleanup guard define for msi_lock_descs()/msi_unlock_descs()
- Use new method to avoid compatible problem.
  Add new command DOORBELL_ENABLE and DOORBELL_DISABLE.
  pcitest -B send DOORBELL_ENABLE first, EP test function driver try to
remap one of BAR_N (except test register bar) to ITS MSI MMIO space. Old
driver don't support new command, so failure return, not side effect.
  After test, DOORBELL_DISABLE command send out to recover original map, so
pcitest bar test can pass as normal.
- Other detail change see each patches's change log
- Link to v3: https://lore.kernel.org/r/20241015-ep-msi-v3-0-cedc89a16c1a@nxp.com

Change from v2 to v3
- Fixed manivannan's comments
- Move common part to pci-ep-msi.c and pci-ep-msi.h
- rebase to 6.12-rc1
- use RevID to distingiush old version

mkdir /sys/kernel/config/pci_ep/functions/pci_epf_test/func1
echo 16 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/msi_interrupts
echo 0x080c > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/deviceid
echo 0x1957 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/vendorid
echo 1 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/revid
^^^^^^ to enable platform msi support.
ln -s /sys/kernel/config/pci_ep/functions/pci_epf_test/func1 /sys/kernel/config/pci_ep/controllers/4c380000.pcie-ep

- use new device ID, which identify support doorbell to avoid broken
compatility.

    Enable doorbell support only for PCI_DEVICE_ID_IMX8_DB, while other devices
    keep the same behavior as before.

           EP side             RC with old driver      RC with new driver
    PCI_DEVICE_ID_IMX8_DB          no probe              doorbell enabled
    Other device ID             doorbell disabled*       doorbell disabled*

    * Behavior remains unchanged.

Change from v1 to v2
- Add missed patch for endpont/pci-epf-test.c
- Move alloc and free to epc driver from epf.
- Provide general help function for EPC driver to alloc platform msi irq.
- Fixed manivannan's comments.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (10):
      PCI: endpoint: Set ID and of_node for function driver
      PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
      PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check
      PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
      PCI: endpoint: pci-epf-test: Add doorbell test support
      misc: pci_endpoint_test: Add doorbell test case
      selftests: pci_endpoint: Add doorbell test case
      pci: imx6: Add helper function imx_pcie_add_lut_by_rid()
      pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode
      arm64: dts: imx95: Add msi-map for pci-ep device

 arch/arm64/boot/dts/freescale/imx95.dtsi           |   1 +
 drivers/misc/pci_endpoint_test.c                   |  82 ++++++++++++
 drivers/pci/controller/dwc/pci-imx6.c              |  25 ++--
 drivers/pci/endpoint/Makefile                      |   1 +
 drivers/pci/endpoint/functions/pci-epf-test.c      | 142 +++++++++++++++++++++
 drivers/pci/endpoint/pci-ep-msi.c                  |  90 +++++++++++++
 drivers/pci/endpoint/pci-epf-core.c                |  48 +++++++
 include/linux/pci-ep-msi.h                         |  28 ++++
 include/linux/pci-epf.h                            |  21 +++
 include/uapi/linux/pcitest.h                       |   1 +
 .../selftests/pci_endpoint/pci_endpoint_test.c     |  28 ++++
 11 files changed, 459 insertions(+), 8 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20241010-ep-msi-8b4cab33b1be

Best regards,
---
Frank Li <Frank.Li@nxp.com>

[PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

Set device ID as 'vfunc_no << 3 | func_no' and use
'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
device.

Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
functions use the EPC's device node, but they should use the EPF one.
For multiple function drivers, IOMMU/MSI should be different for each
function driver.

If multiple function devices share the same EPC device, there will be
no isolation between them. Setting the ID and 'of_node' prepares for
proper support.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- none

change from v13 to v14
new patch
---
 drivers/pci/endpoint/pci-epf-core.c | 4 ++++
 include/linux/pci-epf.h             | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 577a9e490115c..95fb3d7c1d45e 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -120,12 +120,16 @@ int pci_epf_bind(struct pci_epf *epf)
 		epf_vf->sec_epc_func_no = epf->sec_epc_func_no;
 		epf_vf->epc = epf->epc;
 		epf_vf->sec_epc = epf->sec_epc;
+		epf_vf->dev.id = PCI_EPF_DEVID(epf->func_no, vfunc_no);
+		device_set_of_node_from_dev(&epf_vf->dev, epc->dev.parent);
 		ret = epf_vf->driver->ops->bind(epf_vf);
 		if (ret)
 			goto ret;
 		epf_vf->is_bound = true;
 	}
 
+	epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0);
+	device_set_of_node_from_dev(&epf->dev, epc->dev.parent);
 	ret = epf->driver->ops->bind(epf);
 	if (ret)
 		goto ret;
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 749cee0bcf2cc..c0864935c6864 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -216,6 +216,8 @@ static inline void *epf_get_drvdata(struct pci_epf *epf)
 	return dev_get_drvdata(&epf->dev);
 }
 
+#define PCI_EPF_DEVID(func_no, vfunc_no) ((vfunc_no) << 3 | (func_no))
+
 struct pci_epf *pci_epf_create(const char *name);
 void pci_epf_destroy(struct pci_epf *epf);
 int __pci_epf_register_driver(struct pci_epf_driver *driver,

-- 
2.34.1

[PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[Frank Li]

Doorbell feature is implemented by mapping the EP's MSI interrupt
controller message address to a dedicated BAR in the EPC core. It is the
responsibility of the EPF driver to pass the actual message data to be
written by the host to the doorbell BAR region through its own logic.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v15 to v16
- fix rebase conflict

Change from v14 to v15
- check CONFIG_GENERIC_MSI

Fix below build error
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502082204.6PRR3cfG-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
>> drivers/pci/endpoint/pci-ep-msi.c:53:15: error: implicit declaration of function 'platform_device_msi_init_and_alloc_irqs' [-Werror=implicit-function-declaration]
      53 |         ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);

| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502082242.pOq1hB1d-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
>> drivers/pci/endpoint/pci-ep-msi.c:49:14: error: implicit declaration of function 'irq_domain_is_msi_immutable'; did you mean 'irq_domain_is_msi_device'? [-Werror=implicit-function-declaration]
      49 |         if (!irq_domain_is_msi_immutable(dom)) {

Change from v13 to v14
- basic roll back to v9
- use device:id as msi-map input, its will handle it
- use existed platform_device_msi_init_and_alloc_irqs()
- pass down epf->dev point, because epf->dev of-node will be the same as
epc's parent.

Change from v12 to v13
- Use DOMAIN_BUS_DEVICE_PCI_EP_MSI

Change from v10 to v12
- none

Change from v9 to v10
- Create msi domain for each function device.
- Remove only function support limiation. My hardware only support one
function, so not test more than one case.
- use "msi-map" descript msi information

  msi-map = <func_no << 8  | vfunc_no, &its, start_stream_id,  size>;

Chagne from v8 to v9
- sort header file
- use pci_epc_get(dev_name(msi_desc_to_dev(desc)));
- check epf number at pci_epf_alloc_doorbell
- Add comments for miss msi-parent case

change from v5 to v8
-none

Change from v4 to v5
- Remove request_irq() in pci_epc_alloc_doorbell() and leave to EP function
driver, so ep function driver can register differece call back function for
difference doorbell events and set irq affinity to differece CPU core.
- Improve error message when MSI allocate failure.

Change from v3 to v4
- msi change to use msi_get_virq() avoid use msi_for_each_desc().
- add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
- move mutex lock to epc function
- initialize variable at declear place.
- passdown epf to epc*() function to simplify code.
---
 drivers/pci/endpoint/Makefile     |  1 +
 drivers/pci/endpoint/pci-ep-msi.c | 82 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci-ep-msi.h        | 28 +++++++++++++
 include/linux/pci-epf.h           | 16 ++++++++
 4 files changed, 127 insertions(+)

diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
index 95b2fe47e3b06..c502ea7ef367c 100644
--- a/drivers/pci/endpoint/Makefile
+++ b/drivers/pci/endpoint/Makefile
@@ -6,3 +6,4 @@
 obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS)	+= pci-ep-cfs.o
 obj-$(CONFIG_PCI_ENDPOINT)		+= pci-epc-core.o pci-epf-core.o\
 					   pci-epc-mem.o functions/
+obj-$(CONFIG_GENERIC_MSI_IRQ)		+= pci-ep-msi.o
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
new file mode 100644
index 0000000000000..549b55b864d0e
--- /dev/null
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCI Endpoint *Controller* (EPC) MSI library
+ *
+ * Copyright (C) 2025 NXP
+ * Author: Frank Li <Frank.Li@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_irq.h>
+#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
+#include <linux/pci-ep-cfs.h>
+#include <linux/pci-ep-msi.h>
+#include <linux/slab.h>
+
+static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct pci_epf *epf = to_pci_epf(desc->dev);
+
+	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
+		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));
+}
+
+int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
+{
+	struct pci_epc *epc = epf->epc;
+	struct device *dev = &epf->dev;
+	struct irq_domain *dom;
+	void *msg;
+	u32 rid;
+	int ret;
+	int i;
+
+	rid = PCI_EPF_DEVID(epf->func_no, epf->vfunc_no);
+	dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_MSI);
+	if (!dom) {
+		dev_err(dev, "Can't find msi domain\n");
+		return -EINVAL;
+	}
+
+	dev_set_msi_domain(dev, dom);
+
+	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	epf->num_db = num_db;
+	epf->db_msg = msg;
+
+	ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);
+	if (ret) {
+		/*
+		 * The pcie_ep DT node has to specify 'msi-parent' for EP
+		 * doorbell support to work. Right now only GIC ITS is
+		 * supported. If you have GIC ITS and reached this print,
+		 * perhaps you are missing 'msi-map' in DT.
+		 */
+		dev_err(dev, "Failed to allocate MSI\n");
+		kfree(msg);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_db; i++)
+		epf->db_msg[i].virq = msi_get_virq(dev, i);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
+
+void pci_epf_free_doorbell(struct pci_epf *epf)
+{
+	platform_device_msi_free_irqs_all(&epf->dev);
+
+	kfree(epf->db_msg);
+	epf->db_msg = NULL;
+	epf->num_db = 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
new file mode 100644
index 0000000000000..6dfbe9353f0d8
--- /dev/null
+++ b/include/linux/pci-ep-msi.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PCI Endpoint *Function* side MSI header file
+ *
+ * Copyright (C) 2024 NXP
+ * Author: Frank Li <Frank.Li@nxp.com>
+ */
+
+#ifndef __PCI_EP_MSI__
+#define __PCI_EP_MSI__
+
+struct pci_epf;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ
+int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums);
+void pci_epf_free_doorbell(struct pci_epf *epf);
+#else
+static inline int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums)
+{
+	return -EINVAL;
+}
+
+static inline void pci_epf_free_doorbell(struct pci_epf *epf)
+{
+}
+#endif /* CONFIG_GENERIC_MSI_IRQ */
+
+#endif /* __PCI_EP_MSI__ */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index c0864935c6864..9634bf2c1ac06 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -12,6 +12,7 @@
 #include <linux/configfs.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/msi.h>
 #include <linux/pci.h>
 
 struct pci_epf;
@@ -128,6 +129,17 @@ struct pci_epf_bar {
 	int		flags;
 };
 
+/**
+ * struct pci_epf_doorbell_msg - represents doorbell message
+ * @msi_msg: MSI message
+ * @virq: irq number of this doorbell MSI message
+ * @name: irq name for doorbell interrupt
+ */
+struct pci_epf_doorbell_msg {
+	struct msi_msg msg;
+	int virq;
+};
+
 /**
  * struct pci_epf - represents the PCI EPF device
  * @dev: the PCI EPF device
@@ -155,6 +167,8 @@ struct pci_epf_bar {
  * @vfunction_num_map: bitmap to manage virtual function number
  * @pci_vepf: list of virtual endpoint functions associated with this function
  * @event_ops: callbacks for capturing the EPC events
+ * @db_msg: data for MSI from RC side
+ * @num_db: number of doorbells
  */
 struct pci_epf {
 	struct device		dev;
@@ -185,6 +199,8 @@ struct pci_epf {
 	unsigned long		vfunction_num_map;
 	struct list_head	pci_vepf;
 	const struct pci_epc_event_ops *event_ops;
+	struct pci_epf_doorbell_msg *db_msg;
+	u16 num_db;
 };
 
 /**

-- 
2.34.1

[PATCH v19 03/10] PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check

[Frank Li]

Some MSI controller change address/data pair when irq_set_affinity().
Current PCI endpoint can't support this type MSI controller. Call
irq_domain_is_msi_immutable() check if address/data pair immutable.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v18
- update commit message. remove 'include/linux/msi.h' part.

change from v14 to v17
- none

change from  v13 to v14
- bring v10 back

Change from v9 to v10
- new patch
---
 drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
index 549b55b864d0e..c0e2d806ee658 100644
--- a/drivers/pci/endpoint/pci-ep-msi.c
+++ b/drivers/pci/endpoint/pci-ep-msi.c
@@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
 
 	dev_set_msi_domain(dev, dom);
 
+	if (!irq_domain_is_msi_parent(dom))
+		return -EINVAL;
+
+	if (!irq_domain_is_msi_immutable(dom)) {
+		dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
+		return -EINVAL;
+	}
+
 	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;

-- 
2.34.1

[PATCH v19 04/10] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment

[Frank Li]

Introduce the helper function pci_epf_align_inbound_addr() to adjust
addresses according to PCI BAR alignment requirements, converting addresses
into base and offset values.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v15 to v16
- none

Change from v14 to v15
- change out address type to dma_addr_t to fix below build issue

| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502082311.G1hWGggF-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/endpoint/functions/pci-epf-test.c: In function 'pci_epf_test_enable_doorbell':
>> drivers/pci/endpoint/functions/pci-epf-test.c:726:42: error: passing argument 4 of 'pci_epf_align_inbound_addr' from incompatible pointer type [-Werror=incompatible-pointer-types]
     726 |                                          &epf_test->db_bar.phys_addr, &offset);
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                          |
         |                                          dma_addr_t * {aka unsigned int *}
   In file included from include/linux/pci-epc.h:12,

Change form v9 to v14
- none

change from v8 to v9
- pci_epf_align_inbound_addr(), base and off must be not NULL
- rm pci_epf_align_inbound_addr_lo_hi()

change from v7 to v8
- change name to pci_epf_align_inbound_addr()
- update comment said only need for memory, which not allocated by
pci_epf_alloc_space().

change from v6 to v7
- new patch
---
 drivers/pci/endpoint/pci-epf-core.c | 44 +++++++++++++++++++++++++++++++++++++
 include/linux/pci-epf.h             |  3 +++
 2 files changed, 47 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 95fb3d7c1d45e..33e14a6b1549a 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -481,6 +481,50 @@ struct pci_epf *pci_epf_create(const char *name)
 }
 EXPORT_SYMBOL_GPL(pci_epf_create);
 
+/**
+ * pci_epf_align_inbound_addr() - Get base address and offset that match BAR's
+ *			  alignment requirement
+ * @epf: the EPF device
+ * @addr: the address of the memory
+ * @bar: the BAR number corresponding to map addr
+ * @base: return base address, which match BAR's alignment requirement.
+ * @off: return offset.
+ *
+ * Helper function to convert input 'addr' to base and offset, which match
+ * BAR's alignment requirement.
+ *
+ * The pci_epf_alloc_space() function already accounts for alignment. This is
+ * primarily intended for use with other memory regions not allocated by
+ * pci_epf_alloc_space(), such as peripheral register spaces or the trigger
+ * address for a platform MSI controller.
+ */
+int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
+			       u64 addr, dma_addr_t *base, size_t *off)
+{
+	const struct pci_epc_features *epc_features;
+	u64 align;
+
+	if (!base || !off)
+		return -EINVAL;
+
+	epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
+	if (!epc_features) {
+		dev_err(&epf->dev, "epc_features not implemented\n");
+		return -EOPNOTSUPP;
+	}
+
+	align = epc_features->align;
+	align = align ? align : 128;
+	if (epc_features->bar[bar].type == BAR_FIXED)
+		align = max(epc_features->bar[bar].fixed_size, align);
+
+	*base = round_down(addr, align);
+	*off = addr & (align - 1);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_align_inbound_addr);
+
 static void pci_epf_dev_release(struct device *dev)
 {
 	struct pci_epf *epf = to_pci_epf(dev);
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 9634bf2c1ac06..91791480f0488 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -244,6 +244,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 			  enum pci_epc_interface_type type);
 void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
+
+int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
+			       u64 addr, dma_addr_t *base, size_t *off);
 int pci_epf_bind(struct pci_epf *epf);
 void pci_epf_unbind(struct pci_epf *epf);
 int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);

-- 
2.34.1

[PATCH v19 05/10] PCI: endpoint: pci-epf-test: Add doorbell test support

[Frank Li]

Add three registers: doorbell_bar, doorbell_addr, and doorbell_data. Use
pci_epf_alloc_doorbell() to allocate a doorbell address space.

Enable the Root Complex (RC) side driver to trigger pci-epc-test's doorbell
callback handler by writing doorbell_data to the mapped doorbell_bar's
address space.

Set STATUS_DOORBELL_SUCCESS in the doorbell callback to indicate
completion.

Avoid breaking compatibility between host and endpoint, add new command
COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL. Host side need send
COMMAND_ENABLE_DOORBELL to map one bar's inbound address to MSI space.
the command COMMAND_DISABLE_DOORBELL to recovery original inbound address
mapping.

	 	Host side new driver	Host side old driver

EP: new driver      S				F
EP: old driver      F				F

S: If EP side support MSI, 'pci_endpoint_test -f pcie_ep_doorbell' return
success.
   If EP side doesn't support MSI, the same to 'F'.

F: 'pci_endpoint_test -f pcie_ep_doorbell' return failure, other case as
usual.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v15 to v16
- use le32 for doorbell_* register and use cpu_to_le32() and le32_to_cpu()
when use it.

change from v14 to v15
- none

Change from v9 to v14
- update commit message by use pci_endpoint_test -f pcie_ep_doorbell

Change from v8 to v9
- move pci_epf_alloc_doorbell() into pci_epf_{enable/disable}_doorbell().
- remove doorbell_done in commit message.
- rename pci_epf_{enable/disable}_doorbell() to
pci_epf_test_{enable/disable}_doorbell() to align corrent code style.

Change from v7 to v8
- rename to pci_epf_align_inbound_addr_lo_hi()

Change from v6 to v7
- use help function pci_epf_align_addr_lo_hi()

Change from v5 to v6
- rename doorbell_addr to doorbell_offset

Chagne from v4 to v5
- Add doorbell free at unbind function.
- Move msi irq handler to here to more complex user case, such as differece
doorbell can use difference handler function.
- Add Niklas's code to handle fixed bar's case. If need add your signed-off
tag or co-developer tag, please let me know.

change from v3 to v4
- remove revid requirement
- Add command COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
- call pci_epc_set_bar() to map inbound address to MSI space only at
COMMAND_ENABLE_DOORBELL.
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 142 ++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 50eb4106369f4..b9cb1ab218f2b 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -11,12 +11,14 @@
 #include <linux/dmaengine.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/msi.h>
 #include <linux/slab.h>
 #include <linux/pci_ids.h>
 #include <linux/random.h>
 
 #include <linux/pci-epc.h>
 #include <linux/pci-epf.h>
+#include <linux/pci-ep-msi.h>
 #include <linux/pci_regs.h>
 
 #define IRQ_TYPE_INTX			0
@@ -29,6 +31,8 @@
 #define COMMAND_READ			BIT(3)
 #define COMMAND_WRITE			BIT(4)
 #define COMMAND_COPY			BIT(5)
+#define COMMAND_ENABLE_DOORBELL		BIT(6)
+#define COMMAND_DISABLE_DOORBELL	BIT(7)
 
 #define STATUS_READ_SUCCESS		BIT(0)
 #define STATUS_READ_FAIL		BIT(1)
@@ -39,6 +43,11 @@
 #define STATUS_IRQ_RAISED		BIT(6)
 #define STATUS_SRC_ADDR_INVALID		BIT(7)
 #define STATUS_DST_ADDR_INVALID		BIT(8)
+#define STATUS_DOORBELL_SUCCESS		BIT(9)
+#define STATUS_DOORBELL_ENABLE_SUCCESS	BIT(10)
+#define STATUS_DOORBELL_ENABLE_FAIL	BIT(11)
+#define STATUS_DOORBELL_DISABLE_SUCCESS BIT(12)
+#define STATUS_DOORBELL_DISABLE_FAIL	BIT(13)
 
 #define FLAG_USE_DMA			BIT(0)
 
@@ -66,6 +75,7 @@ struct pci_epf_test {
 	bool			dma_supported;
 	bool			dma_private;
 	const struct pci_epc_features *epc_features;
+	struct pci_epf_bar	db_bar;
 };
 
 struct pci_epf_test_reg {
@@ -80,6 +90,9 @@ struct pci_epf_test_reg {
 	__le32 irq_number;
 	__le32 flags;
 	__le32 caps;
+	__le32 doorbell_bar;
+	__le32 doorbell_offset;
+	__le32 doorbell_data;
 } __packed;
 
 static struct pci_epf_header test_header = {
@@ -667,6 +680,126 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 	}
 }
 
+static irqreturn_t pci_epf_test_doorbell_handler(int irq, void *data)
+{
+	struct pci_epf_test *epf_test = data;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	u32 status = le32_to_cpu(reg->status);
+
+	status |= STATUS_DOORBELL_SUCCESS;
+	reg->status = cpu_to_le32(status);
+	pci_epf_test_raise_irq(epf_test, reg);
+
+	return IRQ_HANDLED;
+}
+
+static void pci_epf_test_doorbell_cleanup(struct pci_epf_test *epf_test)
+{
+	struct pci_epf_test_reg *reg = epf_test->reg[epf_test->test_reg_bar];
+	struct pci_epf *epf = epf_test->epf;
+
+	if (le32_to_cpu(reg->doorbell_bar) > 0) {
+		free_irq(epf->db_msg[0].virq, epf_test);
+		reg->doorbell_bar = cpu_to_le32(NO_BAR);
+	}
+
+	if (epf->db_msg)
+		pci_epf_free_doorbell(epf);
+}
+
+static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
+					 struct pci_epf_test_reg *reg)
+{
+	u32 status = le32_to_cpu(reg->status);
+	struct pci_epf *epf = epf_test->epf;
+	struct pci_epc *epc = epf->epc;
+	struct msi_msg *msg;
+	enum pci_barno bar;
+	size_t offset;
+	int ret;
+
+	ret = pci_epf_alloc_doorbell(epf, 1);
+	if (ret) {
+		status |= STATUS_DOORBELL_ENABLE_FAIL;
+		goto set_status;
+	}
+
+	msg = &epf->db_msg[0].msg;
+	bar = pci_epc_get_next_free_bar(epf_test->epc_features, epf_test->test_reg_bar + 1);
+	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
+		status |= STATUS_DOORBELL_ENABLE_FAIL;
+		goto set_status;
+	}
+
+	ret = request_irq(epf->db_msg[0].virq, pci_epf_test_doorbell_handler, 0,
+			  "pci-test-doorbell", epf_test);
+	if (ret) {
+		dev_err(&epf->dev,
+			"Failed to request irq %d, doorbell feature is not supported\n",
+			epf->db_msg[0].virq);
+		status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+		goto set_status;
+	}
+
+	reg->doorbell_data = cpu_to_le32(msg->data);
+	reg->doorbell_bar = cpu_to_le32(bar);
+
+	msg = &epf->db_msg[0].msg;
+	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
+					 &epf_test->db_bar.phys_addr, &offset);
+
+	if (ret) {
+		status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+		goto set_status;
+	}
+
+	reg->doorbell_offset = cpu_to_le32(offset);
+
+	epf_test->db_bar.barno = bar;
+	epf_test->db_bar.size = epf->bar[bar].size;
+	epf_test->db_bar.flags = epf->bar[bar].flags;
+
+	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar);
+	if (ret) {
+		status |= STATUS_DOORBELL_ENABLE_FAIL;
+		pci_epf_test_doorbell_cleanup(epf_test);
+	} else {
+		status |= STATUS_DOORBELL_ENABLE_SUCCESS;
+	}
+
+set_status:
+	reg->status = cpu_to_le32(status);
+}
+
+static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test,
+					  struct pci_epf_test_reg *reg)
+{
+	enum pci_barno bar = le32_to_cpu(reg->doorbell_bar);
+	u32 status = le32_to_cpu(reg->status);
+	struct pci_epf *epf = epf_test->epf;
+	struct pci_epc *epc = epf->epc;
+	int ret;
+
+	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {
+		status |= STATUS_DOORBELL_DISABLE_FAIL;
+		goto set_status;
+	}
+
+	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
+	if (ret)
+		status |= STATUS_DOORBELL_DISABLE_FAIL;
+	else
+		status |= STATUS_DOORBELL_DISABLE_SUCCESS;
+
+	pci_epf_test_doorbell_cleanup(epf_test);
+
+set_status:
+	reg->status = cpu_to_le32(status);
+}
+
 static void pci_epf_test_cmd_handler(struct work_struct *work)
 {
 	u32 command;
@@ -714,6 +847,14 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 		pci_epf_test_copy(epf_test, reg);
 		pci_epf_test_raise_irq(epf_test, reg);
 		break;
+	case COMMAND_ENABLE_DOORBELL:
+		pci_epf_test_enable_doorbell(epf_test, reg);
+		pci_epf_test_raise_irq(epf_test, reg);
+		break;
+	case COMMAND_DISABLE_DOORBELL:
+		pci_epf_test_disable_doorbell(epf_test, reg);
+		pci_epf_test_raise_irq(epf_test, reg);
+		break;
 	default:
 		dev_err(dev, "Invalid command 0x%x\n", command);
 		break;
@@ -987,6 +1128,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 		pci_epf_test_clean_dma_chan(epf_test);
 		pci_epf_test_clear_bar(epf);
 	}
+	pci_epf_test_doorbell_cleanup(epf_test);
 	pci_epf_test_free_space(epf);
 }
 

-- 
2.34.1

[PATCH v19 06/10] misc: pci_endpoint_test: Add doorbell test case

[Frank Li]

Add three registers: PCIE_ENDPOINT_TEST_DB_BAR, PCIE_ENDPOINT_TEST_DB_ADDR,
and PCIE_ENDPOINT_TEST_DB_DATA.

Trigger the doorbell by writing data from PCI_ENDPOINT_TEST_DB_DATA to the
address provided by PCI_ENDPOINT_TEST_DB_OFFSET and wait for endpoint
feedback.

Add two command to COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL
to enable EP side's doorbell support and avoid compatible problem, which
host side driver miss-match with endpoint side function driver. See below
table:

		Host side new driver	Host side old driver
EP: new driver		S			F
EP: old driver		F			F

S: If EP side support MSI, 'pci_endpoint_test -f pcie_ep_doorbell' return
success.
   If EP side doesn't support MSI, the same to 'F'.

F: 'pci_endpoint_test -f pcie_ep_doorbell' return failure, other case as
usual.

Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- none

Change from v13 to v14
- update to use pci_endpoint_test -f pcie_ep_doorbell
- change ioctrl id to fix conflict

Change from v9 to v13
- none

Change from v8 to v9
- change PCITEST_DOORBELL to 0xa

Change form v6 to v8
- none

Change from v5 to v6
- %s/PCI_ENDPOINT_TEST_DB_ADDR/PCI_ENDPOINT_TEST_DB_OFFSET/g

Change from v4 to v5
- remove unused varible
- add irq_type at pci_endpoint_test_doorbell();

change from v3 to v4
- Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
- Remove new DID requirement.
---
 drivers/misc/pci_endpoint_test.c | 82 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/pcitest.h     |  1 +
 2 files changed, 83 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index c4e5e2c977be2..0f3af7adea107 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -37,6 +37,8 @@
 #define COMMAND_READ				BIT(3)
 #define COMMAND_WRITE				BIT(4)
 #define COMMAND_COPY				BIT(5)
+#define COMMAND_ENABLE_DOORBELL			BIT(6)
+#define COMMAND_DISABLE_DOORBELL		BIT(7)
 
 #define PCI_ENDPOINT_TEST_STATUS		0x8
 #define STATUS_READ_SUCCESS			BIT(0)
@@ -48,6 +50,11 @@
 #define STATUS_IRQ_RAISED			BIT(6)
 #define STATUS_SRC_ADDR_INVALID			BIT(7)
 #define STATUS_DST_ADDR_INVALID			BIT(8)
+#define STATUS_DOORBELL_SUCCESS			BIT(9)
+#define STATUS_DOORBELL_ENABLE_SUCCESS		BIT(10)
+#define STATUS_DOORBELL_ENABLE_FAIL		BIT(11)
+#define STATUS_DOORBELL_DISABLE_SUCCESS		BIT(12)
+#define STATUS_DOORBELL_DISABLE_FAIL		BIT(13)
 
 #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0x0c
 #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
@@ -62,6 +69,7 @@
 #define PCI_ENDPOINT_TEST_IRQ_NUMBER		0x28
 
 #define PCI_ENDPOINT_TEST_FLAGS			0x2c
+
 #define FLAG_USE_DMA				BIT(0)
 
 #define PCI_ENDPOINT_TEST_CAPS			0x30
@@ -70,6 +78,10 @@
 #define CAP_MSIX				BIT(2)
 #define CAP_INTX				BIT(3)
 
+#define PCI_ENDPOINT_TEST_DB_BAR		0x34
+#define PCI_ENDPOINT_TEST_DB_OFFSET		0x38
+#define PCI_ENDPOINT_TEST_DB_DATA		0x3c
+
 #define PCI_DEVICE_ID_TI_AM654			0xb00c
 #define PCI_DEVICE_ID_TI_J7200			0xb00f
 #define PCI_DEVICE_ID_TI_AM64			0xb010
@@ -100,6 +112,7 @@ enum pci_barno {
 	BAR_3,
 	BAR_4,
 	BAR_5,
+	NO_BAR = -1,
 };
 
 struct pci_endpoint_test {
@@ -841,6 +854,72 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
 	return 0;
 }
 
+static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
+{
+	struct pci_dev *pdev = test->pdev;
+	struct device *dev = &pdev->dev;
+	int irq_type = test->irq_type;
+	enum pci_barno bar;
+	u32 data, status;
+	u32 addr;
+
+	if (irq_type < PCITEST_IRQ_TYPE_INTX ||
+	    irq_type > PCITEST_IRQ_TYPE_MSIX) {
+		dev_err(dev, "Invalid IRQ type option\n");
+		return -EINVAL;
+	}
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
+				 COMMAND_ENABLE_DOORBELL);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+	if (status & STATUS_DOORBELL_ENABLE_FAIL) {
+		dev_err(dev, "Failed to enable doorbell\n");
+		return -EINVAL;
+	}
+
+	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
+	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET);
+	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0);
+
+	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
+
+	writel(data, test->bar[bar] + addr);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+
+	if (!(status & STATUS_DOORBELL_SUCCESS))
+		dev_err(dev, "Endpoint have not received Doorbell\n");
+
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
+				 COMMAND_DISABLE_DOORBELL);
+
+	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
+
+	status |= pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
+
+	if (status & STATUS_DOORBELL_DISABLE_FAIL) {
+		dev_err(dev, "Failed to disable doorbell\n");
+		return -EINVAL;
+	}
+
+	if (!(status & STATUS_DOORBELL_SUCCESS))
+		return -EINVAL;
+
+	return 0;
+}
+
 static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 				    unsigned long arg)
 {
@@ -891,6 +970,9 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 	case PCITEST_CLEAR_IRQ:
 		ret = pci_endpoint_test_clear_irq(test);
 		break;
+	case PCITEST_DOORBELL:
+		ret = pci_endpoint_test_doorbell(test);
+		break;
 	}
 
 ret:
diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index d3aa8715a525e..d6023a45a9d03 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -21,6 +21,7 @@
 #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
 #define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
 #define PCITEST_BARS		_IO('P', 0xa)
+#define PCITEST_DOORBELL	_IO('P', 0xb)
 #define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
 
 #define PCITEST_IRQ_TYPE_UNDEFINED	-1

-- 
2.34.1

[PATCH v19 07/10] selftests: pci_endpoint: Add doorbell test case

[Frank Li]

Add doorbell test case.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- Add set IRQ type

change from v13 to v14
- merge to selftests framework
---
 .../selftests/pci_endpoint/pci_endpoint_test.c     | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
index ac26481d29d9d..da0db0e7c9693 100644
--- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
+++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
@@ -229,4 +229,32 @@ TEST_F(pci_ep_data_transfer, COPY_TEST)
 					 test_size[i]);
 	}
 }
+
+FIXTURE(pcie_ep_doorbell)
+{
+	int fd;
+};
+
+FIXTURE_SETUP(pcie_ep_doorbell)
+{
+	self->fd = open(test_device, O_RDWR);
+
+	ASSERT_NE(-1, self->fd) TH_LOG("Can't open PCI Endpoint Test device");
+};
+
+FIXTURE_TEARDOWN(pcie_ep_doorbell)
+{
+	close(self->fd);
+};
+
+TEST_F(pcie_ep_doorbell, DOORBELL_TEST)
+{
+	int ret;
+
+	pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_AUTO);
+	ASSERT_EQ(0, ret) TH_LOG("Can't set AUTO IRQ type");
+
+	pci_ep_ioctl(PCITEST_DOORBELL, 0);
+	EXPECT_FALSE(ret) TH_LOG("Test failed for Doorbell\n");
+}
 TEST_HARNESS_MAIN

-- 
2.34.1

[PATCH v19 08/10] pci: imx6: Add helper function imx_pcie_add_lut_by_rid()

[Frank Li]

Add helper function imx_pcie_add_lut_by_rid(), which will be used for
Endpoint mode in the future. No functional change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- none

change from v13 to v14
- new patch
---
 drivers/pci/controller/dwc/pci-imx6.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 5a38cfaf989b1..032b906c44dfa 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1096,18 +1096,14 @@ static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid)
 	}
 }
 
-static int imx_pcie_enable_device(struct pci_host_bridge *bridge,
-				  struct pci_dev *pdev)
+static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
 {
-	struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata));
-	u32 sid_i, sid_m, rid = pci_dev_id(pdev);
+	struct device *dev = imx_pcie->pci->dev;
 	struct device_node *target;
-	struct device *dev;
+	u32 sid_i, sid_m;
 	int err_i, err_m;
 	u32 sid = 0;
 
-	dev = imx_pcie->pci->dev;
-
 	target = NULL;
 	err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask",
 			  &target, &sid_i);
@@ -1182,6 +1178,13 @@ static int imx_pcie_enable_device(struct pci_host_bridge *bridge,
 	return imx_pcie_add_lut(imx_pcie, rid, sid);
 }
 
+static int imx_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev)
+{
+	struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata));
+
+	return imx_pcie_add_lut_by_rid(imx_pcie, pci_dev_id(pdev));
+}
+
 static void imx_pcie_disable_device(struct pci_host_bridge *bridge,
 				    struct pci_dev *pdev)
 {

-- 
2.34.1

[PATCH v19 09/10] pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode

[Frank Li]

Support only one physical function, so call imx_pcie_add_lut_by_rid(0)
to add a single LUT entry when operating in EP mode.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- none

change from v13 to v14
- new patch
---
 drivers/pci/controller/dwc/pci-imx6.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 032b906c44dfa..3123bf49e209c 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1063,7 +1063,10 @@ static int imx_pcie_add_lut(struct imx_pcie *imx_pcie, u16 rid, u8 sid)
 	data1 |= IMX95_PE0_LUT_VLD;
 	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, data1);
 
-	data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
+	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE)
+		data2 = 0x7; /* EP side's RID from RC, only 'D' is meansful */
+	else
+		data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
 	data2 |= FIELD_PREP(IMX95_PE0_LUT_REQID, rid);
 	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, data2);
 
@@ -1767,6 +1770,9 @@ static int imx_pcie_probe(struct platform_device *pdev)
 		ret = imx_add_pcie_ep(imx_pcie, pdev);
 		if (ret < 0)
 			return ret;
+
+		/* Only support one physical function */
+		imx_pcie_add_lut_by_rid(imx_pcie, 0);
 	} else {
 		pci->pp.use_atu_msg = true;
 		ret = dw_pcie_host_init(&pci->pp);

-- 
2.34.1

[PATCH v19 10/10] arm64: dts: imx95: Add msi-map for pci-ep device

[Frank Li]

Add msi-map for pci-ep device.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v14 to v16
- none

change from v13 to v14
- new patch
---
 arch/arm64/boot/dts/freescale/imx95.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 632631a291122..c59d11eb7a581 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -1797,6 +1797,7 @@ pcie1_ep: pcie-ep@4c380000 {
 			assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
 			assigned-clock-parents = <0>, <0>,
 						 <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
+			msi-map = <0x0 &its 0x98 0x1>;
 			power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
 			status = "disabled";
 		};

-- 
2.34.1

Re: [PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[kernel test robot]

Hi Frank,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494]

url:    https://github.com/intel-lab-lkp/linux/commits/Frank-Li/PCI-endpoint-Set-ID-and-of_node-for-function-driver/20250610-003848
base:   19272b37aa4f83ca52bdf9c16d5d81bdd1354494
patch link:    https://lore.kernel.org/r/20250609-ep-msi-v19-2-77362eaa48fa%40nxp.com
patch subject: [PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20250610/202506101649.UpwblcVd-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250610/202506101649.UpwblcVd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506101649.UpwblcVd-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/wireless/st/cw1200/main.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/st/cw1200/pm.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wl1251/main.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/acx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/cmd.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/debugfs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/event.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/io.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/main.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/scan.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wireless/ti/wlcore/tx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/net/wwan/wwan_core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/fdp/fdp.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/mei_phy.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/microread/microread.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/nfcmrvl/main.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/nxp-nci/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/nxp-nci/firmware.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/pn533/pn533.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/pn544/pn544.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/s3fwrn5/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/s3fwrn5/phy_common.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st-nci/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st-nci/ndlc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st-nci/se.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st-nci/vendor_cmds.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st21nfca/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st21nfca/dep.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st21nfca/se.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st21nfca/vendor_cmds.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nfc/st95hf/spi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/ntb/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/ntb/msi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nubus/bus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nubus/nubus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/btt.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/btt_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/bus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/claim.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/dax_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/dimm_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/namespace_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/nd_perf.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/nd_virtio.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/pfn_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvdimm/region_devs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/common/auth.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/common/keyring.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/auth.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/constants.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/fabrics.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/fault_inject.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/fc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/multipath.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/host/sysfs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/target/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvme/target/fc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/nvmem/layouts.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/address.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/base.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/cpu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/device.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/dynamic.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/fdt.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/irq.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/module.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/of_kunit_helpers.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/of_reserved_mem.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/overlay.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/platform.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/property.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/of/resolver.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parisc/eisa.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parisc/gsc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parisc/iosapic.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parisc/sba_iommu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parport/ieee1284.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parport/ieee1284_ops.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parport/parport_pc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/parport/share.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/access.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/bus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/cadence/pcie-cadence-ep.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/cadence/pcie-cadence-host.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/cadence/pcie-cadence.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/dwc/pcie-designware-ep.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/dwc/pcie-designware-host.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/dwc/pcie-designware.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/dwc/pcie-qcom-common.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/pci-host-common.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/pci-hyperv-intf.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/pcie-iproc-msi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/pcie-iproc.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/pcie-rockchip.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/controller/plda/pcie-plda-host.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/devres.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/doe.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/ecam.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/endpoint/pci-ep-cfs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
>> drivers/pci/endpoint/pci-ep-msi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/endpoint/pci-epc-core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/endpoint/pci-epc-mem.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/endpoint/pci-epf-core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/host-bridge.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/acpi_pcihp.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/acpiphp_core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/cpci_hotplug_core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/pci_hotplug_core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/pnv_php.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/rpaphp_core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/hotplug/rpaphp_slot.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/msi/irqdomain.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/of.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/p2pdma.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pci-acpi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pci-bridge-emul.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pci-driver.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pci.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pcie/aer.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pcie/aspm.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pcie/portdrv.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/pcie/ptm.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/probe.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/remove.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/search.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/setup-bus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/slot.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/switch/switchtec.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/tph.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pci/vgaarb.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/cistpl.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/cs.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/ds.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/max1600.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/pcmcia_cis.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/pcmcia_resource.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/pxa2xx_base.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/rsrc_iodyn.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/rsrc_mgr.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/rsrc_nonstatic.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/sa11xx_base.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pcmcia/soc_common.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/peci/cpu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/peci/device.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/perf/arm_cspmu/arm_cspmu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/perf/hisilicon/hisi_uncore_pmu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/perf/riscv_pmu_sbi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/allwinner/phy-sun4i-usb.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/tegra/xusb-tegra124.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/tegra/xusb-tegra186.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/tegra/xusb-tegra210.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/tegra/xusb.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/ti/phy-omap-control.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/phy/ti/phy-omap-usb2.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/devicetree.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/freescale/pinctrl-imx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/freescale/pinctrl-scu.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/intel/pinctrl-intel.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/mediatek/mtk-eint.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/mediatek/pinctrl-paris.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/meson/pinctrl-meson.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/meson/pinctrl-meson8-pmx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/pinconf-generic.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/pinmux.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/pxa/pinctrl-pxa2xx.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/qcom/pinctrl-msm.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/realtek/pinctrl-rtd.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/sprd/pinctrl-sprd.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/pinctrl/tegra/pinctrl-tegra-xusb.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/arm64/huawei-gaokun-ec.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/arm64/lenovo-yoga-c630.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/cros_ec.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/cros_ec_lpc_mec.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/cros_ec_proto.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/cros_ec_sensorhub_ring.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/cros_usbpd_notify.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/chrome/wilco_ec/mailbox.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/cznic/turris-omnia-mcu-base.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/cznic/turris-signing-key.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/olpc/olpc-ec.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/surface/aggregator/bus.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/surface/aggregator/controller.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/surface/aggregator/core.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/surface/aggregator/ssh_packet_layer.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/surface/surface_acpi_notify.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/amd/hsmp/hsmp.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/amd/hsmp/hwmon.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/amd/wbrf.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/asus-wmi.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/dell/dcdbas.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/dell/dell-rbtn.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/dell/dell-smbios-base.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/dell/dell-wmi-descriptor.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/dell/dell-wmi-privacy.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/firmware_attributes_class.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
   drivers/platform/x86/ideapad-laptop.c: warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> Set device ID as 'vfunc_no << 3 | func_no' and use
> 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> device.
> 
> Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> functions use the EPC's device node, but they should use the EPF one.
> For multiple function drivers, IOMMU/MSI should be different for each
> function driver.
> 

We don't define OF node for any function, so device_set_of_node_from_dev() also
ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
I don't understand.

> If multiple function devices share the same EPC device, there will be
> no isolation between them. Setting the ID and 'of_node' prepares for
> proper support.
> 

I don't know who you can provide *isolation* by reusing the EPC OF node. It is
same as using the EPC node directly.

- Mani

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v16
> - none
> 
> change from v13 to v14
> new patch
> ---
>  drivers/pci/endpoint/pci-epf-core.c | 4 ++++
>  include/linux/pci-epf.h             | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 577a9e490115c..95fb3d7c1d45e 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -120,12 +120,16 @@ int pci_epf_bind(struct pci_epf *epf)
>  		epf_vf->sec_epc_func_no = epf->sec_epc_func_no;
>  		epf_vf->epc = epf->epc;
>  		epf_vf->sec_epc = epf->sec_epc;
> +		epf_vf->dev.id = PCI_EPF_DEVID(epf->func_no, vfunc_no);
> +		device_set_of_node_from_dev(&epf_vf->dev, epc->dev.parent);
>  		ret = epf_vf->driver->ops->bind(epf_vf);
>  		if (ret)
>  			goto ret;
>  		epf_vf->is_bound = true;
>  	}
>  
> +	epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0);
> +	device_set_of_node_from_dev(&epf->dev, epc->dev.parent);
>  	ret = epf->driver->ops->bind(epf);
>  	if (ret)
>  		goto ret;
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 749cee0bcf2cc..c0864935c6864 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -216,6 +216,8 @@ static inline void *epf_get_drvdata(struct pci_epf *epf)
>  	return dev_get_drvdata(&epf->dev);
>  }
>  
> +#define PCI_EPF_DEVID(func_no, vfunc_no) ((vfunc_no) << 3 | (func_no))
> +
>  struct pci_epf *pci_epf_create(const char *name);
>  void pci_epf_destroy(struct pci_epf *epf);
>  int __pci_epf_register_driver(struct pci_epf_driver *driver,
> 
> -- 
> 2.34.1
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:14PM GMT, Frank Li wrote:
> Doorbell feature is implemented by mapping the EP's MSI interrupt
> controller message address to a dedicated BAR in the EPC core. It is the
> responsibility of the EPF driver to pass the actual message data to be
> written by the host to the doorbell BAR region through its own logic.
> 
> Tested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v15 to v16
> - fix rebase conflict
> 
> Change from v14 to v15
> - check CONFIG_GENERIC_MSI
> 
> Fix below build error
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202502082204.6PRR3cfG-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
> >> drivers/pci/endpoint/pci-ep-msi.c:53:15: error: implicit declaration of function 'platform_device_msi_init_and_alloc_irqs' [-Werror=implicit-function-declaration]
>       53 |         ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);
> 
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202502082242.pOq1hB1d-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
> >> drivers/pci/endpoint/pci-ep-msi.c:49:14: error: implicit declaration of function 'irq_domain_is_msi_immutable'; did you mean 'irq_domain_is_msi_device'? [-Werror=implicit-function-declaration]
>       49 |         if (!irq_domain_is_msi_immutable(dom)) {
> 
> Change from v13 to v14
> - basic roll back to v9
> - use device:id as msi-map input, its will handle it
> - use existed platform_device_msi_init_and_alloc_irqs()
> - pass down epf->dev point, because epf->dev of-node will be the same as
> epc's parent.
> 
> Change from v12 to v13
> - Use DOMAIN_BUS_DEVICE_PCI_EP_MSI
> 
> Change from v10 to v12
> - none
> 
> Change from v9 to v10
> - Create msi domain for each function device.
> - Remove only function support limiation. My hardware only support one
> function, so not test more than one case.
> - use "msi-map" descript msi information
> 
>   msi-map = <func_no << 8  | vfunc_no, &its, start_stream_id,  size>;
> 
> Chagne from v8 to v9
> - sort header file
> - use pci_epc_get(dev_name(msi_desc_to_dev(desc)));
> - check epf number at pci_epf_alloc_doorbell
> - Add comments for miss msi-parent case
> 
> change from v5 to v8
> -none
> 
> Change from v4 to v5
> - Remove request_irq() in pci_epc_alloc_doorbell() and leave to EP function
> driver, so ep function driver can register differece call back function for
> difference doorbell events and set irq affinity to differece CPU core.
> - Improve error message when MSI allocate failure.
> 
> Change from v3 to v4
> - msi change to use msi_get_virq() avoid use msi_for_each_desc().
> - add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
> - move mutex lock to epc function
> - initialize variable at declear place.
> - passdown epf to epc*() function to simplify code.
> ---
>  drivers/pci/endpoint/Makefile     |  1 +
>  drivers/pci/endpoint/pci-ep-msi.c | 82 +++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-ep-msi.h        | 28 +++++++++++++
>  include/linux/pci-epf.h           | 16 ++++++++
>  4 files changed, 127 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
> index 95b2fe47e3b06..c502ea7ef367c 100644
> --- a/drivers/pci/endpoint/Makefile
> +++ b/drivers/pci/endpoint/Makefile
> @@ -6,3 +6,4 @@
>  obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS)	+= pci-ep-cfs.o
>  obj-$(CONFIG_PCI_ENDPOINT)		+= pci-epc-core.o pci-epf-core.o\
>  					   pci-epc-mem.o functions/
> +obj-$(CONFIG_GENERIC_MSI_IRQ)		+= pci-ep-msi.o

I don't think we should build this driver for generic CONFIG_GENERIC_MSI_IRQ
Kconfig. You should create a new EP specific Kconfig and make it depend on
CONFIG_GENERIC_MSI_IRQ.

> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> new file mode 100644
> index 0000000000000..549b55b864d0e
> --- /dev/null
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCI Endpoint *Controller* (EPC) MSI library
> + *
> + * Copyright (C) 2025 NXP
> + * Author: Frank Li <Frank.Li@nxp.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/msi.h>
> +#include <linux/of_irq.h>
> +#include <linux/pci-epc.h>
> +#include <linux/pci-epf.h>
> +#include <linux/pci-ep-cfs.h>
> +#include <linux/pci-ep-msi.h>
> +#include <linux/slab.h>
> +
> +static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> +{
> +	struct pci_epf *epf = to_pci_epf(desc->dev);
> +
> +	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
> +		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));
> +}
> +
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> +{
> +	struct pci_epc *epc = epf->epc;
> +	struct device *dev = &epf->dev;
> +	struct irq_domain *dom;
> +	void *msg;
> +	u32 rid;
> +	int ret;
> +	int i;
> +
> +	rid = PCI_EPF_DEVID(epf->func_no, epf->vfunc_no);
> +	dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_MSI);
> +	if (!dom) {
> +		dev_err(dev, "Can't find msi domain\n");

s/msi/MSI

Perhaps, "Can't find MSI domain for parent device" to be more explicit that we
are searching for MSI domain in parent device?

> +		return -EINVAL;

-ENODATA

> +	}
> +
> +	dev_set_msi_domain(dev, dom);
> +
> +	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	epf->num_db = num_db;
> +	epf->db_msg = msg;
> +
> +	ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);
> +	if (ret) {
> +		/*
> +		 * The pcie_ep DT node has to specify 'msi-parent' for EP
> +		 * doorbell support to work. Right now only GIC ITS is
> +		 * supported. If you have GIC ITS and reached this print,
> +		 * perhaps you are missing 'msi-map' in DT.
> +		 */

msi-map is a generic property, no need to tie it with GIC ITS. Also, this API
could fail for different reasons. So please get rid of this comment.

> +		dev_err(dev, "Failed to allocate MSI\n");
> +		kfree(msg);
> +		return -ENOMEM;

Why are you overriding the errno and not using 'ret'?

> +	}
> +
> +	for (i = 0; i < num_db; i++)
> +		epf->db_msg[i].virq = msi_get_virq(dev, i);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
> +
> +void pci_epf_free_doorbell(struct pci_epf *epf)
> +{
> +	platform_device_msi_free_irqs_all(&epf->dev);
> +
> +	kfree(epf->db_msg);
> +	epf->db_msg = NULL;
> +	epf->num_db = 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
> diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
> new file mode 100644
> index 0000000000000..6dfbe9353f0d8
> --- /dev/null
> +++ b/include/linux/pci-ep-msi.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PCI Endpoint *Function* side MSI header file
> + *
> + * Copyright (C) 2024 NXP
> + * Author: Frank Li <Frank.Li@nxp.com>
> + */
> +
> +#ifndef __PCI_EP_MSI__
> +#define __PCI_EP_MSI__
> +
> +struct pci_epf;
> +
> +#ifdef CONFIG_GENERIC_MSI_IRQ
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums);
> +void pci_epf_free_doorbell(struct pci_epf *epf);
> +#else
> +static inline int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums)
> +{
> +	return -EINVAL;

-ENODATA

> +}
> +
> +static inline void pci_epf_free_doorbell(struct pci_epf *epf)
> +{
> +}
> +#endif /* CONFIG_GENERIC_MSI_IRQ */
> +
> +#endif /* __PCI_EP_MSI__ */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index c0864935c6864..9634bf2c1ac06 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -12,6 +12,7 @@
>  #include <linux/configfs.h>
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/msi.h>
>  #include <linux/pci.h>
>  
>  struct pci_epf;
> @@ -128,6 +129,17 @@ struct pci_epf_bar {
>  	int		flags;
>  };
>  
> +/**
> + * struct pci_epf_doorbell_msg - represents doorbell message
> + * @msi_msg: MSI message
> + * @virq: irq number of this doorbell MSI message
> + * @name: irq name for doorbell interrupt

'name' is not present in the struct.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 03/10] PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:15PM GMT, Frank Li wrote:
> Some MSI controller change address/data pair when irq_set_affinity().
> Current PCI endpoint can't support this type MSI controller. Call
> irq_domain_is_msi_immutable() check if address/data pair immutable.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v18
> - update commit message. remove 'include/linux/msi.h' part.
> 
> change from v14 to v17
> - none
> 
> change from  v13 to v14
> - bring v10 back
> 
> Change from v9 to v10
> - new patch
> ---
>  drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> index 549b55b864d0e..c0e2d806ee658 100644
> --- a/drivers/pci/endpoint/pci-ep-msi.c
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
>  
>  	dev_set_msi_domain(dev, dom);
>  
> +	if (!irq_domain_is_msi_parent(dom))
> +		return -EINVAL;

This check is not justified in commit message.

> +
> +	if (!irq_domain_is_msi_immutable(dom)) {
> +		dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
> +		return -EINVAL;

GICv3 ITS is an immutable MSI controller. From the earlier patches, I could see
that you have tested this series with ITS. How did that happen if it errors out
here?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[Manivannan Sadhasivam]

On Wed, Jul 02, 2025 at 04:54:12PM GMT, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 12:34:14PM GMT, Frank Li wrote:
> > Doorbell feature is implemented by mapping the EP's MSI interrupt
> > controller message address to a dedicated BAR in the EPC core. It is the
> > responsibility of the EPF driver to pass the actual message data to be
> > written by the host to the doorbell BAR region through its own logic.
> > 
> > Tested-by: Niklas Cassel <cassel@kernel.org>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v15 to v16
> > - fix rebase conflict
> > 
> > Change from v14 to v15
> > - check CONFIG_GENERIC_MSI
> > 
> > Fix below build error
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202502082204.6PRR3cfG-lkp@intel.com/
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
> > >> drivers/pci/endpoint/pci-ep-msi.c:53:15: error: implicit declaration of function 'platform_device_msi_init_and_alloc_irqs' [-Werror=implicit-function-declaration]
> >       53 |         ret = platform_device_msi_init_and_alloc_irqs(&epf->dev, num_db, pci_epf_write_msi_msg);
> > 
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202502082242.pOq1hB1d-lkp@intel.com/
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    drivers/pci/endpoint/pci-ep-msi.c: In function 'pci_epf_alloc_doorbell':
> > >> drivers/pci/endpoint/pci-ep-msi.c:49:14: error: implicit declaration of function 'irq_domain_is_msi_immutable'; did you mean 'irq_domain_is_msi_device'? [-Werror=implicit-function-declaration]
> >       49 |         if (!irq_domain_is_msi_immutable(dom)) {
> > 
> > Change from v13 to v14
> > - basic roll back to v9
> > - use device:id as msi-map input, its will handle it
> > - use existed platform_device_msi_init_and_alloc_irqs()
> > - pass down epf->dev point, because epf->dev of-node will be the same as
> > epc's parent.
> > 
> > Change from v12 to v13
> > - Use DOMAIN_BUS_DEVICE_PCI_EP_MSI
> > 
> > Change from v10 to v12
> > - none
> > 
> > Change from v9 to v10
> > - Create msi domain for each function device.
> > - Remove only function support limiation. My hardware only support one
> > function, so not test more than one case.
> > - use "msi-map" descript msi information
> > 
> >   msi-map = <func_no << 8  | vfunc_no, &its, start_stream_id,  size>;
> > 
> > Chagne from v8 to v9
> > - sort header file
> > - use pci_epc_get(dev_name(msi_desc_to_dev(desc)));
> > - check epf number at pci_epf_alloc_doorbell
> > - Add comments for miss msi-parent case
> > 
> > change from v5 to v8
> > -none
> > 
> > Change from v4 to v5
> > - Remove request_irq() in pci_epc_alloc_doorbell() and leave to EP function
> > driver, so ep function driver can register differece call back function for
> > difference doorbell events and set irq affinity to differece CPU core.
> > - Improve error message when MSI allocate failure.
> > 
> > Change from v3 to v4
> > - msi change to use msi_get_virq() avoid use msi_for_each_desc().
> > - add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
> > - move mutex lock to epc function
> > - initialize variable at declear place.
> > - passdown epf to epc*() function to simplify code.
> > ---
> >  drivers/pci/endpoint/Makefile     |  1 +
> >  drivers/pci/endpoint/pci-ep-msi.c | 82 +++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci-ep-msi.h        | 28 +++++++++++++
> >  include/linux/pci-epf.h           | 16 ++++++++
> >  4 files changed, 127 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
> > index 95b2fe47e3b06..c502ea7ef367c 100644
> > --- a/drivers/pci/endpoint/Makefile
> > +++ b/drivers/pci/endpoint/Makefile
> > @@ -6,3 +6,4 @@
> >  obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS)	+= pci-ep-cfs.o
> >  obj-$(CONFIG_PCI_ENDPOINT)		+= pci-epc-core.o pci-epf-core.o\
> >  					   pci-epc-mem.o functions/
> > +obj-$(CONFIG_GENERIC_MSI_IRQ)		+= pci-ep-msi.o
> 
> I don't think we should build this driver for generic CONFIG_GENERIC_MSI_IRQ
> Kconfig. You should create a new EP specific Kconfig and make it depend on
> CONFIG_GENERIC_MSI_IRQ.
> 
> > diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> > new file mode 100644
> > index 0000000000000..549b55b864d0e
> > --- /dev/null
> > +++ b/drivers/pci/endpoint/pci-ep-msi.c
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCI Endpoint *Controller* (EPC) MSI library
> > + *
> > + * Copyright (C) 2025 NXP
> > + * Author: Frank Li <Frank.Li@nxp.com>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/module.h>
> > +#include <linux/msi.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/pci-epc.h>
> > +#include <linux/pci-epf.h>
> > +#include <linux/pci-ep-cfs.h>
> > +#include <linux/pci-ep-msi.h>
> > +#include <linux/slab.h>
> > +
> > +static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> > +{
> > +	struct pci_epf *epf = to_pci_epf(desc->dev);
> > +
> > +	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
> > +		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));
> > +}
> > +
> > +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> > +{
> > +	struct pci_epc *epc = epf->epc;
> > +	struct device *dev = &epf->dev;
> > +	struct irq_domain *dom;
> > +	void *msg;
> > +	u32 rid;
> > +	int ret;
> > +	int i;
> > +
> > +	rid = PCI_EPF_DEVID(epf->func_no, epf->vfunc_no);
> > +	dom = of_msi_map_get_device_domain(epc->dev.parent, rid, DOMAIN_BUS_PLATFORM_MSI);
> > +	if (!dom) {
> > +		dev_err(dev, "Can't find msi domain\n");
> 
> s/msi/MSI
> 
> Perhaps, "Can't find MSI domain for parent device" to be more explicit that we
> are searching for MSI domain in parent device?
> 
> > +		return -EINVAL;
> 
> -ENODATA

Sorry, -ENODEV here and below.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 04/10] PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:16PM GMT, Frank Li wrote:
> Introduce the helper function pci_epf_align_inbound_addr() to adjust
> addresses according to PCI BAR alignment requirements, converting addresses
> into base and offset values.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v15 to v16
> - none
> 
> Change from v14 to v15
> - change out address type to dma_addr_t to fix below build issue
> 
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202502082311.G1hWGggF-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/pci/endpoint/functions/pci-epf-test.c: In function 'pci_epf_test_enable_doorbell':
> >> drivers/pci/endpoint/functions/pci-epf-test.c:726:42: error: passing argument 4 of 'pci_epf_align_inbound_addr' from incompatible pointer type [-Werror=incompatible-pointer-types]
>      726 |                                          &epf_test->db_bar.phys_addr, &offset);
>          |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>          |                                          |
>          |                                          dma_addr_t * {aka unsigned int *}
>    In file included from include/linux/pci-epc.h:12,
> 
> Change form v9 to v14
> - none
> 
> change from v8 to v9
> - pci_epf_align_inbound_addr(), base and off must be not NULL
> - rm pci_epf_align_inbound_addr_lo_hi()
> 
> change from v7 to v8
> - change name to pci_epf_align_inbound_addr()
> - update comment said only need for memory, which not allocated by
> pci_epf_alloc_space().
> 
> change from v6 to v7
> - new patch
> ---
>  drivers/pci/endpoint/pci-epf-core.c | 44 +++++++++++++++++++++++++++++++++++++
>  include/linux/pci-epf.h             |  3 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 95fb3d7c1d45e..33e14a6b1549a 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -481,6 +481,50 @@ struct pci_epf *pci_epf_create(const char *name)
>  }
>  EXPORT_SYMBOL_GPL(pci_epf_create);
>  
> +/**
> + * pci_epf_align_inbound_addr() - Get base address and offset that match BAR's
> + *			  alignment requirement

'Align the given address based on the BAR alignment requirement'

> + * @epf: the EPF device
> + * @addr: the address of the memory

'inbound address to be aligned'

> + * @bar: the BAR number corresponding to map addr

s/map addr/the given addr

> + * @base: return base address, which match BAR's alignment requirement.

'base address matching the @bar alignment requirement'

> + * @off: return offset.

'offset to be added to the @base address'

> + *
> + * Helper function to convert input 'addr' to base and offset, which match

s/convert/align

> + * BAR's alignment requirement.
> + *
> + * The pci_epf_alloc_space() function already accounts for alignment. This is
> + * primarily intended for use with other memory regions not allocated by
> + * pci_epf_alloc_space(), such as peripheral register spaces or the trigger
> + * address for a platform MSI controller.
> + */
> +int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
> +			       u64 addr, dma_addr_t *base, size_t *off)
> +{
> +	const struct pci_epc_features *epc_features;
> +	u64 align;
> +
> +	if (!base || !off)
> +		return -EINVAL;
> +
> +	epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
> +	if (!epc_features) {
> +		dev_err(&epf->dev, "epc_features not implemented\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	align = epc_features->align;
> +	align = align ? align : 128;

From where this 128 byte alignment comes from?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 05/10] PCI: endpoint: pci-epf-test: Add doorbell test support

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:17PM GMT, Frank Li wrote:

[...]

> +static irqreturn_t pci_epf_test_doorbell_handler(int irq, void *data)
> +{
> +	struct pci_epf_test *epf_test = data;
> +	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +	u32 status = le32_to_cpu(reg->status);
> +
> +	status |= STATUS_DOORBELL_SUCCESS;
> +	reg->status = cpu_to_le32(status);
> +	pci_epf_test_raise_irq(epf_test, reg);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void pci_epf_test_doorbell_cleanup(struct pci_epf_test *epf_test)
> +{
> +	struct pci_epf_test_reg *reg = epf_test->reg[epf_test->test_reg_bar];
> +	struct pci_epf *epf = epf_test->epf;
> +
> +	if (le32_to_cpu(reg->doorbell_bar) > 0) {

Is this check necessary?

> +		free_irq(epf->db_msg[0].virq, epf_test);
> +		reg->doorbell_bar = cpu_to_le32(NO_BAR);
> +	}
> +
> +	if (epf->db_msg)

Same here.

> +		pci_epf_free_doorbell(epf);
> +}
> +
> +static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
> +					 struct pci_epf_test_reg *reg)
> +{
> +	u32 status = le32_to_cpu(reg->status);
> +	struct pci_epf *epf = epf_test->epf;
> +	struct pci_epc *epc = epf->epc;
> +	struct msi_msg *msg;
> +	enum pci_barno bar;
> +	size_t offset;
> +	int ret;
> +
> +	ret = pci_epf_alloc_doorbell(epf, 1);
> +	if (ret) {
> +		status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		goto set_status;

I think you can just set the failure status directly in err path:

	if (ret)
		goto set_err_status;

> +	}
> +
> +	msg = &epf->db_msg[0].msg;
> +	bar = pci_epc_get_next_free_bar(epf_test->epc_features, epf_test->test_reg_bar + 1);
> +	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {

Can 'bar' really be 'epf_test->test_reg_bar' here? You just need to check for
NO_BAR, that's it.

Also 'epf->db_msg' can't be NULL here. You were already dereferencing above, so
this check is pointless.

> +		status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		goto set_status;
> +	}
> +
> +	ret = request_irq(epf->db_msg[0].virq, pci_epf_test_doorbell_handler, 0,
> +			  "pci-test-doorbell", epf_test);

'pci-ep-test-doorbell'

> +	if (ret) {
> +		dev_err(&epf->dev,
> +			"Failed to request irq %d, doorbell feature is not supported\n",

'Failed to request doorbell IRQ: %d\n'

> +			epf->db_msg[0].virq);
> +		status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		pci_epf_test_doorbell_cleanup(epf_test);

this can be moved to a err label:

		goto cleanup_doorbell;

> +		goto set_status;
> +	}
> +
> +	reg->doorbell_data = cpu_to_le32(msg->data);
> +	reg->doorbell_bar = cpu_to_le32(bar);
> +
> +	msg = &epf->db_msg[0].msg;
> +	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
> +					 &epf_test->db_bar.phys_addr, &offset);
> +
> +	if (ret) {
> +		status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		pci_epf_test_doorbell_cleanup(epf_test);
> +		goto set_status;
> +	}
> +
> +	reg->doorbell_offset = cpu_to_le32(offset);
> +
> +	epf_test->db_bar.barno = bar;
> +	epf_test->db_bar.size = epf->bar[bar].size;
> +	epf_test->db_bar.flags = epf->bar[bar].flags;
> +
> +	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar);
> +	if (ret) {
> +		status |= STATUS_DOORBELL_ENABLE_FAIL;
> +		pci_epf_test_doorbell_cleanup(epf_test);
> +	} else {
> +		status |= STATUS_DOORBELL_ENABLE_SUCCESS;
> +	}
> +

Set the success status directly here:

	status |= STATUS_DOORBELL_ENABLE_SUCCESS;
	reg->status = cpu_to_le32(status);

	return;

cleanup_doorbell:
	pci_epf_test_doorbell_cleanup(epf_test);
set_err_status:
	status |= STATUS_DOORBELL_ENABLE_FAIL;
	reg->status = cpu_to_le32(status);

> +set_status:
> +	reg->status = cpu_to_le32(status);
> +}
> +
> +static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test,
> +					  struct pci_epf_test_reg *reg)
> +{
> +	enum pci_barno bar = le32_to_cpu(reg->doorbell_bar);
> +	u32 status = le32_to_cpu(reg->status);
> +	struct pci_epf *epf = epf_test->epf;
> +	struct pci_epc *epc = epf->epc;
> +	int ret;
> +
> +	if (bar < BAR_0 || bar == epf_test->test_reg_bar || !epf->db_msg) {

Same comment about as above for these checks.

> +		status |= STATUS_DOORBELL_DISABLE_FAIL;
> +		goto set_status;
> +	}
> +
> +	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
> +	if (ret)
> +		status |= STATUS_DOORBELL_DISABLE_FAIL;
> +	else
> +		status |= STATUS_DOORBELL_DISABLE_SUCCESS;
> +
> +	pci_epf_test_doorbell_cleanup(epf_test);
> +
> +set_status:
> +	reg->status = cpu_to_le32(status);
> +}
> +
>  static void pci_epf_test_cmd_handler(struct work_struct *work)
>  {
>  	u32 command;
> @@ -714,6 +847,14 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>  		pci_epf_test_copy(epf_test, reg);
>  		pci_epf_test_raise_irq(epf_test, reg);
>  		break;
> +	case COMMAND_ENABLE_DOORBELL:
> +		pci_epf_test_enable_doorbell(epf_test, reg);
> +		pci_epf_test_raise_irq(epf_test, reg);
> +		break;
> +	case COMMAND_DISABLE_DOORBELL:
> +		pci_epf_test_disable_doorbell(epf_test, reg);
> +		pci_epf_test_raise_irq(epf_test, reg);
> +		break;
>  	default:
>  		dev_err(dev, "Invalid command 0x%x\n", command);
>  		break;
> @@ -987,6 +1128,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>  		pci_epf_test_clean_dma_chan(epf_test);
>  		pci_epf_test_clear_bar(epf);
>  	}
> +	pci_epf_test_doorbell_cleanup(epf_test);

Why is this necessary?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 06/10] misc: pci_endpoint_test: Add doorbell test case

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:18PM GMT, Frank Li wrote:
> Add three registers: PCIE_ENDPOINT_TEST_DB_BAR, PCIE_ENDPOINT_TEST_DB_ADDR,
> and PCIE_ENDPOINT_TEST_DB_DATA.
> 
> Trigger the doorbell by writing data from PCI_ENDPOINT_TEST_DB_DATA to the
> address provided by PCI_ENDPOINT_TEST_DB_OFFSET and wait for endpoint
> feedback.
> 
> Add two command to COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL
> to enable EP side's doorbell support and avoid compatible problem, which
> host side driver miss-match with endpoint side function driver. See below
> table:
> 
> 		Host side new driver	Host side old driver
> EP: new driver		S			F
> EP: old driver		F			F
> 
> S: If EP side support MSI, 'pci_endpoint_test -f pcie_ep_doorbell' return
> success.
>    If EP side doesn't support MSI, the same to 'F'.
> 
> F: 'pci_endpoint_test -f pcie_ep_doorbell' return failure, other case as
> usual.
> 
> Tested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v16
> - none
> 
> Change from v13 to v14
> - update to use pci_endpoint_test -f pcie_ep_doorbell
> - change ioctrl id to fix conflict
> 
> Change from v9 to v13
> - none
> 
> Change from v8 to v9
> - change PCITEST_DOORBELL to 0xa
> 
> Change form v6 to v8
> - none
> 
> Change from v5 to v6
> - %s/PCI_ENDPOINT_TEST_DB_ADDR/PCI_ENDPOINT_TEST_DB_OFFSET/g
> 
> Change from v4 to v5
> - remove unused varible
> - add irq_type at pci_endpoint_test_doorbell();
> 
> change from v3 to v4
> - Add COMMAND_ENABLE_DOORBELL and COMMAND_DISABLE_DOORBELL.
> - Remove new DID requirement.
> ---
>  drivers/misc/pci_endpoint_test.c | 82 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/pcitest.h     |  1 +
>  2 files changed, 83 insertions(+)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index c4e5e2c977be2..0f3af7adea107 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -37,6 +37,8 @@
>  #define COMMAND_READ				BIT(3)
>  #define COMMAND_WRITE				BIT(4)
>  #define COMMAND_COPY				BIT(5)
> +#define COMMAND_ENABLE_DOORBELL			BIT(6)
> +#define COMMAND_DISABLE_DOORBELL		BIT(7)
>  
>  #define PCI_ENDPOINT_TEST_STATUS		0x8
>  #define STATUS_READ_SUCCESS			BIT(0)
> @@ -48,6 +50,11 @@
>  #define STATUS_IRQ_RAISED			BIT(6)
>  #define STATUS_SRC_ADDR_INVALID			BIT(7)
>  #define STATUS_DST_ADDR_INVALID			BIT(8)
> +#define STATUS_DOORBELL_SUCCESS			BIT(9)
> +#define STATUS_DOORBELL_ENABLE_SUCCESS		BIT(10)
> +#define STATUS_DOORBELL_ENABLE_FAIL		BIT(11)
> +#define STATUS_DOORBELL_DISABLE_SUCCESS		BIT(12)
> +#define STATUS_DOORBELL_DISABLE_FAIL		BIT(13)
>  
>  #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0x0c
>  #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
> @@ -62,6 +69,7 @@
>  #define PCI_ENDPOINT_TEST_IRQ_NUMBER		0x28
>  
>  #define PCI_ENDPOINT_TEST_FLAGS			0x2c
> +
>  #define FLAG_USE_DMA				BIT(0)
>  
>  #define PCI_ENDPOINT_TEST_CAPS			0x30
> @@ -70,6 +78,10 @@
>  #define CAP_MSIX				BIT(2)
>  #define CAP_INTX				BIT(3)
>  
> +#define PCI_ENDPOINT_TEST_DB_BAR		0x34
> +#define PCI_ENDPOINT_TEST_DB_OFFSET		0x38
> +#define PCI_ENDPOINT_TEST_DB_DATA		0x3c
> +
>  #define PCI_DEVICE_ID_TI_AM654			0xb00c
>  #define PCI_DEVICE_ID_TI_J7200			0xb00f
>  #define PCI_DEVICE_ID_TI_AM64			0xb010
> @@ -100,6 +112,7 @@ enum pci_barno {
>  	BAR_3,
>  	BAR_4,
>  	BAR_5,
> +	NO_BAR = -1,
>  };
>  
>  struct pci_endpoint_test {
> @@ -841,6 +854,72 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
>  	return 0;
>  }
>  
> +static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
> +{
> +	struct pci_dev *pdev = test->pdev;
> +	struct device *dev = &pdev->dev;
> +	int irq_type = test->irq_type;
> +	enum pci_barno bar;
> +	u32 data, status;
> +	u32 addr;
> +
> +	if (irq_type < PCITEST_IRQ_TYPE_INTX ||
> +	    irq_type > PCITEST_IRQ_TYPE_MSIX) {
> +		dev_err(dev, "Invalid IRQ type option\n");

'Invalid IRQ type\n'

> +		return -EINVAL;
> +	}
> +
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> +				 COMMAND_ENABLE_DOORBELL);
> +
> +	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));

You should check for the timeout here and below.

> +
> +	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
> +	if (status & STATUS_DOORBELL_ENABLE_FAIL) {
> +		dev_err(dev, "Failed to enable doorbell\n");
> +		return -EINVAL;
> +	}
> +
> +	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
> +	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET);
> +	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
> +
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> +
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0);
> +
> +	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
> +
> +	writel(data, test->bar[bar] + addr);
> +
> +	wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000));
> +
> +	status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
> +
> +	if (!(status & STATUS_DOORBELL_SUCCESS))
> +		dev_err(dev, "Endpoint have not received Doorbell\n");

'Failed to trigger doorbell in endpoint\n'

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 07/10] selftests: pci_endpoint: Add doorbell test case

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:19PM GMT, Frank Li wrote:
> Add doorbell test case.
> 

This also warrants a documentation change.

- Mani

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v16
> - Add set IRQ type
> 
> change from v13 to v14
> - merge to selftests framework
> ---
>  .../selftests/pci_endpoint/pci_endpoint_test.c     | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> index ac26481d29d9d..da0db0e7c9693 100644
> --- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> +++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> @@ -229,4 +229,32 @@ TEST_F(pci_ep_data_transfer, COPY_TEST)
>  					 test_size[i]);
>  	}
>  }
> +
> +FIXTURE(pcie_ep_doorbell)
> +{
> +	int fd;
> +};
> +
> +FIXTURE_SETUP(pcie_ep_doorbell)
> +{
> +	self->fd = open(test_device, O_RDWR);
> +
> +	ASSERT_NE(-1, self->fd) TH_LOG("Can't open PCI Endpoint Test device");
> +};
> +
> +FIXTURE_TEARDOWN(pcie_ep_doorbell)
> +{
> +	close(self->fd);
> +};
> +
> +TEST_F(pcie_ep_doorbell, DOORBELL_TEST)
> +{
> +	int ret;
> +
> +	pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_AUTO);
> +	ASSERT_EQ(0, ret) TH_LOG("Can't set AUTO IRQ type");
> +
> +	pci_ep_ioctl(PCITEST_DOORBELL, 0);
> +	EXPECT_FALSE(ret) TH_LOG("Test failed for Doorbell\n");
> +}
>  TEST_HARNESS_MAIN
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 08/10] pci: imx6: Add helper function imx_pcie_add_lut_by_rid()

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:20PM GMT, Frank Li wrote:
> Add helper function imx_pcie_add_lut_by_rid(), which will be used for
> Endpoint mode in the future. No functional change.
> 

What do you mean by 'used for Endpoint mode'? Endpoint mode is already supported
by this driver. So be explicit on how the new helper is going to be used.

Also, looks like this patch is independent of the doorbell patches. So this and
9/10 should come first.

- Mani

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v16
> - none
> 
> change from v13 to v14
> - new patch
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 5a38cfaf989b1..032b906c44dfa 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1096,18 +1096,14 @@ static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid)
>  	}
>  }
>  
> -static int imx_pcie_enable_device(struct pci_host_bridge *bridge,
> -				  struct pci_dev *pdev)
> +static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
>  {
> -	struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata));
> -	u32 sid_i, sid_m, rid = pci_dev_id(pdev);
> +	struct device *dev = imx_pcie->pci->dev;
>  	struct device_node *target;
> -	struct device *dev;
> +	u32 sid_i, sid_m;
>  	int err_i, err_m;
>  	u32 sid = 0;
>  
> -	dev = imx_pcie->pci->dev;
> -
>  	target = NULL;
>  	err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask",
>  			  &target, &sid_i);
> @@ -1182,6 +1178,13 @@ static int imx_pcie_enable_device(struct pci_host_bridge *bridge,
>  	return imx_pcie_add_lut(imx_pcie, rid, sid);
>  }
>  
> +static int imx_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev)
> +{
> +	struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata));
> +
> +	return imx_pcie_add_lut_by_rid(imx_pcie, pci_dev_id(pdev));
> +}
> +
>  static void imx_pcie_disable_device(struct pci_host_bridge *bridge,
>  				    struct pci_dev *pdev)
>  {
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 09/10] pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:21PM GMT, Frank Li wrote:
> Support only one physical function, so call imx_pcie_add_lut_by_rid(0)
> to add a single LUT entry when operating in EP mode.
> 

So previously LUT config was not present and endpoint mode continued to work?
Please explain why this is necessary now.

- Mani

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v16
> - none
> 
> change from v13 to v14
> - new patch
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 032b906c44dfa..3123bf49e209c 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1063,7 +1063,10 @@ static int imx_pcie_add_lut(struct imx_pcie *imx_pcie, u16 rid, u8 sid)
>  	data1 |= IMX95_PE0_LUT_VLD;
>  	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, data1);
>  
> -	data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
> +	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE)
> +		data2 = 0x7; /* EP side's RID from RC, only 'D' is meansful */
> +	else
> +		data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
>  	data2 |= FIELD_PREP(IMX95_PE0_LUT_REQID, rid);
>  	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, data2);
>  
> @@ -1767,6 +1770,9 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  		ret = imx_add_pcie_ep(imx_pcie, pdev);
>  		if (ret < 0)
>  			return ret;
> +
> +		/* Only support one physical function */
> +		imx_pcie_add_lut_by_rid(imx_pcie, 0);
>  	} else {
>  		pci->pp.use_atu_msg = true;
>  		ret = dw_pcie_host_init(&pci->pp);
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 10/10] arm64: dts: imx95: Add msi-map for pci-ep device

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:22PM GMT, Frank Li wrote:
> Add msi-map for pci-ep device.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
> change from v14 to v16
> - none
> 
> change from v13 to v14
> - new patch
> ---
>  arch/arm64/boot/dts/freescale/imx95.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> index 632631a291122..c59d11eb7a581 100644
> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> @@ -1797,6 +1797,7 @@ pcie1_ep: pcie-ep@4c380000 {
>  			assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
>  			assigned-clock-parents = <0>, <0>,
>  						 <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> +			msi-map = <0x0 &its 0x98 0x1>;
>  			power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
>  			status = "disabled";
>  		};
> 
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 00/10] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Manivannan Sadhasivam]

On Mon, Jun 09, 2025 at 12:34:12PM GMT, Frank Li wrote:
> ┌────────────┐   ┌───────────────────────────────────┐   ┌────────────────┐
> │            │   │                                   │   │                │
> │            │   │ PCI Endpoint                      │   │ PCI Host       │
> │            │   │                                   │   │                │
> │            │◄──┤ 1.platform_msi_domain_alloc_irqs()│   │                │
> │            │   │                                   │   │                │
> │ MSI        ├──►│ 2.write_msi_msg()                 ├──►├─BAR<n>         │
> │ Controller │   │   update doorbell register address│   │                │
> │            │   │   for BAR                         │   │                │
> │            │   │                                   │   │ 3. Write BAR<n>│
> │            │◄──┼───────────────────────────────────┼───┤                │
> │            │   │                                   │   │                │
> │            ├──►│ 4.Irq Handle                      │   │                │
> │            │   │                                   │   │                │
> │            │   │                                   │   │                │
> └────────────┘   └───────────────────────────────────┘   └────────────────┘
> 
> This patches based on old https://lore.kernel.org/imx/20221124055036.1630573-1-Frank.Li@nxp.com/
> 
> Original patch only target to vntb driver. But actually it is common
> method.
> 
> This patches add new API to pci-epf-core, so any EP driver can use it.
> 

Frank, thanks for your persistence in pushing this series, really appreciated!
I've left some comments, but no real blocker.

Unfortunately, I don't have access to my endpoint setup right now. So I'll go
ahead with the Tested-by tag from Niklas once my comments are addressed.

- Mani

> Previous v2 discussion here.
> https://lore.kernel.org/imx/20230911220920.1817033-1-Frank.Li@nxp.com/
> 
> Changes in v19:
> - irq part already in v6.16-rc1, only missed pcie/dts part
> - rebase to v6.16-rc1
> - update commit message for patch IMMUTABLE check.
> - Link to v18: https://lore.kernel.org/r/20250414-ep-msi-v18-0-f69b49917464@nxp.com
> 
> Changes in v18:
> - pci-ep.yaml: sort property order, fix maxvalue to 0x7ffff for msi-map-mask and
> iommu-map-mask
> - Link to v17: https://lore.kernel.org/r/20250407-ep-msi-v17-0-633ab45a31d0@nxp.com
> 
> Changes in v17:
> - move document part to pci-ep.yaml
> - Link to v16: https://lore.kernel.org/r/20250404-ep-msi-v16-0-d4919d68c0d0@nxp.com
> 
> Changes in v16:
> - remove arm64: dts: imx95-19x19-evk: Add PCIe1 endpoint function overlay file
> because there are better patches, which under review.
> - Add document for pcie-ep msi-map usage
> - other change to see each patch's change log
> About IMMUTABLE (No change for this part, tglx provide feedback)
> > - This IMMUTABLE thing serves no purpose, because you don't randomly
> >   plug this end-point block on any MSI controller. They come as part
> >   of an SoC.
> 
> "Yes and no. The problem is that the EP implementation is meant to be a
> generic library and while GIC-ITS guarantees immutability of the
> address/data pair after setup, there are architectures (x86, loongson,
> riscv) where the base MSI controller does not and immutability is only
> achieved when interrupt remapping is enabled. The latter can be disabled
> at boot-time and then the EP implementation becomes a lottery across
> affinity changes.
> 
> That was my concern about this library implementation and that's why I
> asked for a mechanism to ensure that the underlying irqdomain provides a
> immutable address/data pair.
> 
> So it does not matter for GIC-ITS, but in the larger picture it matters.
> 
> Thanks,
> 
>         tglx
> "
> 
> So it does not matter for GIC-ITS, but in the larger picture it matters.
> 
> - Link to v15: https://lore.kernel.org/r/20250211-ep-msi-v15-0-bcacc1f2b1a9@nxp.com
> 
> Changes in v15:
> - rebase to v6.14-rc1
> - fix build issue find by kernel test robot
> - Link to v14: https://lore.kernel.org/r/20250207-ep-msi-v14-0-9671b136f2b8@nxp.com
> 
> Changes in v14:
> Marc Zyngier raised concerns about adding DOMAIN_BUS_DEVICE_PCI_EP_MSI. As
> a result, the approach has been reverted to the v9 method. However, there
> are several improvements:
> 
> MSI now supports msi-map in addition to msi-parent.
>   - The struct device: id is used as the endpoint function (EPF) device
> identity to map to the stream ID (sideband information).
>   - The EPC device tree source (DTS) utilizes msi-map to provide such
> information.
>   - The EPF device's of_node is set to the EPC controller’s node. This
> approach is commonly used for multi-function device (MFD) platform child
> devices, allowing them to inherit properties from the MFD device’s DTS,
> such as reset-cells and gpio-cells. This method is well-suited for the
> current case, as the EPF is inherently created/binded to the EPC and
> should inherit the EPC’s DTS node properties.
> 
> Additionally:
> 
> Since the basic IMX95 LUT support has already been merged into the
> mainline, a DTS and driver increment patch is added to complete the
> solution. The patch is rebased onto the latest linux-next tree and
> aligned with the new pcitest framework.
> 
> - Link to v13: https://lore.kernel.org/r/20241218-ep-msi-v13-0-646e2192dc24@nxp.com
> 
> Changes in v13:
> - Change to use DOMAIN_BUS_PCI_DEVICE_EP_MSI
> - Change request id as  func | vfunc << 3
> - Remove IRQ_DOMAIN_MSI_IMMUTABLE
> 
> Thomas Gleixner:
> 
> I hope capture all your points in review comments. If missed, let me know.
> 
> - Link to v12: https://lore.kernel.org/r/20241211-ep-msi-v12-0-33d4532fa520@nxp.com
> 
> Changes in v12:
> - Change to use IRQ_DOMAIN_MSI_IMMUTABLE and add help function
> irq_domain_msi_is_immuatble().
> - split PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check to 3 patches
> - Link to v11: https://lore.kernel.org/r/20241209-ep-msi-v11-0-7434fa8397bd@nxp.com
> 
> Changes in v11:
> - Change to use MSI_FLAG_MSG_IMMUTABLE
> - Link to v10: https://lore.kernel.org/r/20241204-ep-msi-v10-0-87c378dbcd6d@nxp.com
> 
> Changes in v10:
> 
> Thomas Gleixner:
> 	There are big change in pci-ep-msi.c. I am sure if go on the
> corrent path. The key improvement is remove only 1 function devices's
> limitation.
> 
> 	I use new patch for imutable check, which relative additional
> feature compared to base enablement patch.
> 
> - Remove patch Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
> - Add new patch irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if provided by msi_domain_info
> - Remove only support 1 endpoint function limiation.
> - Create one MSI domain for each endpoint function devices.
> - Use "msi-map" in pci ep controler node, instead of of msi-parent. first
> argument is
> 	(func_no << 8 | vfunc_no)
> 
> - Link to v9: https://lore.kernel.org/r/20241203-ep-msi-v9-0-a60dbc3f15dd@nxp.com
> 
> Changes in v9
> - Add patch platform-msi: Add msi_remove_device_irq_domain() in platform_device_msi_free_irqs_all()
> - Remove patch PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering
> - Remove API pci_epf_align_inbound_addr_lo_hi
> - Move doorbell_alloc in to doorbell_enable function.
> - Link to v8: https://lore.kernel.org/r/20241116-ep-msi-v8-0-6f1f68ffd1bb@nxp.com
> 
> Changes in v8:
> - update helper function name to pci_epf_align_inbound_addr()
> - Link to v7: https://lore.kernel.org/r/20241114-ep-msi-v7-0-d4ac7aafbd2c@nxp.com
> 
> Changes in v7:
> - Add helper function pci_epf_align_addr();
> - Link to v6: https://lore.kernel.org/r/20241112-ep-msi-v6-0-45f9722e3c2a@nxp.com
> 
> Changes in v6:
> - change doorbell_addr to doorbell_offset
> - use round_down()
> - add Niklas's test by tag
> - rebase to pci/endpoint
> - Link to v5: https://lore.kernel.org/r/20241108-ep-msi-v5-0-a14951c0d007@nxp.com
> 
> Changes in v5:
> - Move request_irq to epf test function driver for more flexiable user case
> - Add fixed size bar handler
> - Some minor improvememtn to see each patches's changelog.
> - Link to v4: https://lore.kernel.org/r/20241031-ep-msi-v4-0-717da2d99b28@nxp.com
> 
> Changes in v4:
> - Remove patch genirq/msi: Add cleanup guard define for msi_lock_descs()/msi_unlock_descs()
> - Use new method to avoid compatible problem.
>   Add new command DOORBELL_ENABLE and DOORBELL_DISABLE.
>   pcitest -B send DOORBELL_ENABLE first, EP test function driver try to
> remap one of BAR_N (except test register bar) to ITS MSI MMIO space. Old
> driver don't support new command, so failure return, not side effect.
>   After test, DOORBELL_DISABLE command send out to recover original map, so
> pcitest bar test can pass as normal.
> - Other detail change see each patches's change log
> - Link to v3: https://lore.kernel.org/r/20241015-ep-msi-v3-0-cedc89a16c1a@nxp.com
> 
> Change from v2 to v3
> - Fixed manivannan's comments
> - Move common part to pci-ep-msi.c and pci-ep-msi.h
> - rebase to 6.12-rc1
> - use RevID to distingiush old version
> 
> mkdir /sys/kernel/config/pci_ep/functions/pci_epf_test/func1
> echo 16 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/msi_interrupts
> echo 0x080c > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/deviceid
> echo 0x1957 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/vendorid
> echo 1 > /sys/kernel/config/pci_ep/functions/pci_epf_test/func1/revid
> ^^^^^^ to enable platform msi support.
> ln -s /sys/kernel/config/pci_ep/functions/pci_epf_test/func1 /sys/kernel/config/pci_ep/controllers/4c380000.pcie-ep
> 
> - use new device ID, which identify support doorbell to avoid broken
> compatility.
> 
>     Enable doorbell support only for PCI_DEVICE_ID_IMX8_DB, while other devices
>     keep the same behavior as before.
> 
>            EP side             RC with old driver      RC with new driver
>     PCI_DEVICE_ID_IMX8_DB          no probe              doorbell enabled
>     Other device ID             doorbell disabled*       doorbell disabled*
> 
>     * Behavior remains unchanged.
> 
> Change from v1 to v2
> - Add missed patch for endpont/pci-epf-test.c
> - Move alloc and free to epc driver from epf.
> - Provide general help function for EPC driver to alloc platform msi irq.
> - Fixed manivannan's comments.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Frank Li (10):
>       PCI: endpoint: Set ID and of_node for function driver
>       PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
>       PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check
>       PCI: endpoint: Add pci_epf_align_inbound_addr() helper for address alignment
>       PCI: endpoint: pci-epf-test: Add doorbell test support
>       misc: pci_endpoint_test: Add doorbell test case
>       selftests: pci_endpoint: Add doorbell test case
>       pci: imx6: Add helper function imx_pcie_add_lut_by_rid()
>       pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode
>       arm64: dts: imx95: Add msi-map for pci-ep device
> 
>  arch/arm64/boot/dts/freescale/imx95.dtsi           |   1 +
>  drivers/misc/pci_endpoint_test.c                   |  82 ++++++++++++
>  drivers/pci/controller/dwc/pci-imx6.c              |  25 ++--
>  drivers/pci/endpoint/Makefile                      |   1 +
>  drivers/pci/endpoint/functions/pci-epf-test.c      | 142 +++++++++++++++++++++
>  drivers/pci/endpoint/pci-ep-msi.c                  |  90 +++++++++++++
>  drivers/pci/endpoint/pci-epf-core.c                |  48 +++++++
>  include/linux/pci-ep-msi.h                         |  28 ++++
>  include/linux/pci-epf.h                            |  21 +++
>  include/uapi/linux/pcitest.h                       |   1 +
>  .../selftests/pci_endpoint/pci_endpoint_test.c     |  28 ++++
>  11 files changed, 459 insertions(+), 8 deletions(-)
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20241010-ep-msi-8b4cab33b1be
> 
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > Set device ID as 'vfunc_no << 3 | func_no' and use
> > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > device.
> >
> > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > functions use the EPC's device node, but they should use the EPF one.
> > For multiple function drivers, IOMMU/MSI should be different for each
> > function driver.
> >
>
> We don't define OF node for any function, so device_set_of_node_from_dev() also
> ends up reusing the EPC node. So how can you make use of it in multi EPF setup?

In mfd devices, children devices reuse parent's of_node
drivers/gpio/gpio-adp5585.c
drivers/input/keyboard/adp5589-keys.c
drivers/pwm/pwm-adp5585.c

multi EPF should be similar to create multi children devices of mfd.

> I don't understand.

>
> > If multiple function devices share the same EPC device, there will be
> > no isolation between them. Setting the ID and 'of_node' prepares for
> > proper support.

Only share the same of_node.

Actually pci host bridge have similar situation, all pci ep devices reuse
bridge's of node. framework use rid to distringuish it. EPF can use device::id
to do similar things.

Actually iommu face the similar problem. So far, there are not EP device enable
iommu yet, because it needs special mapping.

Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
information to each children. But when I see adp5585 case, I think direct
use parent's of_node should be simple and good enough.

In future, I suggest add children dt binding for it. For example: EPF provide
a mailbox interface. how other dts node to refer to this mailbox's phandle?


> >
>
> I don't know who you can provide *isolation* by reusing the EPC OF node. It is
> same as using the EPC node directly.

why it is same?

Frank
>
> - Mani
>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change from v14 to v16
> > - none
> >
> > change from v13 to v14
> > new patch
> > ---
> >  drivers/pci/endpoint/pci-epf-core.c | 4 ++++
> >  include/linux/pci-epf.h             | 2 ++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> > index 577a9e490115c..95fb3d7c1d45e 100644
> > --- a/drivers/pci/endpoint/pci-epf-core.c
> > +++ b/drivers/pci/endpoint/pci-epf-core.c
> > @@ -120,12 +120,16 @@ int pci_epf_bind(struct pci_epf *epf)
> >  		epf_vf->sec_epc_func_no = epf->sec_epc_func_no;
> >  		epf_vf->epc = epf->epc;
> >  		epf_vf->sec_epc = epf->sec_epc;
> > +		epf_vf->dev.id = PCI_EPF_DEVID(epf->func_no, vfunc_no);
> > +		device_set_of_node_from_dev(&epf_vf->dev, epc->dev.parent);
> >  		ret = epf_vf->driver->ops->bind(epf_vf);
> >  		if (ret)
> >  			goto ret;
> >  		epf_vf->is_bound = true;
> >  	}
> >
> > +	epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0);
> > +	device_set_of_node_from_dev(&epf->dev, epc->dev.parent);
> >  	ret = epf->driver->ops->bind(epf);
> >  	if (ret)
> >  		goto ret;
> > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> > index 749cee0bcf2cc..c0864935c6864 100644
> > --- a/include/linux/pci-epf.h
> > +++ b/include/linux/pci-epf.h
> > @@ -216,6 +216,8 @@ static inline void *epf_get_drvdata(struct pci_epf *epf)
> >  	return dev_get_drvdata(&epf->dev);
> >  }
> >
> > +#define PCI_EPF_DEVID(func_no, vfunc_no) ((vfunc_no) << 3 | (func_no))
> > +
> >  struct pci_epf *pci_epf_create(const char *name);
> >  void pci_epf_destroy(struct pci_epf *epf);
> >  int __pci_epf_register_driver(struct pci_epf_driver *driver,
> >
> > --
> > 2.34.1
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 03/10] PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check

[Frank Li]

On Wed, Jul 02, 2025 at 05:00:23PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 12:34:15PM GMT, Frank Li wrote:
> > Some MSI controller change address/data pair when irq_set_affinity().
> > Current PCI endpoint can't support this type MSI controller. Call
> > irq_domain_is_msi_immutable() check if address/data pair immutable.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change in v18
> > - update commit message. remove 'include/linux/msi.h' part.
> >
> > change from v14 to v17
> > - none
> >
> > change from  v13 to v14
> > - bring v10 back
> >
> > Change from v9 to v10
> > - new patch
> > ---
> >  drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> > index 549b55b864d0e..c0e2d806ee658 100644
> > --- a/drivers/pci/endpoint/pci-ep-msi.c
> > +++ b/drivers/pci/endpoint/pci-ep-msi.c
> > @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> >
> >  	dev_set_msi_domain(dev, dom);
> >
> > +	if (!irq_domain_is_msi_parent(dom))
> > +		return -EINVAL;
>
> This check is not justified in commit message.
>
> > +
> > +	if (!irq_domain_is_msi_immutable(dom)) {
> > +		dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
> > +		return -EINVAL;
>
> GICv3 ITS is an immutable MSI controller. From the earlier patches, I could see
> that you have tested this series with ITS. How did that happen if it errors out
> here?

I removed IMMUTASBLE flags in ITS driver to check if go to this error branch.

Frank

>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 09/10] pci: imx6: Add LUT setting for MSI/IOMMU in Endpoint mode

[Frank Li]

On Wed, Jul 02, 2025 at 06:52:01PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 12:34:21PM GMT, Frank Li wrote:
> > Support only one physical function, so call imx_pcie_add_lut_by_rid(0)
> > to add a single LUT entry when operating in EP mode.
> >
>
> So previously LUT config was not present and endpoint mode continued to work?

Yes, LUT only use when enable msi and iommu. I will add such information
at next version.

Frank

> Please explain why this is necessary now.
>
> - Mani
>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change from v14 to v16
> > - none
> >
> > change from v13 to v14
> > - new patch
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 032b906c44dfa..3123bf49e209c 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1063,7 +1063,10 @@ static int imx_pcie_add_lut(struct imx_pcie *imx_pcie, u16 rid, u8 sid)
> >  	data1 |= IMX95_PE0_LUT_VLD;
> >  	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, data1);
> >
> > -	data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
> > +	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE)
> > +		data2 = 0x7; /* EP side's RID from RC, only 'D' is meansful */
> > +	else
> > +		data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */
> >  	data2 |= FIELD_PREP(IMX95_PE0_LUT_REQID, rid);
> >  	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, data2);
> >
> > @@ -1767,6 +1770,9 @@ static int imx_pcie_probe(struct platform_device *pdev)
> >  		ret = imx_add_pcie_ep(imx_pcie, pdev);
> >  		if (ret < 0)
> >  			return ret;
> > +
> > +		/* Only support one physical function */
> > +		imx_pcie_add_lut_by_rid(imx_pcie, 0);
> >  	} else {
> >  		pci->pp.use_atu_msg = true;
> >  		ret = dw_pcie_host_init(&pci->pp);
> >
> > --
> > 2.34.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Manivannan Sadhasivam]

On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > device.
> > >
> > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > functions use the EPC's device node, but they should use the EPF one.
> > > For multiple function drivers, IOMMU/MSI should be different for each
> > > function driver.
> > >
> >
> > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> 
> In mfd devices, children devices reuse parent's of_node
> drivers/gpio/gpio-adp5585.c
> drivers/input/keyboard/adp5589-keys.c
> drivers/pwm/pwm-adp5585.c
> 
> multi EPF should be similar to create multi children devices of mfd.
> 

No, they are not similar. MFD are real physical devices, but EPFs are (so far)
software based entities.

> > I don't understand.
> 
> >
> > > If multiple function devices share the same EPC device, there will be
> > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > proper support.
> 
> Only share the same of_node.
> 
> Actually pci host bridge have similar situation, all pci ep devices reuse
> bridge's of node. framework use rid to distringuish it. EPF can use device::id
> to do similar things.
> 
> Actually iommu face the similar problem. So far, there are not EP device enable
> iommu yet, because it needs special mapping.
> 
> Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> information to each children. But when I see adp5585 case, I think direct
> use parent's of_node should be simple and good enough.
> 
> In future, I suggest add children dt binding for it. For example: EPF provide
> a mailbox interface. how other dts node to refer to this mailbox's phandle?
> 

As I said above, EPFs are not real devices. There is currently only one
exception, MHI, which is backed by a hardware entity. So we cannot add
devicetree nodes for EPF, unless each EPF is a hardware entity.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 03/10] PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check

[Manivannan Sadhasivam]

On Wed, Jul 02, 2025 at 10:42:43AM GMT, Frank Li wrote:
> On Wed, Jul 02, 2025 at 05:00:23PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jun 09, 2025 at 12:34:15PM GMT, Frank Li wrote:
> > > Some MSI controller change address/data pair when irq_set_affinity().
> > > Current PCI endpoint can't support this type MSI controller. Call
> > > irq_domain_is_msi_immutable() check if address/data pair immutable.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > change in v18
> > > - update commit message. remove 'include/linux/msi.h' part.
> > >
> > > change from v14 to v17
> > > - none
> > >
> > > change from  v13 to v14
> > > - bring v10 back
> > >
> > > Change from v9 to v10
> > > - new patch
> > > ---
> > >  drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> > > index 549b55b864d0e..c0e2d806ee658 100644
> > > --- a/drivers/pci/endpoint/pci-ep-msi.c
> > > +++ b/drivers/pci/endpoint/pci-ep-msi.c
> > > @@ -44,6 +44,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> > >
> > >  	dev_set_msi_domain(dev, dom);
> > >
> > > +	if (!irq_domain_is_msi_parent(dom))
> > > +		return -EINVAL;
> >
> > This check is not justified in commit message.
> >
> > > +
> > > +	if (!irq_domain_is_msi_immutable(dom)) {
> > > +		dev_err(dev, "Can't support mutable address/data pair MSI controller\n");
> > > +		return -EINVAL;
> >
> > GICv3 ITS is an immutable MSI controller. From the earlier patches, I could see
> > that you have tested this series with ITS. How did that happen if it errors out
> > here?
> 
> I removed IMMUTASBLE flags in ITS driver to check if go to this error branch.
> 

Sorry, I misread the check and got confused by the error message. Presence of
IMMUTABLE flag is required by this driver, which is fine. Please reword the
error message to,

	"MSI controller not supported\n"

If one bothers to check why, the !irq_domain_is_msi_immutable() check is self
explanatory.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > device.
> > > >
> > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > functions use the EPC's device node, but they should use the EPF one.
> > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > function driver.
> > > >
> > >
> > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> >
> > In mfd devices, children devices reuse parent's of_node
> > drivers/gpio/gpio-adp5585.c
> > drivers/input/keyboard/adp5589-keys.c
> > drivers/pwm/pwm-adp5585.c
> >
> > multi EPF should be similar to create multi children devices of mfd.
> >
>
> No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> software based entities.
>
> > > I don't understand.
> >
> > >
> > > > If multiple function devices share the same EPC device, there will be
> > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > proper support.
> >
> > Only share the same of_node.
> >
> > Actually pci host bridge have similar situation, all pci ep devices reuse
> > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > to do similar things.
> >
> > Actually iommu face the similar problem. So far, there are not EP device enable
> > iommu yet, because it needs special mapping.
> >
> > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > information to each children. But when I see adp5585 case, I think direct
> > use parent's of_node should be simple and good enough.
> >
> > In future, I suggest add children dt binding for it. For example: EPF provide
> > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> >
>
> As I said above, EPFs are not real devices. There is currently only one
> exception, MHI, which is backed by a hardware entity. So we cannot add
> devicetree nodes for EPF, unless each EPF is a hardware entity.

But how resolve this problem, if a DT device need phandle to a EPF? anyway
this is off topic. let go back this doorbell.

It needs an of_node for EPF device, I tried many method before.

Create dymatic of_node for it? MSI framework still go through to parent
of_node to get such information. not big differnece as my view.

Frank

>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

On Wed, Jul 02, 2025 at 11:19:36AM -0400, Frank Li wrote:
> On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > > device.
> > > > >
> > > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > > functions use the EPC's device node, but they should use the EPF one.
> > > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > > function driver.
> > > > >
> > > >
> > > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> > >
> > > In mfd devices, children devices reuse parent's of_node
> > > drivers/gpio/gpio-adp5585.c
> > > drivers/input/keyboard/adp5589-keys.c
> > > drivers/pwm/pwm-adp5585.c
> > >
> > > multi EPF should be similar to create multi children devices of mfd.
> > >
> >
> > No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> > software based entities.
> >
> > > > I don't understand.
> > >
> > > >
> > > > > If multiple function devices share the same EPC device, there will be
> > > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > > proper support.
> > >
> > > Only share the same of_node.
> > >
> > > Actually pci host bridge have similar situation, all pci ep devices reuse
> > > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > > to do similar things.
> > >
> > > Actually iommu face the similar problem. So far, there are not EP device enable
> > > iommu yet, because it needs special mapping.
> > >
> > > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > > information to each children. But when I see adp5585 case, I think direct
> > > use parent's of_node should be simple and good enough.
> > >
> > > In future, I suggest add children dt binding for it. For example: EPF provide
> > > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> > >
> >
> > As I said above, EPFs are not real devices. There is currently only one
> > exception, MHI, which is backed by a hardware entity. So we cannot add
> > devicetree nodes for EPF, unless each EPF is a hardware entity.
>
> But how resolve this problem, if a DT device need phandle to a EPF? anyway
> this is off topic. let go back this doorbell.
>
> It needs an of_node for EPF device, I tried many method before.
>
> Create dymatic of_node for it? MSI framework still go through to parent
> of_node to get such information. not big differnece as my view.

Hi, any idea?  how to move forward? we can dynmatic create of_node for it.

Frank

>
> Frank
>
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

On Wed, Jul 02, 2025 at 11:19:36AM -0400, Frank Li wrote:
> On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > > device.
> > > > >
> > > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > > functions use the EPC's device node, but they should use the EPF one.
> > > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > > function driver.
> > > > >
> > > >
> > > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> > >
> > > In mfd devices, children devices reuse parent's of_node
> > > drivers/gpio/gpio-adp5585.c
> > > drivers/input/keyboard/adp5589-keys.c
> > > drivers/pwm/pwm-adp5585.c
> > >
> > > multi EPF should be similar to create multi children devices of mfd.
> > >
> >
> > No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> > software based entities.
> >
> > > > I don't understand.
> > >
> > > >
> > > > > If multiple function devices share the same EPC device, there will be
> > > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > > proper support.
> > >
> > > Only share the same of_node.
> > >
> > > Actually pci host bridge have similar situation, all pci ep devices reuse
> > > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > > to do similar things.
> > >
> > > Actually iommu face the similar problem. So far, there are not EP device enable
> > > iommu yet, because it needs special mapping.
> > >
> > > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > > information to each children. But when I see adp5585 case, I think direct
> > > use parent's of_node should be simple and good enough.
> > >
> > > In future, I suggest add children dt binding for it. For example: EPF provide
> > > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> > >
> >
> > As I said above, EPFs are not real devices. There is currently only one
> > exception, MHI, which is backed by a hardware entity. So we cannot add
> > devicetree nodes for EPF, unless each EPF is a hardware entity.
>
> But how resolve this problem, if a DT device need phandle to a EPF? anyway
> this is off topic. let go back this doorbell.
>
> It needs an of_node for EPF device, I tried many method before.
>
> Create dymatic of_node for it? MSI framework still go through to parent
> of_node to get such information. not big differnece as my view.

Actually, DMA have simular issues, just 'workaround' it now.

pci_epf_test_read() {
	...
	struct device *dma_dev = epf->epc->dev.parent;
	...
	dst_phys_addr = dma_map_single(dma_dev, buf, map_size,
                                                       DMA_FROM_DEVICE);
					^^^ [1]
	...
}

[1] here direct use epc->dev.parent's of node implicy. If IOMMU enable,
two EPF will share one IOMMU space without isolation. If add of_node(may
dyamatic create one). we should resolve this problem by use epf device
here. Difference EPF will use difference IOMMU space like MSI.

Frank

>
> Frank
>
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 00/10] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Niklas Cassel]

On Wed, Jul 02, 2025 at 06:57:23PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jun 09, 2025 at 12:34:12PM GMT, Frank Li wrote:
> 
> Frank, thanks for your persistence in pushing this series, really appreciated!
> I've left some comments, but no real blocker.
> 
> Unfortunately, I don't have access to my endpoint setup right now. So I'll go
> ahead with the Tested-by tag from Niklas once my comments are addressed.

(snip)

> > Changes in v6:
> > - add Niklas's test by tag

My Tested-by tag was added on v6, now it is v19 :)

To be comfortable of still having my Tested-by tag here,
I decided to test v19.

However I got this:

[ 3255.257047] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
[ 3255.257824] Mem abort info:
[ 3255.258069]   ESR = 0x0000000096000004
[ 3255.258398]   EC = 0x25: DABT (current EL), IL = 32 bits
[ 3255.258862]   SET = 0, FnV = 0
[ 3255.259147]   EA = 0, S1PTW = 0
[ 3255.259423]   FSC = 0x04: level 0 translation fault
[ 3255.259849] Data abort info:
[ 3255.260102]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 3255.260580]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 3255.261020]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 3255.261483] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000100a03000
[ 3255.262045] [0000000000000040] pgd=0000000000000000, p4d=0000000000000000
[ 3255.262639] Internal error: Oops: 0000000096000004 [#1]  SMP
[ 3255.263132] Modules linked in: rk805_pwrkey hantro_vpu v4l2_jpeg v4l2_vp9 v4l2_h264 v4l2_mem2mem videobuf2_v4l2 videobuf2_dma_contig videobuf2_memops videobuf2_common vidf
[ 3255.265357] CPU: 5 UID: 0 PID: 213 Comm: ln Not tainted 6.16.0-rc1+ #233 PREEMPT 
[ 3255.266009] Hardware name: Radxa ROCK 5B (DT)
[ 3255.266388] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 3255.266995] pc : pci_epf_bind+0x160/0x240
[ 3255.267350] lr : pci_epf_bind+0x40/0x240
[ 3255.267694] sp : ffff800081593c30
[ 3255.267983] x29: ffff800081593c30 x28: ffff0001024b2300 x27: ffff000102fc2800
[ 3255.268606] x26: ffff00010191e000 x25: ffff000100504098 x24: ffff000107b8ec80
[ 3255.269228] x23: ffff000104cf3578 x22: 0000000000000000 x21: 0000000000000000
[ 3255.269850] x20: ffff000104cf3000 x19: ffff000104cf3578 x18: 0000000000000000
[ 3255.270472] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[ 3255.271093] x14: 0000000000000000 x13: ffff00010245c037 x12: ffff800081593b94
[ 3255.271715] x11: 0000000528aa6179 x10: 0000000000000002 x9 : ffffa2593ce92b30
[ 3255.272336] x8 : 00000031636e7566 x7 : 00000000ffffbe12 x6 : 0000000000000003
[ 3255.272958] x5 : ffff000102413f78 x4 : ffff000102413f08 x3 : 0000000000000000
[ 3255.273580] x2 : ffff0001024b2300 x1 : 0000000000000000 x0 : ffff000104cf3000
[ 3255.274201] Call trace:
[ 3255.274416]  pci_epf_bind+0x160/0x240 (P)
[ 3255.274767]  pci_epc_epf_link+0x54/0xb0
[ 3255.275104]  configfs_symlink+0x208/0x540
[ 3255.275457]  vfs_symlink+0x158/0x1e0
[ 3255.275770]  do_symlinkat+0x8c/0x138
[ 3255.276083]  __arm64_sys_symlinkat+0x7c/0xc8
[ 3255.276455]  invoke_syscall.constprop.0+0x48/0x100
[ 3255.276874]  el0_svc_common.constprop.0+0x40/0xe8
[ 3255.277285]  do_el0_svc+0x24/0x38
[ 3255.277575]  el0_svc+0x34/0x100
[ 3255.277852]  el0t_64_sync_handler+0x10c/0x140
[ 3255.278233]  el0t_64_sync+0x198/0x1a0
[ 3255.278554] Code: a9446bf9 394ff280 b902aa80 aa1403e0 (f94022a1) 
[ 3255.279085] ---[ end trace 0000000000000000 ]---


Seems to be from patch 1/10:

(gdb) l *(pci_epf_bind+0x160)
0xffff800080892c50 is in pci_epf_bind (drivers/pci/endpoint/pci-epf-core.c:132).
127                             goto ret;
128                     epf_vf->is_bound = true;
129             }
130
131             epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0);
132             device_set_of_node_from_dev(&epf->dev, epc->dev.parent);
133             ret = epf->driver->ops->bind(epf);
134             if (ret)
135                     goto ret;
136             epf->is_bound = true;


I can see that there is a lot of discussion on patch 1/10 already,
but please drop my Tested-by tag until this has been fixed.

Feel free to CC me on v20 of this series, if the problem is fixed,
I will provide my Tested-by tag once again.


Kind regards,
Niklas

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Manivannan Sadhasivam]

On Mon, Jul 07, 2025 at 12:41:22PM GMT, Frank Li wrote:
> On Wed, Jul 02, 2025 at 11:19:36AM -0400, Frank Li wrote:
> > On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > > > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > > > device.
> > > > > >
> > > > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > > > functions use the EPC's device node, but they should use the EPF one.
> > > > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > > > function driver.
> > > > > >
> > > > >
> > > > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> > > >
> > > > In mfd devices, children devices reuse parent's of_node
> > > > drivers/gpio/gpio-adp5585.c
> > > > drivers/input/keyboard/adp5589-keys.c
> > > > drivers/pwm/pwm-adp5585.c
> > > >
> > > > multi EPF should be similar to create multi children devices of mfd.
> > > >
> > >
> > > No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> > > software based entities.
> > >
> > > > > I don't understand.
> > > >
> > > > >
> > > > > > If multiple function devices share the same EPC device, there will be
> > > > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > > > proper support.
> > > >
> > > > Only share the same of_node.
> > > >
> > > > Actually pci host bridge have similar situation, all pci ep devices reuse
> > > > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > > > to do similar things.
> > > >
> > > > Actually iommu face the similar problem. So far, there are not EP device enable
> > > > iommu yet, because it needs special mapping.
> > > >
> > > > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > > > information to each children. But when I see adp5585 case, I think direct
> > > > use parent's of_node should be simple and good enough.
> > > >
> > > > In future, I suggest add children dt binding for it. For example: EPF provide
> > > > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> > > >
> > >
> > > As I said above, EPFs are not real devices. There is currently only one
> > > exception, MHI, which is backed by a hardware entity. So we cannot add
> > > devicetree nodes for EPF, unless each EPF is a hardware entity.
> >
> > But how resolve this problem, if a DT device need phandle to a EPF? anyway
> > this is off topic. let go back this doorbell.
> >
> > It needs an of_node for EPF device, I tried many method before.
> >
> > Create dymatic of_node for it? MSI framework still go through to parent
> > of_node to get such information. not big differnece as my view.
> 
> Actually, DMA have simular issues, just 'workaround' it now.
> 
> pci_epf_test_read() {
> 	...
> 	struct device *dma_dev = epf->epc->dev.parent;
> 	...
> 	dst_phys_addr = dma_map_single(dma_dev, buf, map_size,
>                                                        DMA_FROM_DEVICE);
> 					^^^ [1]
> 	...
> }
> 
> [1] here direct use epc->dev.parent's of node implicy. If IOMMU enable,
> two EPF will share one IOMMU space without isolation. If add of_node(may
> dyamatic create one). we should resolve this problem by use epf device
> here. Difference EPF will use difference IOMMU space like MSI.
> 

Unless your platform comes up with a hardware based EPF, we are not going to
have DT node for any EPF. So all EPFs have to share the same DT node of the EPC.
So right now, it doesn't make a difference if you use a dynamic of_node or copy
the EPC node.

Just reuse the EPC node for now.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 00/10] PCI: EP: Add RC-to-EP doorbell with platform MSI controller

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 01:12:02PM GMT, Niklas Cassel wrote:
> On Wed, Jul 02, 2025 at 06:57:23PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jun 09, 2025 at 12:34:12PM GMT, Frank Li wrote:
> > 
> > Frank, thanks for your persistence in pushing this series, really appreciated!
> > I've left some comments, but no real blocker.
> > 
> > Unfortunately, I don't have access to my endpoint setup right now. So I'll go
> > ahead with the Tested-by tag from Niklas once my comments are addressed.
> 
> (snip)
> 
> > > Changes in v6:
> > > - add Niklas's test by tag
> 
> My Tested-by tag was added on v6, now it is v19 :)
> 

Ouch!

> To be comfortable of still having my Tested-by tag here,
> I decided to test v19.
> 
> However I got this:
> 
> [ 3255.257047] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040

Very well... thanks for testing it. I will also try to get hold of the device in
the meantime.

- Mani

> [ 3255.257824] Mem abort info:
> [ 3255.258069]   ESR = 0x0000000096000004
> [ 3255.258398]   EC = 0x25: DABT (current EL), IL = 32 bits
> [ 3255.258862]   SET = 0, FnV = 0
> [ 3255.259147]   EA = 0, S1PTW = 0
> [ 3255.259423]   FSC = 0x04: level 0 translation fault
> [ 3255.259849] Data abort info:
> [ 3255.260102]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [ 3255.260580]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 3255.261020]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 3255.261483] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000100a03000
> [ 3255.262045] [0000000000000040] pgd=0000000000000000, p4d=0000000000000000
> [ 3255.262639] Internal error: Oops: 0000000096000004 [#1]  SMP
> [ 3255.263132] Modules linked in: rk805_pwrkey hantro_vpu v4l2_jpeg v4l2_vp9 v4l2_h264 v4l2_mem2mem videobuf2_v4l2 videobuf2_dma_contig videobuf2_memops videobuf2_common vidf
> [ 3255.265357] CPU: 5 UID: 0 PID: 213 Comm: ln Not tainted 6.16.0-rc1+ #233 PREEMPT 
> [ 3255.266009] Hardware name: Radxa ROCK 5B (DT)
> [ 3255.266388] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 3255.266995] pc : pci_epf_bind+0x160/0x240
> [ 3255.267350] lr : pci_epf_bind+0x40/0x240
> [ 3255.267694] sp : ffff800081593c30
> [ 3255.267983] x29: ffff800081593c30 x28: ffff0001024b2300 x27: ffff000102fc2800
> [ 3255.268606] x26: ffff00010191e000 x25: ffff000100504098 x24: ffff000107b8ec80
> [ 3255.269228] x23: ffff000104cf3578 x22: 0000000000000000 x21: 0000000000000000
> [ 3255.269850] x20: ffff000104cf3000 x19: ffff000104cf3578 x18: 0000000000000000
> [ 3255.270472] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 3255.271093] x14: 0000000000000000 x13: ffff00010245c037 x12: ffff800081593b94
> [ 3255.271715] x11: 0000000528aa6179 x10: 0000000000000002 x9 : ffffa2593ce92b30
> [ 3255.272336] x8 : 00000031636e7566 x7 : 00000000ffffbe12 x6 : 0000000000000003
> [ 3255.272958] x5 : ffff000102413f78 x4 : ffff000102413f08 x3 : 0000000000000000
> [ 3255.273580] x2 : ffff0001024b2300 x1 : 0000000000000000 x0 : ffff000104cf3000
> [ 3255.274201] Call trace:
> [ 3255.274416]  pci_epf_bind+0x160/0x240 (P)
> [ 3255.274767]  pci_epc_epf_link+0x54/0xb0
> [ 3255.275104]  configfs_symlink+0x208/0x540
> [ 3255.275457]  vfs_symlink+0x158/0x1e0
> [ 3255.275770]  do_symlinkat+0x8c/0x138
> [ 3255.276083]  __arm64_sys_symlinkat+0x7c/0xc8
> [ 3255.276455]  invoke_syscall.constprop.0+0x48/0x100
> [ 3255.276874]  el0_svc_common.constprop.0+0x40/0xe8
> [ 3255.277285]  do_el0_svc+0x24/0x38
> [ 3255.277575]  el0_svc+0x34/0x100
> [ 3255.277852]  el0t_64_sync_handler+0x10c/0x140
> [ 3255.278233]  el0t_64_sync+0x198/0x1a0
> [ 3255.278554] Code: a9446bf9 394ff280 b902aa80 aa1403e0 (f94022a1) 
> [ 3255.279085] ---[ end trace 0000000000000000 ]---
> 
> 
> Seems to be from patch 1/10:
> 
> (gdb) l *(pci_epf_bind+0x160)
> 0xffff800080892c50 is in pci_epf_bind (drivers/pci/endpoint/pci-epf-core.c:132).
> 127                             goto ret;
> 128                     epf_vf->is_bound = true;
> 129             }
> 130
> 131             epf->dev.id = PCI_EPF_DEVID(epf->func_no, 0);
> 132             device_set_of_node_from_dev(&epf->dev, epc->dev.parent);
> 133             ret = epf->driver->ops->bind(epf);
> 134             if (ret)
> 135                     goto ret;
> 136             epf->is_bound = true;
> 
> 
> I can see that there is a lot of discussion on patch 1/10 already,
> but please drop my Tested-by tag until this has been fixed.
> 
> Feel free to CC me on v20 of this series, if the problem is fixed,
> I will provide my Tested-by tag once again.
> 
> 
> Kind regards,
> Niklas

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Frank Li]

On Tue, Jul 08, 2025 at 04:51:55PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jul 07, 2025 at 12:41:22PM GMT, Frank Li wrote:
> > On Wed, Jul 02, 2025 at 11:19:36AM -0400, Frank Li wrote:
> > > On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> > > > On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > > > > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > > > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > > > > device.
> > > > > > >
> > > > > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > > > > functions use the EPC's device node, but they should use the EPF one.
> > > > > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > > > > function driver.
> > > > > > >
> > > > > >
> > > > > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > > > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> > > > >
> > > > > In mfd devices, children devices reuse parent's of_node
> > > > > drivers/gpio/gpio-adp5585.c
> > > > > drivers/input/keyboard/adp5589-keys.c
> > > > > drivers/pwm/pwm-adp5585.c
> > > > >
> > > > > multi EPF should be similar to create multi children devices of mfd.
> > > > >
> > > >
> > > > No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> > > > software based entities.
> > > >
> > > > > > I don't understand.
> > > > >
> > > > > >
> > > > > > > If multiple function devices share the same EPC device, there will be
> > > > > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > > > > proper support.
> > > > >
> > > > > Only share the same of_node.
> > > > >
> > > > > Actually pci host bridge have similar situation, all pci ep devices reuse
> > > > > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > > > > to do similar things.
> > > > >
> > > > > Actually iommu face the similar problem. So far, there are not EP device enable
> > > > > iommu yet, because it needs special mapping.
> > > > >
> > > > > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > > > > information to each children. But when I see adp5585 case, I think direct
> > > > > use parent's of_node should be simple and good enough.
> > > > >
> > > > > In future, I suggest add children dt binding for it. For example: EPF provide
> > > > > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> > > > >
> > > >
> > > > As I said above, EPFs are not real devices. There is currently only one
> > > > exception, MHI, which is backed by a hardware entity. So we cannot add
> > > > devicetree nodes for EPF, unless each EPF is a hardware entity.
> > >
> > > But how resolve this problem, if a DT device need phandle to a EPF? anyway
> > > this is off topic. let go back this doorbell.
> > >
> > > It needs an of_node for EPF device, I tried many method before.
> > >
> > > Create dymatic of_node for it? MSI framework still go through to parent
> > > of_node to get such information. not big differnece as my view.
> >
> > Actually, DMA have simular issues, just 'workaround' it now.
> >
> > pci_epf_test_read() {
> > 	...
> > 	struct device *dma_dev = epf->epc->dev.parent;
> > 	...
> > 	dst_phys_addr = dma_map_single(dma_dev, buf, map_size,
> >                                                        DMA_FROM_DEVICE);
> > 					^^^ [1]
> > 	...
> > }
> >
> > [1] here direct use epc->dev.parent's of node implicy. If IOMMU enable,
> > two EPF will share one IOMMU space without isolation. If add of_node(may
> > dyamatic create one). we should resolve this problem by use epf device
> > here. Difference EPF will use difference IOMMU space like MSI.
> >
>
> Unless your platform comes up with a hardware based EPF, we are not going to
> have DT node for any EPF. So all EPFs have to share the same DT node of the EPC.
> So right now, it doesn't make a difference if you use a dynamic of_node or copy
> the EPC node.
>
> Just reuse the EPC node for now.

It is show-stop issue. The closest version like
https://lore.kernel.org/all/20241204-ep-msi-v10-2-87c378dbcd6d@nxp.com/

Or we just support one EPF. There are not good way to pass down epf ID to MSI
controller.

[1]: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI (like PCI RC bus),
https://lore.kernel.org/all/20250211-ep-msi-v15-0-bcacc1f2b1a9@nxp.com/
rejected by irq mantainer, they think it is too similar with platform msi.

The key problem is MSI controller need known both EPF's ID and EPC's MSI
domain information.

If use EPC, there are no way to pass down EPF's ID. as above dma example,
use EPC devices, dma_map_single() can't distringiush difference EPF. It is
not big issue all EPF share a IO space. but can't do that for MSI. the
different devices can't share the MSI space.

software managed dt property already used in many devices.

Frank

>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v19 01/10] PCI: endpoint: Set ID and of_node for function driver

[Manivannan Sadhasivam]

On Tue, Jul 08, 2025 at 03:06:02PM GMT, Frank Li wrote:
> On Tue, Jul 08, 2025 at 04:51:55PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jul 07, 2025 at 12:41:22PM GMT, Frank Li wrote:
> > > On Wed, Jul 02, 2025 at 11:19:36AM -0400, Frank Li wrote:
> > > > On Wed, Jul 02, 2025 at 08:25:17PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Wed, Jul 02, 2025 at 10:40:53AM GMT, Frank Li wrote:
> > > > > > On Wed, Jul 02, 2025 at 04:30:48PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > On Mon, Jun 09, 2025 at 12:34:13PM GMT, Frank Li wrote:
> > > > > > > > Set device ID as 'vfunc_no << 3 | func_no' and use
> > > > > > > > 'device_set_of_node_from_dev()' to set 'of_node' the same as the EPC parent
> > > > > > > > device.
> > > > > > > >
> > > > > > > > Currently, EPF 'of_node' is NULL, but many functions depend on 'of_node'
> > > > > > > > settings, such as DMA, IOMMU, and MSI. At present, all DMA allocation
> > > > > > > > functions use the EPC's device node, but they should use the EPF one.
> > > > > > > > For multiple function drivers, IOMMU/MSI should be different for each
> > > > > > > > function driver.
> > > > > > > >
> > > > > > >
> > > > > > > We don't define OF node for any function, so device_set_of_node_from_dev() also
> > > > > > > ends up reusing the EPC node. So how can you make use of it in multi EPF setup?
> > > > > >
> > > > > > In mfd devices, children devices reuse parent's of_node
> > > > > > drivers/gpio/gpio-adp5585.c
> > > > > > drivers/input/keyboard/adp5589-keys.c
> > > > > > drivers/pwm/pwm-adp5585.c
> > > > > >
> > > > > > multi EPF should be similar to create multi children devices of mfd.
> > > > > >
> > > > >
> > > > > No, they are not similar. MFD are real physical devices, but EPFs are (so far)
> > > > > software based entities.
> > > > >
> > > > > > > I don't understand.
> > > > > >
> > > > > > >
> > > > > > > > If multiple function devices share the same EPC device, there will be
> > > > > > > > no isolation between them. Setting the ID and 'of_node' prepares for
> > > > > > > > proper support.
> > > > > >
> > > > > > Only share the same of_node.
> > > > > >
> > > > > > Actually pci host bridge have similar situation, all pci ep devices reuse
> > > > > > bridge's of node. framework use rid to distringuish it. EPF can use device::id
> > > > > > to do similar things.
> > > > > >
> > > > > > Actually iommu face the similar problem. So far, there are not EP device enable
> > > > > > iommu yet, because it needs special mapping.
> > > > > >
> > > > > > Prevously, I consider create dymatic of_node for each EPF and copy iommu/msi
> > > > > > information to each children. But when I see adp5585 case, I think direct
> > > > > > use parent's of_node should be simple and good enough.
> > > > > >
> > > > > > In future, I suggest add children dt binding for it. For example: EPF provide
> > > > > > a mailbox interface. how other dts node to refer to this mailbox's phandle?
> > > > > >
> > > > >
> > > > > As I said above, EPFs are not real devices. There is currently only one
> > > > > exception, MHI, which is backed by a hardware entity. So we cannot add
> > > > > devicetree nodes for EPF, unless each EPF is a hardware entity.
> > > >
> > > > But how resolve this problem, if a DT device need phandle to a EPF? anyway
> > > > this is off topic. let go back this doorbell.
> > > >
> > > > It needs an of_node for EPF device, I tried many method before.
> > > >
> > > > Create dymatic of_node for it? MSI framework still go through to parent
> > > > of_node to get such information. not big differnece as my view.
> > >
> > > Actually, DMA have simular issues, just 'workaround' it now.
> > >
> > > pci_epf_test_read() {
> > > 	...
> > > 	struct device *dma_dev = epf->epc->dev.parent;
> > > 	...
> > > 	dst_phys_addr = dma_map_single(dma_dev, buf, map_size,
> > >                                                        DMA_FROM_DEVICE);
> > > 					^^^ [1]
> > > 	...
> > > }
> > >
> > > [1] here direct use epc->dev.parent's of node implicy. If IOMMU enable,
> > > two EPF will share one IOMMU space without isolation. If add of_node(may
> > > dyamatic create one). we should resolve this problem by use epf device
> > > here. Difference EPF will use difference IOMMU space like MSI.
> > >
> >
> > Unless your platform comes up with a hardware based EPF, we are not going to
> > have DT node for any EPF. So all EPFs have to share the same DT node of the EPC.
> > So right now, it doesn't make a difference if you use a dynamic of_node or copy
> > the EPC node.
> >
> > Just reuse the EPC node for now.
> 
> It is show-stop issue. The closest version like
> https://lore.kernel.org/all/20241204-ep-msi-v10-2-87c378dbcd6d@nxp.com/
> 
> Or we just support one EPF. There are not good way to pass down epf ID to MSI
> controller.
> 
> [1]: Add DOMAIN_BUS_DEVICE_PCI_EP_MSI (like PCI RC bus),
> https://lore.kernel.org/all/20250211-ep-msi-v15-0-bcacc1f2b1a9@nxp.com/
> rejected by irq mantainer, they think it is too similar with platform msi.
> 
> The key problem is MSI controller need known both EPF's ID and EPC's MSI
> domain information.
> 
> If use EPC, there are no way to pass down EPF's ID. as above dma example,
> use EPC devices, dma_map_single() can't distringiush difference EPF. It is
> not big issue all EPF share a IO space. but can't do that for MSI. the
> different devices can't share the MSI space.
> 
> software managed dt property already used in many devices.
> 

You need not just a property, but a whole new DT node. This won't fly with the
DT maintainers, so it is not my call.

I'd suggest that we merge this initial implementation that supports only one EPF
for doorbell and tackle the multi-EPF implementation in future patches. I do not
want this series to be in a limbo for another 5-6 releases.

- Mani

-- 
மணிவண்ணன் சதாசிவம்