| Message ID | 20220603074137.1849892-5-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: dwc: Fix higher MSI vectors handling | expand |
On Fri, Jun 03, 2022 at 10:41:34AM +0300, Dmitry Baryshkov wrote: > 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 <dmitry.baryshkov@linaro.org> > --- > .../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? You need to define msi_name as: char msi_name[] = "msiX"; > + 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
On 06/06/2022 19:27, Johan Hovold wrote: > On Fri, Jun 03, 2022 at 10:41:34AM +0300, Dmitry Baryshkov wrote: >> 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 <dmitry.baryshkov@linaro.org> >> --- >> .../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"; > >> + 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
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; + 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 there were no "msiN" IRQs at all, fallback to the standard "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 pcie_port *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -297,21 +337,32 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp) int ret; u32 ctrl, num_ctrls; - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL; - for (ctrl = 0; ctrl < num_ctrls; ctrl++) + 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]) { int irq = platform_get_irq_byname_optional(pdev, "msi"); if (irq < 0) { irq = platform_get_irq(pdev, 0); if (irq < 0) - return irq; + return dev_err_probe(dev, irq, "Failed to parse MSI irq\n"); } pp->msi_irq[0] = irq; } + 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); @@ -409,7 +460,11 @@ int dw_pcie_host_init(struct pcie_port *pp) of_property_read_bool(np, "msi-parent") || of_property_read_bool(np, "msi-map")); - if (!pp->num_vectors) { + /* + * 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) { pp->num_vectors = MSI_DEF_NUM_VECTORS; } else if (pp->num_vectors > MAX_MSI_IRQS) { dev_err(dev, "Invalid number of vectors\n");