Re: [PATCH v13 4/7] PCI: dwc: Handle MSIs routed to multiple GIC interrupts

On 06/06/2022 19:27, Johan Hovold wrote:

On Fri, Jun 03, 2022 at 10:41:34AM +0300, Dmitry Baryshkov wrote:

quoted

On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
separate GIC interrupt. Implement support for such configurations by
parsing "msi0" ... "msiN" interrupts and attaching them to the chained
handler.

Note, that if DT doesn't list an array of MSI interrupts and uses single
"msi" IRQ, the driver will limit the amount of supported MSI vectors
accordingly (to 32).

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Dmitry Baryshkov <redacted>
---
  .../pci/controller/dwc/pcie-designware-host.c | 63 +++++++++++++++++--
  1 file changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 85c1160792e1..d1f9e20df903 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c

@@ -289,6 +289,46 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
  	dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
  }
  
+static int dw_pcie_parse_split_msi_irq(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int irq;
+	u32 ctrl, max_vectors;
+
+	/* Parse as many IRQs as described in the devicetree. */
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
+		char *msi_name = "msiX";
+
+		msi_name[3] = '0' + ctrl;

This oopses here as the string constant is read only:

	[   19.787973] Unable to handle kernel write to read-only memory at virtual address ffffaa14f831afd3

Did you not test the series before posting?

Interesting enough the posted series works for me. Maybe I have a different set of debugging options. But thanks for spotting this. I'll post v14.

You need to define msi_name as:

	char msi_name[] = "msiX";

quoted

+		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;
+	}

Johan

-- 
With best wishes
Dmitry