public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Bjorn Helgaas <bhelgaas@google.com>, <linux-cxl@vger.kernel.org>,
	<linux-pci@vger.kernel.org>
Cc: "Davidlohr Bueso" <dave@stgolabs.net>,
	"Dave Jiang" <dave.jiang@intel.com>,
	"Alison Schofield" <alison.schofield@intel.com>,
	"Vishal Verma" <vishal.l.verma@intel.com>,
	"Ira Weiny" <ira.weiny@intel.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Will Deacon" <will@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	linuxarm@huawei.com, terry.bowman@amd.com,
	"Kuppuswamy Sathyanarayanan"
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [RFC PATCH 9/9] pci: pcie: portdrv: aer: Switch to auxiliary_bus
Date: Wed, 29 May 2024 17:41:03 +0100	[thread overview]
Message-ID: <20240529164103.31671-10-Jonathan.Cameron@huawei.com> (raw)
In-Reply-To: <20240529164103.31671-1-Jonathan.Cameron@huawei.com>

Not sure if this is a good idea or not. (probably not!)
Is it useful to add a bus device for a feature that there is
only ever one of?  Maybe or perhaps better option is to convert
this to directly called library code rather than a device + separate
driver.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
Discussion of options / why in the cover letter.

 drivers/pci/pcie/aer.c     | 67 +++++++++++++++++++++++++-------------
 drivers/pci/pcie/portdrv.c | 34 ++++++++++++++++---
 2 files changed, 75 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index e07c5bf71372..2df996d80849 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -15,6 +15,7 @@
 #define pr_fmt(fmt) "AER: " fmt
 #define dev_fmt pr_fmt
 
+#include <linux/auxiliary_bus.h>
 #include <linux/bitops.h>
 #include <linux/cper.h>
 #include <linux/pci.h>
@@ -1327,8 +1328,9 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
  */
 static irqreturn_t aer_isr(int irq, void *context)
 {
-	struct pcie_device *dev = (struct pcie_device *)context;
-	struct aer_rpc *rpc = dev_get_drvdata(&dev->device);
+	struct pcie_port_aux_dev *pcie_adev = context;
+	struct device *dev = &pcie_adev->adev.dev;
+	struct aer_rpc *rpc = dev_get_drvdata(dev);
 	struct aer_err_source e_src;
 
 	if (kfifo_is_empty(&rpc->aer_fifo))
@@ -1348,8 +1350,9 @@ static irqreturn_t aer_isr(int irq, void *context)
  */
 static irqreturn_t aer_irq(int irq, void *context)
 {
-	struct pcie_device *pdev = (struct pcie_device *)context;
-	struct aer_rpc *rpc = dev_get_drvdata(&pdev->device);
+	struct pcie_port_aux_dev *pcie_adev = context;
+	struct device *dev = &pcie_adev->adev.dev;
+	struct aer_rpc *rpc = dev_get_drvdata(dev);
 	struct pci_dev *rp = rpc->rpd;
 	int aer = rp->aer_cap;
 	struct aer_err_source e_src = {};
@@ -1442,58 +1445,74 @@ static void aer_disable_rootport(struct aer_rpc *rpc)
 
 /**
  * aer_remove - clean up resources
- * @dev: pointer to the pcie_dev data structure
+ * @adev: pointer to the auxiliary device within the pcie_port_aux_dev
  *
  * Invoked when PCI Express bus unloads or AER probe fails.
  */
-static void aer_remove(struct pcie_device *dev)
+static void aer_remove(struct auxiliary_device *adev)
 {
-	struct aer_rpc *rpc = dev_get_drvdata(&dev->device);
-
+	struct aer_rpc *rpc = dev_get_drvdata(&adev->dev);
 	aer_disable_rootport(rpc);
 }
 
 /**
  * aer_probe - initialize resources
- * @dev: pointer to the pcie_dev data structure
+ * @adev: pointer to the auxiliary device within the pcie_port_aux_dev
+ * @id: id table entry that matched.
  *
  * Invoked when PCI Express bus loads AER service driver.
  */
-static int aer_probe(struct pcie_device *dev)
+static int aer_probe(struct auxiliary_device *adev,
+		     const struct auxiliary_device_id *id)
 {
+	struct pcie_port_aux_dev *pcie_adev = to_pcie_port_aux_dev(adev);
 	int status;
 	struct aer_rpc *rpc;
-	struct device *device = &dev->device;
-	struct pci_dev *port = dev->port;
+	struct device *dev = &adev->dev;
+	struct pci_dev *port = to_pci_dev(dev->parent);
+	u32 pos, reg32;
+	int irq_num, irq;
 
 	BUILD_BUG_ON(ARRAY_SIZE(aer_correctable_error_string) <
 		     AER_MAX_TYPEOF_COR_ERRS);
 	BUILD_BUG_ON(ARRAY_SIZE(aer_uncorrectable_error_string) <
 		     AER_MAX_TYPEOF_UNCOR_ERRS);
 
+	pos = port->aer_cap;
+	if (pos) {
+		pci_read_config_dword(port, pos + PCI_ERR_ROOT_STATUS,
+				&reg32);
+		irq_num = FIELD_GET(PCI_ERR_ROOT_AER_IRQ, reg32);
+		/* To check - is this sufficient to detect legacy case? */
+		if (pci_dev_msi_enabled(port))
+			irq = pci_irq_vector(port, irq_num);
+		else
+			irq = pci_irq_vector(port, 0);
+	}
+
 	/* Limit to Root Ports or Root Complex Event Collectors */
 	if ((pci_pcie_type(port) != PCI_EXP_TYPE_RC_EC) &&
 	    (pci_pcie_type(port) != PCI_EXP_TYPE_ROOT_PORT))
 		return -ENODEV;
 
-	rpc = devm_kzalloc(device, sizeof(struct aer_rpc), GFP_KERNEL);
+	rpc = devm_kzalloc(dev, sizeof(struct aer_rpc), GFP_KERNEL);
 	if (!rpc)
 		return -ENOMEM;
 
 	rpc->rpd = port;
 	INIT_KFIFO(rpc->aer_fifo);
-	dev_set_drvdata(&dev->device, rpc);
+	dev_set_drvdata(&adev->dev, rpc);
 
-	status = devm_request_threaded_irq(device, dev->irq, aer_irq, aer_isr,
-					   IRQF_SHARED, "aerdrv", dev);
+	status = devm_request_threaded_irq(dev, irq, aer_irq, aer_isr,
+					   IRQF_SHARED, "aerdrv", pcie_adev);
 	if (status) {
-		pci_err(port, "request AER IRQ %d failed\n", dev->irq);
+		pci_err(port, "request AER IRQ %d failed\n", irq);
 		return status;
 	}
 
 	cxl_rch_enable_rcec(port);
 	aer_enable_rootport(rpc);
-	pci_info(port, "enabled with IRQ %d\n", dev->irq);
+	pci_info(port, "enabled with IRQ %d\n", irq);
 	return 0;
 }
 
@@ -1555,12 +1574,16 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
 	return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
 }
 
-static struct pcie_port_service_driver aerdriver = {
-	.name		= "aer",
-	.service	= PCIE_PORT_SERVICE_AER,
+static const struct auxiliary_device_id aer_ids[] = {
+	{ .name = "pcieportdrv.aer" },
+	{ }
+};
 
+static struct auxiliary_driver aerdriver = {
+	.name		= "aer",
 	.probe		= aer_probe,
 	.remove		= aer_remove,
+	.id_table	= aer_ids,
 };
 
 /**
@@ -1572,5 +1595,5 @@ int __init pcie_aer_init(void)
 {
 	if (!pci_aer_available())
 		return -ENXIO;
-	return pcie_port_service_register(&aerdriver);
+	return auxiliary_driver_register(&aerdriver);
 }
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 7274ee55a8c3..14aedce3be3b 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -162,9 +162,6 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 		irqs[PCIE_PORT_SERVICE_BWNOTIF_SHIFT] = pcie_irq;
 	}
 
-	if (mask & PCIE_PORT_SERVICE_AER)
-		irqs[PCIE_PORT_SERVICE_AER_SHIFT] = pci_irq_vector(dev, aer);
-
 	if (mask & PCIE_PORT_SERVICE_DPC)
 		irqs[PCIE_PORT_SERVICE_DPC_SHIFT] = pci_irq_vector(dev, dpc);
 
@@ -210,6 +207,15 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 	return 0;
 }
 
+DEFINE_IDA(pcie_portdrv_aer_ida);
+
+static void pcie_port_aer_adev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	ida_free(&pcie_portdrv_aer_ida, adev->id);
+}
+
 /**
  * get_port_device_capability - discover capabilities of a PCI Express port
  * @dev: PCI Express port to examine
@@ -245,8 +251,28 @@ static int get_port_device_capability(struct pci_dev *dev,
 	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
              pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
 	    dev->aer_cap && pci_aer_available() &&
-	    (pcie_ports_native || host->native_aer))
+	    (pcie_ports_native || host->native_aer)) {
+		struct pcie_port_aux_dev *pcie_adev;
+		int id, ret;
+
+		pcie_adev = devm_kzalloc(&dev->dev, sizeof(*pcie_adev), GFP_KERNEL);
+		if (!pcie_adev)
+			return -ENOMEM;
+
+		id = ida_alloc(&pcie_portdrv_aer_ida, GFP_KERNEL);
+		if (id < 0)
+			return id;
+		pcie_adev->adev.name = "aer";
+		pcie_adev->adev.id = id;
+		pcie_adev->adev.dev.parent = &dev->dev;
+		pcie_adev->adev.dev.release = pcie_port_aer_adev_release;
+
+		ret = devm_pcie_port_aux_dev_init(&dev->dev, pcie_adev);
+		if (ret)
+			return ret;
+		list_add(&pcie_adev->node, aux_dev_list);
 		services |= PCIE_PORT_SERVICE_AER;
+	}
 #endif
 
 	/* Root Ports and Root Complex Event Collectors may generate PMEs */
-- 
2.39.2


  parent reply	other threads:[~2024-05-29 16:45 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29 16:40 [RFC PATCH 0/9] pci: portdrv: Add auxiliary bus and register CXL PMUs (and aer) Jonathan Cameron
2024-05-29 16:40 ` [RFC PATCH 1/9] pci: pcie: Drop priv_data from struct pcie_device and use dev_get/set_drvdata() instead Jonathan Cameron
2024-05-29 16:40 ` [RFC PATCH 2/9] pci: portdrv: Drop driver field for port type Jonathan Cameron
2024-05-29 16:40 ` [RFC PATCH 3/9] pci: pcie: portdrv: Use managed device handling to simplify error and remove flows Jonathan Cameron
2024-05-29 16:40 ` [RFC PATCH 4/9] auxiliary_bus: expose auxiliary_bus_type Jonathan Cameron
2024-05-29 16:40 ` [RFC PATCH 5/9] pci: pcie: portdrv: Add a auxiliary_bus Jonathan Cameron
2024-05-29 16:41 ` [RFC PATCH 6/9] cxl: Move CPMU register definitions to header Jonathan Cameron
2024-05-29 16:41 ` [RFC PATCH 7/9] pci: pcie/cxl: Register an auxiliary device for each CPMU instance Jonathan Cameron
2024-05-29 16:41 ` [RFC PATCH 8/9] perf: cxl: Make the cpmu driver also work with auxiliary_devices Jonathan Cameron
2024-05-29 16:41 ` Jonathan Cameron [this message]
2024-06-05 18:04 ` [RFC PATCH 0/9] pci: portdrv: Add auxiliary bus and register CXL PMUs (and aer) Bjorn Helgaas
2024-06-05 19:44   ` Jonathan Cameron
2024-06-06 12:57     ` Jonathan Cameron
2024-06-06 13:21       ` Lukas Wunner
2024-08-23 11:05         ` Jonathan Cameron
2024-08-28 21:11           ` Thomas Gleixner
2024-08-29 12:17             ` Jonathan Cameron
2024-09-05 11:23             ` Jonathan Cameron
2024-09-06 10:11               ` Thomas Gleixner
2024-09-06 17:18                 ` Jonathan Cameron
2024-09-10 16:47                   ` Jonathan Cameron
2024-09-10 17:37                     ` Jonathan Cameron
2024-09-10 20:04                     ` Thomas Gleixner
2024-09-12 16:37                       ` Jonathan Cameron
2024-09-12 17:34                         ` Jonathan Cameron
2024-09-13 16:24                           ` Thomas Gleixner
2024-06-17  7:03       ` Ilpo Järvinen
2024-07-04 16:14       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240529164103.31671-10-Jonathan.Cameron@huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=lpieralisi@kernel.org \
    --cc=mahesh@linux.ibm.com \
    --cc=mark.rutland@arm.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=terry.bowman@amd.com \
    --cc=vishal.l.verma@intel.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox