[PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller

Pragnesh Patel posted 1 patch 2 weeks, 3 days ago
MAINTAINERS                             |   6 +
drivers/pci/controller/Kconfig          |   9 +
drivers/pci/controller/Makefile         |   1 +
drivers/pci/controller/pci-octeon-pem.c | 278 ++++++++++++++++++++++++
4 files changed, 294 insertions(+)
create mode 100644 drivers/pci/controller/pci-octeon-pem.c
[PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
Posted by Pragnesh Patel 2 weeks, 3 days ago
From: Suneel Garapati <sgarapati@marvell.com>

This driver adds support for link-down interrupt in PCIe MAC (PEM)
controller in Root complex mode to support hot-plug removal of
endpoint devices.

An interrupt handler is registered for RST_INT msix vector
which is triggered with link going down. This handler
performs cleanup of root bridge and its children and
re-initializes root bridge to kernel for next link-up event.

Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
Signed-off-by: Pragnesh Patel <pragneshp@marvell.com>
---
 MAINTAINERS                             |   6 +
 drivers/pci/controller/Kconfig          |   9 +
 drivers/pci/controller/Makefile         |   1 +
 drivers/pci/controller/pci-octeon-pem.c | 278 ++++++++++++++++++++++++
 4 files changed, 294 insertions(+)
 create mode 100644 drivers/pci/controller/pci-octeon-pem.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f1b020588597..9af8f676c027 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15363,6 +15363,12 @@ R:	Vamsi Attunuru <vattunuru@marvell.com>
 S:	Supported
 F:	drivers/pci/hotplug/octep_hp.c
 
+MARVELL OCTEON PEM CONTROLLER DRIVER
+R:	George Cherian <gcherian@marvell.com>
+R:	Pragnesh Patel <pragneshp@marvell.com>
+S:	Supported
+F:	drivers/pci/controller/pci-octeon-pem.c
+
 MATROX FRAMEBUFFER DRIVER
 L:	linux-fbdev@vger.kernel.org
 S:	Orphan
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index c254d2b8bf17..5251dc8b1188 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -231,6 +231,15 @@ config PCI_HYPERV_INTERFACE
 	  drivers to have a common interface with the Hyper-V PCI frontend
 	  driver.
 
+config PCI_OCTEON_PEM
+	bool "Marvell Octeon PEM (PCIe MAC) controller"
+	depends on ARM64 || COMPILE_TEST
+	depends on PCI_MSI
+	depends on HOTPLUG_PCI_PCIE
+	help
+	  Say Y here if you want PEM controller support for Marvell ARM64 Octeon SoCs
+	  in root complex mode.
+
 config PCI_TEGRA
 	bool "NVIDIA Tegra PCIe controller"
 	depends on ARCH_TEGRA || COMPILE_TEST
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 229929a945c2..dd1dab50dd07 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o
 obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
 obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
 obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
+obj-$(CONFIG_PCI_OCTEON_PEM) += pci-octeon-pem.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_MSI) += pcie-iproc-msi.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
diff --git a/drivers/pci/controller/pci-octeon-pem.c b/drivers/pci/controller/pci-octeon-pem.c
new file mode 100644
index 000000000000..2d67e2b16e9e
--- /dev/null
+++ b/drivers/pci/controller/pci-octeon-pem.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Octeon PEM driver
+ *
+ * Copyright (C) 2021 Marvell.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/sysfs.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include "../hotplug/pciehp.h"
+#include "../pci.h"
+
+#define DRV_NAME	"octeon-pem"
+#define DRV_VERSION	"1.0"
+
+#define PCI_DEVID_OCTEON_PEM	0xA06C
+
+#define ID_SHIFT		36
+#define DOMAIN_OFFSET		0x3
+#define ON_OFFSET		0xE0
+#define RST_SOFT_PERST_OFFSET	0x298
+#define RST_INT_OFFSET		0x300
+#define RST_INT_ENA_W1C_OFFSET	0x310
+#define RST_INT_ENA_W1S_OFFSET	0x318
+#define RST_INT_LINKDOWN	BIT(1)
+
+struct pem_ctlr {
+	int			index;
+	char			irq_name[32];
+	void __iomem		*base;
+	struct pci_dev		*pdev;
+	struct work_struct	recover_rc_work;
+};
+
+static struct pcie_device *to_pciehp_dev(struct pci_dev *dev)
+{
+	struct device *device;
+
+	device = pcie_port_find_device(dev, PCIE_PORT_SERVICE_HP);
+	if (!device)
+		return NULL;
+	return to_pcie_device(device);
+}
+
+static void pem_recover_rc_link(struct work_struct *ws)
+{
+	struct pem_ctlr *pem = container_of(ws, struct pem_ctlr,
+					    recover_rc_work);
+	struct pci_dev *pem_dev = pem->pdev;
+	struct pci_dev *root_port;
+	struct pci_bus *bus;
+	struct pcie_device *pcie;
+	struct controller *ctrl;
+	int rc_domain, timeout = 100;
+	u64 pem_reg;
+
+	rc_domain = pem->index + DOMAIN_OFFSET;
+
+	root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
+	if (!root_port) {
+		dev_err(&pem_dev->dev, "failed to get root port\n");
+		return;
+	}
+
+	dev_dbg(&pem_dev->dev, "PEM%d rcvr work\n", pem->index);
+
+	/* Check if HP interrupt thread is in progress
+	 * and wait for it to complete
+	 */
+	pcie = to_pciehp_dev(root_port);
+	if (!pcie)
+		return;
+	ctrl = get_service_data(pcie);
+	wait_event(ctrl->requester,
+		   !atomic_read(&ctrl->pending_events) &&
+		   !ctrl->ist_running);
+	dev_dbg(&pem_dev->dev, "PEM%d HP ist done\n", pem->index);
+
+	/* Disable hot-plug interrupt
+	 * Removal and rescan below would setup again.
+	 */
+	pcie_disable_interrupt(ctrl);
+	dev_dbg(&pem_dev->dev, "PEM%d Disable interrupt\n", pem->index);
+
+	pci_lock_rescan_remove();
+
+	pci_walk_bus(root_port->subordinate, pci_dev_set_disconnected, NULL);
+
+	/* Clean-up device and RC bridge */
+	pci_stop_and_remove_bus_device(root_port);
+
+	pci_unlock_rescan_remove();
+
+	usleep_range(100, 200);
+
+	writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
+
+	while (timeout--) {
+		/* Check for PEM_OOR to be set */
+		pem_reg = readq(pem->base + ON_OFFSET);
+		if (pem_reg & BIT(1))
+			break;
+		usleep_range(1000, 2000);
+	}
+	if (!timeout) {
+		dev_warn(&pem_dev->dev,
+			 "PEM failed to get out of reset\n");
+		return;
+	}
+
+	pci_lock_rescan_remove();
+
+	/*
+	 * Hardware resets and initializes config space of RC bridge
+	 * on every link down event with auto-mode in use.
+	 * Re-scan will setup RC bridge cleanly in kernel
+	 * after removal and to be ready for next link-up event.
+	 */
+	bus = NULL;
+	while ((bus = pci_find_next_bus(bus)) != NULL)
+		if (bus->domain_nr == rc_domain)
+			pci_rescan_bus(bus);
+	pci_unlock_rescan_remove();
+	pci_dev_put(root_port);
+
+	/* Ack interrupt */
+	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_OFFSET);
+	/* Enable RST_INT[LINKDOWN] interrupt */
+	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
+}
+
+static irqreturn_t pem_irq_handler(int irq, void *dev_id)
+{
+	struct pem_ctlr *pem = (struct pem_ctlr *)dev_id;
+
+	/* Disable RST_INT[LINKDOWN] interrupt */
+	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1C_OFFSET);
+	schedule_work(&pem->recover_rc_work);
+
+	return IRQ_HANDLED;
+}
+
+static int pem_register_interrupts(struct pci_dev *pdev)
+{
+	struct pem_ctlr *pem = pci_get_drvdata(pdev);
+	int nvec, err;
+
+	nvec = pci_msix_vec_count(pdev);
+	/* Some earlier silicon versions do not support RST vector
+	 * so check on table size before registering otherwise
+	 * return with info message.
+	 */
+	if (nvec != 10) {
+		dev_info(&pdev->dev,
+			 "No RST MSI-X vector support on silicon\n");
+		return 0;
+	}
+	err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX);
+	if (err < 0) {
+		dev_err(&pdev->dev, "pci_alloc_irq_vectors() failed %d\n",
+			nvec);
+		return -ENOSPC;
+	}
+
+	snprintf(pem->irq_name, 32, "PEM%d RST_INT", pem->index);
+
+	/* register interrupt for RST_INT */
+	return devm_request_irq(&pdev->dev, pci_irq_vector(pdev, 9),
+				pem_irq_handler, 0,
+				pem->irq_name, pem);
+}
+
+static int pem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct device *dev = &pdev->dev;
+	struct pem_ctlr *pem;
+	struct pci_dev *root_port;
+	int err, rc_domain;
+
+	pem = devm_kzalloc(dev, sizeof(struct pem_ctlr), GFP_KERNEL);
+	if (!pem)
+		return -ENOMEM;
+
+	pem->pdev = pdev;
+	pci_set_drvdata(pdev, pem);
+
+	err = pcim_enable_device(pdev);
+	if (err) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		goto enable_failed;
+	}
+
+	err = pci_request_regions(pdev, DRV_NAME);
+	if (err) {
+		dev_err(dev, "PCI request regions failed 0x%x\n", err);
+		goto region_failed;
+	}
+
+	pci_set_master(pdev);
+
+	/* CSR Space mapping */
+	pem->base = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
+	if (!pem->base) {
+		dev_err(&pdev->dev, "Unable to map BAR0\n");
+		err = -ENODEV;
+		goto bar0_map_failed;
+	}
+	pem->index = ((u64)pci_resource_start(pdev, 0) >> ID_SHIFT) & 0xf;
+
+	rc_domain = pem->index + DOMAIN_OFFSET;
+
+	root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
+	if (!root_port) {
+		dev_err(&pdev->dev, "failed to get root port\n");
+		goto bar0_map_failed;
+	}
+	if (!root_port->is_hotplug_bridge) {
+		dev_info(&pdev->dev, "Hot-plug disabled skip registration\n");
+		goto bar0_map_failed;
+	}
+
+	err = pem_register_interrupts(pdev);
+	if (err < 0) {
+		dev_err(dev, "Register interrupt failed\n");
+		goto irq_failed;
+	}
+
+	INIT_WORK(&pem->recover_rc_work, pem_recover_rc_link);
+
+	/* Enable RST_INT[LINKDOWN] interrupt */
+	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
+
+	dev_info(&pdev->dev, "PEM%d probed\n", pem->index);
+	return 0;
+
+irq_failed:
+bar0_map_failed:
+	pci_release_regions(pdev);
+region_failed:
+enable_failed:
+	pci_set_drvdata(pdev, NULL);
+	return err;
+}
+
+static void pem_remove(struct pci_dev *pdev)
+{
+	pci_release_regions(pdev);
+}
+
+/* Supported devices */
+static const struct pci_device_id pem_id_table[] = {
+	{PCI_VDEVICE(CAVIUM, PCI_DEVID_OCTEON_PEM)},
+	{0} /* end of table */
+};
+
+static struct pci_driver pem_driver = {
+	.name = DRV_NAME,
+	.id_table = pem_id_table,
+	.probe = pem_probe,
+	.remove = pem_remove,
+};
+
+module_pci_driver(pem_driver);
+
+MODULE_AUTHOR("Marvell Inc.");
+MODULE_DESCRIPTION("Marvell Octeon PEM Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+MODULE_DEVICE_TABLE(pci, pem_id_table);
-- 
2.43.0
Re: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
Posted by kernel test robot 2 weeks, 3 days ago
Hi Pragnesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/for-linus]
[also build test ERROR on linus/master v6.19-rc6 next-20260120]
[cannot apply to pci/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pragnesh-Patel/PCI-octeon-Add-link-down-handler-support-for-PCIe-MAC-controller/20260121-131806
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link:    https://lore.kernel.org/r/20260121051439.1882086-1-pragneshp%40marvell.com
patch subject: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20260122/202601220216.k4L3t8eH-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260122/202601220216.k4L3t8eH-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/202601220216.k4L3t8eH-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/controller/pci-octeon-pem.c: In function 'pem_recover_rc_link':
   drivers/pci/controller/pci-octeon-pem.c:105:9: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Wimplicit-function-declaration]
     105 |         writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
         |         ^~~~~~
         |         writel
   drivers/pci/controller/pci-octeon-pem.c:109:27: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Wimplicit-function-declaration]
     109 |                 pem_reg = readq(pem->base + ON_OFFSET);
         |                           ^~~~~
         |                           readl
>> drivers/pci/controller/pci-octeon-pem.c:130:24: error: 'struct pci_bus' has no member named 'domain_nr'
     130 |                 if (bus->domain_nr == rc_domain)
         |                        ^~


vim +130 drivers/pci/controller/pci-octeon-pem.c

    53	
    54	static void pem_recover_rc_link(struct work_struct *ws)
    55	{
    56		struct pem_ctlr *pem = container_of(ws, struct pem_ctlr,
    57						    recover_rc_work);
    58		struct pci_dev *pem_dev = pem->pdev;
    59		struct pci_dev *root_port;
    60		struct pci_bus *bus;
    61		struct pcie_device *pcie;
    62		struct controller *ctrl;
    63		int rc_domain, timeout = 100;
    64		u64 pem_reg;
    65	
    66		rc_domain = pem->index + DOMAIN_OFFSET;
    67	
    68		root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
    69		if (!root_port) {
    70			dev_err(&pem_dev->dev, "failed to get root port\n");
    71			return;
    72		}
    73	
    74		dev_dbg(&pem_dev->dev, "PEM%d rcvr work\n", pem->index);
    75	
    76		/* Check if HP interrupt thread is in progress
    77		 * and wait for it to complete
    78		 */
    79		pcie = to_pciehp_dev(root_port);
    80		if (!pcie)
    81			return;
    82		ctrl = get_service_data(pcie);
    83		wait_event(ctrl->requester,
    84			   !atomic_read(&ctrl->pending_events) &&
    85			   !ctrl->ist_running);
    86		dev_dbg(&pem_dev->dev, "PEM%d HP ist done\n", pem->index);
    87	
    88		/* Disable hot-plug interrupt
    89		 * Removal and rescan below would setup again.
    90		 */
    91		pcie_disable_interrupt(ctrl);
    92		dev_dbg(&pem_dev->dev, "PEM%d Disable interrupt\n", pem->index);
    93	
    94		pci_lock_rescan_remove();
    95	
    96		pci_walk_bus(root_port->subordinate, pci_dev_set_disconnected, NULL);
    97	
    98		/* Clean-up device and RC bridge */
    99		pci_stop_and_remove_bus_device(root_port);
   100	
   101		pci_unlock_rescan_remove();
   102	
   103		usleep_range(100, 200);
   104	
   105		writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
   106	
   107		while (timeout--) {
   108			/* Check for PEM_OOR to be set */
   109			pem_reg = readq(pem->base + ON_OFFSET);
   110			if (pem_reg & BIT(1))
   111				break;
   112			usleep_range(1000, 2000);
   113		}
   114		if (!timeout) {
   115			dev_warn(&pem_dev->dev,
   116				 "PEM failed to get out of reset\n");
   117			return;
   118		}
   119	
   120		pci_lock_rescan_remove();
   121	
   122		/*
   123		 * Hardware resets and initializes config space of RC bridge
   124		 * on every link down event with auto-mode in use.
   125		 * Re-scan will setup RC bridge cleanly in kernel
   126		 * after removal and to be ready for next link-up event.
   127		 */
   128		bus = NULL;
   129		while ((bus = pci_find_next_bus(bus)) != NULL)
 > 130			if (bus->domain_nr == rc_domain)
   131				pci_rescan_bus(bus);
   132		pci_unlock_rescan_remove();
   133		pci_dev_put(root_port);
   134	
   135		/* Ack interrupt */
   136		writeq(RST_INT_LINKDOWN, pem->base + RST_INT_OFFSET);
   137		/* Enable RST_INT[LINKDOWN] interrupt */
   138		writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
   139	}
   140	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
Posted by Bjorn Helgaas 2 weeks, 3 days ago
On Tue, Jan 20, 2026 at 09:14:29PM -0800, Pragnesh Patel wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> This driver adds support for link-down interrupt in PCIe MAC (PEM)
> controller in Root complex mode to support hot-plug removal of
> endpoint devices.

PEM?

> An interrupt handler is registered for RST_INT msix vector
> which is triggered with link going down. This handler
> performs cleanup of root bridge and its children and
> re-initializes root bridge to kernel for next link-up event.

Wrap to fill 75 columns.

> +config PCI_OCTEON_PEM
> +	bool "Marvell Octeon PEM (PCIe MAC) controller"
> +	depends on ARM64 || COMPILE_TEST
> +	depends on PCI_MSI
> +	depends on HOTPLUG_PCI_PCIE
> +	help
> +	  Say Y here if you want PEM controller support for Marvell ARM64 Octeon SoCs
> +	  in root complex mode.

Wrap to fit in 80 columns like the rest of the file.

> +++ b/drivers/pci/controller/pci-octeon-pem.c

This looks like a PCIe controller, so name it "pcie-octeon-pem.c".
There are some older drivers for PCIe controllers that use "pci-", but
that was a mistake.

> +#include "../hotplug/pciehp.h"

This looks like a red flag, see below.

> +#define PCI_DEVID_OCTEON_PEM	0xA06C

Typical names are PCI_DEVICE_ID_<vendor>_<device> (see
include/linux/pci_ids.h).

In this case, I guess it would be "PCI_DEVICE_ID_CAVIUM_OCTEON" or
similar.

> +#define ID_SHIFT		36

Use GENMASK() instead of shift/mask, so you don't need #defines like
this.

> +#define DOMAIN_OFFSET		0x3
> +#define ON_OFFSET		0xE0
> +#define RST_SOFT_PERST_OFFSET	0x298
> +#define RST_INT_OFFSET		0x300
> +#define RST_INT_ENA_W1C_OFFSET	0x310
> +#define RST_INT_ENA_W1S_OFFSET	0x318
> +#define RST_INT_LINKDOWN	BIT(1)

The "_OFFSET" suffixes look unnecessarily wordy.

> +static void pem_recover_rc_link(struct work_struct *ws)
> +{

> +	/* Check if HP interrupt thread is in progress
> +	 * and wait for it to complete
> +	 */

Multi-line comment style in drivers/pci/ is:

  /*
   * Check ...
   */

Wrap to fill 78 columns or so.

> +	dev_dbg(&pem_dev->dev, "PEM%d HP ist done\n", pem->index);

s/ist/is/

> +	/* Clean-up device and RC bridge */
> +	pci_stop_and_remove_bus_device(root_port);

I'm doubtful about removing a Root Port.  I don't know of any standard
PCIe mechanism for doing that.

> +	pci_unlock_rescan_remove();
> +
> +	usleep_range(100, 200);

Where does this delay come from?  If it's something in the PCIe spec,
we should have a #define in drivers/pci/pci.h for it.  If it's in the
Octeon spec, a reference to that would be helpful.

> +	writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
> +
> +	while (timeout--) {
> +		/* Check for PEM_OOR to be set */
> +		pem_reg = readq(pem->base + ON_OFFSET);
> +		if (pem_reg & BIT(1))

Add a #define for this BIT(1).

> +			break;
> +		usleep_range(1000, 2000);

Same delay question as above.

> +	}
> +	if (!timeout) {
> +		dev_warn(&pem_dev->dev,
> +			 "PEM failed to get out of reset\n");
> +		return;
> +	}
> +
> +	pci_lock_rescan_remove();

I haven't reviewed this in detail, but it looks like unusual knowledge
of pciehp internals that might not be appropriate for a controller
driver.

> +static int pem_register_interrupts(struct pci_dev *pdev)
> +{
> +	struct pem_ctlr *pem = pci_get_drvdata(pdev);
> +	int nvec, err;
> +
> +	nvec = pci_msix_vec_count(pdev);

Add blank line before comment, use multi-line comment style.

> +	/* Some earlier silicon versions do not support RST vector
> +	 * so check on table size before registering otherwise
> +	 * return with info message.
> +	 */

> +/* Supported devices */
> +static const struct pci_device_id pem_id_table[] = {
> +	{PCI_VDEVICE(CAVIUM, PCI_DEVID_OCTEON_PEM)},

Same question as Mani, I guess.  PCI controllers are not themselves
PCI devices; they're platform_devices normally discovered via ACPI or
DT.

Root Ports *are* PCIe devices, and normally don't need device specific
code since they are PCI-to-PCI bridges.  The device-specific code is
usually in a PCIe controller (Root Complex) driver that claims the
relevant platform_device.

It's problematic to claim Root Ports, because portdrv normally claims
them so it can support AER, pciehp, etc.
Re: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
Posted by Manivannan Sadhasivam 2 weeks, 3 days ago
On Tue, Jan 20, 2026 at 09:14:29PM -0800, Pragnesh Patel wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> This driver adds support for link-down interrupt in PCIe MAC (PEM)
> controller in Root complex mode to support hot-plug removal of
> endpoint devices.
> 

What is this device actually? Is this really a Root Complex device? Root complex
(host bridge) is the one that exposes the Root bus and not gets plugged into the
bus. This looks very strange.

> An interrupt handler is registered for RST_INT msix vector
> which is triggered with link going down. This handler
> performs cleanup of root bridge and its children and
> re-initializes root bridge to kernel for next link-up event.
> 

I have a series [1] (needs respin) that allows the controller drivers to reset
the Root Ports in the event of link down. You might want to take a look at that.
I'll try to respin asap.

- Mani

[1] https://lore.kernel.org/linux-pci/20250715-pci-port-reset-v6-0-6f9cce94e7bb@oss.qualcomm.com/

> Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
> Signed-off-by: Pragnesh Patel <pragneshp@marvell.com>
> ---
>  MAINTAINERS                             |   6 +
>  drivers/pci/controller/Kconfig          |   9 +
>  drivers/pci/controller/Makefile         |   1 +
>  drivers/pci/controller/pci-octeon-pem.c | 278 ++++++++++++++++++++++++
>  4 files changed, 294 insertions(+)
>  create mode 100644 drivers/pci/controller/pci-octeon-pem.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1b020588597..9af8f676c027 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15363,6 +15363,12 @@ R:	Vamsi Attunuru <vattunuru@marvell.com>
>  S:	Supported
>  F:	drivers/pci/hotplug/octep_hp.c
>  
> +MARVELL OCTEON PEM CONTROLLER DRIVER
> +R:	George Cherian <gcherian@marvell.com>
> +R:	Pragnesh Patel <pragneshp@marvell.com>
> +S:	Supported
> +F:	drivers/pci/controller/pci-octeon-pem.c
> +
>  MATROX FRAMEBUFFER DRIVER
>  L:	linux-fbdev@vger.kernel.org
>  S:	Orphan
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index c254d2b8bf17..5251dc8b1188 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -231,6 +231,15 @@ config PCI_HYPERV_INTERFACE
>  	  drivers to have a common interface with the Hyper-V PCI frontend
>  	  driver.
>  
> +config PCI_OCTEON_PEM
> +	bool "Marvell Octeon PEM (PCIe MAC) controller"
> +	depends on ARM64 || COMPILE_TEST
> +	depends on PCI_MSI
> +	depends on HOTPLUG_PCI_PCIE
> +	help
> +	  Say Y here if you want PEM controller support for Marvell ARM64 Octeon SoCs
> +	  in root complex mode.
> +
>  config PCI_TEGRA
>  	bool "NVIDIA Tegra PCIe controller"
>  	depends on ARCH_TEGRA || COMPILE_TEST
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index 229929a945c2..dd1dab50dd07 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o
>  obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>  obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>  obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
> +obj-$(CONFIG_PCI_OCTEON_PEM) += pci-octeon-pem.o
>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>  obj-$(CONFIG_PCIE_IPROC_MSI) += pcie-iproc-msi.o
>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
> diff --git a/drivers/pci/controller/pci-octeon-pem.c b/drivers/pci/controller/pci-octeon-pem.c
> new file mode 100644
> index 000000000000..2d67e2b16e9e
> --- /dev/null
> +++ b/drivers/pci/controller/pci-octeon-pem.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Octeon PEM driver
> + *
> + * Copyright (C) 2021 Marvell.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/sysfs.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include "../hotplug/pciehp.h"
> +#include "../pci.h"
> +
> +#define DRV_NAME	"octeon-pem"
> +#define DRV_VERSION	"1.0"
> +
> +#define PCI_DEVID_OCTEON_PEM	0xA06C
> +
> +#define ID_SHIFT		36
> +#define DOMAIN_OFFSET		0x3
> +#define ON_OFFSET		0xE0
> +#define RST_SOFT_PERST_OFFSET	0x298
> +#define RST_INT_OFFSET		0x300
> +#define RST_INT_ENA_W1C_OFFSET	0x310
> +#define RST_INT_ENA_W1S_OFFSET	0x318
> +#define RST_INT_LINKDOWN	BIT(1)
> +
> +struct pem_ctlr {
> +	int			index;
> +	char			irq_name[32];
> +	void __iomem		*base;
> +	struct pci_dev		*pdev;
> +	struct work_struct	recover_rc_work;
> +};
> +
> +static struct pcie_device *to_pciehp_dev(struct pci_dev *dev)
> +{
> +	struct device *device;
> +
> +	device = pcie_port_find_device(dev, PCIE_PORT_SERVICE_HP);
> +	if (!device)
> +		return NULL;
> +	return to_pcie_device(device);
> +}
> +
> +static void pem_recover_rc_link(struct work_struct *ws)
> +{
> +	struct pem_ctlr *pem = container_of(ws, struct pem_ctlr,
> +					    recover_rc_work);
> +	struct pci_dev *pem_dev = pem->pdev;
> +	struct pci_dev *root_port;
> +	struct pci_bus *bus;
> +	struct pcie_device *pcie;
> +	struct controller *ctrl;
> +	int rc_domain, timeout = 100;
> +	u64 pem_reg;
> +
> +	rc_domain = pem->index + DOMAIN_OFFSET;
> +
> +	root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
> +	if (!root_port) {
> +		dev_err(&pem_dev->dev, "failed to get root port\n");
> +		return;
> +	}
> +
> +	dev_dbg(&pem_dev->dev, "PEM%d rcvr work\n", pem->index);
> +
> +	/* Check if HP interrupt thread is in progress
> +	 * and wait for it to complete
> +	 */
> +	pcie = to_pciehp_dev(root_port);
> +	if (!pcie)
> +		return;
> +	ctrl = get_service_data(pcie);
> +	wait_event(ctrl->requester,
> +		   !atomic_read(&ctrl->pending_events) &&
> +		   !ctrl->ist_running);
> +	dev_dbg(&pem_dev->dev, "PEM%d HP ist done\n", pem->index);
> +
> +	/* Disable hot-plug interrupt
> +	 * Removal and rescan below would setup again.
> +	 */
> +	pcie_disable_interrupt(ctrl);
> +	dev_dbg(&pem_dev->dev, "PEM%d Disable interrupt\n", pem->index);
> +
> +	pci_lock_rescan_remove();
> +
> +	pci_walk_bus(root_port->subordinate, pci_dev_set_disconnected, NULL);
> +
> +	/* Clean-up device and RC bridge */
> +	pci_stop_and_remove_bus_device(root_port);
> +
> +	pci_unlock_rescan_remove();
> +
> +	usleep_range(100, 200);
> +
> +	writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
> +
> +	while (timeout--) {
> +		/* Check for PEM_OOR to be set */
> +		pem_reg = readq(pem->base + ON_OFFSET);
> +		if (pem_reg & BIT(1))
> +			break;
> +		usleep_range(1000, 2000);
> +	}
> +	if (!timeout) {
> +		dev_warn(&pem_dev->dev,
> +			 "PEM failed to get out of reset\n");
> +		return;
> +	}
> +
> +	pci_lock_rescan_remove();
> +
> +	/*
> +	 * Hardware resets and initializes config space of RC bridge
> +	 * on every link down event with auto-mode in use.
> +	 * Re-scan will setup RC bridge cleanly in kernel
> +	 * after removal and to be ready for next link-up event.
> +	 */
> +	bus = NULL;
> +	while ((bus = pci_find_next_bus(bus)) != NULL)
> +		if (bus->domain_nr == rc_domain)
> +			pci_rescan_bus(bus);
> +	pci_unlock_rescan_remove();
> +	pci_dev_put(root_port);
> +
> +	/* Ack interrupt */
> +	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_OFFSET);
> +	/* Enable RST_INT[LINKDOWN] interrupt */
> +	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
> +}
> +
> +static irqreturn_t pem_irq_handler(int irq, void *dev_id)
> +{
> +	struct pem_ctlr *pem = (struct pem_ctlr *)dev_id;
> +
> +	/* Disable RST_INT[LINKDOWN] interrupt */
> +	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1C_OFFSET);
> +	schedule_work(&pem->recover_rc_work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int pem_register_interrupts(struct pci_dev *pdev)
> +{
> +	struct pem_ctlr *pem = pci_get_drvdata(pdev);
> +	int nvec, err;
> +
> +	nvec = pci_msix_vec_count(pdev);
> +	/* Some earlier silicon versions do not support RST vector
> +	 * so check on table size before registering otherwise
> +	 * return with info message.
> +	 */
> +	if (nvec != 10) {
> +		dev_info(&pdev->dev,
> +			 "No RST MSI-X vector support on silicon\n");
> +		return 0;
> +	}
> +	err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "pci_alloc_irq_vectors() failed %d\n",
> +			nvec);
> +		return -ENOSPC;
> +	}
> +
> +	snprintf(pem->irq_name, 32, "PEM%d RST_INT", pem->index);
> +
> +	/* register interrupt for RST_INT */
> +	return devm_request_irq(&pdev->dev, pci_irq_vector(pdev, 9),
> +				pem_irq_handler, 0,
> +				pem->irq_name, pem);
> +}
> +
> +static int pem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct pem_ctlr *pem;
> +	struct pci_dev *root_port;
> +	int err, rc_domain;
> +
> +	pem = devm_kzalloc(dev, sizeof(struct pem_ctlr), GFP_KERNEL);
> +	if (!pem)
> +		return -ENOMEM;
> +
> +	pem->pdev = pdev;
> +	pci_set_drvdata(pdev, pem);
> +
> +	err = pcim_enable_device(pdev);
> +	if (err) {
> +		dev_err(dev, "Failed to enable PCI device\n");
> +		goto enable_failed;
> +	}
> +
> +	err = pci_request_regions(pdev, DRV_NAME);
> +	if (err) {
> +		dev_err(dev, "PCI request regions failed 0x%x\n", err);
> +		goto region_failed;
> +	}
> +
> +	pci_set_master(pdev);
> +
> +	/* CSR Space mapping */
> +	pem->base = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
> +	if (!pem->base) {
> +		dev_err(&pdev->dev, "Unable to map BAR0\n");
> +		err = -ENODEV;
> +		goto bar0_map_failed;
> +	}
> +	pem->index = ((u64)pci_resource_start(pdev, 0) >> ID_SHIFT) & 0xf;
> +
> +	rc_domain = pem->index + DOMAIN_OFFSET;
> +
> +	root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
> +	if (!root_port) {
> +		dev_err(&pdev->dev, "failed to get root port\n");
> +		goto bar0_map_failed;
> +	}
> +	if (!root_port->is_hotplug_bridge) {
> +		dev_info(&pdev->dev, "Hot-plug disabled skip registration\n");
> +		goto bar0_map_failed;
> +	}
> +
> +	err = pem_register_interrupts(pdev);
> +	if (err < 0) {
> +		dev_err(dev, "Register interrupt failed\n");
> +		goto irq_failed;
> +	}
> +
> +	INIT_WORK(&pem->recover_rc_work, pem_recover_rc_link);
> +
> +	/* Enable RST_INT[LINKDOWN] interrupt */
> +	writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
> +
> +	dev_info(&pdev->dev, "PEM%d probed\n", pem->index);
> +	return 0;
> +
> +irq_failed:
> +bar0_map_failed:
> +	pci_release_regions(pdev);
> +region_failed:
> +enable_failed:
> +	pci_set_drvdata(pdev, NULL);
> +	return err;
> +}
> +
> +static void pem_remove(struct pci_dev *pdev)
> +{
> +	pci_release_regions(pdev);
> +}
> +
> +/* Supported devices */
> +static const struct pci_device_id pem_id_table[] = {
> +	{PCI_VDEVICE(CAVIUM, PCI_DEVID_OCTEON_PEM)},
> +	{0} /* end of table */
> +};
> +
> +static struct pci_driver pem_driver = {
> +	.name = DRV_NAME,
> +	.id_table = pem_id_table,
> +	.probe = pem_probe,
> +	.remove = pem_remove,
> +};
> +
> +module_pci_driver(pem_driver);
> +
> +MODULE_AUTHOR("Marvell Inc.");
> +MODULE_DESCRIPTION("Marvell Octeon PEM Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_DEVICE_TABLE(pci, pem_id_table);
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
Posted by kernel test robot 2 weeks, 3 days ago
Hi Pragnesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/for-linus]
[also build test ERROR on linus/master v6.19-rc6 next-20260120]
[cannot apply to pci/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pragnesh-Patel/PCI-octeon-Add-link-down-handler-support-for-PCIe-MAC-controller/20260121-131806
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link:    https://lore.kernel.org/r/20260121051439.1882086-1-pragneshp%40marvell.com
patch subject: [PATCH] PCI: octeon: Add link down handler support for PCIe MAC controller
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20260121/202601211935.bHNggPkG-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260121/202601211935.bHNggPkG-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/202601211935.bHNggPkG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/pci/controller/pci-octeon-pem.c:130:12: error: no member named 'domain_nr' in 'struct pci_bus'
     130 |                 if (bus->domain_nr == rc_domain)
         |                     ~~~  ^
   1 error generated.


vim +130 drivers/pci/controller/pci-octeon-pem.c

    53	
    54	static void pem_recover_rc_link(struct work_struct *ws)
    55	{
    56		struct pem_ctlr *pem = container_of(ws, struct pem_ctlr,
    57						    recover_rc_work);
    58		struct pci_dev *pem_dev = pem->pdev;
    59		struct pci_dev *root_port;
    60		struct pci_bus *bus;
    61		struct pcie_device *pcie;
    62		struct controller *ctrl;
    63		int rc_domain, timeout = 100;
    64		u64 pem_reg;
    65	
    66		rc_domain = pem->index + DOMAIN_OFFSET;
    67	
    68		root_port = pci_get_domain_bus_and_slot(rc_domain, 0, 0);
    69		if (!root_port) {
    70			dev_err(&pem_dev->dev, "failed to get root port\n");
    71			return;
    72		}
    73	
    74		dev_dbg(&pem_dev->dev, "PEM%d rcvr work\n", pem->index);
    75	
    76		/* Check if HP interrupt thread is in progress
    77		 * and wait for it to complete
    78		 */
    79		pcie = to_pciehp_dev(root_port);
    80		if (!pcie)
    81			return;
    82		ctrl = get_service_data(pcie);
    83		wait_event(ctrl->requester,
    84			   !atomic_read(&ctrl->pending_events) &&
    85			   !ctrl->ist_running);
    86		dev_dbg(&pem_dev->dev, "PEM%d HP ist done\n", pem->index);
    87	
    88		/* Disable hot-plug interrupt
    89		 * Removal and rescan below would setup again.
    90		 */
    91		pcie_disable_interrupt(ctrl);
    92		dev_dbg(&pem_dev->dev, "PEM%d Disable interrupt\n", pem->index);
    93	
    94		pci_lock_rescan_remove();
    95	
    96		pci_walk_bus(root_port->subordinate, pci_dev_set_disconnected, NULL);
    97	
    98		/* Clean-up device and RC bridge */
    99		pci_stop_and_remove_bus_device(root_port);
   100	
   101		pci_unlock_rescan_remove();
   102	
   103		usleep_range(100, 200);
   104	
   105		writeq(0x0, pem->base + RST_SOFT_PERST_OFFSET);
   106	
   107		while (timeout--) {
   108			/* Check for PEM_OOR to be set */
   109			pem_reg = readq(pem->base + ON_OFFSET);
   110			if (pem_reg & BIT(1))
   111				break;
   112			usleep_range(1000, 2000);
   113		}
   114		if (!timeout) {
   115			dev_warn(&pem_dev->dev,
   116				 "PEM failed to get out of reset\n");
   117			return;
   118		}
   119	
   120		pci_lock_rescan_remove();
   121	
   122		/*
   123		 * Hardware resets and initializes config space of RC bridge
   124		 * on every link down event with auto-mode in use.
   125		 * Re-scan will setup RC bridge cleanly in kernel
   126		 * after removal and to be ready for next link-up event.
   127		 */
   128		bus = NULL;
   129		while ((bus = pci_find_next_bus(bus)) != NULL)
 > 130			if (bus->domain_nr == rc_domain)
   131				pci_rescan_bus(bus);
   132		pci_unlock_rescan_remove();
   133		pci_dev_put(root_port);
   134	
   135		/* Ack interrupt */
   136		writeq(RST_INT_LINKDOWN, pem->base + RST_INT_OFFSET);
   137		/* Enable RST_INT[LINKDOWN] interrupt */
   138		writeq(RST_INT_LINKDOWN, pem->base + RST_INT_ENA_W1S_OFFSET);
   139	}
   140	

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