[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <kife5sc4dmoxt7b6werw7l3dcq46rx634wmwxyuenjkp6xstqe@np6g6z55r7sz>
Date: Wed, 2 Jul 2025 17:01:10 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Frank Li <Frank.Li@....com>
Cc: Kishon Vijay Abraham I <kishon@...nel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Anup Patel <apatel@...tanamicro.com>, Marc Zyngier <maz@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>, Shuah Khan <shuah@...nel.org>,
Richard Zhu <hongxing.zhu@....com>, Lucas Stach <l.stach@...gutronix.de>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Rob Herring <robh@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>,
Niklas Cassel <cassel@...nel.org>, dlemoal@...nel.org, jdmason@...zu.us,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
linux-kselftest@...r.kernel.org, imx@...ts.linux.dev, devicetree@...r.kernel.org
Subject: Re: [PATCH v19 02/10] PCI: endpoint: Add RC-to-EP doorbell support
using platform MSI controller
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@...nel.org>
> > Signed-off-by: Frank Li <Frank.Li@....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@...el.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@...el.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@....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
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists