diff mbox series

[v8,06/10] PCI: dwc: Handle MSIs routed to multiple GIC interrupts

Message ID 20220512104545.2204523-7-dmitry.baryshkov@linaro.org
State New
Headers show
Series PCI: qcom: Fix higher MSI vectors handling | expand

Commit Message

Dmitry Baryshkov May 12, 2022, 10:45 a.m. UTC
On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
separate GIC interrupt. Implement support for such configuraions by
parsing "msi0" ... "msi7" 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 | 33 ++++++++++++++++++-
 drivers/pci/controller/dwc/pcie-designware.h  |  1 +
 2 files changed, 33 insertions(+), 1 deletion(-)

Comments

Bjorn Helgaas May 12, 2022, 6:55 p.m. UTC | #1
On Thu, May 12, 2022 at 01:45:41PM +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 configuraions by
> parsing "msi0" ... "msi7" interrupts and attaching them to the chained
> handler.

Again, only if you have some other reason to repost:

s/configuraions/configurations/

> 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 | 33 ++++++++++++++++++-
>  drivers/pci/controller/dwc/pcie-designware.h  |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 6b0c7b75391f..258bafa306dc 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -291,7 +291,8 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -	struct platform_device *pdev = to_platform_device(pci->dev);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
>  	int ret;
>  	u32 ctrl, num_ctrls;
>  
> @@ -299,6 +300,36 @@ 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) {
> +		char irq_name[] = "msiXX";
> +		int irq;
> +
> +		if (!pp->msi_irq[0]) {
> +			irq = platform_get_irq_byname_optional(pdev, irq_name);
> +			if (irq == -ENXIO) {
> +				num_ctrls = 1;
> +				pp->num_vectors = min((u32)MAX_MSI_IRQS_PER_CTRL, pp->num_vectors);
> +				dev_warn(dev, "No additional MSI IRQs, limiting amount of MSI vectors to %d\n",
> +					 pp->num_vectors);
> +			} else {
> +				pp->msi_irq[0] = irq;
> +			}
> +		}
> +
> +		/* If we fallback to the single MSI ctrl IRQ, this loop will be skipped as num_ctrls is 1 */
> +		for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
> +			if (pp->msi_irq[ctrl])
> +				continue;
> +
> +			snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl);
> +			irq = platform_get_irq_byname(pdev, irq_name);
> +			if (irq < 0)
> +				return irq;
> +
> +			pp->msi_irq[ctrl] = irq;
> +		}
> +	}
> +
>  	if (!pp->msi_irq[0]) {
>  		int irq = platform_get_irq_byname_optional(pdev, "msi");
>  
> 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;
> -- 
> 2.35.1
>
Johan Hovold May 13, 2022, 11:52 a.m. UTC | #2
On Thu, May 12, 2022 at 01:45:41PM +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 configuraions by
> parsing "msi0" ... "msi7" 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 | 33 ++++++++++++++++++-
>  drivers/pci/controller/dwc/pcie-designware.h  |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 6b0c7b75391f..258bafa306dc 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -291,7 +291,8 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -	struct platform_device *pdev = to_platform_device(pci->dev);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
>  	int ret;
>  	u32 ctrl, num_ctrls;
>  
> @@ -299,6 +300,36 @@ 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) {
> +		char irq_name[] = "msiXX";
> +		int irq;
> +
> +		if (!pp->msi_irq[0]) {
> +			irq = platform_get_irq_byname_optional(pdev, irq_name);

This looks broken; you're requesting "msiXX", not "msi0".

> +			if (irq == -ENXIO) {
> +				num_ctrls = 1;
> +				pp->num_vectors = min((u32)MAX_MSI_IRQS_PER_CTRL, pp->num_vectors);
> +				dev_warn(dev, "No additional MSI IRQs, limiting amount of MSI vectors to %d\n",
> +					 pp->num_vectors);
> +			} else {
> +				pp->msi_irq[0] = irq;
> +			}
> +		}
> +
> +		/* If we fallback to the single MSI ctrl IRQ, this loop will be skipped as num_ctrls is 1 */
> +		for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
> +			if (pp->msi_irq[ctrl])
> +				continue;
> +
> +			snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl);
> +			irq = platform_get_irq_byname(pdev, irq_name);
> +			if (irq < 0)
> +				return irq;
> +
> +			pp->msi_irq[ctrl] = irq;
> +		}
> +	}
> +
>  	if (!pp->msi_irq[0]) {
>  		int irq = platform_get_irq_byname_optional(pdev, "msi");

Johan
Dmitry Baryshkov May 13, 2022, 12:19 p.m. UTC | #3
On Fri, 13 May 2022 at 14:52, Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, May 12, 2022 at 01:45:41PM +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 configuraions by
> > parsing "msi0" ... "msi7" 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 | 33 ++++++++++++++++++-
> >  drivers/pci/controller/dwc/pcie-designware.h  |  1 +
> >  2 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 6b0c7b75391f..258bafa306dc 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -291,7 +291,8 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
> >  static int dw_pcie_msi_host_init(struct pcie_port *pp)
> >  {
> >       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > -     struct platform_device *pdev = to_platform_device(pci->dev);
> > +     struct device *dev = pci->dev;
> > +     struct platform_device *pdev = to_platform_device(dev);
> >       int ret;
> >       u32 ctrl, num_ctrls;
> >
> > @@ -299,6 +300,36 @@ 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) {
> > +             char irq_name[] = "msiXX";
> > +             int irq;
> > +
> > +             if (!pp->msi_irq[0]) {
> > +                     irq = platform_get_irq_byname_optional(pdev, irq_name);
>
> This looks broken; you're requesting "msiXX", not "msi0".

Yes, I'm preparing the updated patch.

>
> > +                     if (irq == -ENXIO) {
> > +                             num_ctrls = 1;
> > +                             pp->num_vectors = min((u32)MAX_MSI_IRQS_PER_CTRL, pp->num_vectors);
> > +                             dev_warn(dev, "No additional MSI IRQs, limiting amount of MSI vectors to %d\n",
> > +                                      pp->num_vectors);
> > +                     } else {
> > +                             pp->msi_irq[0] = irq;
> > +                     }
> > +             }
> > +
> > +             /* If we fallback to the single MSI ctrl IRQ, this loop will be skipped as num_ctrls is 1 */
> > +             for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
> > +                     if (pp->msi_irq[ctrl])
> > +                             continue;
> > +
> > +                     snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl);
> > +                     irq = platform_get_irq_byname(pdev, irq_name);
> > +                     if (irq < 0)
> > +                             return irq;
> > +
> > +                     pp->msi_irq[ctrl] = irq;
> > +             }
> > +     }
> > +
> >       if (!pp->msi_irq[0]) {
> >               int irq = platform_get_irq_byname_optional(pdev, "msi");
>
> Johan
Johan Hovold May 13, 2022, 12:33 p.m. UTC | #4
On Thu, May 12, 2022 at 01:45:41PM +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 configuraions by
> parsing "msi0" ... "msi7" 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 | 33 ++++++++++++++++++-
>  drivers/pci/controller/dwc/pcie-designware.h  |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 6b0c7b75391f..258bafa306dc 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -291,7 +291,8 @@ static void dw_pcie_msi_init(struct pcie_port *pp)
>  static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -	struct platform_device *pdev = to_platform_device(pci->dev);
> +	struct device *dev = pci->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
>  	int ret;
>  	u32 ctrl, num_ctrls;
>  
> @@ -299,6 +300,36 @@ 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) {
> +		char irq_name[] = "msiXX";
> +		int irq;
> +
> +		if (!pp->msi_irq[0]) {
> +			irq = platform_get_irq_byname_optional(pdev, irq_name);
> +			if (irq == -ENXIO) {
> +				num_ctrls = 1;
> +				pp->num_vectors = min((u32)MAX_MSI_IRQS_PER_CTRL, pp->num_vectors);

min_t()?

> +				dev_warn(dev, "No additional MSI IRQs, limiting amount of MSI vectors to %d\n",
> +					 pp->num_vectors);

We already print the number of vectors used a bit further down in this
function, no need to repeat it here.

This will also be printed when booting with devicetrees which only have
a single "msi" interrupt (which isn't deprecated).

Perhaps a debug printk is sufficient, or at least something less verbose.

> +			} else {
> +				pp->msi_irq[0] = irq;
> +			}
> +		}
> +
> +		/* If we fallback to the single MSI ctrl IRQ, this loop will be skipped as num_ctrls is 1 */

Please break lines at 80 columns unless not doing so improves
readability.

> +		for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
> +			if (pp->msi_irq[ctrl])
> +				continue;
> +
> +			snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl);
> +			irq = platform_get_irq_byname(pdev, irq_name);
> +			if (irq < 0)
> +				return irq;
> +
> +			pp->msi_irq[ctrl] = irq;
> +		}
> +	}
> +
>  	if (!pp->msi_irq[0]) {
>  		int irq = platform_get_irq_byname_optional(pdev, "msi");

Johan
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 6b0c7b75391f..258bafa306dc 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -291,7 +291,8 @@  static void dw_pcie_msi_init(struct pcie_port *pp)
 static int dw_pcie_msi_host_init(struct pcie_port *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-	struct platform_device *pdev = to_platform_device(pci->dev);
+	struct device *dev = pci->dev;
+	struct platform_device *pdev = to_platform_device(dev);
 	int ret;
 	u32 ctrl, num_ctrls;
 
@@ -299,6 +300,36 @@  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) {
+		char irq_name[] = "msiXX";
+		int irq;
+
+		if (!pp->msi_irq[0]) {
+			irq = platform_get_irq_byname_optional(pdev, irq_name);
+			if (irq == -ENXIO) {
+				num_ctrls = 1;
+				pp->num_vectors = min((u32)MAX_MSI_IRQS_PER_CTRL, pp->num_vectors);
+				dev_warn(dev, "No additional MSI IRQs, limiting amount of MSI vectors to %d\n",
+					 pp->num_vectors);
+			} else {
+				pp->msi_irq[0] = irq;
+			}
+		}
+
+		/* If we fallback to the single MSI ctrl IRQ, this loop will be skipped as num_ctrls is 1 */
+		for (ctrl = 1; ctrl < num_ctrls; ctrl++) {
+			if (pp->msi_irq[ctrl])
+				continue;
+
+			snprintf(irq_name, sizeof(irq_name), "msi%d", ctrl);
+			irq = platform_get_irq_byname(pdev, irq_name);
+			if (irq < 0)
+				return irq;
+
+			pp->msi_irq[ctrl] = irq;
+		}
+	}
+
 	if (!pp->msi_irq[0]) {
 		int irq = platform_get_irq_byname_optional(pdev, "msi");
 
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;