mbox series

[v9,0/3] PCI: dwc: Enables MSI-X driver support

Message ID cover.1519834018.git.gustavo.pimentel@synopsys.com
Headers show
Series PCI: dwc: Enables MSI-X driver support | expand

Message

Gustavo Pimentel Feb. 28, 2018, 4:10 p.m. UTC
Changes into a new IRQ handler mechanism (multiplexed IRQ domain hierarchy)
more suitable and appropriate to use on pcie-designware and on each SoC
specific driver in order to allow new and more complex features like MSI-X.

Adds Synopsys Root Complex driver support for MSI-X feature.

Expands the maximum number of IRQs from 32 to 256 distributed by
a maximum of 8 controller registers.

The patch set was made against the Bjorn's master branch (v4.16-rc1).

Gustavo Pimentel (3):
  PCI: dwc: Add new IRQ API
  PCI: dwc: Remove old IRQ API
  PCI: dwc: Expand maximum number of IRQs from 32 to 256

 drivers/pci/dwc/pci-exynos.c           |  18 --
 drivers/pci/dwc/pci-imx6.c             |  18 --
 drivers/pci/dwc/pci-keystone-dw.c      |  91 +-------
 drivers/pci/dwc/pci-keystone.c         |   1 +
 drivers/pci/dwc/pci-keystone.h         |   4 +-
 drivers/pci/dwc/pci-layerscape.c       |   3 +-
 drivers/pci/dwc/pcie-artpec6.c         |  18 --
 drivers/pci/dwc/pcie-designware-host.c | 398 +++++++++++++++++++--------------
 drivers/pci/dwc/pcie-designware-plat.c |  16 --
 drivers/pci/dwc/pcie-designware.h      |  30 ++-
 drivers/pci/dwc/pcie-qcom.c            |  16 --
 11 files changed, 258 insertions(+), 355 deletions(-)

Comments

Marc Zyngier Feb. 28, 2018, 6:12 p.m. UTC | #1
On 28/02/18 16:10, Gustavo Pimentel wrote:
> Changes into a new IRQ handler mechanism (multiplexed IRQ domain hierarchy)
> more suitable and appropriate to use on pcie-designware and on each SoC
> specific driver in order to allow new and more complex features like MSI-X.
> 
> Adds Synopsys Root Complex driver support for MSI-X feature.
> 
> Expands the maximum number of IRQs from 32 to 256 distributed by
> a maximum of 8 controller registers.
> 
> The patch set was made against the Bjorn's master branch (v4.16-rc1).
> 
> Gustavo Pimentel (3):
>   PCI: dwc: Add new IRQ API
>   PCI: dwc: Remove old IRQ API
>   PCI: dwc: Expand maximum number of IRQs from 32 to 256
> 
>  drivers/pci/dwc/pci-exynos.c           |  18 --
>  drivers/pci/dwc/pci-imx6.c             |  18 --
>  drivers/pci/dwc/pci-keystone-dw.c      |  91 +-------
>  drivers/pci/dwc/pci-keystone.c         |   1 +
>  drivers/pci/dwc/pci-keystone.h         |   4 +-
>  drivers/pci/dwc/pci-layerscape.c       |   3 +-
>  drivers/pci/dwc/pcie-artpec6.c         |  18 --
>  drivers/pci/dwc/pcie-designware-host.c | 398 +++++++++++++++++++--------------
>  drivers/pci/dwc/pcie-designware-plat.c |  16 --
>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>  drivers/pci/dwc/pcie-qcom.c            |  16 --
>  11 files changed, 258 insertions(+), 355 deletions(-)
> 

For the whole series:

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

There is certainly some more cleanups to be done, but that's a huge step
in the right direction. Many thanks Gustavo and Joao for having stuck
with it.

	M.
Lorenzo Pieralisi March 1, 2018, 11:35 a.m. UTC | #2
On Wed, Feb 28, 2018 at 04:10:22PM +0000, Gustavo Pimentel wrote:
> Changes into a new IRQ handler mechanism (multiplexed IRQ domain hierarchy)
> more suitable and appropriate to use on pcie-designware and on each SoC
> specific driver in order to allow new and more complex features like MSI-X.
> 
> Adds Synopsys Root Complex driver support for MSI-X feature.
> 
> Expands the maximum number of IRQs from 32 to 256 distributed by
> a maximum of 8 controller registers.
> 
> The patch set was made against the Bjorn's master branch (v4.16-rc1).
> 
> Gustavo Pimentel (3):
>   PCI: dwc: Add new IRQ API
>   PCI: dwc: Remove old IRQ API
>   PCI: dwc: Expand maximum number of IRQs from 32 to 256
> 
>  drivers/pci/dwc/pci-exynos.c           |  18 --
>  drivers/pci/dwc/pci-imx6.c             |  18 --
>  drivers/pci/dwc/pci-keystone-dw.c      |  91 +-------
>  drivers/pci/dwc/pci-keystone.c         |   1 +
>  drivers/pci/dwc/pci-keystone.h         |   4 +-
>  drivers/pci/dwc/pci-layerscape.c       |   3 +-
>  drivers/pci/dwc/pcie-artpec6.c         |  18 --
>  drivers/pci/dwc/pcie-designware-host.c | 398 +++++++++++++++++++--------------
>  drivers/pci/dwc/pcie-designware-plat.c |  16 --
>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>  drivers/pci/dwc/pcie-qcom.c            |  16 --
>  11 files changed, 258 insertions(+), 355 deletions(-)

Patches queued with Marc's ACK in pci/dwc-msi for v4.17, please
have a look to check everything is in order.

Thank you very much for putting it together, it is a nice stepping
stone towards getting rid of struct msi_controller.

Thanks !
Lorenzo
Lorenzo Pieralisi March 2, 2018, 6:03 p.m. UTC | #3
[+cc Shawn, Vignesh, Pratyush]
On Wed, Feb 28, 2018 at 04:10:22PM +0000, Gustavo Pimentel wrote:
> Changes into a new IRQ handler mechanism (multiplexed IRQ domain hierarchy)
> more suitable and appropriate to use on pcie-designware and on each SoC
> specific driver in order to allow new and more complex features like MSI-X.
> 
> Adds Synopsys Root Complex driver support for MSI-X feature.
> 
> Expands the maximum number of IRQs from 32 to 256 distributed by
> a maximum of 8 controller registers.
> 
> The patch set was made against the Bjorn's master branch (v4.16-rc1).
> 
> Gustavo Pimentel (3):
>   PCI: dwc: Add new IRQ API
>   PCI: dwc: Remove old IRQ API
>   PCI: dwc: Expand maximum number of IRQs from 32 to 256
> 
>  drivers/pci/dwc/pci-exynos.c           |  18 --
>  drivers/pci/dwc/pci-imx6.c             |  18 --
>  drivers/pci/dwc/pci-keystone-dw.c      |  91 +-------
>  drivers/pci/dwc/pci-keystone.c         |   1 +
>  drivers/pci/dwc/pci-keystone.h         |   4 +-
>  drivers/pci/dwc/pci-layerscape.c       |   3 +-
>  drivers/pci/dwc/pcie-artpec6.c         |  18 --
>  drivers/pci/dwc/pcie-designware-host.c | 398 +++++++++++++++++++--------------

Hi Gustavo,

I was reviewing a patch and this raised a question:

-  why pcie-histb.c is not in the list of converted drivers above,
   given that it uses dw_handle_msi_irq() the same way as others
   you converted ?

Is it correct to leave pcie-histb.c as-is ? I do not think it is.

I also would like to get this tested on DRA7 and SPEAR13XX which
are the other host drivers using dw_handle_msi_irq() (even though
in a different way than pcie-histb.c).

The difference here seems to be struct pcie_port->msi_irq usage, that's
extremely confusing how it is used (or not used) in the drivers I
mentioned above.

I have CC'ed the related maintainers I need feedback and this needs
testing; this series code is in my branch:

pci/dwc-msi

Thanks,
Lorenzo

>  drivers/pci/dwc/pcie-designware-plat.c |  16 --
>  drivers/pci/dwc/pcie-designware.h      |  30 ++-
>  drivers/pci/dwc/pcie-qcom.c            |  16 --
>  11 files changed, 258 insertions(+), 355 deletions(-)
> 
> -- 
> 2.7.4
> 
>
Gustavo Pimentel March 5, 2018, 6:22 p.m. UTC | #4
Hi Lorenzo,

On 02/03/2018 18:03, Lorenzo Pieralisi wrote:
> [+cc Shawn, Vignesh, Pratyush]
> On Wed, Feb 28, 2018 at 04:10:22PM +0000, Gustavo Pimentel wrote:
>> Changes into a new IRQ handler mechanism (multiplexed IRQ domain hierarchy)
>> more suitable and appropriate to use on pcie-designware and on each SoC
>> specific driver in order to allow new and more complex features like MSI-X.
>>
>> Adds Synopsys Root Complex driver support for MSI-X feature.
>>
>> Expands the maximum number of IRQs from 32 to 256 distributed by
>> a maximum of 8 controller registers.
>>
>> The patch set was made against the Bjorn's master branch (v4.16-rc1).
>>
>> Gustavo Pimentel (3):
>>   PCI: dwc: Add new IRQ API
>>   PCI: dwc: Remove old IRQ API
>>   PCI: dwc: Expand maximum number of IRQs from 32 to 256
>>
>>  drivers/pci/dwc/pci-exynos.c           |  18 --
>>  drivers/pci/dwc/pci-imx6.c             |  18 --
>>  drivers/pci/dwc/pci-keystone-dw.c      |  91 +-------
>>  drivers/pci/dwc/pci-keystone.c         |   1 +
>>  drivers/pci/dwc/pci-keystone.h         |   4 +-
>>  drivers/pci/dwc/pci-layerscape.c       |   3 +-
>>  drivers/pci/dwc/pcie-artpec6.c         |  18 --
>>  drivers/pci/dwc/pcie-designware-host.c | 398 +++++++++++++++++++--------------
> 
> Hi Gustavo,
> 
> I was reviewing a patch and this raised a question:
> 
> -  why pcie-histb.c is not in the list of converted drivers above,
>    given that it uses dw_handle_msi_irq() the same way as others
>    you converted ?
> 
> Is it correct to leave pcie-histb.c as-is ? I do not think it is.

At the time the first series of this patch came out there wasn't such driver and
I just start working on this topic on December... Nevertheless, you are right it
should follow the same procedure that others.

diff --git a/drivers/pci/dwc/pcie-histb.c b/drivers/pci/dwc/pcie-histb.c
index 70b5c0b..5d47b90 100644
--- a/drivers/pci/dwc/pcie-histb.c
+++ b/drivers/pci/dwc/pcie-histb.c
@@ -207,13 +207,6 @@ static struct dw_pcie_host_ops histb_pcie_host_ops = {
 	.host_init = histb_pcie_host_init,
 };

-static irqreturn_t histb_pcie_msi_irq_handler(int irq, void *arg)
-{
-	struct pcie_port *pp = arg;
-
-	return dw_handle_msi_irq(pp);
-}
-
 static void histb_pcie_host_disable(struct histb_pcie *hipcie)
 {
 	reset_control_assert(hipcie->soft_reset);
@@ -393,14 +386,6 @@ static int histb_pcie_probe(struct platform_device *pdev)
 			dev_err(dev, "Failed to get MSI IRQ\n");
 			return pp->msi_irq;
 		}
-
-		ret = devm_request_irq(dev, pp->msi_irq,
-				       histb_pcie_msi_irq_handler,
-				       IRQF_SHARED, "histb-pcie-msi", pp);
-		if (ret) {
-			dev_err(dev, "cannot request MSI IRQ\n");
-			return ret;
-		}
 	}

 	hipcie->phy = devm_phy_get(dev, "phy");
Shawn Guo March 6, 2018, 8:23 a.m. UTC | #5
On Mon, Mar 05, 2018 at 06:22:53PM +0000, Gustavo Pimentel wrote:
> On 02/03/2018 18:03, Lorenzo Pieralisi wrote:
> > [+cc Shawn, Vignesh, Pratyush]
...
> > Is it correct to leave pcie-histb.c as-is ? I do not think it is.
> 
> At the time the first series of this patch came out there wasn't such driver and
> I just start working on this topic on December... Nevertheless, you are right it
> should follow the same procedure that others.
> 
> diff --git a/drivers/pci/dwc/pcie-histb.c b/drivers/pci/dwc/pcie-histb.c
> index 70b5c0b..5d47b90 100644
> --- a/drivers/pci/dwc/pcie-histb.c
> +++ b/drivers/pci/dwc/pcie-histb.c
> @@ -207,13 +207,6 @@ static struct dw_pcie_host_ops histb_pcie_host_ops = {
>  	.host_init = histb_pcie_host_init,
>  };
> 
> -static irqreturn_t histb_pcie_msi_irq_handler(int irq, void *arg)
> -{
> -	struct pcie_port *pp = arg;
> -
> -	return dw_handle_msi_irq(pp);
> -}
> -
>  static void histb_pcie_host_disable(struct histb_pcie *hipcie)
>  {
>  	reset_control_assert(hipcie->soft_reset);
> @@ -393,14 +386,6 @@ static int histb_pcie_probe(struct platform_device *pdev)
>  			dev_err(dev, "Failed to get MSI IRQ\n");
>  			return pp->msi_irq;
>  		}
> -
> -		ret = devm_request_irq(dev, pp->msi_irq,
> -				       histb_pcie_msi_irq_handler,
> -				       IRQF_SHARED, "histb-pcie-msi", pp);
> -		if (ret) {
> -			dev_err(dev, "cannot request MSI IRQ\n");
> -			return ret;
> -		}
>  	}
> 
>  	hipcie->phy = devm_phy_get(dev, "phy");

I manually applied the changes on top of Lorenzo's pci/dwc-msi branch,
and pcie-histb driver still works fine.

Tested-by: Shawn Guo <shawn.guo@linaro.org>