Hi Kishon,
On Mon, Mar 25, 2019 at 03:09:23PM +0530, Kishon Vijay Abraham I wrote:
> pci-keystone driver uses irq_of_parse_and_map to get irq number of
> error_irq. Use platform_get_irq instead and move platform_get_irq()
> and request_irq() of error_irq from ks_pcie_add_pcie_port to ks_pcie_probe
> since error_irq is common to both RC mode and EP mode.
Does this have any DT implications?
It's not obvious that platform_get_irq() and irq_of_parse_and_map() work
similarly or that they get the same result from DT.
I'm sure they *do*, but it would be nice to have some hints in the commit
log about why that's the case.
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> ---
> drivers/pci/controller/dwc/pci-keystone.c | 43 +++++++++--------------
> 1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 07f55b355d75..e50f8773e768 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -98,8 +98,6 @@ struct keystone_pcie {
> struct irq_domain *legacy_irq_domain;
> struct device_node *np;
>
> - int error_irq;
> -
> /* Application register space */
> void __iomem *va_app_base; /* DT 1st resource */
> struct resource app;
> @@ -743,12 +741,6 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> return ret;
> }
>
> -static void ks_pcie_setup_interrupts(struct keystone_pcie *ks_pcie)
> -{
> - if (ks_pcie->error_irq > 0)
> - ks_pcie_enable_error_irq(ks_pcie);
> -}
> -
> /*
> * When a PCI device does not exist during config cycles, keystone host gets a
> * bus error instead of returning 0xffffffff. This handler always returns 0
> @@ -810,7 +802,6 @@ static int __init ks_pcie_host_init(struct pcie_port *pp)
>
> ks_pcie_stop_link(pci);
> ks_pcie_setup_rc_app_regs(ks_pcie);
> - ks_pcie_setup_interrupts(ks_pcie);
> writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
> pci->dbi_base + PCI_IO_BASE);
>
> @@ -854,23 +845,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
> struct device *dev = &pdev->dev;
> int ret;
>
> - /*
> - * Index 0 is the platform interrupt for error interrupt
> - * from RC. This is optional.
> - */
> - ks_pcie->error_irq = irq_of_parse_and_map(ks_pcie->np, 0);
> - if (ks_pcie->error_irq <= 0)
> - dev_info(dev, "no error IRQ defined\n");
> - else {
> - ret = request_irq(ks_pcie->error_irq, ks_pcie_err_irq_handler,
> - IRQF_SHARED, "pcie-error-irq", ks_pcie);
> - if (ret < 0) {
> - dev_err(dev, "failed to request error IRQ %d\n",
> - ks_pcie->error_irq);
> - return ret;
> - }
> - }
> -
> pp->ops = &ks_pcie_host_ops;
> ret = ks_pcie_dw_host_init(ks_pcie);
> if (ret) {
> @@ -946,6 +920,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
> u32 num_lanes;
> char name[10];
> int ret;
> + int irq;
> int i;
>
> ks_pcie = devm_kzalloc(dev, sizeof(*ks_pcie), GFP_KERNEL);
> @@ -965,6 +940,20 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
> return ret;
> }
>
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "missing IRQ resource: %d\n", irq);
> + return irq;
> + }
> +
> + ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> + "ks-pcie-error-irq", ks_pcie);
> + if (ret < 0) {
> + dev_err(dev, "failed to request error IRQ %d\n",
> + irq);
> + return ret;
> + }
> +
> ret = of_property_read_u32(np, "num-lanes", &num_lanes);
> if (ret)
> num_lanes = 1;
> @@ -1020,6 +1009,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_get_sync;
>
> + ks_pcie_enable_error_irq(ks_pcie);
> +
> return 0;
>
> err_get_sync:
> --
> 2.17.1
>
