| Message ID | 20220513172622.2968887-7-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: Fix higher MSI vectors handling | expand |
On Fri, May 13, 2022 at 08:26:18PM +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). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++++++++++- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 70f0435907c1..320a968dd366 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -288,6 +288,11 @@ 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 const char * const split_msi_names[] = { > + "msi0", "msi1", "msi2", "msi3", > + "msi4", "msi5", "msi6", "msi7", > +}; > + > static int dw_pcie_msi_host_init(struct pcie_port *pp) > { > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > @@ -300,17 +305,48 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp) > for (ctrl = 0; ctrl < num_ctrls; ctrl++) > pp->irq_mask[ctrl] = ~0; > > + if (pp->has_split_msi_irq) { You don't need to add this configuration parameter as it can be inferred from the devicetree (e.g. if "msi0" is specified). > + /* > + * Parse as many IRQs as described in the DTS. If there are > + * none, fallback to the single "msi" IRQ. > + */ > + for (ctrl = 0; ctrl < num_ctrls; ctrl++) { > + int irq; > + > + if (pp->msi_irq[ctrl]) > + continue; > + > + irq = platform_get_irq_byname(pdev, split_msi_names[ctrl]); You need to use platform_get_irq_byname_optional() here or an error will still printed if the number of "msi" interrupts is less than 8. > + if (irq == -ENXIO) { > + num_ctrls = ctrl; > + break; > + } else if (irq < 0) { > + return dev_err_probe(dev, irq, > + "Failed to parse MSI IRQ '%s'\n", > + split_msi_names[ctrl]); > + } > + > + pp->msi_irq[ctrl] = irq; > + } > + > + if (num_ctrls == 0) > + num_ctrls = 1; > + } > + > 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; > } > > + pp->num_vectors = min_t(u32, pp->num_vectors, num_ctrls * MAX_MSI_IRQS_PER_CTRL); > + dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors); Can you rework the handling of num_vectors == 0 (in dw_pcie_host_init()) so that the number is always inferred from the number of "msi" interrupts without having to pass in num_vectors == MAX_MSI_IRQS? That is num_vectors == 0 && "msi" => num_vectors = MSI_DEF_NUM_VECTORS (32) num_vectors == 0 && "msi0".."msin" => num_vectors = n * MAX_MSI_IRQS_PER_CTRL (n * 32) > + > pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; > > ret = dw_pcie_allocate_domains(pp); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 9c1a38b0a6b3..3aa840a5b19c 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -179,6 +179,7 @@ struct dw_pcie_host_ops { > > struct pcie_port { > bool has_msi_ctrl:1; > + bool has_split_msi_irq:1; > u64 cfg0_base; > void __iomem *va_cfg0_base; > u32 cfg0_size; Johan
On 18/05/2022 12:43, Johan Hovold wrote: > On Fri, May 13, 2022 at 08:26:18PM +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). >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++++++++++- >> drivers/pci/controller/dwc/pcie-designware.h | 1 + >> 2 files changed, 38 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c >> index 70f0435907c1..320a968dd366 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >> @@ -288,6 +288,11 @@ 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 const char * const split_msi_names[] = { >> + "msi0", "msi1", "msi2", "msi3", >> + "msi4", "msi5", "msi6", "msi7", >> +}; >> + >> static int dw_pcie_msi_host_init(struct pcie_port *pp) >> { >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> @@ -300,17 +305,48 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp) >> for (ctrl = 0; ctrl < num_ctrls; ctrl++) >> pp->irq_mask[ctrl] = ~0; >> >> + if (pp->has_split_msi_irq) { > > You don't need to add this configuration parameter as it can be inferred > from the devicetree (e.g. if "msi0" is specified). > >> + /* >> + * Parse as many IRQs as described in the DTS. If there are >> + * none, fallback to the single "msi" IRQ. >> + */ >> + for (ctrl = 0; ctrl < num_ctrls; ctrl++) { >> + int irq; >> + >> + if (pp->msi_irq[ctrl]) >> + continue; >> + >> + irq = platform_get_irq_byname(pdev, split_msi_names[ctrl]); > > You need to use platform_get_irq_byname_optional() here or an error will > still printed if the number of "msi" interrupts is less than 8. > >> + if (irq == -ENXIO) { >> + num_ctrls = ctrl; >> + break; >> + } else if (irq < 0) { >> + return dev_err_probe(dev, irq, >> + "Failed to parse MSI IRQ '%s'\n", >> + split_msi_names[ctrl]); >> + } >> + >> + pp->msi_irq[ctrl] = irq; >> + } >> + >> + if (num_ctrls == 0) >> + num_ctrls = 1; >> + } >> + >> 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; >> } >> >> + pp->num_vectors = min_t(u32, pp->num_vectors, num_ctrls * MAX_MSI_IRQS_PER_CTRL); >> + dev_dbg(dev, "Using %d MSI vectors\n", pp->num_vectors); > > Can you rework the handling of num_vectors == 0 (in dw_pcie_host_init()) > so that the number is always inferred from the number of "msi" > interrupts without having to pass in num_vectors == MAX_MSI_IRQS? It wasn't that easy, but I think I ended up doing it properly. > > That is > > num_vectors == 0 && "msi" => num_vectors = MSI_DEF_NUM_VECTORS (32) > num_vectors == 0 && "msi0".."msin" => num_vectors = n * MAX_MSI_IRQS_PER_CTRL (n * 32) > >> + >> pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; >> >> ret = dw_pcie_allocate_domains(pp); >> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h >> index 9c1a38b0a6b3..3aa840a5b19c 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware.h >> +++ b/drivers/pci/controller/dwc/pcie-designware.h >> @@ -179,6 +179,7 @@ struct dw_pcie_host_ops { >> >> struct pcie_port { >> bool has_msi_ctrl:1; >> + bool has_split_msi_irq:1; >> u64 cfg0_base; >> void __iomem *va_cfg0_base; >> u32 cfg0_size; > > Johan
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 70f0435907c1..320a968dd366 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -288,6 +288,11 @@ 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 const char * const split_msi_names[] = { + "msi0", "msi1", "msi2", "msi3", + "msi4", "msi5", "msi6", "msi7", +}; + static int dw_pcie_msi_host_init(struct pcie_port *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -300,17 +305,48 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp) for (ctrl = 0; ctrl < num_ctrls; ctrl++) pp->irq_mask[ctrl] = ~0; + if (pp->has_split_msi_irq) { + /* + * Parse as many IRQs as described in the DTS. If there are + * none, fallback to the single "msi" IRQ. + */ + for (ctrl = 0; ctrl < num_ctrls; ctrl++) { + int irq; + + if (pp->msi_irq[ctrl]) + continue; + + irq = platform_get_irq_byname(pdev, split_msi_names[ctrl]); + if (irq == -ENXIO) { + num_ctrls = ctrl; + break; + } else if (irq < 0) { + return dev_err_probe(dev, irq, + "Failed to parse MSI IRQ '%s'\n", + split_msi_names[ctrl]); + } + + pp->msi_irq[ctrl] = irq; + } + + if (num_ctrls == 0) + num_ctrls = 1; + } + 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; } + pp->num_vectors = min_t(u32, pp->num_vectors, num_ctrls * MAX_MSI_IRQS_PER_CTRL); + 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); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 9c1a38b0a6b3..3aa840a5b19c 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -179,6 +179,7 @@ struct dw_pcie_host_ops { struct pcie_port { bool has_msi_ctrl:1; + bool has_split_msi_irq:1; u64 cfg0_base; void __iomem *va_cfg0_base; u32 cfg0_size;
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). Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++++++++++- drivers/pci/controller/dwc/pcie-designware.h | 1 + 2 files changed, 38 insertions(+), 1 deletion(-)