* [PATCH V2 1/7] PCI: dwc: Move MSI functionality related code to separate file
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-31 16:40 ` Manivannan Sadhasivam
2024-07-15 18:13 ` [PATCH V222/7] PCI: dwc: Add msi_ops to allow DBI based MSI register access Mayank Rana
` (7 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
This change moves dwc PCIe controller specific MSI functionality
into separate file in preparation to allow MSI functionality with
PCIe ECAM driver. Update existing drivers to accommodate this change.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/dwc/Makefile | 2 +-
drivers/pci/controller/dwc/pci-keystone.c | 12 +-
drivers/pci/controller/dwc/pcie-designware-host.c | 420 +---------------------
drivers/pci/controller/dwc/pcie-designware-msi.c | 409 +++++++++++++++++++++
drivers/pci/controller/dwc/pcie-designware-msi.h | 43 +++
drivers/pci/controller/dwc/pcie-designware.c | 1 +
drivers/pci/controller/dwc/pcie-designware.h | 26 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 1 +
drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
9 files changed, 484 insertions(+), 435 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.c
create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.h
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index bac103f..2ecc603 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PCIE_DW) += pcie-designware.o
-obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
+obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o pcie-designware-msi.o
obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index cd0e002..b95d319 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -307,8 +307,14 @@ static int ks_pcie_msi_host_init(struct dw_pcie_rp *pp)
*/
dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, ks_pcie->app.start);
- pp->msi_irq_chip = &ks_pcie_msi_irq_chip;
- return dw_pcie_allocate_domains(pp);
+ pp->msi = devm_kzalloc(pci->dev, sizeof(struct dw_msi *), GFP_KERNEL);
+ if (pp->msi == NULL)
+ return -ENOMEM;
+
+ pp->msi->msi_irq_chip = &ks_pcie_msi_irq_chip;
+ pp->msi->dev = pci->dev;
+ pp->msi->private_data = pp;
+ return dw_pcie_allocate_domains(pp->msi);
}
static void ks_pcie_handle_intx_irq(struct keystone_pcie *ks_pcie,
@@ -322,7 +328,7 @@ static void ks_pcie_handle_intx_irq(struct keystone_pcie *ks_pcie,
if (BIT(0) & pending) {
dev_dbg(dev, ": irq: irq_offset %d", offset);
- generic_handle_domain_irq(ks_pcie->intx_irq_domain, offset);
+ generic_handle_domain_irq(ks_pcie->msi->intx_irq_domain, offset);
}
/* EOI the INTx interrupt */
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index a0822d5..3dcf88a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -9,9 +9,6 @@
*/
#include <linux/iopoll.h>
-#include <linux/irqchip/chained_irq.h>
-#include <linux/irqdomain.h>
-#include <linux/msi.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
#include <linux/pci_regs.h>
@@ -19,385 +16,11 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#include "pcie-designware-msi.h"
static struct pci_ops dw_pcie_ops;
static struct pci_ops dw_child_pcie_ops;
-static void dw_msi_ack_irq(struct irq_data *d)
-{
- irq_chip_ack_parent(d);
-}
-
-static void dw_msi_mask_irq(struct irq_data *d)
-{
- pci_msi_mask_irq(d);
- irq_chip_mask_parent(d);
-}
-
-static void dw_msi_unmask_irq(struct irq_data *d)
-{
- pci_msi_unmask_irq(d);
- irq_chip_unmask_parent(d);
-}
-
-static struct irq_chip dw_pcie_msi_irq_chip = {
- .name = "PCI-MSI",
- .irq_ack = dw_msi_ack_irq,
- .irq_mask = dw_msi_mask_irq,
- .irq_unmask = dw_msi_unmask_irq,
-};
-
-static struct msi_domain_info dw_pcie_msi_domain_info = {
- .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
- .chip = &dw_pcie_msi_irq_chip,
-};
-
-/* MSI int handler */
-irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
-{
- int i, pos;
- unsigned long val;
- u32 status, num_ctrls;
- irqreturn_t ret = IRQ_NONE;
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-
- num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-
- for (i = 0; i < num_ctrls; i++) {
- status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
- (i * MSI_REG_CTRL_BLOCK_SIZE));
- if (!status)
- continue;
-
- ret = IRQ_HANDLED;
- val = status;
- pos = 0;
- while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
- pos)) != MAX_MSI_IRQS_PER_CTRL) {
- generic_handle_domain_irq(pp->irq_domain,
- (i * MAX_MSI_IRQS_PER_CTRL) +
- pos);
- pos++;
- }
- }
-
- return ret;
-}
-
-/* Chained MSI interrupt service routine */
-static void dw_chained_msi_isr(struct irq_desc *desc)
-{
- struct irq_chip *chip = irq_desc_get_chip(desc);
- struct dw_pcie_rp *pp;
-
- chained_irq_enter(chip, desc);
-
- pp = irq_desc_get_handler_data(desc);
- dw_handle_msi_irq(pp);
-
- chained_irq_exit(chip, desc);
-}
-
-static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
-{
- struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- u64 msi_target;
-
- msi_target = (u64)pp->msi_data;
-
- msg->address_lo = lower_32_bits(msi_target);
- msg->address_hi = upper_32_bits(msi_target);
-
- msg->data = d->hwirq;
-
- dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
- (int)d->hwirq, msg->address_hi, msg->address_lo);
-}
-
-static int dw_pci_msi_set_affinity(struct irq_data *d,
- const struct cpumask *mask, bool force)
-{
- return -EINVAL;
-}
-
-static void dw_pci_bottom_mask(struct irq_data *d)
-{
- struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- unsigned int res, bit, ctrl;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&pp->lock, flags);
-
- ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
- res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
- bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
-
- pp->irq_mask[ctrl] |= BIT(bit);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, pp->irq_mask[ctrl]);
-
- raw_spin_unlock_irqrestore(&pp->lock, flags);
-}
-
-static void dw_pci_bottom_unmask(struct irq_data *d)
-{
- struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- unsigned int res, bit, ctrl;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&pp->lock, flags);
-
- ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
- res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
- bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
-
- pp->irq_mask[ctrl] &= ~BIT(bit);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, pp->irq_mask[ctrl]);
-
- raw_spin_unlock_irqrestore(&pp->lock, flags);
-}
-
-static void dw_pci_bottom_ack(struct irq_data *d)
-{
- struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- unsigned int res, bit, ctrl;
-
- ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
- res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
- bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
-
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
-}
-
-static struct irq_chip dw_pci_msi_bottom_irq_chip = {
- .name = "DWPCI-MSI",
- .irq_ack = dw_pci_bottom_ack,
- .irq_compose_msi_msg = dw_pci_setup_msi_msg,
- .irq_set_affinity = dw_pci_msi_set_affinity,
- .irq_mask = dw_pci_bottom_mask,
- .irq_unmask = dw_pci_bottom_unmask,
-};
-
-static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
- unsigned int virq, unsigned int nr_irqs,
- void *args)
-{
- struct dw_pcie_rp *pp = domain->host_data;
- unsigned long flags;
- u32 i;
- int bit;
-
- raw_spin_lock_irqsave(&pp->lock, flags);
-
- bit = bitmap_find_free_region(pp->msi_irq_in_use, pp->num_vectors,
- order_base_2(nr_irqs));
-
- raw_spin_unlock_irqrestore(&pp->lock, flags);
-
- if (bit < 0)
- return -ENOSPC;
-
- for (i = 0; i < nr_irqs; i++)
- irq_domain_set_info(domain, virq + i, bit + i,
- pp->msi_irq_chip,
- pp, handle_edge_irq,
- NULL, NULL);
-
- return 0;
-}
-
-static void dw_pcie_irq_domain_free(struct irq_domain *domain,
- unsigned int virq, unsigned int nr_irqs)
-{
- struct irq_data *d = irq_domain_get_irq_data(domain, virq);
- struct dw_pcie_rp *pp = domain->host_data;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&pp->lock, flags);
-
- bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
- order_base_2(nr_irqs));
-
- raw_spin_unlock_irqrestore(&pp->lock, flags);
-}
-
-static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
- .alloc = dw_pcie_irq_domain_alloc,
- .free = dw_pcie_irq_domain_free,
-};
-
-int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
-{
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
-
- pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors,
- &dw_pcie_msi_domain_ops, pp);
- if (!pp->irq_domain) {
- dev_err(pci->dev, "Failed to create IRQ domain\n");
- return -ENOMEM;
- }
-
- irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
-
- pp->msi_domain = pci_msi_create_irq_domain(fwnode,
- &dw_pcie_msi_domain_info,
- pp->irq_domain);
- if (!pp->msi_domain) {
- dev_err(pci->dev, "Failed to create MSI domain\n");
- irq_domain_remove(pp->irq_domain);
- return -ENOMEM;
- }
-
- return 0;
-}
-
-static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
-{
- u32 ctrl;
-
- for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
- if (pp->msi_irq[ctrl] > 0)
- irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
- NULL, NULL);
- }
-
- irq_domain_remove(pp->msi_domain);
- irq_domain_remove(pp->irq_domain);
-}
-
-static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
-{
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- u64 msi_target = (u64)pp->msi_data;
-
- if (!pci_msi_enabled() || !pp->has_msi_ctrl)
- return;
-
- /* Program the msi_data */
- dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
- dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
-}
-
-static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
-{
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct device *dev = pci->dev;
- struct platform_device *pdev = to_platform_device(dev);
- u32 ctrl, max_vectors;
- int irq;
-
- /* Parse any "msiX" IRQs described in the devicetree */
- for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
- char msi_name[] = "msiX";
-
- msi_name[3] = '0' + ctrl;
- irq = platform_get_irq_byname_optional(pdev, msi_name);
- if (irq == -ENXIO)
- break;
- if (irq < 0)
- return dev_err_probe(dev, irq,
- "Failed to parse MSI IRQ '%s'\n",
- msi_name);
-
- pp->msi_irq[ctrl] = irq;
- }
-
- /* If no "msiX" IRQs, caller should fallback to "msi" IRQ */
- if (ctrl == 0)
- return -ENXIO;
-
- max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
- if (pp->num_vectors > max_vectors) {
- dev_warn(dev, "Exceeding number of MSI vectors, limiting to %u\n",
- max_vectors);
- pp->num_vectors = max_vectors;
- }
- if (!pp->num_vectors)
- pp->num_vectors = max_vectors;
-
- return 0;
-}
-
-static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
-{
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct device *dev = pci->dev;
- struct platform_device *pdev = to_platform_device(dev);
- u64 *msi_vaddr = NULL;
- int ret;
- u32 ctrl, num_ctrls;
-
- for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
- pp->irq_mask[ctrl] = ~0;
-
- if (!pp->msi_irq[0]) {
- ret = dw_pcie_parse_split_msi_irq(pp);
- if (ret < 0 && ret != -ENXIO)
- return ret;
- }
-
- if (!pp->num_vectors)
- pp->num_vectors = MSI_DEF_NUM_VECTORS;
- num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-
- if (!pp->msi_irq[0]) {
- pp->msi_irq[0] = platform_get_irq_byname_optional(pdev, "msi");
- if (pp->msi_irq[0] < 0) {
- pp->msi_irq[0] = platform_get_irq(pdev, 0);
- if (pp->msi_irq[0] < 0)
- return pp->msi_irq[0];
- }
- }
-
- dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors);
-
- pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
-
- ret = dw_pcie_allocate_domains(pp);
- if (ret)
- return ret;
-
- for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
- if (pp->msi_irq[ctrl] > 0)
- irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
- dw_chained_msi_isr, pp);
- }
-
- /*
- * Even though the iMSI-RX Module supports 64-bit addresses some
- * peripheral PCIe devices may lack 64-bit message support. In
- * order not to miss MSI TLPs from those devices the MSI target
- * address has to be within the lowest 4GB.
- *
- * Note until there is a better alternative found the reservation is
- * done by allocating from the artificially limited DMA-coherent
- * memory.
- */
- ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
- if (!ret)
- msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
- GFP_KERNEL);
-
- if (!msi_vaddr) {
- dev_warn(dev, "Failed to allocate 32-bit MSI address\n");
- dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
- msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
- GFP_KERNEL);
- if (!msi_vaddr) {
- dev_err(dev, "Failed to allocate MSI address\n");
- dw_pcie_free_msi(pp);
- return -ENOMEM;
- }
- }
-
- return 0;
-}
-
static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -433,6 +56,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
struct resource_entry *win;
struct pci_host_bridge *bridge;
struct resource *res;
+ bool has_msi_ctrl;
int ret;
raw_spin_lock_init(&pp->lock);
@@ -479,15 +103,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
}
if (pci_msi_enabled()) {
- pp->has_msi_ctrl = !(pp->ops->msi_init ||
- of_property_read_bool(np, "msi-parent") ||
- of_property_read_bool(np, "msi-map"));
+ has_msi_ctrl = !(pp->ops->msi_init ||
+ of_property_read_bool(np, "msi-parent") ||
+ of_property_read_bool(np, "msi-map"));
/*
* For the has_msi_ctrl case the default assignment is handled
* in the dw_pcie_msi_host_init().
*/
- if (!pp->has_msi_ctrl && !pp->num_vectors) {
+ if (!has_msi_ctrl && !pp->num_vectors) {
pp->num_vectors = MSI_DEF_NUM_VECTORS;
} else if (pp->num_vectors > MAX_MSI_IRQS) {
dev_err(dev, "Invalid number of vectors\n");
@@ -499,10 +123,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
ret = pp->ops->msi_init(pp);
if (ret < 0)
goto err_deinit_host;
- } else if (pp->has_msi_ctrl) {
- ret = dw_pcie_msi_host_init(pp);
- if (ret < 0)
+ } else if (has_msi_ctrl) {
+ pp->msi = dw_pcie_msi_host_init(pdev, pp, pp->num_vectors);
+ if (IS_ERR(pp->msi)) {
+ ret = PTR_ERR(pp->msi);
goto err_deinit_host;
+ }
}
}
@@ -557,8 +183,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
dw_pcie_edma_remove(pci);
err_free_msi:
- if (pp->has_msi_ctrl)
- dw_pcie_free_msi(pp);
+ dw_pcie_free_msi(pp->msi);
err_deinit_host:
if (pp->ops->deinit)
@@ -579,8 +204,7 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
dw_pcie_edma_remove(pci);
- if (pp->has_msi_ctrl)
- dw_pcie_free_msi(pp);
+ dw_pcie_free_msi(pp->msi);
if (pp->ops->deinit)
pp->ops->deinit(pp);
@@ -808,7 +432,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- u32 val, ctrl, num_ctrls;
+ u32 val;
int ret;
/*
@@ -819,21 +443,7 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
dw_pcie_setup(pci);
- if (pp->has_msi_ctrl) {
- num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-
- /* Initialize IRQ Status array */
- for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- pp->irq_mask[ctrl]);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- ~0);
- }
- }
-
- dw_pcie_msi_init(pp);
+ dw_pcie_msi_init(pp->msi);
/* Setup RC BARs */
dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0x00000004);
diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.c b/drivers/pci/controller/dwc/pcie-designware-msi.c
new file mode 100644
index 0000000..39fe5be
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-designware-msi.c
@@ -0,0 +1,409 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * https://www.samsung.com
+ *
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ */
+
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+#include <linux/platform_device.h>
+
+#include "pcie-designware.h"
+#include "pcie-designware-msi.h"
+
+static void dw_msi_ack_irq(struct irq_data *d)
+{
+ irq_chip_ack_parent(d);
+}
+
+static void dw_msi_mask_irq(struct irq_data *d)
+{
+ pci_msi_mask_irq(d);
+ irq_chip_mask_parent(d);
+}
+
+static void dw_msi_unmask_irq(struct irq_data *d)
+{
+ pci_msi_unmask_irq(d);
+ irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip dw_pcie_msi_irq_chip = {
+ .name = "PCI-MSI",
+ .irq_ack = dw_msi_ack_irq,
+ .irq_mask = dw_msi_mask_irq,
+ .irq_unmask = dw_msi_unmask_irq,
+};
+
+static struct msi_domain_info dw_pcie_msi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+ MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
+ .chip = &dw_pcie_msi_irq_chip,
+};
+
+/* MSI int handler */
+irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
+{
+ int i, pos;
+ unsigned long val;
+ u32 status, num_ctrls;
+ irqreturn_t ret = IRQ_NONE;
+ struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
+
+ num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+ for (i = 0; i < num_ctrls; i++) {
+ status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
+ (i * MSI_REG_CTRL_BLOCK_SIZE));
+ if (!status)
+ continue;
+
+ ret = IRQ_HANDLED;
+ val = status;
+ pos = 0;
+ while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
+ pos)) != MAX_MSI_IRQS_PER_CTRL) {
+ generic_handle_domain_irq(msi->irq_domain,
+ (i * MAX_MSI_IRQS_PER_CTRL) +
+ pos);
+ pos++;
+ }
+ }
+
+ return ret;
+}
+
+/* Chained MSI interrupt service routine */
+static void dw_chained_msi_isr(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct dw_msi *msi;
+
+ chained_irq_enter(chip, desc);
+
+ msi = irq_desc_get_handler_data(desc);
+ dw_handle_msi_irq(msi);
+
+ chained_irq_exit(chip, desc);
+}
+
+static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+ struct dw_msi *msi = irq_data_get_irq_chip_data(d);
+ u64 msi_target;
+
+ msi_target = (u64)msi->msi_data;
+
+ msg->address_lo = lower_32_bits(msi_target);
+ msg->address_hi = upper_32_bits(msi_target);
+
+ msg->data = d->hwirq;
+
+ dev_dbg(msi->dev, "msi#%d address_hi %#x address_lo %#x\n",
+ (int)d->hwirq, msg->address_hi, msg->address_lo);
+}
+
+static int dw_pci_msi_set_affinity(struct irq_data *d,
+ const struct cpumask *mask, bool force)
+{
+ return -EINVAL;
+}
+
+static void dw_pci_bottom_mask(struct irq_data *d)
+{
+ struct dw_msi *msi = irq_data_get_irq_chip_data(d);
+ struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
+ unsigned int res, bit, ctrl;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&msi->lock, flags);
+
+ ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
+
+ msi->irq_mask[ctrl] |= BIT(bit);
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
+
+ raw_spin_unlock_irqrestore(&msi->lock, flags);
+}
+
+static void dw_pci_bottom_unmask(struct irq_data *d)
+{
+ struct dw_msi *msi = irq_data_get_irq_chip_data(d);
+ struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
+ unsigned int res, bit, ctrl;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&msi->lock, flags);
+
+ ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
+
+ msi->irq_mask[ctrl] &= ~BIT(bit);
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
+
+ raw_spin_unlock_irqrestore(&msi->lock, flags);
+}
+
+static void dw_pci_bottom_ack(struct irq_data *d)
+{
+ struct dw_msi *msi = irq_data_get_irq_chip_data(d);
+ struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
+ unsigned int res, bit, ctrl;
+
+ ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
+
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
+}
+
+static struct irq_chip dw_pci_msi_bottom_irq_chip = {
+ .name = "DWPCI-MSI",
+ .irq_ack = dw_pci_bottom_ack,
+ .irq_compose_msi_msg = dw_pci_setup_msi_msg,
+ .irq_set_affinity = dw_pci_msi_set_affinity,
+ .irq_mask = dw_pci_bottom_mask,
+ .irq_unmask = dw_pci_bottom_unmask,
+};
+
+static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs,
+ void *args)
+{
+ struct dw_msi *msi = domain->host_data;
+ unsigned long flags;
+ u32 i;
+ int bit;
+
+ raw_spin_lock_irqsave(&msi->lock, flags);
+
+ bit = bitmap_find_free_region(msi->msi_irq_in_use, msi->num_vectors,
+ order_base_2(nr_irqs));
+
+ raw_spin_unlock_irqrestore(&msi->lock, flags);
+
+ if (bit < 0)
+ return -ENOSPC;
+
+ for (i = 0; i < nr_irqs; i++)
+ irq_domain_set_info(domain, virq + i, bit + i,
+ msi->msi_irq_chip,
+ msi, handle_edge_irq,
+ NULL, NULL);
+
+ return 0;
+}
+
+static void dw_pcie_irq_domain_free(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs)
+{
+ struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+ struct dw_msi *msi = domain->host_data;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&msi->lock, flags);
+
+ bitmap_release_region(msi->msi_irq_in_use, d->hwirq,
+ order_base_2(nr_irqs));
+
+ raw_spin_unlock_irqrestore(&msi->lock, flags);
+}
+
+static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
+ .alloc = dw_pcie_irq_domain_alloc,
+ .free = dw_pcie_irq_domain_free,
+};
+
+int dw_pcie_allocate_domains(struct dw_msi *msi)
+{
+ struct fwnode_handle *fwnode = of_node_to_fwnode(msi->dev->of_node);
+
+ msi->irq_domain = irq_domain_create_linear(fwnode, msi->num_vectors,
+ &dw_pcie_msi_domain_ops,
+ msi->private_data ? msi->private_data : msi);
+ if (!msi->irq_domain) {
+ dev_err(msi->dev, "Failed to create IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ irq_domain_update_bus_token(msi->irq_domain, DOMAIN_BUS_NEXUS);
+
+ msi->msi_domain = pci_msi_create_irq_domain(fwnode,
+ &dw_pcie_msi_domain_info,
+ msi->irq_domain);
+ if (!msi->msi_domain) {
+ dev_err(msi->dev, "Failed to create MSI domain\n");
+ irq_domain_remove(msi->irq_domain);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+void dw_pcie_free_msi(struct dw_msi *msi)
+{
+ u32 ctrl;
+
+ for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
+ if (msi->msi_irq[ctrl] > 0)
+ irq_set_chained_handler_and_data(msi->msi_irq[ctrl],
+ NULL, NULL);
+ }
+
+ irq_domain_remove(msi->msi_domain);
+ irq_domain_remove(msi->irq_domain);
+}
+
+void dw_pcie_msi_init(struct dw_msi *msi)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
+ u32 ctrl, num_ctrls;
+ u64 msi_target;
+
+ if (!pci_msi_enabled() || !msi->has_msi_ctrl)
+ return;
+
+ msi_target = (u64)msi->msi_data;
+ num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+ /* Initialize IRQ Status array */
+ for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
+ msi->irq_mask[ctrl]);
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE), ~0);
+ }
+
+ /* Program the msi_data */
+ dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
+ dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
+}
+
+static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ u32 ctrl, max_vectors;
+ int irq;
+
+ /* Parse any "msiX" IRQs described in the devicetree */
+ for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
+ char msi_name[] = "msiX";
+
+ msi_name[3] = '0' + ctrl;
+ irq = platform_get_irq_byname_optional(pdev, msi_name);
+ if (irq == -ENXIO)
+ break;
+ if (irq < 0)
+ return dev_err_probe(dev, irq,
+ "Failed to parse MSI IRQ '%s'\n",
+ msi_name);
+
+ msi->msi_irq[ctrl] = irq;
+ }
+
+ /* If no "msiX" IRQs, caller should fallback to "msi" IRQ */
+ if (ctrl == 0)
+ return -ENXIO;
+
+ max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
+ if (msi->num_vectors > max_vectors) {
+ dev_warn(dev, "Exceeding number of MSI vectors, limiting to %u\n",
+ max_vectors);
+ msi->num_vectors = max_vectors;
+ }
+ if (!msi->num_vectors)
+ msi->num_vectors = max_vectors;
+
+ return 0;
+}
+
+struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
+ void *pp, u32 num_vectors)
+{
+ struct device *dev = &pdev->dev;
+ u64 *msi_vaddr = NULL;
+ u32 ctrl, num_ctrls;
+ struct dw_msi *msi;
+ int ret;
+
+ msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
+ if (msi == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
+ msi->irq_mask[ctrl] = ~0;
+
+ raw_spin_lock_init(&msi->lock);
+ msi->dev = dev;
+ msi->pp = pp;
+ msi->has_msi_ctrl = true;
+ msi->num_vectors = num_vectors;
+
+ if (!msi->msi_irq[0]) {
+ ret = dw_pcie_parse_split_msi_irq(msi, pdev);
+ if (ret < 0 && ret != -ENXIO)
+ return ERR_PTR(ret);
+ }
+
+ if (!msi->num_vectors)
+ msi->num_vectors = MSI_DEF_NUM_VECTORS;
+ num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+ if (!msi->msi_irq[0]) {
+ msi->msi_irq[0] = platform_get_irq_byname_optional(pdev, "msi");
+ if (msi->msi_irq[0] < 0) {
+ msi->msi_irq[0] = platform_get_irq(pdev, 0);
+ if (msi->msi_irq[0] < 0)
+ return ERR_PTR(msi->msi_irq[0]);
+ }
+ }
+
+ dev_dbg(dev, "Using %d MSI vectors\n", msi->num_vectors);
+
+ msi->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
+
+ ret = dw_pcie_allocate_domains(msi);
+ if (ret)
+ return ERR_PTR(ret);
+
+ for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
+ if (msi->msi_irq[ctrl] > 0)
+ irq_set_chained_handler_and_data(msi->msi_irq[ctrl],
+ dw_chained_msi_isr, msi);
+ }
+
+ /*
+ * Even though the iMSI-RX Module supports 64-bit addresses some
+ * peripheral PCIe devices may lack 64-bit message support. In
+ * order not to miss MSI TLPs from those devices the MSI target
+ * address has to be within the lowest 4GB.
+ *
+ * Note until there is a better alternative found the reservation is
+ * done by allocating from the artificially limited DMA-coherent
+ * memory.
+ */
+ ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+ if (!ret)
+ msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &msi->msi_data,
+ GFP_KERNEL);
+
+ if (!msi_vaddr) {
+ dev_warn(dev, "Failed to allocate 32-bit MSI address\n");
+ dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
+ msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &msi->msi_data,
+ GFP_KERNEL);
+ if (!msi_vaddr) {
+ dev_err(dev, "Failed to allocate MSI address\n");
+ dw_pcie_free_msi(msi);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+
+ return msi;
+}
diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
new file mode 100644
index 0000000..633156e
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synopsys DesignWare PCIe host controller driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * https://www.samsung.com
+ *
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ */
+#ifndef _PCIE_DESIGNWARE_MSI_H
+#define _PCIE_DESIGNWARE_MSI_H
+
+#include "../../pci.h"
+
+#define MAX_MSI_IRQS 256
+#define MAX_MSI_IRQS_PER_CTRL 32
+#define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
+#define MSI_REG_CTRL_BLOCK_SIZE 12
+#define MSI_DEF_NUM_VECTORS 32
+
+struct dw_msi {
+ struct device *dev;
+ struct irq_domain *irq_domain;
+ struct irq_domain *msi_domain;
+ struct irq_chip *msi_irq_chip;
+ int msi_irq[MAX_MSI_CTRLS];
+ dma_addr_t msi_data;
+ u32 num_vectors;
+ u32 irq_mask[MAX_MSI_CTRLS];
+ raw_spinlock_t lock;
+ DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
+ bool has_msi_ctrl;
+ void *private_data;
+ void *pp;
+};
+
+struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
+ void *pp, u32 num_vectors);
+int dw_pcie_allocate_domains(struct dw_msi *msi);
+void dw_pcie_msi_init(struct dw_msi *msi);
+void dw_pcie_free_msi(struct dw_msi *msi);
+irqreturn_t dw_handle_msi_irq(struct dw_msi *msi);
+#endif /* _PCIE_DESIGNWARE_MSI_H */
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 1b5aba1..e84298e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -22,6 +22,7 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#include "pcie-designware-msi.h"
static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
[DW_PCIE_DBI_CLK] = "dbi",
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ef84931..8b7fddf 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -232,12 +232,6 @@
#define DEFAULT_DBI_ATU_OFFSET (0x3 << 20)
#define DEFAULT_DBI_DMA_OFFSET PCIE_DMA_UNROLL_BASE
-#define MAX_MSI_IRQS 256
-#define MAX_MSI_IRQS_PER_CTRL 32
-#define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
-#define MSI_REG_CTRL_BLOCK_SIZE 12
-#define MSI_DEF_NUM_VECTORS 32
-
/* Maximum number of inbound/outbound iATUs */
#define MAX_IATU_IN 256
#define MAX_IATU_OUT 256
@@ -319,7 +313,6 @@ struct dw_pcie_host_ops {
};
struct dw_pcie_rp {
- bool has_msi_ctrl:1;
bool cfg0_io_shared:1;
u64 cfg0_base;
void __iomem *va_cfg0_base;
@@ -329,16 +322,10 @@ struct dw_pcie_rp {
u32 io_size;
int irq;
const struct dw_pcie_host_ops *ops;
- int msi_irq[MAX_MSI_CTRLS];
- struct irq_domain *irq_domain;
- struct irq_domain *msi_domain;
- dma_addr_t msi_data;
- struct irq_chip *msi_irq_chip;
u32 num_vectors;
- u32 irq_mask[MAX_MSI_CTRLS];
+ struct dw_msi *msi;
struct pci_host_bridge *bridge;
raw_spinlock_t lock;
- DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
bool use_atu_msg;
int msg_atu_index;
struct resource *msg_res;
@@ -639,19 +626,12 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
}
#ifdef CONFIG_PCIE_DW_HOST
-irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
int dw_pcie_host_init(struct dw_pcie_rp *pp);
void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
-int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
int where);
#else
-static inline irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
-{
- return IRQ_NONE;
-}
-
static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
{
return 0;
@@ -666,10 +646,6 @@ static inline void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
{
}
-static inline int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
-{
- return 0;
-}
static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
unsigned int devfn,
int where)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index d99e12f..6139330 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -16,6 +16,7 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#include "pcie-designware-msi.h"
/* Renesas-specific */
/* PCIe Mode Setting Register 0 */
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 804341b..f415fa1 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -34,6 +34,7 @@
#include <soc/tegra/bpmp.h>
#include <soc/tegra/bpmp-abi.h>
#include "../../pci.h"
+#include "pcie-designware-msi.h"
#define TEGRA194_DWC_IP_VER 0x490A
#define TEGRA234_DWC_IP_VER 0x562A
@@ -2407,6 +2408,7 @@ static int tegra_pcie_dw_resume_early(struct device *dev)
static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
{
struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
+ struct dw_msi *msi;
if (pcie->of_data->mode == DW_PCIE_RC_TYPE) {
if (!pcie->link_state)
@@ -2415,9 +2417,10 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
debugfs_remove_recursive(pcie->debugfs);
tegra_pcie_downstream_dev_to_D0(pcie);
+ msi = pcie->pci.pp.msi;
disable_irq(pcie->pci.pp.irq);
if (IS_ENABLED(CONFIG_PCI_MSI))
- disable_irq(pcie->pci.pp.msi_irq[0]);
+ disable_irq(msi->msi_irq[0]);
tegra_pcie_dw_pme_turnoff(pcie);
tegra_pcie_unconfig_controller(pcie);
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V2 1/7] PCI: dwc: Move MSI functionality related code to separate file
2024-07-15 18:13 ` [PATCH V2 1/7] PCI: dwc: Move MSI functionality related code to separate file Mayank Rana
@ 2024-07-31 16:40 ` Manivannan Sadhasivam
0 siblings, 0 replies; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-31 16:40 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:29AM -0700, Mayank Rana wrote:
> This change moves dwc PCIe controller specific MSI functionality
> into separate file in preparation to allow MSI functionality with
> PCIe ECAM driver. Update existing drivers to accommodate this change.
>
Could you please add more info on how the move is happening? For sure you are
not just copy pasting stuff...
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/dwc/Makefile | 2 +-
> drivers/pci/controller/dwc/pci-keystone.c | 12 +-
> drivers/pci/controller/dwc/pcie-designware-host.c | 420 +---------------------
> drivers/pci/controller/dwc/pcie-designware-msi.c | 409 +++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-designware-msi.h | 43 +++
> drivers/pci/controller/dwc/pcie-designware.c | 1 +
> drivers/pci/controller/dwc/pcie-designware.h | 26 +-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
> 9 files changed, 484 insertions(+), 435 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.c
> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.h
>
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bac103f..2ecc603 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> -obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> +obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o pcie-designware-msi.o
> obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index cd0e002..b95d319 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -307,8 +307,14 @@ static int ks_pcie_msi_host_init(struct dw_pcie_rp *pp)
> */
> dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, ks_pcie->app.start);
>
> - pp->msi_irq_chip = &ks_pcie_msi_irq_chip;
> - return dw_pcie_allocate_domains(pp);
> + pp->msi = devm_kzalloc(pci->dev, sizeof(struct dw_msi *), GFP_KERNEL);
> + if (pp->msi == NULL)
> + return -ENOMEM;
> +
> + pp->msi->msi_irq_chip = &ks_pcie_msi_irq_chip;
> + pp->msi->dev = pci->dev;
> + pp->msi->private_data = pp;
It'd be better to have an API to allocate 'struct dw_msi' and populate these
parameters, like:
pp->msi = dw_pcie_alloc_msi(dev, &ks_pcie_msi_irq_chip, pp);
This API could then be used in dw_pcie_msi_host_init() also.
Rest LGTM!
- Mani
> + return dw_pcie_allocate_domains(pp->msi);
> }
>
> static void ks_pcie_handle_intx_irq(struct keystone_pcie *ks_pcie,
> @@ -322,7 +328,7 @@ static void ks_pcie_handle_intx_irq(struct keystone_pcie *ks_pcie,
>
> if (BIT(0) & pending) {
> dev_dbg(dev, ": irq: irq_offset %d", offset);
> - generic_handle_domain_irq(ks_pcie->intx_irq_domain, offset);
> + generic_handle_domain_irq(ks_pcie->msi->intx_irq_domain, offset);
> }
>
> /* EOI the INTx interrupt */
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a0822d5..3dcf88a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -9,9 +9,6 @@
> */
>
> #include <linux/iopoll.h>
> -#include <linux/irqchip/chained_irq.h>
> -#include <linux/irqdomain.h>
> -#include <linux/msi.h>
> #include <linux/of_address.h>
> #include <linux/of_pci.h>
> #include <linux/pci_regs.h>
> @@ -19,385 +16,11 @@
>
> #include "../../pci.h"
> #include "pcie-designware.h"
> +#include "pcie-designware-msi.h"
>
> static struct pci_ops dw_pcie_ops;
> static struct pci_ops dw_child_pcie_ops;
>
> -static void dw_msi_ack_irq(struct irq_data *d)
> -{
> - irq_chip_ack_parent(d);
> -}
> -
> -static void dw_msi_mask_irq(struct irq_data *d)
> -{
> - pci_msi_mask_irq(d);
> - irq_chip_mask_parent(d);
> -}
> -
> -static void dw_msi_unmask_irq(struct irq_data *d)
> -{
> - pci_msi_unmask_irq(d);
> - irq_chip_unmask_parent(d);
> -}
> -
> -static struct irq_chip dw_pcie_msi_irq_chip = {
> - .name = "PCI-MSI",
> - .irq_ack = dw_msi_ack_irq,
> - .irq_mask = dw_msi_mask_irq,
> - .irq_unmask = dw_msi_unmask_irq,
> -};
> -
> -static struct msi_domain_info dw_pcie_msi_domain_info = {
> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> - MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
> - .chip = &dw_pcie_msi_irq_chip,
> -};
> -
> -/* MSI int handler */
> -irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
> -{
> - int i, pos;
> - unsigned long val;
> - u32 status, num_ctrls;
> - irqreturn_t ret = IRQ_NONE;
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -
> - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> -
> - for (i = 0; i < num_ctrls; i++) {
> - status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> - (i * MSI_REG_CTRL_BLOCK_SIZE));
> - if (!status)
> - continue;
> -
> - ret = IRQ_HANDLED;
> - val = status;
> - pos = 0;
> - while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> - pos)) != MAX_MSI_IRQS_PER_CTRL) {
> - generic_handle_domain_irq(pp->irq_domain,
> - (i * MAX_MSI_IRQS_PER_CTRL) +
> - pos);
> - pos++;
> - }
> - }
> -
> - return ret;
> -}
> -
> -/* Chained MSI interrupt service routine */
> -static void dw_chained_msi_isr(struct irq_desc *desc)
> -{
> - struct irq_chip *chip = irq_desc_get_chip(desc);
> - struct dw_pcie_rp *pp;
> -
> - chained_irq_enter(chip, desc);
> -
> - pp = irq_desc_get_handler_data(desc);
> - dw_handle_msi_irq(pp);
> -
> - chained_irq_exit(chip, desc);
> -}
> -
> -static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
> -{
> - struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - u64 msi_target;
> -
> - msi_target = (u64)pp->msi_data;
> -
> - msg->address_lo = lower_32_bits(msi_target);
> - msg->address_hi = upper_32_bits(msi_target);
> -
> - msg->data = d->hwirq;
> -
> - dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
> - (int)d->hwirq, msg->address_hi, msg->address_lo);
> -}
> -
> -static int dw_pci_msi_set_affinity(struct irq_data *d,
> - const struct cpumask *mask, bool force)
> -{
> - return -EINVAL;
> -}
> -
> -static void dw_pci_bottom_mask(struct irq_data *d)
> -{
> - struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - unsigned int res, bit, ctrl;
> - unsigned long flags;
> -
> - raw_spin_lock_irqsave(&pp->lock, flags);
> -
> - ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> - res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> - bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> -
> - pp->irq_mask[ctrl] |= BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, pp->irq_mask[ctrl]);
> -
> - raw_spin_unlock_irqrestore(&pp->lock, flags);
> -}
> -
> -static void dw_pci_bottom_unmask(struct irq_data *d)
> -{
> - struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - unsigned int res, bit, ctrl;
> - unsigned long flags;
> -
> - raw_spin_lock_irqsave(&pp->lock, flags);
> -
> - ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> - res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> - bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> -
> - pp->irq_mask[ctrl] &= ~BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, pp->irq_mask[ctrl]);
> -
> - raw_spin_unlock_irqrestore(&pp->lock, flags);
> -}
> -
> -static void dw_pci_bottom_ack(struct irq_data *d)
> -{
> - struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - unsigned int res, bit, ctrl;
> -
> - ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> - res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> - bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> -
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> -}
> -
> -static struct irq_chip dw_pci_msi_bottom_irq_chip = {
> - .name = "DWPCI-MSI",
> - .irq_ack = dw_pci_bottom_ack,
> - .irq_compose_msi_msg = dw_pci_setup_msi_msg,
> - .irq_set_affinity = dw_pci_msi_set_affinity,
> - .irq_mask = dw_pci_bottom_mask,
> - .irq_unmask = dw_pci_bottom_unmask,
> -};
> -
> -static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
> - unsigned int virq, unsigned int nr_irqs,
> - void *args)
> -{
> - struct dw_pcie_rp *pp = domain->host_data;
> - unsigned long flags;
> - u32 i;
> - int bit;
> -
> - raw_spin_lock_irqsave(&pp->lock, flags);
> -
> - bit = bitmap_find_free_region(pp->msi_irq_in_use, pp->num_vectors,
> - order_base_2(nr_irqs));
> -
> - raw_spin_unlock_irqrestore(&pp->lock, flags);
> -
> - if (bit < 0)
> - return -ENOSPC;
> -
> - for (i = 0; i < nr_irqs; i++)
> - irq_domain_set_info(domain, virq + i, bit + i,
> - pp->msi_irq_chip,
> - pp, handle_edge_irq,
> - NULL, NULL);
> -
> - return 0;
> -}
> -
> -static void dw_pcie_irq_domain_free(struct irq_domain *domain,
> - unsigned int virq, unsigned int nr_irqs)
> -{
> - struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> - struct dw_pcie_rp *pp = domain->host_data;
> - unsigned long flags;
> -
> - raw_spin_lock_irqsave(&pp->lock, flags);
> -
> - bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
> - order_base_2(nr_irqs));
> -
> - raw_spin_unlock_irqrestore(&pp->lock, flags);
> -}
> -
> -static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
> - .alloc = dw_pcie_irq_domain_alloc,
> - .free = dw_pcie_irq_domain_free,
> -};
> -
> -int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> -{
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
> -
> - pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors,
> - &dw_pcie_msi_domain_ops, pp);
> - if (!pp->irq_domain) {
> - dev_err(pci->dev, "Failed to create IRQ domain\n");
> - return -ENOMEM;
> - }
> -
> - irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> -
> - pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> - &dw_pcie_msi_domain_info,
> - pp->irq_domain);
> - if (!pp->msi_domain) {
> - dev_err(pci->dev, "Failed to create MSI domain\n");
> - irq_domain_remove(pp->irq_domain);
> - return -ENOMEM;
> - }
> -
> - return 0;
> -}
> -
> -static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
> -{
> - u32 ctrl;
> -
> - for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> - if (pp->msi_irq[ctrl] > 0)
> - irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> - NULL, NULL);
> - }
> -
> - irq_domain_remove(pp->msi_domain);
> - irq_domain_remove(pp->irq_domain);
> -}
> -
> -static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
> -{
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - u64 msi_target = (u64)pp->msi_data;
> -
> - if (!pci_msi_enabled() || !pp->has_msi_ctrl)
> - return;
> -
> - /* Program the msi_data */
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> -}
> -
> -static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
> -{
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - struct device *dev = pci->dev;
> - struct platform_device *pdev = to_platform_device(dev);
> - u32 ctrl, max_vectors;
> - int irq;
> -
> - /* Parse any "msiX" IRQs described in the devicetree */
> - for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> - char msi_name[] = "msiX";
> -
> - msi_name[3] = '0' + ctrl;
> - irq = platform_get_irq_byname_optional(pdev, msi_name);
> - if (irq == -ENXIO)
> - break;
> - if (irq < 0)
> - return dev_err_probe(dev, irq,
> - "Failed to parse MSI IRQ '%s'\n",
> - msi_name);
> -
> - pp->msi_irq[ctrl] = irq;
> - }
> -
> - /* If no "msiX" IRQs, caller should fallback to "msi" IRQ */
> - if (ctrl == 0)
> - return -ENXIO;
> -
> - max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
> - if (pp->num_vectors > max_vectors) {
> - dev_warn(dev, "Exceeding number of MSI vectors, limiting to %u\n",
> - max_vectors);
> - pp->num_vectors = max_vectors;
> - }
> - if (!pp->num_vectors)
> - pp->num_vectors = max_vectors;
> -
> - return 0;
> -}
> -
> -static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> -{
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - struct device *dev = pci->dev;
> - struct platform_device *pdev = to_platform_device(dev);
> - u64 *msi_vaddr = NULL;
> - int ret;
> - u32 ctrl, num_ctrls;
> -
> - for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
> - pp->irq_mask[ctrl] = ~0;
> -
> - if (!pp->msi_irq[0]) {
> - ret = dw_pcie_parse_split_msi_irq(pp);
> - if (ret < 0 && ret != -ENXIO)
> - return ret;
> - }
> -
> - if (!pp->num_vectors)
> - pp->num_vectors = MSI_DEF_NUM_VECTORS;
> - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> -
> - if (!pp->msi_irq[0]) {
> - pp->msi_irq[0] = platform_get_irq_byname_optional(pdev, "msi");
> - if (pp->msi_irq[0] < 0) {
> - pp->msi_irq[0] = platform_get_irq(pdev, 0);
> - if (pp->msi_irq[0] < 0)
> - return pp->msi_irq[0];
> - }
> - }
> -
> - dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors);
> -
> - pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
> -
> - ret = dw_pcie_allocate_domains(pp);
> - if (ret)
> - return ret;
> -
> - for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> - if (pp->msi_irq[ctrl] > 0)
> - irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> - dw_chained_msi_isr, pp);
> - }
> -
> - /*
> - * Even though the iMSI-RX Module supports 64-bit addresses some
> - * peripheral PCIe devices may lack 64-bit message support. In
> - * order not to miss MSI TLPs from those devices the MSI target
> - * address has to be within the lowest 4GB.
> - *
> - * Note until there is a better alternative found the reservation is
> - * done by allocating from the artificially limited DMA-coherent
> - * memory.
> - */
> - ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> - if (!ret)
> - msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
> - GFP_KERNEL);
> -
> - if (!msi_vaddr) {
> - dev_warn(dev, "Failed to allocate 32-bit MSI address\n");
> - dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
> - msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &pp->msi_data,
> - GFP_KERNEL);
> - if (!msi_vaddr) {
> - dev_err(dev, "Failed to allocate MSI address\n");
> - dw_pcie_free_msi(pp);
> - return -ENOMEM;
> - }
> - }
> -
> - return 0;
> -}
> -
> static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -433,6 +56,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> struct resource_entry *win;
> struct pci_host_bridge *bridge;
> struct resource *res;
> + bool has_msi_ctrl;
> int ret;
>
> raw_spin_lock_init(&pp->lock);
> @@ -479,15 +103,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> }
>
> if (pci_msi_enabled()) {
> - pp->has_msi_ctrl = !(pp->ops->msi_init ||
> - of_property_read_bool(np, "msi-parent") ||
> - of_property_read_bool(np, "msi-map"));
> + has_msi_ctrl = !(pp->ops->msi_init ||
> + of_property_read_bool(np, "msi-parent") ||
> + of_property_read_bool(np, "msi-map"));
>
> /*
> * For the has_msi_ctrl case the default assignment is handled
> * in the dw_pcie_msi_host_init().
> */
> - if (!pp->has_msi_ctrl && !pp->num_vectors) {
> + if (!has_msi_ctrl && !pp->num_vectors) {
> pp->num_vectors = MSI_DEF_NUM_VECTORS;
> } else if (pp->num_vectors > MAX_MSI_IRQS) {
> dev_err(dev, "Invalid number of vectors\n");
> @@ -499,10 +123,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> ret = pp->ops->msi_init(pp);
> if (ret < 0)
> goto err_deinit_host;
> - } else if (pp->has_msi_ctrl) {
> - ret = dw_pcie_msi_host_init(pp);
> - if (ret < 0)
> + } else if (has_msi_ctrl) {
> + pp->msi = dw_pcie_msi_host_init(pdev, pp, pp->num_vectors);
> + if (IS_ERR(pp->msi)) {
> + ret = PTR_ERR(pp->msi);
> goto err_deinit_host;
> + }
> }
> }
>
> @@ -557,8 +183,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> dw_pcie_edma_remove(pci);
>
> err_free_msi:
> - if (pp->has_msi_ctrl)
> - dw_pcie_free_msi(pp);
> + dw_pcie_free_msi(pp->msi);
>
> err_deinit_host:
> if (pp->ops->deinit)
> @@ -579,8 +204,7 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>
> dw_pcie_edma_remove(pci);
>
> - if (pp->has_msi_ctrl)
> - dw_pcie_free_msi(pp);
> + dw_pcie_free_msi(pp->msi);
>
> if (pp->ops->deinit)
> pp->ops->deinit(pp);
> @@ -808,7 +432,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - u32 val, ctrl, num_ctrls;
> + u32 val;
> int ret;
>
> /*
> @@ -819,21 +443,7 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>
> dw_pcie_setup(pci);
>
> - if (pp->has_msi_ctrl) {
> - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> -
> - /* Initialize IRQ Status array */
> - for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> - pp->irq_mask[ctrl]);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> - ~0);
> - }
> - }
> -
> - dw_pcie_msi_init(pp);
> + dw_pcie_msi_init(pp->msi);
>
> /* Setup RC BARs */
> dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0x00000004);
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.c b/drivers/pci/controller/dwc/pcie-designware-msi.c
> new file mode 100644
> index 0000000..39fe5be
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * https://www.samsung.com
> + *
> + * Author: Jingoo Han <jg1.han@samsung.com>
> + */
> +
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/msi.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-designware.h"
> +#include "pcie-designware-msi.h"
> +
> +static void dw_msi_ack_irq(struct irq_data *d)
> +{
> + irq_chip_ack_parent(d);
> +}
> +
> +static void dw_msi_mask_irq(struct irq_data *d)
> +{
> + pci_msi_mask_irq(d);
> + irq_chip_mask_parent(d);
> +}
> +
> +static void dw_msi_unmask_irq(struct irq_data *d)
> +{
> + pci_msi_unmask_irq(d);
> + irq_chip_unmask_parent(d);
> +}
> +
> +static struct irq_chip dw_pcie_msi_irq_chip = {
> + .name = "PCI-MSI",
> + .irq_ack = dw_msi_ack_irq,
> + .irq_mask = dw_msi_mask_irq,
> + .irq_unmask = dw_msi_unmask_irq,
> +};
> +
> +static struct msi_domain_info dw_pcie_msi_domain_info = {
> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> + MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
> + .chip = &dw_pcie_msi_irq_chip,
> +};
> +
> +/* MSI int handler */
> +irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
> +{
> + int i, pos;
> + unsigned long val;
> + u32 status, num_ctrls;
> + irqreturn_t ret = IRQ_NONE;
> + struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> +
> + num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> + for (i = 0; i < num_ctrls; i++) {
> + status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> + (i * MSI_REG_CTRL_BLOCK_SIZE));
> + if (!status)
> + continue;
> +
> + ret = IRQ_HANDLED;
> + val = status;
> + pos = 0;
> + while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> + pos)) != MAX_MSI_IRQS_PER_CTRL) {
> + generic_handle_domain_irq(msi->irq_domain,
> + (i * MAX_MSI_IRQS_PER_CTRL) +
> + pos);
> + pos++;
> + }
> + }
> +
> + return ret;
> +}
> +
> +/* Chained MSI interrupt service routine */
> +static void dw_chained_msi_isr(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct dw_msi *msi;
> +
> + chained_irq_enter(chip, desc);
> +
> + msi = irq_desc_get_handler_data(desc);
> + dw_handle_msi_irq(msi);
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
> +{
> + struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> + u64 msi_target;
> +
> + msi_target = (u64)msi->msi_data;
> +
> + msg->address_lo = lower_32_bits(msi_target);
> + msg->address_hi = upper_32_bits(msi_target);
> +
> + msg->data = d->hwirq;
> +
> + dev_dbg(msi->dev, "msi#%d address_hi %#x address_lo %#x\n",
> + (int)d->hwirq, msg->address_hi, msg->address_lo);
> +}
> +
> +static int dw_pci_msi_set_affinity(struct irq_data *d,
> + const struct cpumask *mask, bool force)
> +{
> + return -EINVAL;
> +}
> +
> +static void dw_pci_bottom_mask(struct irq_data *d)
> +{
> + struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> + struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> + unsigned int res, bit, ctrl;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&msi->lock, flags);
> +
> + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> +
> + msi->irq_mask[ctrl] |= BIT(bit);
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> +
> + raw_spin_unlock_irqrestore(&msi->lock, flags);
> +}
> +
> +static void dw_pci_bottom_unmask(struct irq_data *d)
> +{
> + struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> + struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> + unsigned int res, bit, ctrl;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&msi->lock, flags);
> +
> + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> +
> + msi->irq_mask[ctrl] &= ~BIT(bit);
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> +
> + raw_spin_unlock_irqrestore(&msi->lock, flags);
> +}
> +
> +static void dw_pci_bottom_ack(struct irq_data *d)
> +{
> + struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> + struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> + unsigned int res, bit, ctrl;
> +
> + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
> +
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> +}
> +
> +static struct irq_chip dw_pci_msi_bottom_irq_chip = {
> + .name = "DWPCI-MSI",
> + .irq_ack = dw_pci_bottom_ack,
> + .irq_compose_msi_msg = dw_pci_setup_msi_msg,
> + .irq_set_affinity = dw_pci_msi_set_affinity,
> + .irq_mask = dw_pci_bottom_mask,
> + .irq_unmask = dw_pci_bottom_unmask,
> +};
> +
> +static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs,
> + void *args)
> +{
> + struct dw_msi *msi = domain->host_data;
> + unsigned long flags;
> + u32 i;
> + int bit;
> +
> + raw_spin_lock_irqsave(&msi->lock, flags);
> +
> + bit = bitmap_find_free_region(msi->msi_irq_in_use, msi->num_vectors,
> + order_base_2(nr_irqs));
> +
> + raw_spin_unlock_irqrestore(&msi->lock, flags);
> +
> + if (bit < 0)
> + return -ENOSPC;
> +
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_info(domain, virq + i, bit + i,
> + msi->msi_irq_chip,
> + msi, handle_edge_irq,
> + NULL, NULL);
> +
> + return 0;
> +}
> +
> +static void dw_pcie_irq_domain_free(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs)
> +{
> + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> + struct dw_msi *msi = domain->host_data;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&msi->lock, flags);
> +
> + bitmap_release_region(msi->msi_irq_in_use, d->hwirq,
> + order_base_2(nr_irqs));
> +
> + raw_spin_unlock_irqrestore(&msi->lock, flags);
> +}
> +
> +static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
> + .alloc = dw_pcie_irq_domain_alloc,
> + .free = dw_pcie_irq_domain_free,
> +};
> +
> +int dw_pcie_allocate_domains(struct dw_msi *msi)
> +{
> + struct fwnode_handle *fwnode = of_node_to_fwnode(msi->dev->of_node);
> +
> + msi->irq_domain = irq_domain_create_linear(fwnode, msi->num_vectors,
> + &dw_pcie_msi_domain_ops,
> + msi->private_data ? msi->private_data : msi);
> + if (!msi->irq_domain) {
> + dev_err(msi->dev, "Failed to create IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + irq_domain_update_bus_token(msi->irq_domain, DOMAIN_BUS_NEXUS);
> +
> + msi->msi_domain = pci_msi_create_irq_domain(fwnode,
> + &dw_pcie_msi_domain_info,
> + msi->irq_domain);
> + if (!msi->msi_domain) {
> + dev_err(msi->dev, "Failed to create MSI domain\n");
> + irq_domain_remove(msi->irq_domain);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +void dw_pcie_free_msi(struct dw_msi *msi)
> +{
> + u32 ctrl;
> +
> + for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> + if (msi->msi_irq[ctrl] > 0)
> + irq_set_chained_handler_and_data(msi->msi_irq[ctrl],
> + NULL, NULL);
> + }
> +
> + irq_domain_remove(msi->msi_domain);
> + irq_domain_remove(msi->irq_domain);
> +}
> +
> +void dw_pcie_msi_init(struct dw_msi *msi)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> + u32 ctrl, num_ctrls;
> + u64 msi_target;
> +
> + if (!pci_msi_enabled() || !msi->has_msi_ctrl)
> + return;
> +
> + msi_target = (u64)msi->msi_data;
> + num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> + /* Initialize IRQ Status array */
> + for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> + msi->irq_mask[ctrl]);
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE), ~0);
> + }
> +
> + /* Program the msi_data */
> + dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> + dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> +}
> +
> +static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + u32 ctrl, max_vectors;
> + int irq;
> +
> + /* Parse any "msiX" IRQs described in the devicetree */
> + for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> + char msi_name[] = "msiX";
> +
> + msi_name[3] = '0' + ctrl;
> + irq = platform_get_irq_byname_optional(pdev, msi_name);
> + if (irq == -ENXIO)
> + break;
> + if (irq < 0)
> + return dev_err_probe(dev, irq,
> + "Failed to parse MSI IRQ '%s'\n",
> + msi_name);
> +
> + msi->msi_irq[ctrl] = irq;
> + }
> +
> + /* If no "msiX" IRQs, caller should fallback to "msi" IRQ */
> + if (ctrl == 0)
> + return -ENXIO;
> +
> + max_vectors = ctrl * MAX_MSI_IRQS_PER_CTRL;
> + if (msi->num_vectors > max_vectors) {
> + dev_warn(dev, "Exceeding number of MSI vectors, limiting to %u\n",
> + max_vectors);
> + msi->num_vectors = max_vectors;
> + }
> + if (!msi->num_vectors)
> + msi->num_vectors = max_vectors;
> +
> + return 0;
> +}
> +
> +struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> + void *pp, u32 num_vectors)
> +{
> + struct device *dev = &pdev->dev;
> + u64 *msi_vaddr = NULL;
> + u32 ctrl, num_ctrls;
> + struct dw_msi *msi;
> + int ret;
> +
> + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
> + if (msi == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
> + msi->irq_mask[ctrl] = ~0;
> +
> + raw_spin_lock_init(&msi->lock);
> + msi->dev = dev;
> + msi->pp = pp;
> + msi->has_msi_ctrl = true;
> + msi->num_vectors = num_vectors;
> +
> + if (!msi->msi_irq[0]) {
> + ret = dw_pcie_parse_split_msi_irq(msi, pdev);
> + if (ret < 0 && ret != -ENXIO)
> + return ERR_PTR(ret);
> + }
> +
> + if (!msi->num_vectors)
> + msi->num_vectors = MSI_DEF_NUM_VECTORS;
> + num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> + if (!msi->msi_irq[0]) {
> + msi->msi_irq[0] = platform_get_irq_byname_optional(pdev, "msi");
> + if (msi->msi_irq[0] < 0) {
> + msi->msi_irq[0] = platform_get_irq(pdev, 0);
> + if (msi->msi_irq[0] < 0)
> + return ERR_PTR(msi->msi_irq[0]);
> + }
> + }
> +
> + dev_dbg(dev, "Using %d MSI vectors\n", msi->num_vectors);
> +
> + msi->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
> +
> + ret = dw_pcie_allocate_domains(msi);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> + if (msi->msi_irq[ctrl] > 0)
> + irq_set_chained_handler_and_data(msi->msi_irq[ctrl],
> + dw_chained_msi_isr, msi);
> + }
> +
> + /*
> + * Even though the iMSI-RX Module supports 64-bit addresses some
> + * peripheral PCIe devices may lack 64-bit message support. In
> + * order not to miss MSI TLPs from those devices the MSI target
> + * address has to be within the lowest 4GB.
> + *
> + * Note until there is a better alternative found the reservation is
> + * done by allocating from the artificially limited DMA-coherent
> + * memory.
> + */
> + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> + if (!ret)
> + msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &msi->msi_data,
> + GFP_KERNEL);
> +
> + if (!msi_vaddr) {
> + dev_warn(dev, "Failed to allocate 32-bit MSI address\n");
> + dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
> + msi_vaddr = dmam_alloc_coherent(dev, sizeof(u64), &msi->msi_data,
> + GFP_KERNEL);
> + if (!msi_vaddr) {
> + dev_err(dev, "Failed to allocate MSI address\n");
> + dw_pcie_free_msi(msi);
> + return ERR_PTR(-ENOMEM);
> + }
> + }
> +
> + return msi;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
> new file mode 100644
> index 0000000..633156e
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synopsys DesignWare PCIe host controller driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * https://www.samsung.com
> + *
> + * Author: Jingoo Han <jg1.han@samsung.com>
> + */
> +#ifndef _PCIE_DESIGNWARE_MSI_H
> +#define _PCIE_DESIGNWARE_MSI_H
> +
> +#include "../../pci.h"
> +
> +#define MAX_MSI_IRQS 256
> +#define MAX_MSI_IRQS_PER_CTRL 32
> +#define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
> +#define MSI_REG_CTRL_BLOCK_SIZE 12
> +#define MSI_DEF_NUM_VECTORS 32
> +
> +struct dw_msi {
> + struct device *dev;
> + struct irq_domain *irq_domain;
> + struct irq_domain *msi_domain;
> + struct irq_chip *msi_irq_chip;
> + int msi_irq[MAX_MSI_CTRLS];
> + dma_addr_t msi_data;
> + u32 num_vectors;
> + u32 irq_mask[MAX_MSI_CTRLS];
> + raw_spinlock_t lock;
> + DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> + bool has_msi_ctrl;
> + void *private_data;
> + void *pp;
> +};
> +
> +struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> + void *pp, u32 num_vectors);
> +int dw_pcie_allocate_domains(struct dw_msi *msi);
> +void dw_pcie_msi_init(struct dw_msi *msi);
> +void dw_pcie_free_msi(struct dw_msi *msi);
> +irqreturn_t dw_handle_msi_irq(struct dw_msi *msi);
> +#endif /* _PCIE_DESIGNWARE_MSI_H */
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 1b5aba1..e84298e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -22,6 +22,7 @@
>
> #include "../../pci.h"
> #include "pcie-designware.h"
> +#include "pcie-designware-msi.h"
>
> static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
> [DW_PCIE_DBI_CLK] = "dbi",
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ef84931..8b7fddf 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -232,12 +232,6 @@
> #define DEFAULT_DBI_ATU_OFFSET (0x3 << 20)
> #define DEFAULT_DBI_DMA_OFFSET PCIE_DMA_UNROLL_BASE
>
> -#define MAX_MSI_IRQS 256
> -#define MAX_MSI_IRQS_PER_CTRL 32
> -#define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
> -#define MSI_REG_CTRL_BLOCK_SIZE 12
> -#define MSI_DEF_NUM_VECTORS 32
> -
> /* Maximum number of inbound/outbound iATUs */
> #define MAX_IATU_IN 256
> #define MAX_IATU_OUT 256
> @@ -319,7 +313,6 @@ struct dw_pcie_host_ops {
> };
>
> struct dw_pcie_rp {
> - bool has_msi_ctrl:1;
> bool cfg0_io_shared:1;
> u64 cfg0_base;
> void __iomem *va_cfg0_base;
> @@ -329,16 +322,10 @@ struct dw_pcie_rp {
> u32 io_size;
> int irq;
> const struct dw_pcie_host_ops *ops;
> - int msi_irq[MAX_MSI_CTRLS];
> - struct irq_domain *irq_domain;
> - struct irq_domain *msi_domain;
> - dma_addr_t msi_data;
> - struct irq_chip *msi_irq_chip;
> u32 num_vectors;
> - u32 irq_mask[MAX_MSI_CTRLS];
> + struct dw_msi *msi;
> struct pci_host_bridge *bridge;
> raw_spinlock_t lock;
> - DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> bool use_atu_msg;
> int msg_atu_index;
> struct resource *msg_res;
> @@ -639,19 +626,12 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> }
>
> #ifdef CONFIG_PCIE_DW_HOST
> -irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> int dw_pcie_host_init(struct dw_pcie_rp *pp);
> void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
> -int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
> void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
> int where);
> #else
> -static inline irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
> -{
> - return IRQ_NONE;
> -}
> -
> static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> {
> return 0;
> @@ -666,10 +646,6 @@ static inline void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
> {
> }
>
> -static inline int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> -{
> - return 0;
> -}
> static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> unsigned int devfn,
> int where)
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index d99e12f..6139330 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -16,6 +16,7 @@
>
> #include "../../pci.h"
> #include "pcie-designware.h"
> +#include "pcie-designware-msi.h"
>
> /* Renesas-specific */
> /* PCIe Mode Setting Register 0 */
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 804341b..f415fa1 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -34,6 +34,7 @@
> #include <soc/tegra/bpmp.h>
> #include <soc/tegra/bpmp-abi.h>
> #include "../../pci.h"
> +#include "pcie-designware-msi.h"
>
> #define TEGRA194_DWC_IP_VER 0x490A
> #define TEGRA234_DWC_IP_VER 0x562A
> @@ -2407,6 +2408,7 @@ static int tegra_pcie_dw_resume_early(struct device *dev)
> static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
> {
> struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
> + struct dw_msi *msi;
>
> if (pcie->of_data->mode == DW_PCIE_RC_TYPE) {
> if (!pcie->link_state)
> @@ -2415,9 +2417,10 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
> debugfs_remove_recursive(pcie->debugfs);
> tegra_pcie_downstream_dev_to_D0(pcie);
>
> + msi = pcie->pci.pp.msi;
> disable_irq(pcie->pci.pp.irq);
> if (IS_ENABLED(CONFIG_PCI_MSI))
> - disable_irq(pcie->pci.pp.msi_irq[0]);
> + disable_irq(msi->msi_irq[0]);
>
> tegra_pcie_dw_pme_turnoff(pcie);
> tegra_pcie_unconfig_controller(pcie);
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V222/7] PCI: dwc: Add msi_ops to allow DBI based MSI register access
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
2024-07-15 18:13 ` [PATCH V2 1/7] PCI: dwc: Move MSI functionality related code to separate file Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-31 17:17 ` Manivannan Sadhasivam
2024-07-15 18:13 ` [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option Mayank Rana
` (6 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
PCIe ECAM driver do not have dw_pcie data structure populated and DBI
access related APIs. Hence add msi_ops as part of dw_msi structure to
allow populating DBI based MSI register access.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 20 ++++++++++++-
drivers/pci/controller/dwc/pcie-designware-msi.c | 36 +++++++++++++----------
drivers/pci/controller/dwc/pcie-designware-msi.h | 10 +++++--
3 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3dcf88a..7a1eb1f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -47,6 +47,16 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
}
}
+static u32 dw_pcie_readl_msi_dbi(void *pci, u32 reg)
+{
+ return dw_pcie_readl_dbi((struct dw_pcie *)pci, reg);
+}
+
+static void dw_pcie_writel_msi_dbi(void *pci, u32 reg, u32 val)
+{
+ dw_pcie_writel_dbi((struct dw_pcie *)pci, reg, val);
+}
+
int dw_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -55,6 +65,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
struct platform_device *pdev = to_platform_device(dev);
struct resource_entry *win;
struct pci_host_bridge *bridge;
+ struct dw_msi_ops *msi_ops;
struct resource *res;
bool has_msi_ctrl;
int ret;
@@ -124,7 +135,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
if (ret < 0)
goto err_deinit_host;
} else if (has_msi_ctrl) {
- pp->msi = dw_pcie_msi_host_init(pdev, pp, pp->num_vectors);
+ msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
+ if (msi_ops == NULL)
+ goto err_deinit_host;
+
+ msi_ops->pp = pci;
+ msi_ops->readl_msi = dw_pcie_readl_msi_dbi,
+ msi_ops->writel_msi = dw_pcie_writel_msi_dbi,
+ pp->msi = dw_pcie_msi_host_init(pdev, msi_ops, pp->num_vectors);
if (IS_ERR(pp->msi)) {
ret = PTR_ERR(pp->msi);
goto err_deinit_host;
diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.c b/drivers/pci/controller/dwc/pcie-designware-msi.c
index 39fe5be..dbfffce 100644
--- a/drivers/pci/controller/dwc/pcie-designware-msi.c
+++ b/drivers/pci/controller/dwc/pcie-designware-msi.c
@@ -44,6 +44,16 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
.chip = &dw_pcie_msi_irq_chip,
};
+static u32 dw_msi_readl(struct dw_msi *msi, u32 reg)
+{
+ return msi->msi_ops->readl_msi(msi->msi_ops->pp, reg);
+}
+
+static void dw_msi_writel(struct dw_msi *msi, u32 reg, u32 val)
+{
+ msi->msi_ops->writel_msi(msi->msi_ops->pp, reg, val);
+}
+
/* MSI int handler */
irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
{
@@ -51,13 +61,11 @@ irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
unsigned long val;
u32 status, num_ctrls;
irqreturn_t ret = IRQ_NONE;
- struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
for (i = 0; i < num_ctrls; i++) {
- status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
- (i * MSI_REG_CTRL_BLOCK_SIZE));
+ status = dw_msi_readl(msi, PCIE_MSI_INTR0_STATUS + (i * MSI_REG_CTRL_BLOCK_SIZE));
if (!status)
continue;
@@ -115,7 +123,6 @@ static int dw_pci_msi_set_affinity(struct irq_data *d,
static void dw_pci_bottom_mask(struct irq_data *d)
{
struct dw_msi *msi = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
unsigned int res, bit, ctrl;
unsigned long flags;
@@ -126,7 +133,7 @@ static void dw_pci_bottom_mask(struct irq_data *d)
bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
msi->irq_mask[ctrl] |= BIT(bit);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
+ dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
raw_spin_unlock_irqrestore(&msi->lock, flags);
}
@@ -134,7 +141,6 @@ static void dw_pci_bottom_mask(struct irq_data *d)
static void dw_pci_bottom_unmask(struct irq_data *d)
{
struct dw_msi *msi = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
unsigned int res, bit, ctrl;
unsigned long flags;
@@ -145,7 +151,7 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
msi->irq_mask[ctrl] &= ~BIT(bit);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
+ dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
raw_spin_unlock_irqrestore(&msi->lock, flags);
}
@@ -153,14 +159,13 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
static void dw_pci_bottom_ack(struct irq_data *d)
{
struct dw_msi *msi = irq_data_get_irq_chip_data(d);
- struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
unsigned int res, bit, ctrl;
ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
+ dw_msi_writel(msi, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
}
static struct irq_chip dw_pci_msi_bottom_irq_chip = {
@@ -262,7 +267,6 @@ void dw_pcie_free_msi(struct dw_msi *msi)
void dw_pcie_msi_init(struct dw_msi *msi)
{
- struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
u32 ctrl, num_ctrls;
u64 msi_target;
@@ -273,16 +277,16 @@ void dw_pcie_msi_init(struct dw_msi *msi)
num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
/* Initialize IRQ Status array */
for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
+ dw_msi_writel(msi, PCIE_MSI_INTR0_MASK +
(ctrl * MSI_REG_CTRL_BLOCK_SIZE),
msi->irq_mask[ctrl]);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
+ dw_msi_writel(msi, PCIE_MSI_INTR0_ENABLE +
(ctrl * MSI_REG_CTRL_BLOCK_SIZE), ~0);
}
/* Program the msi_data */
- dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
- dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
+ dw_msi_writel(msi, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
+ dw_msi_writel(msi, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
}
static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_device *pdev)
@@ -324,7 +328,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_devic
}
struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
- void *pp, u32 num_vectors)
+ struct dw_msi_ops *ops, u32 num_vectors)
{
struct device *dev = &pdev->dev;
u64 *msi_vaddr = NULL;
@@ -341,7 +345,7 @@ struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
raw_spin_lock_init(&msi->lock);
msi->dev = dev;
- msi->pp = pp;
+ msi->msi_ops = ops;
msi->has_msi_ctrl = true;
msi->num_vectors = num_vectors;
diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
index 633156e..cf5c612 100644
--- a/drivers/pci/controller/dwc/pcie-designware-msi.h
+++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
@@ -18,8 +18,15 @@
#define MSI_REG_CTRL_BLOCK_SIZE 12
#define MSI_DEF_NUM_VECTORS 32
+struct dw_msi_ops {
+ void *pp;
+ u32 (*readl_msi)(void *pp, u32 reg);
+ void (*writel_msi)(void *pp, u32 reg, u32 val);
+};
+
struct dw_msi {
struct device *dev;
+ struct dw_msi_ops *msi_ops;
struct irq_domain *irq_domain;
struct irq_domain *msi_domain;
struct irq_chip *msi_irq_chip;
@@ -31,11 +38,10 @@ struct dw_msi {
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
bool has_msi_ctrl;
void *private_data;
- void *pp;
};
struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
- void *pp, u32 num_vectors);
+ struct dw_msi_ops *ops, u32 num_vectors);
int dw_pcie_allocate_domains(struct dw_msi *msi);
void dw_pcie_msi_init(struct dw_msi *msi);
void dw_pcie_free_msi(struct dw_msi *msi);
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V222/7] PCI: dwc: Add msi_ops to allow DBI based MSI register access
2024-07-15 18:13 ` [PATCH V222/7] PCI: dwc: Add msi_ops to allow DBI based MSI register access Mayank Rana
@ 2024-07-31 17:17 ` Manivannan Sadhasivam
0 siblings, 0 replies; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-31 17:17 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:30AM -0700, Mayank Rana wrote:
> PCIe ECAM driver do not have dw_pcie data structure populated and DBI
> access related APIs. Hence add msi_ops as part of dw_msi structure to
> allow populating DBI based MSI register access.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 20 ++++++++++++-
> drivers/pci/controller/dwc/pcie-designware-msi.c | 36 +++++++++++++----------
> drivers/pci/controller/dwc/pcie-designware-msi.h | 10 +++++--
> 3 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3dcf88a..7a1eb1f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -47,6 +47,16 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> }
> }
>
> +static u32 dw_pcie_readl_msi_dbi(void *pci, u32 reg)
> +{
> + return dw_pcie_readl_dbi((struct dw_pcie *)pci, reg);
> +}
> +
> +static void dw_pcie_writel_msi_dbi(void *pci, u32 reg, u32 val)
> +{
> + dw_pcie_writel_dbi((struct dw_pcie *)pci, reg, val);
> +}
> +
There is nothing MSI specific in dw_pcie_{writel/readl}_msi_dbi(). This just
writes/reads the registers. So this should be called as dw_pcie_write_reg()/
dw_pcie_write_reg().
> int dw_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -55,6 +65,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> struct platform_device *pdev = to_platform_device(dev);
> struct resource_entry *win;
> struct pci_host_bridge *bridge;
> + struct dw_msi_ops *msi_ops;
> struct resource *res;
> bool has_msi_ctrl;
> int ret;
> @@ -124,7 +135,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret < 0)
> goto err_deinit_host;
> } else if (has_msi_ctrl) {
> - pp->msi = dw_pcie_msi_host_init(pdev, pp, pp->num_vectors);
> + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
> + if (msi_ops == NULL)
> + goto err_deinit_host;
> +
> + msi_ops->pp = pci;
Stuffing private data inside ops structure looks weird. Also the fact that
allocating memory for ops...
> + msi_ops->readl_msi = dw_pcie_readl_msi_dbi,
> + msi_ops->writel_msi = dw_pcie_writel_msi_dbi,
Same for the callback name.
> + pp->msi = dw_pcie_msi_host_init(pdev, msi_ops, pp->num_vectors);
> if (IS_ERR(pp->msi)) {
> ret = PTR_ERR(pp->msi);
> goto err_deinit_host;
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.c b/drivers/pci/controller/dwc/pcie-designware-msi.c
> index 39fe5be..dbfffce 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-msi.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.c
> @@ -44,6 +44,16 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
> .chip = &dw_pcie_msi_irq_chip,
> };
>
> +static u32 dw_msi_readl(struct dw_msi *msi, u32 reg)
> +{
> + return msi->msi_ops->readl_msi(msi->msi_ops->pp, reg);
> +}
> +
> +static void dw_msi_writel(struct dw_msi *msi, u32 reg, u32 val)
> +{
> + msi->msi_ops->writel_msi(msi->msi_ops->pp, reg, val);
> +}
> +
These could be:
dw_msi_read_reg()
dw_msi_write_reg()
- Mani
> /* MSI int handler */
> irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
> {
> @@ -51,13 +61,11 @@ irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
> unsigned long val;
> u32 status, num_ctrls;
> irqreturn_t ret = IRQ_NONE;
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
>
> num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>
> for (i = 0; i < num_ctrls; i++) {
> - status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> - (i * MSI_REG_CTRL_BLOCK_SIZE));
> + status = dw_msi_readl(msi, PCIE_MSI_INTR0_STATUS + (i * MSI_REG_CTRL_BLOCK_SIZE));
> if (!status)
> continue;
>
> @@ -115,7 +123,6 @@ static int dw_pci_msi_set_affinity(struct irq_data *d,
> static void dw_pci_bottom_mask(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
> unsigned long flags;
>
> @@ -126,7 +133,7 @@ static void dw_pci_bottom_mask(struct irq_data *d)
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> msi->irq_mask[ctrl] |= BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
>
> raw_spin_unlock_irqrestore(&msi->lock, flags);
> }
> @@ -134,7 +141,6 @@ static void dw_pci_bottom_mask(struct irq_data *d)
> static void dw_pci_bottom_unmask(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
> unsigned long flags;
>
> @@ -145,7 +151,7 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> msi->irq_mask[ctrl] &= ~BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
>
> raw_spin_unlock_irqrestore(&msi->lock, flags);
> }
> @@ -153,14 +159,13 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
> static void dw_pci_bottom_ack(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
>
> ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> + dw_msi_writel(msi, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> }
>
> static struct irq_chip dw_pci_msi_bottom_irq_chip = {
> @@ -262,7 +267,6 @@ void dw_pcie_free_msi(struct dw_msi *msi)
>
> void dw_pcie_msi_init(struct dw_msi *msi)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> u32 ctrl, num_ctrls;
> u64 msi_target;
>
> @@ -273,16 +277,16 @@ void dw_pcie_msi_init(struct dw_msi *msi)
> num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> /* Initialize IRQ Status array */
> for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK +
> (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> msi->irq_mask[ctrl]);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> + dw_msi_writel(msi, PCIE_MSI_INTR0_ENABLE +
> (ctrl * MSI_REG_CTRL_BLOCK_SIZE), ~0);
> }
>
> /* Program the msi_data */
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> + dw_msi_writel(msi, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> + dw_msi_writel(msi, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> }
>
> static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_device *pdev)
> @@ -324,7 +328,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_devic
> }
>
> struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> - void *pp, u32 num_vectors)
> + struct dw_msi_ops *ops, u32 num_vectors)
> {
> struct device *dev = &pdev->dev;
> u64 *msi_vaddr = NULL;
> @@ -341,7 +345,7 @@ struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
>
> raw_spin_lock_init(&msi->lock);
> msi->dev = dev;
> - msi->pp = pp;
> + msi->msi_ops = ops;
> msi->has_msi_ctrl = true;
> msi->num_vectors = num_vectors;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
> index 633156e..cf5c612 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-msi.h
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
> @@ -18,8 +18,15 @@
> #define MSI_REG_CTRL_BLOCK_SIZE 12
> #define MSI_DEF_NUM_VECTORS 32
>
> +struct dw_msi_ops {
> + void *pp;
> + u32 (*readl_msi)(void *pp, u32 reg);
> + void (*writel_msi)(void *pp, u32 reg, u32 val);
> +};
> +
> struct dw_msi {
> struct device *dev;
> + struct dw_msi_ops *msi_ops;
> struct irq_domain *irq_domain;
> struct irq_domain *msi_domain;
> struct irq_chip *msi_irq_chip;
> @@ -31,11 +38,10 @@ struct dw_msi {
> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> bool has_msi_ctrl;
> void *private_data;
> - void *pp;
> };
>
> struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> - void *pp, u32 num_vectors);
> + struct dw_msi_ops *ops, u32 num_vectors);
> int dw_pcie_allocate_domains(struct dw_msi *msi);
> void dw_pcie_msi_init(struct dw_msi *msi);
> void dw_pcie_free_msi(struct dw_msi *msi);
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
2024-07-15 18:13 ` [PATCH V2 1/7] PCI: dwc: Move MSI functionality related code to separate file Mayank Rana
2024-07-15 18:13 ` [PATCH V222/7] PCI: dwc: Add msi_ops to allow DBI based MSI register access Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-15 18:39 ` Andrew Lunn
2024-07-15 18:13 ` [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding Mayank Rana
` (5 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
PCIe designware MSI driver (pcie-designware-msi.c) shall be used without
enabling pcie-designware core drivers (e.g. usage with ECAM driver). Hence
add Kconfig option to enable pcie-designware-msi driver as separate module.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/dwc/Kconfig | 8 ++++++++
drivers/pci/controller/dwc/Makefile | 3 ++-
drivers/pci/controller/dwc/pcie-designware-msi.h | 14 ++++++++++++++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 8afacc9..a4c8920 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -6,8 +6,16 @@ menu "DesignWare-based PCIe controllers"
config PCIE_DW
bool
+config PCIE_DW_MSI
+ bool "DWC PCIe based MSI controller"
+ depends on PCI_MSI
+ help
+ Say Y here to enable DWC PCIe based MSI controller to support
+ MSI functionality.
+
config PCIE_DW_HOST
bool
+ select PCIE_DW_MSI
select PCIE_DW
config PCIE_DW_EP
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 2ecc603..9e8e4515 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PCIE_DW) += pcie-designware.o
-obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o pcie-designware-msi.o
+obj-$(CONFIG_PCIE_DW_MSI) += pcie-designware-msi.o
+obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
index cf5c612..2872775f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-msi.h
+++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
@@ -40,10 +40,24 @@ struct dw_msi {
void *private_data;
};
+#if IS_ENABLED(CONFIG_PCIE_DW_MSI)
struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
struct dw_msi_ops *ops, u32 num_vectors);
int dw_pcie_allocate_domains(struct dw_msi *msi);
void dw_pcie_msi_init(struct dw_msi *msi);
void dw_pcie_free_msi(struct dw_msi *msi);
irqreturn_t dw_handle_msi_irq(struct dw_msi *msi);
+#else
+static inline struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
+ struct dw_msi_ops *ops, u32 num_vectors)
+{ return ERR_PTR(-ENODEV); }
+static inline int dw_pcie_allocate_domains(struct dw_msi *msi)
+{ return -ENODEV; }
+static inline void dw_pcie_msi_init(struct dw_msi *msi)
+{ }
+static inline void dw_pcie_free_msi(struct dw_msi *msi)
+{ }
+static inline irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
+{ return IRQ_NONE; }
+#endif
#endif /* _PCIE_DESIGNWARE_MSI_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option
2024-07-15 18:13 ` [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option Mayank Rana
@ 2024-07-15 18:39 ` Andrew Lunn
2024-07-16 0:04 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Andrew Lunn @ 2024-07-15 18:39 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:31AM -0700, Mayank Rana wrote:
> PCIe designware MSI driver (pcie-designware-msi.c) shall be used without
> enabling pcie-designware core drivers (e.g. usage with ECAM driver). Hence
> add Kconfig option to enable pcie-designware-msi driver as separate module.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 8 ++++++++
> drivers/pci/controller/dwc/Makefile | 3 ++-
> drivers/pci/controller/dwc/pcie-designware-msi.h | 14 ++++++++++++++
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 8afacc9..a4c8920 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -6,8 +6,16 @@ menu "DesignWare-based PCIe controllers"
> config PCIE_DW
> bool
>
> +config PCIE_DW_MSI
> + bool "DWC PCIe based MSI controller"
> + depends on PCI_MSI
> + help
> + Say Y here to enable DWC PCIe based MSI controller to support
> + MSI functionality.
> +
Nit picking, but in the commit message you say separate module. But it
is a bool, not a tristate, so it cannot be built as a module.
Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option
2024-07-15 18:39 ` Andrew Lunn
@ 2024-07-16 0:04 ` Mayank Rana
0 siblings, 0 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-16 0:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
Hi Andrew
On 7/15/2024 11:39 AM, Andrew Lunn wrote:
> On Mon, Jul 15, 2024 at 11:13:31AM -0700, Mayank Rana wrote:
>> PCIe designware MSI driver (pcie-designware-msi.c) shall be used without
>> enabling pcie-designware core drivers (e.g. usage with ECAM driver). Hence
>> add Kconfig option to enable pcie-designware-msi driver as separate module.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 8 ++++++++
>> drivers/pci/controller/dwc/Makefile | 3 ++-
>> drivers/pci/controller/dwc/pcie-designware-msi.h | 14 ++++++++++++++
>> 3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 8afacc9..a4c8920 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -6,8 +6,16 @@ menu "DesignWare-based PCIe controllers"
>> config PCIE_DW
>> bool
>>
>> +config PCIE_DW_MSI
>> + bool "DWC PCIe based MSI controller"
>> + depends on PCI_MSI
>> + help
>> + Say Y here to enable DWC PCIe based MSI controller to support
>> + MSI functionality.
>> +
>
> Nit picking, but in the commit message you say separate module. But it
> is a bool, not a tristate, so it cannot be built as a module.
I don't mean to make this driver as loadable module here. Saying this
I agree that commit text is saying as separate module. I shall update
commit text by replacing "separate module" as "separate driver".
> Andrew
>
Regards,
Mayank
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (2 preceding siblings ...)
2024-07-15 18:13 ` [PATCH V2 3/7] PCI: dwc: Add pcie-designware-msi driver related Kconfig option Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-16 7:25 ` Krzysztof Kozlowski
2024-07-15 18:13 ` [PATCH V2 5/7] PCI: host-generic: Add power domain based handling for PCIe controller Mayank Rana
` (4 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
Add "power-domains" usage (optional) related binding to power up ECAM
compliant PCIe root complex.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
index 3484e0b..9c714fa 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
@@ -110,6 +110,12 @@ properties:
iommu-map-mask: true
msi-parent: true
+ power-domains:
+ maxItems: 1
+ description:
+ A phandle to the node that controls power or/and system resource or interface to firmware
+ to enable ECAM compliant PCIe root complex.
+
required:
- compatible
- reg
@@ -172,6 +178,7 @@ examples:
// PCI_DEVICE(3) INT#(1)
interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
+ power-domains = <&scmi5_pd 0>;
};
};
...
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding
2024-07-15 18:13 ` [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding Mayank Rana
@ 2024-07-16 7:25 ` Krzysztof Kozlowski
2024-07-16 21:47 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-16 7:25 UTC (permalink / raw)
To: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
On 15/07/2024 20:13, Mayank Rana wrote:
> Add "power-domains" usage (optional) related binding to power up ECAM
> compliant PCIe root complex.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> index 3484e0b..9c714fa 100644
> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> @@ -110,6 +110,12 @@ properties:
> iommu-map-mask: true
> msi-parent: true
>
> + power-domains:
> + maxItems: 1
> + description:
> + A phandle to the node that controls power or/and system resource or interface to firmware
Wrap how Coding Style asks (so 80).
I am sorry, but power domains are power domains, not interface to
firmware to enable your hardware. Rephrase it to actually describe the
hardware.
Also, drop all redundant information. It cannot be anything else than
phandle to the node...
> + to enable ECAM compliant PCIe root complex.
> +
Anyway, there are no DTS users with such power domain. Look at the
binding and its compatibles. Does any of these devices have power
domain? No.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding
2024-07-16 7:25 ` Krzysztof Kozlowski
@ 2024-07-16 21:47 ` Mayank Rana
0 siblings, 0 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-16 21:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, will, lpieralisi, kw, robh, bhelgaas,
jingoohan1, manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh,
s-vadapalli, u.kleine-koenig, dlemoal, amishin, thierry.reding,
jonathanh, Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas,
krzk+dt, conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
Hi Krzysztof
Appreciate your quick review comments.
On 7/16/2024 12:25 AM, Krzysztof Kozlowski wrote:
> On 15/07/2024 20:13, Mayank Rana wrote:
>> Add "power-domains" usage (optional) related binding to power up ECAM
>> compliant PCIe root complex.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> index 3484e0b..9c714fa 100644
>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> @@ -110,6 +110,12 @@ properties:
>> iommu-map-mask: true
>> msi-parent: true
>>
>> + power-domains:
>> + maxItems: 1
>> + description:
>> + A phandle to the node that controls power or/and system resource or interface to firmware
>
> Wrap how Coding Style asks (so 80).
My bad, I shall fix it into next patchset.
> I am sorry, but power domains are power domains, not interface to
> firmware to enable your hardware. Rephrase it to actually describe the
> hardware.
Agree with your above first part of comment.
I understood that you are suggesting to describe all steps in terms of
what happen with usage of power domain instead of mentioning generic
abstraction with proposed solution here. I mentioned generic way as
power domain is not tied with specific compatible string or platform
as optional usage with this generic driver.
Power domain shall be doing possible below implementation:
1. controls power -> can be just regulators
2. system resource -> can be PCIe related all system resource like
GDSC/Clock/regulators/gpio
3. Interface to firmware -> including all system resource handling in
addition to PCIe PHY and controller programming to PCIe ECAM RC mode
with D0 state.
Cover letter is showing high level architecture and usage here although
it would be lost. So to document here I can add more information. Below
are steps which is being performed:
1. Handle all system resources (GDSC/CLOCKs/regulators/bus (interconnect
voting))
2. Bring PCIe PHY and Controller out of reset
3. Program PCIe PHY and controller to get PCIe link up
Power domain interface is also used to perform D3cold based
functionality with system suspend case to turn off resources after
performing PCIe level handshake (i.e. PME turn off and L23 ready).
I can rework/reword above provided power domain binding information but
not sure shall I keep it generic information or capture specific above
usage with proposed solution. Please let me know what do suggest or
prefer here ?
> Also, drop all redundant information. It cannot be anything else than
> phandle to the node...
ACK
>> + to enable ECAM compliant PCIe root complex.
>> +
>
> Anyway, there are no DTS users with such power domain. Look at the
> binding and its compatibles. Does any of these devices have power
> domain? No.
Agree. Work in progress, and based on outcome of that I shall add user
of it as part of next patchset.
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V2 5/7] PCI: host-generic: Add power domain based handling for PCIe controller
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (3 preceding siblings ...)
2024-07-15 18:13 ` [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-15 18:13 ` [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding Mayank Rana
` (3 subsequent siblings)
8 siblings, 0 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
Add usage of power domain to control PCIe controller enumeration. This is
needed to allow interaction with firmware to vote/unvote system resources,
PCIe PHY and configuration of PCIe controller into ECAM mode. This feature
support system suspend and resume to put PCIe into D3 cold and D0.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/pci-host-generic.c | 42 +++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
index 41cb6a0..c2c027f 100644
--- a/drivers/pci/controller/pci-host-generic.c
+++ b/drivers/pci/controller/pci-host-generic.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
static const struct pci_ecam_ops gen_pci_cfg_cam_bus_ops = {
.bus_shift = 16,
@@ -76,13 +77,50 @@ static const struct of_device_id gen_pci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, gen_pci_of_match);
+static int gen_pcie_ecam_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int ret = 0;
+
+ if (!IS_ERR_OR_NULL(dev->pm_domain)) {
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0) {
+ dev_err(dev, "fail to enable pcie controller:%d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = pci_host_common_probe(pdev);
+ if (ret) {
+ dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
+ goto err;
+ }
+
+ return ret;
+err:
+ if (!IS_ERR_OR_NULL(dev->pm_domain))
+ pm_runtime_put_sync(dev);
+ return ret;
+}
+
+static void gen_pcie_ecam_remove(struct platform_device *pdev)
+{
+ pci_host_common_remove(pdev);
+ if (pdev->dev.pm_domain)
+ pm_runtime_put_sync(&pdev->dev);
+}
+
static struct platform_driver gen_pci_driver = {
.driver = {
.name = "pci-host-generic",
.of_match_table = gen_pci_of_match,
},
- .probe = pci_host_common_probe,
- .remove_new = pci_host_common_remove,
+ .probe = gen_pcie_ecam_probe,
+ .remove_new = gen_pcie_ecam_remove,
};
module_platform_driver(gen_pci_driver);
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (4 preceding siblings ...)
2024-07-15 18:13 ` [PATCH V2 5/7] PCI: host-generic: Add power domain based handling for PCIe controller Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-15 19:43 ` Rob Herring (Arm)
` (2 more replies)
2024-07-15 18:13 ` [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage Mayank Rana
` (2 subsequent siblings)
8 siblings, 3 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
To support MSI functionality using Synopsys DesignWare PCIe controller
based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
compatible binding which uses provided SPIs to support MSI functionality.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
.../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
index 9c714fa..9e860d5 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
@@ -81,6 +81,12 @@ properties:
- marvell,armada8k-pcie-ecam
- socionext,synquacer-pcie-ecam
- const: snps,dw-pcie-ecam
+ - description: |
+ Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
+ ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
+ controller for MSI functionality, this compatible is used.
+ items:
+ - const: snps,dw-pcie-ecam-msi
- description:
CAM or ECAM compliant PCI host controllers without any quirks
enum:
@@ -116,6 +122,20 @@ properties:
A phandle to the node that controls power or/and system resource or interface to firmware
to enable ECAM compliant PCIe root complex.
+ interrupts:
+ description:
+ DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
+ minItems: 1
+ maxItems: 8
+
+ interrupt-names:
+ description:
+ MSI interrupt names
+ minItems: 1
+ maxItems: 8
+ items:
+ pattern: '^msi[0-9]+$'
+
required:
- compatible
- reg
@@ -146,11 +166,22 @@ allOf:
reg:
maxItems: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: snps,dw-pcie-ecam-msi
+ then:
+ required:
+ - interrupts
+ - interrupt-names
+
unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
bus {
#address-cells = <2>;
#size-cells = <2>;
@@ -180,5 +211,31 @@ examples:
interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
power-domains = <&scmi5_pd 0>;
};
+
+ pcie0: pci@1c00000 {
+ compatible = "snps,dw-pcie-ecam-msi";
+ reg = <0x4 0x00000000 0 0x10000000>;
+ device_type = "pci";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x02000000 0x0 0x40100000 0x0 0x40100000 0x0 0x1ff00000>,
+ <0x43000000 0x4 0x10100000 0x4 0x10100000 0x0 0x40000000>;
+ bus-range = <0x00 0xff>;
+ dma-coherent;
+ linux,pci-domain = <0>;
+ power-domains = <&scmi5_pd 0>;
+ iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
+ <0x100 &pcie_smmu 0x0001 0x1>;
+
+ interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi0", "msi1", "msi2", "msi3", "msi4", "msi5", "msi6", "msi7";
+ };
};
...
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-15 18:13 ` [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding Mayank Rana
@ 2024-07-15 19:43 ` Rob Herring (Arm)
2024-07-16 7:28 ` Krzysztof Kozlowski
2024-07-31 17:26 ` Manivannan Sadhasivam
2 siblings, 0 replies; 37+ messages in thread
From: Rob Herring (Arm) @ 2024-07-15 19:43 UTC (permalink / raw)
To: Mayank Rana
Cc: krzk+dt, vidyas, thierry.reding, s-vadapalli, will, linux-pci,
Frank.Li, marek.vasut+renesas, jingoohan1, conor+dt, cassel,
quic_nkela, quic_nitegupt, lpieralisi, linux-arm-kernel,
jonathanh, yoshihiro.shimoda.uh, bhelgaas, quic_msarkar, dlemoal,
quic_ramkri, quic_shazhuss, amishin, kw, u.kleine-koenig,
devicetree, manivannan.sadhasivam, ilpo.jarvinen
On Mon, 15 Jul 2024 11:13:34 -0700, Mayank Rana wrote:
> To support MSI functionality using Synopsys DesignWare PCIe controller
> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
> compatible binding which uses provided SPIs to support MSI functionality.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/pci/host-generic-pci.yaml:137:9: [warning] wrong indentation: expected 6 but found 8 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1721067215-5832-7-git-send-email-quic_mrana@quicinc.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-15 18:13 ` [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding Mayank Rana
2024-07-15 19:43 ` Rob Herring (Arm)
@ 2024-07-16 7:28 ` Krzysztof Kozlowski
2024-07-16 22:09 ` Mayank Rana
2024-07-31 17:26 ` Manivannan Sadhasivam
2 siblings, 1 reply; 37+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-16 7:28 UTC (permalink / raw)
To: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
On 15/07/2024 20:13, Mayank Rana wrote:
> To support MSI functionality using Synopsys DesignWare PCIe controller
> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
> compatible binding which uses provided SPIs to support MSI functionality.
To support MSI, you add MSI support... That's a tautology. Describe
hardware instead.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> index 9c714fa..9e860d5 100644
> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> @@ -81,6 +81,12 @@ properties:
> - marvell,armada8k-pcie-ecam
> - socionext,synquacer-pcie-ecam
> - const: snps,dw-pcie-ecam
> + - description: |
> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
> + controller for MSI functionality, this compatible is used.
> + items:
> + - const: snps,dw-pcie-ecam-msi
MSI is already present in the binding, isn't it? Anyway, aren't you
forgetting specific compatible? Please open your internal (quite
comprehensive) guideline on bindings and DTS.
> - description:
> CAM or ECAM compliant PCI host controllers without any quirks
> enum:
> @@ -116,6 +122,20 @@ properties:
> A phandle to the node that controls power or/and system resource or interface to firmware
> to enable ECAM compliant PCIe root complex.
>
> + interrupts:
> + description:
> + DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
> + minItems: 1
> + maxItems: 8
> +
> + interrupt-names:
> + description:
> + MSI interrupt names
> + minItems: 1
> + maxItems: 8
> + items:
> + pattern: '^msi[0-9]+$'
Why the same devices have variable numbers?
> +
> required:
> - compatible
> - reg
> @@ -146,11 +166,22 @@ allOf:
> reg:
> maxItems: 1
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-16 7:28 ` Krzysztof Kozlowski
@ 2024-07-16 22:09 ` Mayank Rana
2024-07-17 6:47 ` Krzysztof Kozlowski
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-16 22:09 UTC (permalink / raw)
To: Krzysztof Kozlowski, will, lpieralisi, kw, robh, bhelgaas,
jingoohan1, manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh,
s-vadapalli, u.kleine-koenig, dlemoal, amishin, thierry.reding,
jonathanh, Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas,
krzk+dt, conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
Hi Krzysztof
On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
> On 15/07/2024 20:13, Mayank Rana wrote:
>> To support MSI functionality using Synopsys DesignWare PCIe controller
>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>> compatible binding which uses provided SPIs to support MSI functionality.
>
> To support MSI, you add MSI support... That's a tautology. Describe
> hardware instead.
Ok. let me repharse it to provide more useful information.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> index 9c714fa..9e860d5 100644
>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>> @@ -81,6 +81,12 @@ properties:
>> - marvell,armada8k-pcie-ecam
>> - socionext,synquacer-pcie-ecam
>> - const: snps,dw-pcie-ecam
>> + - description: |
>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>> + controller for MSI functionality, this compatible is used.
>> + items:
>> + - const: snps,dw-pcie-ecam-msi
>
> MSI is already present in the binding, isn't it?
It is mentioning as msi-parent usage which could be different MSI
controller (GIC or vendor specific one) not related to designware PCIe
controller based MSI controller.
>Anyway, aren't you
> forgetting specific compatible? Please open your internal (quite
> comprehensive) guideline on bindings and DTS.
Here I am trying to define Designware based PCIe ECAM controller
supporting MSIcontroller based device. Hence I am not mentioning vendor
specific compatible usage
and keeping generic compatible binding for such device.
>
>> - description:
>> CAM or ECAM compliant PCI host controllers without any quirks
>> enum:
>> @@ -116,6 +122,20 @@ properties:
>> A phandle to the node that controls power or/and system resource or interface to firmware
>> to enable ECAM compliant PCIe root complex.
>>
>> + interrupts:
>> + description:
>> + DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
>> + minItems: 1
>> + maxItems: 8
>> +
>> + interrupt-names:
>> + description:
>> + MSI interrupt names
>> + minItems: 1
>> + maxItems: 8
>> + items:
>> + pattern: '^msi[0-9]+$'
>
> Why the same devices have variable numbers?
Max supported MSI with designware PCIe controller is 8 Only, and it
depends if those all are
used or some of used on specific vendor based device. Hence I have kept
it here variable names. Although here it should be [0 - 7] instead of
[0 - 9].
>> +
>> required:
>> - compatible
>> - reg
>> @@ -146,11 +166,22 @@ allOf:
>> reg:
>> maxItems: 1
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-16 22:09 ` Mayank Rana
@ 2024-07-17 6:47 ` Krzysztof Kozlowski
2024-07-17 17:20 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-17 6:47 UTC (permalink / raw)
To: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
On 17/07/2024 00:09, Mayank Rana wrote:
> Hi Krzysztof
>
> On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
>> On 15/07/2024 20:13, Mayank Rana wrote:
>>> To support MSI functionality using Synopsys DesignWare PCIe controller
>>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>>> compatible binding which uses provided SPIs to support MSI functionality.
>>
>> To support MSI, you add MSI support... That's a tautology. Describe
>> hardware instead.
> Ok. let me repharse it to provide more useful information.
>>>
>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>> ---
>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>>> 1 file changed, 57 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>> index 9c714fa..9e860d5 100644
>>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>> @@ -81,6 +81,12 @@ properties:
>>> - marvell,armada8k-pcie-ecam
>>> - socionext,synquacer-pcie-ecam
>>> - const: snps,dw-pcie-ecam
>>> + - description: |
>>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>>> + controller for MSI functionality, this compatible is used.
>>> + items:
>>> + - const: snps,dw-pcie-ecam-msi
>>
>> MSI is already present in the binding, isn't it?
> It is mentioning as msi-parent usage which could be different MSI
> controller (GIC or vendor specific one) not related to designware PCIe
> controller based MSI controller.
>
>> Anyway, aren't you
>> forgetting specific compatible? Please open your internal (quite
>> comprehensive) guideline on bindings and DTS.
> Here I am trying to define Designware based PCIe ECAM controller
> supporting MSIcontroller based device. Hence I am not mentioning vendor
> specific compatible usage
> and keeping generic compatible binding for such device.
I know what you try, yet it feels simply wrong. Read your guideline.
Are you sure you work on Designware core itself, not on one used in
Qualcomm? I would expect people from Designware to design Designware
cores and people from Qualcomm only to design licensed cores.
>>
>>> - description:
>>> CAM or ECAM compliant PCI host controllers without any quirks
>>> enum:
>>> @@ -116,6 +122,20 @@ properties:
>>> A phandle to the node that controls power or/and system resource or interface to firmware
>>> to enable ECAM compliant PCIe root complex.
>>>
>>> + interrupts:
>>> + description:
>>> + DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
>>> + minItems: 1
>>> + maxItems: 8
>>> +
>>> + interrupt-names:
>>> + description:
>>> + MSI interrupt names
>>> + minItems: 1
>>> + maxItems: 8
>>> + items:
>>> + pattern: '^msi[0-9]+$'
>>
>> Why the same devices have variable numbers?
> Max supported MSI with designware PCIe controller is 8 Only, and it
> depends if those all are
> used or some of used on specific vendor based device. Hence I have kept
> it here variable names. Although here it should be [0 - 7] instead of
> [0 - 9].
Wait, you just said there is no specific vendor device.
Sorry, bring some sanity to this.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-17 6:47 ` Krzysztof Kozlowski
@ 2024-07-17 17:20 ` Mayank Rana
2024-07-18 6:05 ` Krzysztof Kozlowski
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-17 17:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, will, lpieralisi, kw, robh, bhelgaas,
jingoohan1, manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh,
s-vadapalli, u.kleine-koenig, dlemoal, amishin, thierry.reding,
jonathanh, Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas,
krzk+dt, conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
Hi Krzysztof
On 7/16/2024 11:47 PM, Krzysztof Kozlowski wrote:
> On 17/07/2024 00:09, Mayank Rana wrote:
>> Hi Krzysztof
>>
>> On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
>>> On 15/07/2024 20:13, Mayank Rana wrote:
>>>> To support MSI functionality using Synopsys DesignWare PCIe controller
>>>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>>>> compatible binding which uses provided SPIs to support MSI functionality.
>>>
>>> To support MSI, you add MSI support... That's a tautology. Describe
>>> hardware instead.
>> Ok. let me repharse it to provide more useful information.
>>>>
>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>> ---
>>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>>>> 1 file changed, 57 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>> index 9c714fa..9e860d5 100644
>>>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>> @@ -81,6 +81,12 @@ properties:
>>>> - marvell,armada8k-pcie-ecam
>>>> - socionext,synquacer-pcie-ecam
>>>> - const: snps,dw-pcie-ecam
>>>> + - description: |
>>>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>>>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>>>> + controller for MSI functionality, this compatible is used.
>>>> + items:
>>>> + - const: snps,dw-pcie-ecam-msi
>>>
>>> MSI is already present in the binding, isn't it?
>> It is mentioning as msi-parent usage which could be different MSI
>> controller (GIC or vendor specific one) not related to designware PCIe
>> controller based MSI controller.
>>
>>> Anyway, aren't you
>>> forgetting specific compatible? Please open your internal (quite
>>> comprehensive) guideline on bindings and DTS.
>> Here I am trying to define Designware based PCIe ECAM controller
>> supporting MSIcontroller based device. Hence I am not mentioning vendor
>> specific compatible usage
>> and keeping generic compatible binding for such device.
>
> I know what you try, yet it feels simply wrong. Read your guideline.
> Are you sure you work on Designware core itself, not on one used in
> Qualcomm? I would expect people from Designware to design Designware
> cores and people from Qualcomm only to design licensed cores.
Ok. let me not make generic comment here. I refereed how it is done with
other
snps based IP usage for example USB, and would follow same.
>>>
>>>> - description:
>>>> CAM or ECAM compliant PCI host controllers without any quirks
>>>> enum:
>>>> @@ -116,6 +122,20 @@ properties:
>>>> A phandle to the node that controls power or/and system resource or interface to firmware
>>>> to enable ECAM compliant PCIe root complex.
>>>>
>>>> + interrupts:
>>>> + description:
>>>> + DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
>>>> + minItems: 1
>>>> + maxItems: 8
>>>> +
>>>> + interrupt-names:
>>>> + description:
>>>> + MSI interrupt names
>>>> + minItems: 1
>>>> + maxItems: 8
>>>> + items:
>>>> + pattern: '^msi[0-9]+$'
>>>
>>> Why the same devices have variable numbers?
>> Max supported MSI with designware PCIe controller is 8 Only, and it
>> depends if those all are
>> used or some of used on specific vendor based device. Hence I have kept
>> it here variable names. Although here it should be [0 - 7] instead of
>> [0 - 9].
>
> Wait, you just said there is no specific vendor device.
>
> Sorry, bring some sanity to this
I understood that you are having concern about generic vs platform
specific usage here in binding document. I would try avoid mixing those,
and will try to do just platform specific usage. Thanks for having
patience with me, and helping me to here.
Regards,
Mayank
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-17 17:20 ` Mayank Rana
@ 2024-07-18 6:05 ` Krzysztof Kozlowski
2024-07-18 23:19 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-18 6:05 UTC (permalink / raw)
To: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
On 17/07/2024 19:20, Mayank Rana wrote:
> Hi Krzysztof
>
> On 7/16/2024 11:47 PM, Krzysztof Kozlowski wrote:
>> On 17/07/2024 00:09, Mayank Rana wrote:
>>> Hi Krzysztof
>>>
>>> On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
>>>> On 15/07/2024 20:13, Mayank Rana wrote:
>>>>> To support MSI functionality using Synopsys DesignWare PCIe controller
>>>>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>>>>> compatible binding which uses provided SPIs to support MSI functionality.
>>>>
>>>> To support MSI, you add MSI support... That's a tautology. Describe
>>>> hardware instead.
>>> Ok. let me repharse it to provide more useful information.
>>>>>
>>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>>> ---
>>>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>>>>> 1 file changed, 57 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>> index 9c714fa..9e860d5 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>> @@ -81,6 +81,12 @@ properties:
>>>>> - marvell,armada8k-pcie-ecam
>>>>> - socionext,synquacer-pcie-ecam
>>>>> - const: snps,dw-pcie-ecam
>>>>> + - description: |
>>>>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>>>>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>>>>> + controller for MSI functionality, this compatible is used.
>>>>> + items:
>>>>> + - const: snps,dw-pcie-ecam-msi
>>>>
>>>> MSI is already present in the binding, isn't it?
>>> It is mentioning as msi-parent usage which could be different MSI
>>> controller (GIC or vendor specific one) not related to designware PCIe
>>> controller based MSI controller.
>>>
>>>> Anyway, aren't you
>>>> forgetting specific compatible? Please open your internal (quite
>>>> comprehensive) guideline on bindings and DTS.
>>> Here I am trying to define Designware based PCIe ECAM controller
>>> supporting MSIcontroller based device. Hence I am not mentioning vendor
>>> specific compatible usage
>>> and keeping generic compatible binding for such device.
>>
>> I know what you try, yet it feels simply wrong. Read your guideline.
>> Are you sure you work on Designware core itself, not on one used in
>> Qualcomm? I would expect people from Designware to design Designware
>> cores and people from Qualcomm only to design licensed cores.
> Ok. let me not make generic comment here. I refereed how it is done with
> other
> snps based IP usage for example USB, and would follow same.
Well, it is not. If you read their bindings or any reviews related to
such cores, you would see that single Designware compatible is always
never appropriate. Such cores always have customization per user.
You can also look at the binding you are changing. Do you see Designware
alone? No.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-18 6:05 ` Krzysztof Kozlowski
@ 2024-07-18 23:19 ` Mayank Rana
2024-07-20 18:30 ` Krzysztof Kozlowski
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-18 23:19 UTC (permalink / raw)
To: Krzysztof Kozlowski, will, lpieralisi, kw, robh, bhelgaas,
jingoohan1, manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh,
s-vadapalli, u.kleine-koenig, dlemoal, amishin, thierry.reding,
jonathanh, Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas,
krzk+dt, conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
Hi Krzysztof
On 7/17/2024 11:05 PM, Krzysztof Kozlowski wrote:
> On 17/07/2024 19:20, Mayank Rana wrote:
>> Hi Krzysztof
>>
>> On 7/16/2024 11:47 PM, Krzysztof Kozlowski wrote:
>>> On 17/07/2024 00:09, Mayank Rana wrote:
>>>> Hi Krzysztof
>>>>
>>>> On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
>>>>> On 15/07/2024 20:13, Mayank Rana wrote:
>>>>>> To support MSI functionality using Synopsys DesignWare PCIe controller
>>>>>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>>>>>> compatible binding which uses provided SPIs to support MSI functionality.
>>>>>
>>>>> To support MSI, you add MSI support... That's a tautology. Describe
>>>>> hardware instead.
>>>> Ok. let me repharse it to provide more useful information.
>>>>>>
>>>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>>>> ---
>>>>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>>>>>> 1 file changed, 57 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>> index 9c714fa..9e860d5 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>> @@ -81,6 +81,12 @@ properties:
>>>>>> - marvell,armada8k-pcie-ecam
>>>>>> - socionext,synquacer-pcie-ecam
>>>>>> - const: snps,dw-pcie-ecam
>>>>>> + - description: |
>>>>>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>>>>>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>>>>>> + controller for MSI functionality, this compatible is used.
>>>>>> + items:
>>>>>> + - const: snps,dw-pcie-ecam-msi
>>>>>
>>>>> MSI is already present in the binding, isn't it?
>>>> It is mentioning as msi-parent usage which could be different MSI
>>>> controller (GIC or vendor specific one) not related to designware PCIe
>>>> controller based MSI controller.
>>>>
>>>>> Anyway, aren't you
>>>>> forgetting specific compatible? Please open your internal (quite
>>>>> comprehensive) guideline on bindings and DTS.
>>>> Here I am trying to define Designware based PCIe ECAM controller
>>>> supporting MSIcontroller based device. Hence I am not mentioning vendor
>>>> specific compatible usage
>>>> and keeping generic compatible binding for such device.
>>>
>>> I know what you try, yet it feels simply wrong. Read your guideline.
>>> Are you sure you work on Designware core itself, not on one used in
>>> Qualcomm? I would expect people from Designware to design Designware
>>> cores and people from Qualcomm only to design licensed cores.
>> Ok. let me not make generic comment here. I refereed how it is done with
>> other
>> snps based IP usage for example USB, and would follow same.
>
> Well, it is not. If you read their bindings or any reviews related to
> such cores, you would see that single Designware compatible is always
> never appropriate. Such cores always have customization per user.
>
> You can also look at the binding you are changing. Do you see Designware
> alone? No.
I found reference in this binding as below:
79 items:
80 - enum:
81 - marvell,armada8k-pcie-ecam
82 - socionext,synquacer-pcie-ecam
83 - const: snps,dw-pcie-ecam
And as you mentioned in previous emails about how to add such usage, I
ACKed it but let me put reason why I tried to add differently to start
with it.
This specific driver under-discussion is really not vendor specific
driver. It can work with any PCIe controller which is already configured
in ECAM mode by firmware (i.e. PCIe controller from SNPS or any other
vendor). There are few quirks only added to get specific vendor based
SOC configuration for SNPS PCIe controller by different SOC vendors.
90 - description:
91 CAM or ECAM compliant PCI host controllers without any quirks
92 enum:
93 - pci-host-cam-generic
94 - pci-host-ecam-generic
Above enum based usage works for SA8775P platform which is having SNPS
PCIe controller which doesn't need any quirks, and firmware is able to
configure PCIe controller into ECAM mode. Here I tried adding PCIe SNPS
controller based MSI support as SA8775P doesn't support LPI/ITS for MSI.
I need to differentiate it hence added generic enum as MSI controller is
part of SNPS PCIe controller, and not separate MSI IP here. Although how
many MSI can be supported it depends on how many SPIs are available/used
with MSI controller depend on particular SOC. Hence put as
snps,dw-pcie-ecam-msi as usage and variable number of MSI. hopefully I
am able to convey why this driver binding modified differently.
Regards,
Mayank
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-18 23:19 ` Mayank Rana
@ 2024-07-20 18:30 ` Krzysztof Kozlowski
0 siblings, 0 replies; 37+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-20 18:30 UTC (permalink / raw)
To: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
On 19/07/2024 01:19, Mayank Rana wrote:
> Hi Krzysztof
>
> On 7/17/2024 11:05 PM, Krzysztof Kozlowski wrote:
>> On 17/07/2024 19:20, Mayank Rana wrote:
>>> Hi Krzysztof
>>>
>>> On 7/16/2024 11:47 PM, Krzysztof Kozlowski wrote:
>>>> On 17/07/2024 00:09, Mayank Rana wrote:
>>>>> Hi Krzysztof
>>>>>
>>>>> On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
>>>>>> On 15/07/2024 20:13, Mayank Rana wrote:
>>>>>>> To support MSI functionality using Synopsys DesignWare PCIe controller
>>>>>>> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
>>>>>>> compatible binding which uses provided SPIs to support MSI functionality.
>>>>>>
>>>>>> To support MSI, you add MSI support... That's a tautology. Describe
>>>>>> hardware instead.
>>>>> Ok. let me repharse it to provide more useful information.
>>>>>>>
>>>>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
>>>>>>> 1 file changed, 57 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>>> index 9c714fa..9e860d5 100644
>>>>>>> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>>>>>>> @@ -81,6 +81,12 @@ properties:
>>>>>>> - marvell,armada8k-pcie-ecam
>>>>>>> - socionext,synquacer-pcie-ecam
>>>>>>> - const: snps,dw-pcie-ecam
>>>>>>> + - description: |
>>>>>>> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
>>>>>>> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
>>>>>>> + controller for MSI functionality, this compatible is used.
>>>>>>> + items:
>>>>>>> + - const: snps,dw-pcie-ecam-msi
>>>>>>
>>>>>> MSI is already present in the binding, isn't it?
>>>>> It is mentioning as msi-parent usage which could be different MSI
>>>>> controller (GIC or vendor specific one) not related to designware PCIe
>>>>> controller based MSI controller.
>>>>>
>>>>>> Anyway, aren't you
>>>>>> forgetting specific compatible? Please open your internal (quite
>>>>>> comprehensive) guideline on bindings and DTS.
>>>>> Here I am trying to define Designware based PCIe ECAM controller
>>>>> supporting MSIcontroller based device. Hence I am not mentioning vendor
>>>>> specific compatible usage
>>>>> and keeping generic compatible binding for such device.
>>>>
>>>> I know what you try, yet it feels simply wrong. Read your guideline.
>>>> Are you sure you work on Designware core itself, not on one used in
>>>> Qualcomm? I would expect people from Designware to design Designware
>>>> cores and people from Qualcomm only to design licensed cores.
>>> Ok. let me not make generic comment here. I refereed how it is done with
>>> other
>>> snps based IP usage for example USB, and would follow same.
>>
>> Well, it is not. If you read their bindings or any reviews related to
>> such cores, you would see that single Designware compatible is always
>> never appropriate. Such cores always have customization per user.
>>
>> You can also look at the binding you are changing. Do you see Designware
>> alone? No.
> I found reference in this binding as below:
> 79 items:
> 80 - enum:
> 81 - marvell,armada8k-pcie-ecam
> 82 - socionext,synquacer-pcie-ecam
> 83 - const: snps,dw-pcie-ecam
>
> And as you mentioned in previous emails about how to add such usage, I
> ACKed it but let me put reason why I tried to add differently to start
> with it.
>
> This specific driver under-discussion is really not vendor specific
> driver. It can work with any PCIe controller which is already configured
> in ECAM mode by firmware (i.e. PCIe controller from SNPS or any other
> vendor). There are few quirks only added to get specific vendor based
> SOC configuration for SNPS PCIe controller by different SOC vendors.
>
> 90 - description:
> 91 CAM or ECAM compliant PCI host controllers without any quirks
> 92 enum:
> 93 - pci-host-cam-generic
> 94 - pci-host-ecam-generic
>
> Above enum based usage works for SA8775P platform which is having SNPS
> PCIe controller which doesn't need any quirks, and firmware is able to
> configure PCIe controller into ECAM mode. Here I tried adding PCIe SNPS
> controller based MSI support as SA8775P doesn't support LPI/ITS for MSI.
> I need to differentiate it hence added generic enum as MSI controller is
> part of SNPS PCIe controller, and not separate MSI IP here. Although how
> many MSI can be supported it depends on how many SPIs are available/used
> with MSI controller depend on particular SOC. Hence put as
> snps,dw-pcie-ecam-msi as usage and variable number of MSI. hopefully I
> am able to convey why this driver binding modified differently.
Your binding already defines some specifics for specific device, but you
still claim all of them are identical.
Sorry, I am confused. Read carefully writing bindings, consult internal
guideline (go/upstream - it is really detailed!) and then come with a
solution.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
2024-07-15 18:13 ` [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding Mayank Rana
2024-07-15 19:43 ` Rob Herring (Arm)
2024-07-16 7:28 ` Krzysztof Kozlowski
@ 2024-07-31 17:26 ` Manivannan Sadhasivam
2 siblings, 0 replies; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-31 17:26 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:34AM -0700, Mayank Rana wrote:
> To support MSI functionality using Synopsys DesignWare PCIe controller
> based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
> compatible binding which uses provided SPIs to support MSI functionality.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../devicetree/bindings/pci/host-generic-pci.yaml | 57 ++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> index 9c714fa..9e860d5 100644
> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> @@ -81,6 +81,12 @@ properties:
> - marvell,armada8k-pcie-ecam
> - socionext,synquacer-pcie-ecam
> - const: snps,dw-pcie-ecam
> + - description: |
> + Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
> + ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
> + controller for MSI functionality, this compatible is used.
> + items:
> + - const: snps,dw-pcie-ecam-msi
There is no MSI ECAM. You can have Qcom specific ECAM implementation. Even
generalising this as DWC ECAM is wrong, since it won't work on DWC based
systems (especially with SCMI power domain).
- Mani
> - description:
> CAM or ECAM compliant PCI host controllers without any quirks
> enum:
> @@ -116,6 +122,20 @@ properties:
> A phandle to the node that controls power or/and system resource or interface to firmware
> to enable ECAM compliant PCIe root complex.
>
> + interrupts:
> + description:
> + DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
> + minItems: 1
> + maxItems: 8
> +
> + interrupt-names:
> + description:
> + MSI interrupt names
> + minItems: 1
> + maxItems: 8
> + items:
> + pattern: '^msi[0-9]+$'
> +
> required:
> - compatible
> - reg
> @@ -146,11 +166,22 @@ allOf:
> reg:
> maxItems: 1
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: snps,dw-pcie-ecam-msi
> + then:
> + required:
> + - interrupts
> + - interrupt-names
> +
> unevaluatedProperties: false
>
> examples:
> - |
>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> bus {
> #address-cells = <2>;
> #size-cells = <2>;
> @@ -180,5 +211,31 @@ examples:
> interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
> power-domains = <&scmi5_pd 0>;
> };
> +
> + pcie0: pci@1c00000 {
> + compatible = "snps,dw-pcie-ecam-msi";
> + reg = <0x4 0x00000000 0 0x10000000>;
> + device_type = "pci";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x02000000 0x0 0x40100000 0x0 0x40100000 0x0 0x1ff00000>,
> + <0x43000000 0x4 0x10100000 0x4 0x10100000 0x0 0x40000000>;
> + bus-range = <0x00 0xff>;
> + dma-coherent;
> + linux,pci-domain = <0>;
> + power-domains = <&scmi5_pd 0>;
> + iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
> + <0x100 &pcie_smmu 0x0001 0x1>;
> +
> + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi0", "msi1", "msi2", "msi3", "msi4", "msi5", "msi6", "msi7";
> + };
> };
> ...
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (5 preceding siblings ...)
2024-07-15 18:13 ` [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding Mayank Rana
@ 2024-07-15 18:13 ` Mayank Rana
2024-07-16 8:58 ` Will Deacon
2024-07-24 2:13 ` [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Dmitry Baryshkov
2024-07-29 17:19 ` Mayank Rana
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-15 18:13 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt, Mayank Rana
Add usage of Synopsys Designware PCIe controller based MSI controller to
support MSI functionality with ECAM compliant Synopsys Designware PCIe
controller. To use this functionality add device compatible string as
"snps,dw-pcie-ecam-msi".
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/pci-host-generic.c | 92 ++++++++++++++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
index c2c027f..457ae44 100644
--- a/drivers/pci/controller/pci-host-generic.c
+++ b/drivers/pci/controller/pci-host-generic.c
@@ -8,13 +8,73 @@
* Author: Will Deacon <will.deacon@arm.com>
*/
-#include <linux/kernel.h>
#include <linux/init.h>
+#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of_address.h>
#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include "dwc/pcie-designware-msi.h"
+
+struct dw_ecam_pcie {
+ void __iomem *cfg;
+ struct dw_msi *msi;
+ struct pci_host_bridge *bridge;
+};
+
+static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
+{
+ struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
+
+ return readl(ecam_pcie->cfg + reg);
+}
+
+static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
+{
+ struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
+
+ writel(val, ecam_pcie->cfg + reg);
+}
+
+static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dw_ecam_pcie *ecam_pcie;
+ struct dw_msi_ops *msi_ops;
+ u64 addr;
+
+ ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
+ if (!ecam_pcie)
+ return ERR_PTR(-ENOMEM);
+
+ if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
+ dev_err(dev, "Failed to get reg address\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
+ if (ecam_pcie->cfg == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
+ if (!msi_ops)
+ return ERR_PTR(-ENOMEM);
+
+ msi_ops->readl_msi = dw_ecam_pcie_readl;
+ msi_ops->writel_msi = dw_ecam_pcie_writel;
+ msi_ops->pp = ecam_pcie;
+ ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
+ if (IS_ERR(ecam_pcie->msi)) {
+ dev_err(dev, "dw_pcie_msi_host_init() failed\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ dw_pcie_msi_init(ecam_pcie->msi);
+ return ecam_pcie;
+}
+
static const struct pci_ecam_ops gen_pci_cfg_cam_bus_ops = {
.bus_shift = 16,
.pci_ops = {
@@ -73,6 +133,9 @@ static const struct of_device_id gen_pci_of_match[] = {
{ .compatible = "snps,dw-pcie-ecam",
.data = &pci_dw_ecam_bus_ops },
+ { .compatible = "snps,dw-pcie-ecam-msi",
+ .data = &pci_generic_ecam_ops },
+
{ },
};
MODULE_DEVICE_TABLE(of, gen_pci_of_match);
@@ -80,6 +143,7 @@ MODULE_DEVICE_TABLE(of, gen_pci_of_match);
static int gen_pcie_ecam_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct dw_ecam_pcie *ecam_pcie = NULL;
int ret = 0;
if (!IS_ERR_OR_NULL(dev->pm_domain)) {
@@ -94,14 +158,30 @@ static int gen_pcie_ecam_probe(struct platform_device *pdev)
}
}
+ if (of_device_is_compatible(dev->of_node, "snps,dw-pcie-ecam-msi")) {
+ ecam_pcie = dw_pcie_ecam_msi(pdev);
+ if (IS_ERR(ecam_pcie)) {
+ ret = -ENODEV;
+ goto err;
+ }
+ }
+
ret = pci_host_common_probe(pdev);
if (ret) {
dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
goto err;
}
+ if (ecam_pcie) {
+ ecam_pcie->bridge = platform_get_drvdata(pdev);
+ platform_set_drvdata(pdev, ecam_pcie);
+ }
+
return ret;
err:
+ if (!IS_ERR_OR_NULL(ecam_pcie))
+ dw_pcie_free_msi(ecam_pcie->msi);
+
if (!IS_ERR_OR_NULL(dev->pm_domain))
pm_runtime_put_sync(dev);
return ret;
@@ -109,7 +189,17 @@ static int gen_pcie_ecam_probe(struct platform_device *pdev)
static void gen_pcie_ecam_remove(struct platform_device *pdev)
{
+ struct dw_ecam_pcie *ecam_pcie = NULL;
+
+ if (of_device_is_compatible(pdev->dev.of_node, "snps,dw-pcie-ecam-msi")) {
+ ecam_pcie = platform_get_drvdata(pdev);
+ platform_set_drvdata(pdev, ecam_pcie->bridge);
+ }
+
pci_host_common_remove(pdev);
+ if (ecam_pcie)
+ dw_pcie_free_msi(ecam_pcie->msi);
+
if (pdev->dev.pm_domain)
pm_runtime_put_sync(&pdev->dev);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-15 18:13 ` [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage Mayank Rana
@ 2024-07-16 8:58 ` Will Deacon
2024-07-16 13:42 ` Rob Herring
0 siblings, 1 reply; 37+ messages in thread
From: Will Deacon @ 2024-07-16 8:58 UTC (permalink / raw)
To: Mayank Rana
Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, manivannan.sadhasivam,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:35AM -0700, Mayank Rana wrote:
> Add usage of Synopsys Designware PCIe controller based MSI controller to
> support MSI functionality with ECAM compliant Synopsys Designware PCIe
> controller. To use this functionality add device compatible string as
> "snps,dw-pcie-ecam-msi".
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/pci-host-generic.c | 92 ++++++++++++++++++++++++++++++-
> 1 file changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
> index c2c027f..457ae44 100644
> --- a/drivers/pci/controller/pci-host-generic.c
> +++ b/drivers/pci/controller/pci-host-generic.c
> @@ -8,13 +8,73 @@
> * Author: Will Deacon <will.deacon@arm.com>
> */
>
> -#include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of_address.h>
> #include <linux/pci-ecam.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
>
> +#include "dwc/pcie-designware-msi.h"
> +
> +struct dw_ecam_pcie {
> + void __iomem *cfg;
> + struct dw_msi *msi;
> + struct pci_host_bridge *bridge;
> +};
> +
> +static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
> +{
> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> +
> + return readl(ecam_pcie->cfg + reg);
> +}
> +
> +static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
> +{
> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> +
> + writel(val, ecam_pcie->cfg + reg);
> +}
> +
> +static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_ecam_pcie *ecam_pcie;
> + struct dw_msi_ops *msi_ops;
> + u64 addr;
> +
> + ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
> + if (!ecam_pcie)
> + return ERR_PTR(-ENOMEM);
> +
> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
> + dev_err(dev, "Failed to get reg address\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
> + if (ecam_pcie->cfg == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
> + if (!msi_ops)
> + return ERR_PTR(-ENOMEM);
> +
> + msi_ops->readl_msi = dw_ecam_pcie_readl;
> + msi_ops->writel_msi = dw_ecam_pcie_writel;
> + msi_ops->pp = ecam_pcie;
> + ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
> + if (IS_ERR(ecam_pcie->msi)) {
> + dev_err(dev, "dw_pcie_msi_host_init() failed\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + dw_pcie_msi_init(ecam_pcie->msi);
> + return ecam_pcie;
> +}
Hmm. This looks like quite a lot of not-very-generic code to be adding
to pci-host-generic.c. The file is now, what, 50% designware logic?
Will
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-16 8:58 ` Will Deacon
@ 2024-07-16 13:42 ` Rob Herring
2024-07-16 22:32 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Rob Herring @ 2024-07-16 13:42 UTC (permalink / raw)
To: Will Deacon
Cc: Mayank Rana, lpieralisi, kw, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
On Tue, Jul 16, 2024 at 09:58:12AM +0100, Will Deacon wrote:
> On Mon, Jul 15, 2024 at 11:13:35AM -0700, Mayank Rana wrote:
> > Add usage of Synopsys Designware PCIe controller based MSI controller to
> > support MSI functionality with ECAM compliant Synopsys Designware PCIe
> > controller. To use this functionality add device compatible string as
> > "snps,dw-pcie-ecam-msi".
> >
> > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> > ---
> > drivers/pci/controller/pci-host-generic.c | 92 ++++++++++++++++++++++++++++++-
> > 1 file changed, 91 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
> > index c2c027f..457ae44 100644
> > --- a/drivers/pci/controller/pci-host-generic.c
> > +++ b/drivers/pci/controller/pci-host-generic.c
> > @@ -8,13 +8,73 @@
> > * Author: Will Deacon <will.deacon@arm.com>
> > */
> >
> > -#include <linux/kernel.h>
> > #include <linux/init.h>
> > +#include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/of_address.h>
> > #include <linux/pci-ecam.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> >
> > +#include "dwc/pcie-designware-msi.h"
> > +
> > +struct dw_ecam_pcie {
> > + void __iomem *cfg;
> > + struct dw_msi *msi;
> > + struct pci_host_bridge *bridge;
> > +};
> > +
> > +static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
> > +{
> > + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> > +
> > + return readl(ecam_pcie->cfg + reg);
> > +}
> > +
> > +static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
> > +{
> > + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> > +
> > + writel(val, ecam_pcie->cfg + reg);
> > +}
> > +
> > +static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct dw_ecam_pcie *ecam_pcie;
> > + struct dw_msi_ops *msi_ops;
> > + u64 addr;
> > +
> > + ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
> > + if (!ecam_pcie)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
Using this function on MMIO addresses is wrong. It is an untranslated
address.
> > + dev_err(dev, "Failed to get reg address\n");
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
> > + if (ecam_pcie->cfg == NULL)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
> > + if (!msi_ops)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + msi_ops->readl_msi = dw_ecam_pcie_readl;
> > + msi_ops->writel_msi = dw_ecam_pcie_writel;
> > + msi_ops->pp = ecam_pcie;
> > + ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
> > + if (IS_ERR(ecam_pcie->msi)) {
> > + dev_err(dev, "dw_pcie_msi_host_init() failed\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + dw_pcie_msi_init(ecam_pcie->msi);
> > + return ecam_pcie;
> > +}
>
> Hmm. This looks like quite a lot of not-very-generic code to be adding
> to pci-host-generic.c. The file is now, what, 50% designware logic?
Agreed.
I would suggest you add ECAM support to the DW/QCom driver reusing some
of the common ECAM support code.
I suppose another option would be to define a node and driver which is
just the DW MSI controller. That might not work given the power domain
being added (which is not very generic either).
Rob
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-16 13:42 ` Rob Herring
@ 2024-07-16 22:32 ` Mayank Rana
2024-07-23 22:56 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-16 22:32 UTC (permalink / raw)
To: Rob Herring, Will Deacon
Cc: lpieralisi, kw, bhelgaas, jingoohan1, manivannan.sadhasivam,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
Hi Will and Rob
Thank you for your quick review comments.
On 7/16/2024 6:42 AM, Rob Herring wrote:
> On Tue, Jul 16, 2024 at 09:58:12AM +0100, Will Deacon wrote:
>> On Mon, Jul 15, 2024 at 11:13:35AM -0700, Mayank Rana wrote:
>>> Add usage of Synopsys Designware PCIe controller based MSI controller to
>>> support MSI functionality with ECAM compliant Synopsys Designware PCIe
>>> controller. To use this functionality add device compatible string as
>>> "snps,dw-pcie-ecam-msi".
>>>
>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>> ---
>>> drivers/pci/controller/pci-host-generic.c | 92 ++++++++++++++++++++++++++++++-
>>> 1 file changed, 91 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
>>> index c2c027f..457ae44 100644
>>> --- a/drivers/pci/controller/pci-host-generic.c
>>> +++ b/drivers/pci/controller/pci-host-generic.c
>>> @@ -8,13 +8,73 @@
>>> * Author: Will Deacon <will.deacon@arm.com>
>>> */
>>>
>>> -#include <linux/kernel.h>
>>> #include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> #include <linux/module.h>
>>> +#include <linux/of_address.h>
>>> #include <linux/pci-ecam.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/pm_runtime.h>
>>>
>>> +#include "dwc/pcie-designware-msi.h"
>>> +
>>> +struct dw_ecam_pcie {
>>> + void __iomem *cfg;
>>> + struct dw_msi *msi;
>>> + struct pci_host_bridge *bridge;
>>> +};
>>> +
>>> +static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
>>> +{
>>> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
>>> +
>>> + return readl(ecam_pcie->cfg + reg);
>>> +}
>>> +
>>> +static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
>>> +{
>>> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
>>> +
>>> + writel(val, ecam_pcie->cfg + reg);
>>> +}
>>> +
>>> +static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct dw_ecam_pcie *ecam_pcie;
>>> + struct dw_msi_ops *msi_ops;
>>> + u64 addr;
>>> +
>>> + ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
>>> + if (!ecam_pcie)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
>
> Using this function on MMIO addresses is wrong. It is an untranslated
> address.
ok. do you prefer me to use of_address_to_resource() instead here ?
>>> + dev_err(dev, "Failed to get reg address\n");
>>> + return ERR_PTR(-ENODEV);
>>> + }
>>> +
>>> + ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
>>> + if (ecam_pcie->cfg == NULL)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
>>> + if (!msi_ops)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + msi_ops->readl_msi = dw_ecam_pcie_readl;
>>> + msi_ops->writel_msi = dw_ecam_pcie_writel;
>>> + msi_ops->pp = ecam_pcie;
>>> + ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
>>> + if (IS_ERR(ecam_pcie->msi)) {
>>> + dev_err(dev, "dw_pcie_msi_host_init() failed\n");
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>> +
>>> + dw_pcie_msi_init(ecam_pcie->msi);
>>> + return ecam_pcie;
>>> +}
>>
>> Hmm. This looks like quite a lot of not-very-generic code to be adding
>> to pci-host-generic.c. The file is now, what, 50% designware logic?
>
> Agreed.
>
> I would suggest you add ECAM support to the DW/QCom driver reusing some
> of the common ECAM support code.
I can try although there is very limited reusage of code with
pcie-qcom.c and pcie-designware-host.c except reusing MSI functionality.
That would make more new OPs within pcie-designware-host.c and
pcie-qcom.c just to perform few operation. As now MSI functionality is
available outside pcie core designware driver (although those changes
are under review), will you be ok to allow separate Qualcomm PCIe ECAM
driver as previously submitted RFC as
https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
I can modify above ECAM driver to call into PCIe designware module based
MSI ops as doing here and that would allow reusing of MSI functionality
at same time allowing separate driver for handling firmware VM based
implementation.
>
> I suppose another option would be to define a node and driver which is
> just the DW MSI controller. That might not work given the power domain
> being added (which is not very generic either).
yes, I did consider this approach, and haven't used this due to concern
as you mentioned, and also that ask for modifying devicetree usage for
existing user of PCIe Designware controller based MSI controller.
Regards,
Mayank
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-16 22:32 ` Mayank Rana
@ 2024-07-23 22:56 ` Mayank Rana
2024-07-24 9:54 ` Manivannan Sadhasivam
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-23 22:56 UTC (permalink / raw)
To: Rob Herring, Will Deacon
Cc: lpieralisi, kw, bhelgaas, jingoohan1, manivannan.sadhasivam,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt, linux-arm-msm
Hi Rob
On 7/16/2024 3:32 PM, Mayank Rana wrote:
> Hi Will and Rob
>
> Thank you for your quick review comments.
>
> On 7/16/2024 6:42 AM, Rob Herring wrote:
>> On Tue, Jul 16, 2024 at 09:58:12AM +0100, Will Deacon wrote:
>>> On Mon, Jul 15, 2024 at 11:13:35AM -0700, Mayank Rana wrote:
>>>> Add usage of Synopsys Designware PCIe controller based MSI
>>>> controller to
>>>> support MSI functionality with ECAM compliant Synopsys Designware PCIe
>>>> controller. To use this functionality add device compatible string as
>>>> "snps,dw-pcie-ecam-msi".
>>>>
>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>> ---
>>>> drivers/pci/controller/pci-host-generic.c | 92
>>>> ++++++++++++++++++++++++++++++-
>>>> 1 file changed, 91 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/controller/pci-host-generic.c
>>>> b/drivers/pci/controller/pci-host-generic.c
>>>> index c2c027f..457ae44 100644
>>>> --- a/drivers/pci/controller/pci-host-generic.c
>>>> +++ b/drivers/pci/controller/pci-host-generic.c
>>>> @@ -8,13 +8,73 @@
>>>> * Author: Will Deacon <will.deacon@arm.com>
>>>> */
>>>> -#include <linux/kernel.h>
>>>> #include <linux/init.h>
>>>> +#include <linux/kernel.h>
>>>> #include <linux/module.h>
>>>> +#include <linux/of_address.h>
>>>> #include <linux/pci-ecam.h>
>>>> #include <linux/platform_device.h>
>>>> #include <linux/pm_runtime.h>
>>>> +#include "dwc/pcie-designware-msi.h"
>>>> +
>>>> +struct dw_ecam_pcie {
>>>> + void __iomem *cfg;
>>>> + struct dw_msi *msi;
>>>> + struct pci_host_bridge *bridge;
>>>> +};
>>>> +
>>>> +static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
>>>> +{
>>>> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
>>>> +
>>>> + return readl(ecam_pcie->cfg + reg);
>>>> +}
>>>> +
>>>> +static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
>>>> +{
>>>> + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
>>>> +
>>>> + writel(val, ecam_pcie->cfg + reg);
>>>> +}
>>>> +
>>>> +static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct platform_device
>>>> *pdev)
>>>> +{
>>>> + struct device *dev = &pdev->dev;
>>>> + struct dw_ecam_pcie *ecam_pcie;
>>>> + struct dw_msi_ops *msi_ops;
>>>> + u64 addr;
>>>> +
>>>> + ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
>>>> + if (!ecam_pcie)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
>>
>> Using this function on MMIO addresses is wrong. It is an untranslated
>> address.
> ok. do you prefer me to use of_address_to_resource() instead here ?
>
>>>> + dev_err(dev, "Failed to get reg address\n");
>>>> + return ERR_PTR(-ENODEV);
>>>> + }
>>>> +
>>>> + ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
>>>> + if (ecam_pcie->cfg == NULL)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
>>>> + if (!msi_ops)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + msi_ops->readl_msi = dw_ecam_pcie_readl;
>>>> + msi_ops->writel_msi = dw_ecam_pcie_writel;
>>>> + msi_ops->pp = ecam_pcie;
>>>> + ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
>>>> + if (IS_ERR(ecam_pcie->msi)) {
>>>> + dev_err(dev, "dw_pcie_msi_host_init() failed\n");
>>>> + return ERR_PTR(-EINVAL);
>>>> + }
>>>> +
>>>> + dw_pcie_msi_init(ecam_pcie->msi);
>>>> + return ecam_pcie;
>>>> +}
>>>
>>> Hmm. This looks like quite a lot of not-very-generic code to be adding
>>> to pci-host-generic.c. The file is now, what, 50% designware logic?
>>
>> Agreed.
>>
>> I would suggest you add ECAM support to the DW/QCom driver reusing some
>> of the common ECAM support code.
> I can try although there is very limited reusage of code with
> pcie-qcom.c and pcie-designware-host.c except reusing MSI functionality.
> That would make more new OPs within pcie-designware-host.c and
> pcie-qcom.c just to perform few operation. As now MSI functionality is
> available outside pcie core designware driver (although those changes
> are under review), will you be ok to allow separate Qualcomm PCIe ECAM
> driver as previously submitted RFC as
> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>
> I can modify above ECAM driver to call into PCIe designware module based
> MSI ops as doing here and that would allow reusing of MSI functionality
> at same time allowing separate driver for handling firmware VM based
> implementation.
Can you consider above request to have separate driver here ?
Please suggest on this.
>>
>> I suppose another option would be to define a node and driver which is
>> just the DW MSI controller. That might not work given the power domain
>> being added (which is not very generic either).
> yes, I did consider this approach, and haven't used this due to concern
> as you mentioned, and also that ask for modifying devicetree usage for
> existing user of PCIe Designware controller based MSI controller.
>
> Regards,
> Mayank
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage
2024-07-23 22:56 ` Mayank Rana
@ 2024-07-24 9:54 ` Manivannan Sadhasivam
0 siblings, 0 replies; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-24 9:54 UTC (permalink / raw)
To: Mayank Rana
Cc: Rob Herring, Will Deacon, lpieralisi, kw, bhelgaas, jingoohan1,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt, linux-arm-msm
On Tue, Jul 23, 2024 at 03:56:35PM -0700, Mayank Rana wrote:
> Hi Rob
>
> On 7/16/2024 3:32 PM, Mayank Rana wrote:
> > Hi Will and Rob
> >
> > Thank you for your quick review comments.
> >
> > On 7/16/2024 6:42 AM, Rob Herring wrote:
> > > On Tue, Jul 16, 2024 at 09:58:12AM +0100, Will Deacon wrote:
> > > > On Mon, Jul 15, 2024 at 11:13:35AM -0700, Mayank Rana wrote:
> > > > > Add usage of Synopsys Designware PCIe controller based MSI
> > > > > controller to
> > > > > support MSI functionality with ECAM compliant Synopsys Designware PCIe
> > > > > controller. To use this functionality add device compatible string as
> > > > > "snps,dw-pcie-ecam-msi".
> > > > >
> > > > > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> > > > > ---
> > > > > drivers/pci/controller/pci-host-generic.c | 92
> > > > > ++++++++++++++++++++++++++++++-
> > > > > 1 file changed, 91 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/pci-host-generic.c
> > > > > b/drivers/pci/controller/pci-host-generic.c
> > > > > index c2c027f..457ae44 100644
> > > > > --- a/drivers/pci/controller/pci-host-generic.c
> > > > > +++ b/drivers/pci/controller/pci-host-generic.c
> > > > > @@ -8,13 +8,73 @@
> > > > > * Author: Will Deacon <will.deacon@arm.com>
> > > > > */
> > > > > -#include <linux/kernel.h>
> > > > > #include <linux/init.h>
> > > > > +#include <linux/kernel.h>
> > > > > #include <linux/module.h>
> > > > > +#include <linux/of_address.h>
> > > > > #include <linux/pci-ecam.h>
> > > > > #include <linux/platform_device.h>
> > > > > #include <linux/pm_runtime.h>
> > > > > +#include "dwc/pcie-designware-msi.h"
> > > > > +
> > > > > +struct dw_ecam_pcie {
> > > > > + void __iomem *cfg;
> > > > > + struct dw_msi *msi;
> > > > > + struct pci_host_bridge *bridge;
> > > > > +};
> > > > > +
> > > > > +static u32 dw_ecam_pcie_readl(void *p_data, u32 reg)
> > > > > +{
> > > > > + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> > > > > +
> > > > > + return readl(ecam_pcie->cfg + reg);
> > > > > +}
> > > > > +
> > > > > +static void dw_ecam_pcie_writel(void *p_data, u32 reg, u32 val)
> > > > > +{
> > > > > + struct dw_ecam_pcie *ecam_pcie = (struct dw_ecam_pcie *)p_data;
> > > > > +
> > > > > + writel(val, ecam_pcie->cfg + reg);
> > > > > +}
> > > > > +
> > > > > +static struct dw_ecam_pcie *dw_pcie_ecam_msi(struct
> > > > > platform_device *pdev)
> > > > > +{
> > > > > + struct device *dev = &pdev->dev;
> > > > > + struct dw_ecam_pcie *ecam_pcie;
> > > > > + struct dw_msi_ops *msi_ops;
> > > > > + u64 addr;
> > > > > +
> > > > > + ecam_pcie = devm_kzalloc(dev, sizeof(*ecam_pcie), GFP_KERNEL);
> > > > > + if (!ecam_pcie)
> > > > > + return ERR_PTR(-ENOMEM);
> > > > > +
> > > > > + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
> > >
> > > Using this function on MMIO addresses is wrong. It is an untranslated
> > > address.
> > ok. do you prefer me to use of_address_to_resource() instead here ?
> >
> > > > > + dev_err(dev, "Failed to get reg address\n");
> > > > > + return ERR_PTR(-ENODEV);
> > > > > + }
> > > > > +
> > > > > + ecam_pcie->cfg = devm_ioremap(dev, addr, PAGE_SIZE);
> > > > > + if (ecam_pcie->cfg == NULL)
> > > > > + return ERR_PTR(-ENOMEM);
> > > > > +
> > > > > + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
> > > > > + if (!msi_ops)
> > > > > + return ERR_PTR(-ENOMEM);
> > > > > +
> > > > > + msi_ops->readl_msi = dw_ecam_pcie_readl;
> > > > > + msi_ops->writel_msi = dw_ecam_pcie_writel;
> > > > > + msi_ops->pp = ecam_pcie;
> > > > > + ecam_pcie->msi = dw_pcie_msi_host_init(pdev, msi_ops, 0);
> > > > > + if (IS_ERR(ecam_pcie->msi)) {
> > > > > + dev_err(dev, "dw_pcie_msi_host_init() failed\n");
> > > > > + return ERR_PTR(-EINVAL);
> > > > > + }
> > > > > +
> > > > > + dw_pcie_msi_init(ecam_pcie->msi);
> > > > > + return ecam_pcie;
> > > > > +}
> > > >
> > > > Hmm. This looks like quite a lot of not-very-generic code to be adding
> > > > to pci-host-generic.c. The file is now, what, 50% designware logic?
> > >
> > > Agreed.
> > >
> > > I would suggest you add ECAM support to the DW/QCom driver reusing some
> > > of the common ECAM support code.
> > I can try although there is very limited reusage of code with
> > pcie-qcom.c and pcie-designware-host.c except reusing MSI functionality.
> > That would make more new OPs within pcie-designware-host.c and
> > pcie-qcom.c just to perform few operation. As now MSI functionality is
> > available outside pcie core designware driver (although those changes
> > are under review), will you be ok to allow separate Qualcomm PCIe ECAM
> > driver as previously submitted RFC as https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
> >
> > I can modify above ECAM driver to call into PCIe designware module based
> > MSI ops as doing here and that would allow reusing of MSI functionality
> > at same time allowing separate driver for handling firmware VM based
> > implementation.
> Can you consider above request to have separate driver here ?
> Please suggest on this.
>
Generic ECAM driver is already supporting some DWC based ECAM implementations
like the ones added in commit 58fb207fb100 ("PCI: generic: Add support for
Synopsys DesignWare RC in ECAM mode").
From that perspective, I think it makes sense to add Qcom ECAM driver as a part
of this. But at the same time, you can also add the ECAM mode to the existing
DWC Qcom driver and reuse existing codes like MSI. Considering the amount of
Qcom specific features you are going to add (like safety interrupts etc...), it
won't look like a generic ECAM driver anyway.
So I guess there is no *ideal* location for this driver. IMO as long as you
avoid code duplication, I'm fine with whatever location.
From a quick look, I think it you go with Rob's suggestion, you can reuse
existing MSI and future RASDES functionalities, isn't it?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (6 preceding siblings ...)
2024-07-15 18:13 ` [PATCH V2 7/7] PCI: host-generic: Add dwc PCIe controller based MSI controller usage Mayank Rana
@ 2024-07-24 2:13 ` Dmitry Baryshkov
2024-07-24 3:58 ` Mayank Rana
2024-07-29 17:19 ` Mayank Rana
8 siblings, 1 reply; 37+ messages in thread
From: Dmitry Baryshkov @ 2024-07-24 2:13 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
> Based on previously received feedback, this patch series adds functionalities
> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> host root complex functionality on Qualcomm SA8775P auto platform.
>
> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>
> 1. Interface to allow requesting firmware to manage system resources and performing
> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> framework based power domain controls these operations)
> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
Please excuse me my ignorance if this is described somewhere. Why are
you using DWC-specific MSI handling instead of using GIC ITS?
> Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
> controller based functionality. Here firmware VM based PCIe driver takes care of resource
> management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
> host generic driver uses power domain to request firmware VM to perform these operations
> using SCMI interface.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-24 2:13 ` [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Dmitry Baryshkov
@ 2024-07-24 3:58 ` Mayank Rana
2024-07-24 7:12 ` Dmitry Baryshkov
0 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-24 3:58 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
On 7/23/2024 7:13 PM, Dmitry Baryshkov wrote:
> On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
>> Based on previously received feedback, this patch series adds functionalities
>> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
>> host root complex functionality on Qualcomm SA8775P auto platform.
>>
>> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
>> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>>
>> 1. Interface to allow requesting firmware to manage system resources and performing
>> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
>> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
>> framework based power domain controls these operations)
>> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
>> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
>> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
>
> Please excuse me my ignorance if this is described somewhere. Why are
> you using DWC-specific MSI handling instead of using GIC ITS?
Due to usage of GIC v3 on SA8775p with Gunyah hypervisor, we have
limitation of not supporting GIC ITS
functionality. We considered other approach as usage of free SPIs (not
available, limitation in terms of mismatch between number of SPIs
available with physical GIC vs hypervisor) and extended SPIs (not
supported with GIC hardware). Hence we just left with DWC-specific MSI
controller here for MSI functionality.
>> Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
>> controller based functionality. Here firmware VM based PCIe driver takes care of resource
>> management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
>> host generic driver uses power domain to request firmware VM to perform these operations
>> using SCMI interface.
>
Regards,
Mayank
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-24 3:58 ` Mayank Rana
@ 2024-07-24 7:12 ` Dmitry Baryshkov
2024-07-24 13:31 ` Manivannan Sadhasivam
0 siblings, 1 reply; 37+ messages in thread
From: Dmitry Baryshkov @ 2024-07-24 7:12 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree, quic_ramkri,
quic_nkela, quic_shazhuss, quic_msarkar, quic_nitegupt
On Wed, 24 Jul 2024 at 06:58, Mayank Rana <quic_mrana@quicinc.com> wrote:
>
>
>
> On 7/23/2024 7:13 PM, Dmitry Baryshkov wrote:
> > On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
> >> Based on previously received feedback, this patch series adds functionalities
> >> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> >> host root complex functionality on Qualcomm SA8775P auto platform.
> >>
> >> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> >> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
> >>
> >> 1. Interface to allow requesting firmware to manage system resources and performing
> >> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> >> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> >> framework based power domain controls these operations)
> >> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> >> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> >> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
> >
> > Please excuse me my ignorance if this is described somewhere. Why are
> > you using DWC-specific MSI handling instead of using GIC ITS?
> Due to usage of GIC v3 on SA8775p with Gunyah hypervisor, we have
> limitation of not supporting GIC ITS
> functionality. We considered other approach as usage of free SPIs (not
> available, limitation in terms of mismatch between number of SPIs
> available with physical GIC vs hypervisor) and extended SPIs (not
> supported with GIC hardware). Hence we just left with DWC-specific MSI
> controller here for MSI functionality.
... or extend Gunyah to support GIC ITS. I'd say it is a significant
deficiency if one can not use GIC ITS on Gunyah platforms.
> >> Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
> >> controller based functionality. Here firmware VM based PCIe driver takes care of resource
> >> management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
> >> host generic driver uses power domain to request firmware VM to perform these operations
> >> using SCMI interface.
> >
> Regards,
> Mayank
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-24 7:12 ` Dmitry Baryshkov
@ 2024-07-24 13:31 ` Manivannan Sadhasivam
2024-07-24 13:34 ` Dmitry Baryshkov
0 siblings, 1 reply; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-24 13:31 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Wed, Jul 24, 2024 at 10:12:17AM +0300, Dmitry Baryshkov wrote:
> On Wed, 24 Jul 2024 at 06:58, Mayank Rana <quic_mrana@quicinc.com> wrote:
> >
> >
> >
> > On 7/23/2024 7:13 PM, Dmitry Baryshkov wrote:
> > > On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
> > >> Based on previously received feedback, this patch series adds functionalities
> > >> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> > >> host root complex functionality on Qualcomm SA8775P auto platform.
> > >>
> > >> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> > >> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
> > >>
> > >> 1. Interface to allow requesting firmware to manage system resources and performing
> > >> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> > >> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> > >> framework based power domain controls these operations)
> > >> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> > >> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> > >> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
> > >
> > > Please excuse me my ignorance if this is described somewhere. Why are
> > > you using DWC-specific MSI handling instead of using GIC ITS?
> > Due to usage of GIC v3 on SA8775p with Gunyah hypervisor, we have
> > limitation of not supporting GIC ITS
> > functionality. We considered other approach as usage of free SPIs (not
> > available, limitation in terms of mismatch between number of SPIs
> > available with physical GIC vs hypervisor) and extended SPIs (not
> > supported with GIC hardware). Hence we just left with DWC-specific MSI
> > controller here for MSI functionality.
>
> ... or extend Gunyah to support GIC ITS. I'd say it is a significant
> deficiency if one can not use GIC ITS on Gunyah platforms.
>
It if were possible, Qcom would've went with that. Unfortunately, it is not.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-24 13:31 ` Manivannan Sadhasivam
@ 2024-07-24 13:34 ` Dmitry Baryshkov
2024-07-24 16:51 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Dmitry Baryshkov @ 2024-07-24 13:34 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Mayank Rana, will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
cassel, yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig,
dlemoal, amishin, thierry.reding, jonathanh, Frank.Li,
ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt, conor+dt,
linux-pci, linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Wed, 24 Jul 2024 at 16:31, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Wed, Jul 24, 2024 at 10:12:17AM +0300, Dmitry Baryshkov wrote:
> > On Wed, 24 Jul 2024 at 06:58, Mayank Rana <quic_mrana@quicinc.com> wrote:
> > >
> > >
> > >
> > > On 7/23/2024 7:13 PM, Dmitry Baryshkov wrote:
> > > > On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
> > > >> Based on previously received feedback, this patch series adds functionalities
> > > >> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> > > >> host root complex functionality on Qualcomm SA8775P auto platform.
> > > >>
> > > >> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> > > >> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
> > > >>
> > > >> 1. Interface to allow requesting firmware to manage system resources and performing
> > > >> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> > > >> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> > > >> framework based power domain controls these operations)
> > > >> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> > > >> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> > > >> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
> > > >
> > > > Please excuse me my ignorance if this is described somewhere. Why are
> > > > you using DWC-specific MSI handling instead of using GIC ITS?
> > > Due to usage of GIC v3 on SA8775p with Gunyah hypervisor, we have
> > > limitation of not supporting GIC ITS
> > > functionality. We considered other approach as usage of free SPIs (not
> > > available, limitation in terms of mismatch between number of SPIs
> > > available with physical GIC vs hypervisor) and extended SPIs (not
> > > supported with GIC hardware). Hence we just left with DWC-specific MSI
> > > controller here for MSI functionality.
> >
> > ... or extend Gunyah to support GIC ITS. I'd say it is a significant
> > deficiency if one can not use GIC ITS on Gunyah platforms.
> >
>
> It if were possible, Qcom would've went with that. Unfortunately, it is not.
Ack.
Mayank, if the patch gets resent for any reason, please add this to
the commit message.
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-24 13:34 ` Dmitry Baryshkov
@ 2024-07-24 16:51 ` Mayank Rana
0 siblings, 0 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-24 16:51 UTC (permalink / raw)
To: Dmitry Baryshkov, Manivannan Sadhasivam
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On 7/24/2024 6:34 AM, Dmitry Baryshkov wrote:
> On Wed, 24 Jul 2024 at 16:31, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
>>
>> On Wed, Jul 24, 2024 at 10:12:17AM +0300, Dmitry Baryshkov wrote:
>>> On Wed, 24 Jul 2024 at 06:58, Mayank Rana <quic_mrana@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 7/23/2024 7:13 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, Jul 15, 2024 at 11:13:28AM GMT, Mayank Rana wrote:
>>>>>> Based on previously received feedback, this patch series adds functionalities
>>>>>> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
>>>>>> host root complex functionality on Qualcomm SA8775P auto platform.
>>>>>>
>>>>>> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
>>>>>> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>>>>>>
>>>>>> 1. Interface to allow requesting firmware to manage system resources and performing
>>>>>> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
>>>>>> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
>>>>>> framework based power domain controls these operations)
>>>>>> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
>>>>>> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
>>>>>> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
>>>>>
>>>>> Please excuse me my ignorance if this is described somewhere. Why are
>>>>> you using DWC-specific MSI handling instead of using GIC ITS?
>>>> Due to usage of GIC v3 on SA8775p with Gunyah hypervisor, we have
>>>> limitation of not supporting GIC ITS
>>>> functionality. We considered other approach as usage of free SPIs (not
>>>> available, limitation in terms of mismatch between number of SPIs
>>>> available with physical GIC vs hypervisor) and extended SPIs (not
>>>> supported with GIC hardware). Hence we just left with DWC-specific MSI
>>>> controller here for MSI functionality.
>>>
>>> ... or extend Gunyah to support GIC ITS. I'd say it is a significant
>>> deficiency if one can not use GIC ITS on Gunyah platforms.
>>>
>>
>> It if were possible, Qcom would've went with that. Unfortunately, it is not.
>
> Ack.
> Mayank, if the patch gets resent for any reason, please add this to
> the commit message.
ACK.
Regards,
Mayank
>>
>> - Mani
>>
>> --
>> மணிவண்ணன் சதாசிவம்
>
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-15 18:13 [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Mayank Rana
` (7 preceding siblings ...)
2024-07-24 2:13 ` [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver Dmitry Baryshkov
@ 2024-07-29 17:19 ` Mayank Rana
2024-07-30 5:34 ` Manivannan Sadhasivam
8 siblings, 1 reply; 37+ messages in thread
From: Mayank Rana @ 2024-07-29 17:19 UTC (permalink / raw)
To: will, lpieralisi, kw, robh, bhelgaas, jingoohan1,
manivannan.sadhasivam, cassel, yoshihiro.shimoda.uh, s-vadapalli,
u.kleine-koenig, dlemoal, amishin, thierry.reding, jonathanh,
Frank.Li, ilpo.jarvinen, vidyas, marek.vasut+renesas, krzk+dt,
conor+dt, linux-pci, linux-arm-kernel, devicetree
Cc: quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
Hi Bjorn / Mani
Gentle ping for your review/feedback on this series.
Thank you.
Regards,
Mayank
On 7/15/2024 11:13 AM, Mayank Rana wrote:
> Based on previously received feedback, this patch series adds functionalities
> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> host root complex functionality on Qualcomm SA8775P auto platform.
>
> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>
> 1. Interface to allow requesting firmware to manage system resources and performing
> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> framework based power domain controls these operations)
> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
>
> Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
> controller based functionality. Here firmware VM based PCIe driver takes care of resource
> management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
> host generic driver uses power domain to request firmware VM to perform these operations
> using SCMI interface.
> ----------------
>
>
> ┌────────────────────────┐
> │ │
> ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
> │ Firmware VM │ │ │ │ Linux VM │
> │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
> │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE host │ │
> │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ generic driver│ │
> │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
> │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
> │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
> │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
> │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
> │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
> │ │ │ │ │ │ │ │ │ │
> └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
> │ │ │ │ │ │
> │ │ └────────────────────────┘ │ │
> │ │ │ │
> │ │ │ │
> │ │ │ │
> │ │ │IRQ │HVC
> IRQ │ │HVC │ │
> │ │ │ │
> │ │ │ │
> │ │ │ │
> ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
> │ │
> │ │
> │ HYPERVISOR │
> │ │
> │ │
> │ │
> └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
>
> ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
> │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
> │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
> └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
>
> ----------
> Changes in V2:
> - Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
> - Add power domain based functionality within existing ECAM driver
>
> Tested:
> - Validated NVME functionality with PCIe0 and PCIe1 on SA8775P-RIDE platform
>
> Mayank Rana (7):
> PCI: dwc: Move MSI related code to separate file
> PCI: dwc: Add msi_ops to allow DBI based MSI register access
> PCI: dwc: Add pcie-designware-msi driver kconfig option
> dt-bindings: PCI: host-generic-pci: Add power-domains binding
> PCI: host-generic: Add power domain based handling for PCIe controller
> dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
> PCI: host-generic: Add dwc MSI based MSI functionality
>
> .../devicetree/bindings/pci/host-generic-pci.yaml | 64 +++
> drivers/pci/controller/dwc/Kconfig | 8 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pci-keystone.c | 12 +-
> drivers/pci/controller/dwc/pcie-designware-host.c | 438 ++-------------------
> drivers/pci/controller/dwc/pcie-designware-msi.c | 413 +++++++++++++++++++
> drivers/pci/controller/dwc/pcie-designware-msi.h | 63 +++
> drivers/pci/controller/dwc/pcie-designware.c | 1 +
> drivers/pci/controller/dwc/pcie-designware.h | 26 +-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
> drivers/pci/controller/pci-host-generic.c | 127 +++++-
> 12 files changed, 723 insertions(+), 436 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.c
> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.h
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-29 17:19 ` Mayank Rana
@ 2024-07-30 5:34 ` Manivannan Sadhasivam
2024-07-30 16:16 ` Mayank Rana
0 siblings, 1 reply; 37+ messages in thread
From: Manivannan Sadhasivam @ 2024-07-30 5:34 UTC (permalink / raw)
To: Mayank Rana
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On Mon, Jul 29, 2024 at 10:19:45AM -0700, Mayank Rana wrote:
> Hi Bjorn / Mani
>
> Gentle ping for your review/feedback on this series.
> Thank you.
>
I was waiting for your reply for my comment [1]. Because that will have
influence on this series.
- Mani
[1] https://lore.kernel.org/linux-pci/20240724095407.GA2347@thinkpad/
> Regards,
> Mayank
>
> On 7/15/2024 11:13 AM, Mayank Rana wrote:
> > Based on previously received feedback, this patch series adds functionalities
> > with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
> > host root complex functionality on Qualcomm SA8775P auto platform.
> >
> > Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
> > https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
> >
> > 1. Interface to allow requesting firmware to manage system resources and performing
> > PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
> > 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
> > framework based power domain controls these operations)
> > 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
> > This MSI functionality is used with PCIe host generic driver after splitting existing MSI
> > functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
> >
> > Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
> > controller based functionality. Here firmware VM based PCIe driver takes care of resource
> > management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
> > host generic driver uses power domain to request firmware VM to perform these operations
> > using SCMI interface.
> > ----------------
> >
> >
> > ┌────────────────────────┐
> > │ │
> > ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
> > │ Firmware VM │ │ │ │ Linux VM │
> > │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
> > │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE host │ │
> > │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ generic driver│ │
> > │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
> > │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
> > │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
> > │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
> > │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
> > │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
> > │ │ │ │ │ │ │ │ │ │
> > └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
> > │ │ │ │ │ │
> > │ │ └────────────────────────┘ │ │
> > │ │ │ │
> > │ │ │ │
> > │ │ │ │
> > │ │ │IRQ │HVC
> > IRQ │ │HVC │ │
> > │ │ │ │
> > │ │ │ │
> > │ │ │ │
> > ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
> > │ │
> > │ │
> > │ HYPERVISOR │
> > │ │
> > │ │
> > │ │
> > └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
> > ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
> > │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
> > │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
> > └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
> > ----------
> > Changes in V2:
> > - Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
> > - Add power domain based functionality within existing ECAM driver
> >
> > Tested:
> > - Validated NVME functionality with PCIe0 and PCIe1 on SA8775P-RIDE platform
> >
> > Mayank Rana (7):
> > PCI: dwc: Move MSI related code to separate file
> > PCI: dwc: Add msi_ops to allow DBI based MSI register access
> > PCI: dwc: Add pcie-designware-msi driver kconfig option
> > dt-bindings: PCI: host-generic-pci: Add power-domains binding
> > PCI: host-generic: Add power domain based handling for PCIe controller
> > dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
> > PCI: host-generic: Add dwc MSI based MSI functionality
> >
> > .../devicetree/bindings/pci/host-generic-pci.yaml | 64 +++
> > drivers/pci/controller/dwc/Kconfig | 8 +
> > drivers/pci/controller/dwc/Makefile | 1 +
> > drivers/pci/controller/dwc/pci-keystone.c | 12 +-
> > drivers/pci/controller/dwc/pcie-designware-host.c | 438 ++-------------------
> > drivers/pci/controller/dwc/pcie-designware-msi.c | 413 +++++++++++++++++++
> > drivers/pci/controller/dwc/pcie-designware-msi.h | 63 +++
> > drivers/pci/controller/dwc/pcie-designware.c | 1 +
> > drivers/pci/controller/dwc/pcie-designware.h | 26 +-
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 1 +
> > drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
> > drivers/pci/controller/pci-host-generic.c | 127 +++++-
> > 12 files changed, 723 insertions(+), 436 deletions(-)
> > create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.c
> > create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.h
> >
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH V2 0/7] Add power domain and MSI functionality with PCIe host generic ECAM driver
2024-07-30 5:34 ` Manivannan Sadhasivam
@ 2024-07-30 16:16 ` Mayank Rana
0 siblings, 0 replies; 37+ messages in thread
From: Mayank Rana @ 2024-07-30 16:16 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: will, lpieralisi, kw, robh, bhelgaas, jingoohan1, cassel,
yoshihiro.shimoda.uh, s-vadapalli, u.kleine-koenig, dlemoal,
amishin, thierry.reding, jonathanh, Frank.Li, ilpo.jarvinen,
vidyas, marek.vasut+renesas, krzk+dt, conor+dt, linux-pci,
linux-arm-kernel, devicetree, quic_ramkri, quic_nkela,
quic_shazhuss, quic_msarkar, quic_nitegupt
On 7/29/2024 10:34 PM, Manivannan Sadhasivam wrote:
> On Mon, Jul 29, 2024 at 10:19:45AM -0700, Mayank Rana wrote:
>> Hi Bjorn / Mani
>>
>> Gentle ping for your review/feedback on this series.
>> Thank you.
>>
>
> I was waiting for your reply for my comment [1]. Because that will have
> influence on this series.
>
> - Mani
>
> [1] https://lore.kernel.org/linux-pci/20240724095407.GA2347@thinkpad/
ok. Thanks for above response, and suggesting that you are ok with
either new ECAM driver or updating PCIE qcom driver with ECAM mode as
well utilizing DWC specific functionality. I still believe having
separate PCIe QCOM ecam driver would be more useful and allow decoupling
dwc host specific code base.
I am requesting above if you can review current MSI split functionality,
and provide feedback on that.
Regards,
Mayank
>
>> Regards,
>> Mayank
>>
>> On 7/15/2024 11:13 AM, Mayank Rana wrote:
>>> Based on previously received feedback, this patch series adds functionalities
>>> with existing PCIe host generic ECAM driver (pci-host-generic.c) to get PCIe
>>> host root complex functionality on Qualcomm SA8775P auto platform.
>>>
>>> Previously sent RFC patchset to have separate Qualcomm PCIe ECAM platform driver:
>>> https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>>>
>>> 1. Interface to allow requesting firmware to manage system resources and performing
>>> PCIe Link up (devicetree binding in terms of power domain and runtime PM APIs is used in driver)
>>> 2. Performing D3 cold with system suspend and D0 with system resume (usage of GenPD
>>> framework based power domain controls these operations)
>>> 3. SA8775P is using Synopsys Designware PCIe controller which supports MSI controller.
>>> This MSI functionality is used with PCIe host generic driver after splitting existing MSI
>>> functionality from pcie-designware-host.c file into pcie-designware-msi.c file.
>>>
>>> Below architecture is used on Qualcomm SA8775P auto platform to get ECAM compliant PCIe
>>> controller based functionality. Here firmware VM based PCIe driver takes care of resource
>>> management and performing PCIe link related handling (D0 and D3cold). Linux VM based PCIe
>>> host generic driver uses power domain to request firmware VM to perform these operations
>>> using SCMI interface.
>>> ----------------
>>>
>>>
>>> ┌────────────────────────┐
>>> │ │
>>> ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
>>> │ Firmware VM │ │ │ │ Linux VM │
>>> │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
>>> │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE host │ │
>>> │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ generic driver│ │
>>> │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
>>> │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
>>> │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
>>> │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
>>> │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
>>> │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
>>> │ │ │ │ │ │ │ │ │ │
>>> └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
>>> │ │ │ │ │ │
>>> │ │ └────────────────────────┘ │ │
>>> │ │ │ │
>>> │ │ │ │
>>> │ │ │ │
>>> │ │ │IRQ │HVC
>>> IRQ │ │HVC │ │
>>> │ │ │ │
>>> │ │ │ │
>>> │ │ │ │
>>> ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
>>> │ │
>>> │ │
>>> │ HYPERVISOR │
>>> │ │
>>> │ │
>>> │ │
>>> └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
>>> ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
>>> │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
>>> │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
>>> └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
>>> ----------
>>> Changes in V2:
>>> - Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
>>> - Add power domain based functionality within existing ECAM driver
>>>
>>> Tested:
>>> - Validated NVME functionality with PCIe0 and PCIe1 on SA8775P-RIDE platform
>>>
>>> Mayank Rana (7):
>>> PCI: dwc: Move MSI related code to separate file
>>> PCI: dwc: Add msi_ops to allow DBI based MSI register access
>>> PCI: dwc: Add pcie-designware-msi driver kconfig option
>>> dt-bindings: PCI: host-generic-pci: Add power-domains binding
>>> PCI: host-generic: Add power domain based handling for PCIe controller
>>> dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding
>>> PCI: host-generic: Add dwc MSI based MSI functionality
>>>
>>> .../devicetree/bindings/pci/host-generic-pci.yaml | 64 +++
>>> drivers/pci/controller/dwc/Kconfig | 8 +
>>> drivers/pci/controller/dwc/Makefile | 1 +
>>> drivers/pci/controller/dwc/pci-keystone.c | 12 +-
>>> drivers/pci/controller/dwc/pcie-designware-host.c | 438 ++-------------------
>>> drivers/pci/controller/dwc/pcie-designware-msi.c | 413 +++++++++++++++++++
>>> drivers/pci/controller/dwc/pcie-designware-msi.h | 63 +++
>>> drivers/pci/controller/dwc/pcie-designware.c | 1 +
>>> drivers/pci/controller/dwc/pcie-designware.h | 26 +-
>>> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 1 +
>>> drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
>>> drivers/pci/controller/pci-host-generic.c | 127 +++++-
>>> 12 files changed, 723 insertions(+), 436 deletions(-)
>>> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.c
>>> create mode 100644 drivers/pci/controller/dwc/pcie-designware-msi.h
>>>
>
^ permalink raw reply [flat|nested] 37+ messages in thread