2022-04-13 15:00:06

by Peter Geis

[permalink] [raw]
Subject: [PATCH v2 2/4] PCI: dwc: rockchip: add legacy interrupt support

The legacy interrupts on the rk356x pcie controller are handled by a
single muxed interrupt. Add irq domain support to the pcie-dw-rockchip
driver to support the virtual domain.

Signed-off-by: Peter Geis <[email protected]>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 123 +++++++++++++++++-
1 file changed, 121 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index c9b341e55cbb..a8b1dc03d3cc 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -10,9 +10,12 @@

#include <linux/clk.h>
#include <linux/gpio/consumer.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -36,10 +39,13 @@
#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
#define PCIE_L0S_ENTRY 0x11
#define PCIE_CLIENT_GENERAL_CONTROL 0x0
+#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
+#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
#define PCIE_CLIENT_GENERAL_DEBUG 0x104
-#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
+#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
#define PCIE_CLIENT_LTSSM_STATUS 0x300
-#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
+#define PCIE_LEGACY_INT_ENABLE GENMASK(3, 0)
+#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)

struct rockchip_pcie {
@@ -51,6 +57,8 @@ struct rockchip_pcie {
struct reset_control *rst;
struct gpio_desc *rst_gpio;
struct regulator *vpcie3v3;
+ struct irq_domain *irq_domain;
+ raw_spinlock_t irq_lock;
};

static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip,
@@ -65,6 +73,105 @@ static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip,
writel_relaxed(val, rockchip->apb_base + reg);
}

+static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
+ struct device *dev = rockchip->pci.dev;
+ u32 reg;
+ u32 hwirq;
+ u32 virq;
+
+ chained_irq_enter(chip, desc);
+
+ reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_LEGACY);
+
+ while (reg) {
+ hwirq = ffs(reg) - 1;
+ reg &= ~BIT(hwirq);
+
+ virq = irq_find_mapping(rockchip->irq_domain, hwirq);
+ if (virq)
+ generic_handle_irq(virq);
+ else
+ dev_err(dev, "unexpected IRQ, INT%d\n", hwirq);
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static void rockchip_intx_mask(struct irq_data *data)
+{
+ struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
+ unsigned long flags;
+ u32 val;
+
+ /* disable legacy interrupts */
+ raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
+ val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
+ val |= PCIE_LEGACY_INT_ENABLE;
+ rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_LEGACY);
+ raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
+};
+
+static void rockchip_intx_unmask(struct irq_data *data)
+{
+ struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
+ unsigned long flags;
+ u32 val;
+
+ /* enable legacy interrupts */
+ raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
+ val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
+ val &= ~PCIE_LEGACY_INT_ENABLE;
+ rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_LEGACY);
+ raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
+};
+
+static struct irq_chip rockchip_intx_irq_chip = {
+ .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
+ .irq_mask = rockchip_intx_mask,
+ .irq_unmask = rockchip_intx_unmask,
+ .name = "INTx",
+};
+
+static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &rockchip_intx_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+ .map = rockchip_pcie_intx_map,
+};
+
+static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
+{
+ struct device *dev = rockchip->pci.dev;
+ struct device_node *intc;
+
+ raw_spin_lock_init(&rockchip->irq_lock);
+
+ intc = of_get_child_by_name(dev->of_node, "legacy-interrupt-controller");
+ if (!intc) {
+ dev_err(dev, "missing child interrupt-controller node\n");
+ return -EINVAL;
+ }
+
+ rockchip->irq_domain = irq_domain_add_linear(intc, PCI_NUM_INTX,
+ &intx_domain_ops, rockchip);
+ of_node_put(intc);
+ if (!rockchip->irq_domain) {
+ dev_err(dev, "failed to get a INTx IRQ domain\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
{
rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
@@ -111,7 +218,19 @@ static int rockchip_pcie_host_init(struct pcie_port *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+ struct device *dev = rockchip->pci.dev;
u32 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
+ int irq, ret;
+
+ irq = of_irq_get_byname(dev->of_node, "legacy");
+ if (irq < 0)
+ return irq;
+
+ irq_set_chained_handler_and_data(irq, rockchip_pcie_legacy_int_handler, rockchip);
+
+ ret = rockchip_pcie_init_irq_domain(rockchip);
+ if (ret < 0)
+ dev_err(dev, "failed to init irq domain\n");

/* LTSSM enable control mode */
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
--
2.25.1


2022-04-16 01:25:00

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] PCI: dwc: rockchip: add legacy interrupt support

On Thu, Apr 14, 2022 at 4:36 PM Marc Zyngier <[email protected]> wrote:
>
> On 2022-04-13 14:37, Peter Geis wrote:
> > The legacy interrupts on the rk356x pcie controller are handled by a
> > single muxed interrupt. Add irq domain support to the pcie-dw-rockchip
> > driver to support the virtual domain.
> >
> > Signed-off-by: Peter Geis <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 123 +++++++++++++++++-
> > 1 file changed, 121 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index c9b341e55cbb..a8b1dc03d3cc 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > @@ -10,9 +10,12 @@
> >
> > #include <linux/clk.h>
> > #include <linux/gpio/consumer.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdomain.h>
> > #include <linux/mfd/syscon.h>
> > #include <linux/module.h>
> > #include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> > @@ -36,10 +39,13 @@
> > #define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
> > #define PCIE_L0S_ENTRY 0x11
> > #define PCIE_CLIENT_GENERAL_CONTROL 0x0
> > +#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
> > +#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
> > #define PCIE_CLIENT_GENERAL_DEBUG 0x104
> > -#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > #define PCIE_CLIENT_LTSSM_STATUS 0x300
> > -#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
> > +#define PCIE_LEGACY_INT_ENABLE GENMASK(3, 0)
> > +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
> > #define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)
> >
> > struct rockchip_pcie {
> > @@ -51,6 +57,8 @@ struct rockchip_pcie {
> > struct reset_control *rst;
> > struct gpio_desc *rst_gpio;
> > struct regulator *vpcie3v3;
> > + struct irq_domain *irq_domain;
> > + raw_spinlock_t irq_lock;
> > };
> >
> > static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip,
> > @@ -65,6 +73,105 @@ static void rockchip_pcie_writel_apb(struct
> > rockchip_pcie *rockchip,
> > writel_relaxed(val, rockchip->apb_base + reg);
> > }
> >
> > +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
> > +{
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
> > + struct device *dev = rockchip->pci.dev;
> > + u32 reg;
> > + u32 hwirq;
> > + u32 virq;
> > +
> > + chained_irq_enter(chip, desc);
> > +
> > + reg = rockchip_pcie_readl_apb(rockchip,
> > PCIE_CLIENT_INTR_STATUS_LEGACY);
> > +
> > + while (reg) {
> > + hwirq = ffs(reg) - 1;
> > + reg &= ~BIT(hwirq);
>
> The whole construct would be better served by for_each_set_bit().

Ah, that's much nicer, thanks!

>
> > +
> > + virq = irq_find_mapping(rockchip->irq_domain, hwirq);
> > + if (virq)
> > + generic_handle_irq(virq);
>
> Please replace this with generic_handle_domain_irq().

I see the bulk conversion was done after I created this patch.
I will make this change.

>
> > + else
> > + dev_err(dev, "unexpected IRQ, INT%d\n", hwirq);
>
> This hardly serves any purpose. At best, this is a debug statement.
> At worse, this is a DoS. In any case, please remove it.

Done!

>
> > + }
> > +
> > + chained_irq_exit(chip, desc);
> > +}
> > +
> > +static void rockchip_intx_mask(struct irq_data *data)
> > +{
> > + struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
> > + unsigned long flags;
> > + u32 val;
> > +
> > + /* disable legacy interrupts */
> > + raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
> > + val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
> > + val |= PCIE_LEGACY_INT_ENABLE;
> > + rockchip_pcie_writel_apb(rockchip, val,
> > PCIE_CLIENT_INTR_MASK_LEGACY);
> > + raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
> > +};
> > +
> > +static void rockchip_intx_unmask(struct irq_data *data)
> > +{
> > + struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
> > + unsigned long flags;
> > + u32 val;
> > +
> > + /* enable legacy interrupts */
> > + raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
> > + val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
> > + val &= ~PCIE_LEGACY_INT_ENABLE;
> > + rockchip_pcie_writel_apb(rockchip, val,
> > PCIE_CLIENT_INTR_MASK_LEGACY);
> > + raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
> > +};
> > +
> > +static struct irq_chip rockchip_intx_irq_chip = {
> > + .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
> > + .irq_mask = rockchip_intx_mask,
> > + .irq_unmask = rockchip_intx_unmask,
> > + .name = "INTx",
>
> For consistency, please place 'name' at the top, and 'flags' at the end.

Will do.

>
> > +};
> > +
> > +static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned
> > int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + irq_set_chip_and_handler(irq, &rockchip_intx_irq_chip,
> > handle_simple_irq);
>
> Why isn't this a *level* handler, as per the PCI spec?

Fixed.

>
> > + irq_set_chip_data(irq, domain->host_data);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct irq_domain_ops intx_domain_ops = {
> > + .map = rockchip_pcie_intx_map,
> > +};
> > +
> > +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie
> > *rockchip)
> > +{
> > + struct device *dev = rockchip->pci.dev;
> > + struct device_node *intc;
> > +
> > + raw_spin_lock_init(&rockchip->irq_lock);
> > +
> > + intc = of_get_child_by_name(dev->of_node,
> > "legacy-interrupt-controller");
> > + if (!intc) {
> > + dev_err(dev, "missing child interrupt-controller node\n");
> > + return -EINVAL;
> > + }
> > +
> > + rockchip->irq_domain = irq_domain_add_linear(intc, PCI_NUM_INTX,
> > + &intx_domain_ops, rockchip);
> > + of_node_put(intc);
> > + if (!rockchip->irq_domain) {
> > + dev_err(dev, "failed to get a INTx IRQ domain\n");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
> > {
> > rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
> > @@ -111,7 +218,19 @@ static int rockchip_pcie_host_init(struct
> > pcie_port *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > + struct device *dev = rockchip->pci.dev;
> > u32 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
> > + int irq, ret;
> > +
> > + irq = of_irq_get_byname(dev->of_node, "legacy");
> > + if (irq < 0)
> > + return irq;
> > +
> > + irq_set_chained_handler_and_data(irq,
> > rockchip_pcie_legacy_int_handler, rockchip);
> > +
>
> Installing the handler before the domain is instantiated is
> unlikely to end well if you have a pending interrupt...

While the interrupts are masked at this stage, you make a valid point,
I'll move this a little south.

>
> > + ret = rockchip_pcie_init_irq_domain(rockchip);
> > + if (ret < 0)
> > + dev_err(dev, "failed to init irq domain\n");
> >
> > /* LTSSM enable control mode */
> > rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...

Thank you for your review.

2022-04-16 02:06:58

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] PCI: dwc: rockchip: add legacy interrupt support

On 2022-04-13 14:37, Peter Geis wrote:
> The legacy interrupts on the rk356x pcie controller are handled by a
> single muxed interrupt. Add irq domain support to the pcie-dw-rockchip
> driver to support the virtual domain.
>
> Signed-off-by: Peter Geis <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 123 +++++++++++++++++-
> 1 file changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index c9b341e55cbb..a8b1dc03d3cc 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -10,9 +10,12 @@
>
> #include <linux/clk.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> +#include <linux/of_irq.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -36,10 +39,13 @@
> #define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
> #define PCIE_L0S_ENTRY 0x11
> #define PCIE_CLIENT_GENERAL_CONTROL 0x0
> +#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
> +#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
> #define PCIE_CLIENT_GENERAL_DEBUG 0x104
> -#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> #define PCIE_CLIENT_LTSSM_STATUS 0x300
> -#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
> +#define PCIE_LEGACY_INT_ENABLE GENMASK(3, 0)
> +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
> #define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)
>
> struct rockchip_pcie {
> @@ -51,6 +57,8 @@ struct rockchip_pcie {
> struct reset_control *rst;
> struct gpio_desc *rst_gpio;
> struct regulator *vpcie3v3;
> + struct irq_domain *irq_domain;
> + raw_spinlock_t irq_lock;
> };
>
> static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip,
> @@ -65,6 +73,105 @@ static void rockchip_pcie_writel_apb(struct
> rockchip_pcie *rockchip,
> writel_relaxed(val, rockchip->apb_base + reg);
> }
>
> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
> + struct device *dev = rockchip->pci.dev;
> + u32 reg;
> + u32 hwirq;
> + u32 virq;
> +
> + chained_irq_enter(chip, desc);
> +
> + reg = rockchip_pcie_readl_apb(rockchip,
> PCIE_CLIENT_INTR_STATUS_LEGACY);
> +
> + while (reg) {
> + hwirq = ffs(reg) - 1;
> + reg &= ~BIT(hwirq);

The whole construct would be better served by for_each_set_bit().

> +
> + virq = irq_find_mapping(rockchip->irq_domain, hwirq);
> + if (virq)
> + generic_handle_irq(virq);

Please replace this with generic_handle_domain_irq().

> + else
> + dev_err(dev, "unexpected IRQ, INT%d\n", hwirq);

This hardly serves any purpose. At best, this is a debug statement.
At worse, this is a DoS. In any case, please remove it.

> + }
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static void rockchip_intx_mask(struct irq_data *data)
> +{
> + struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
> + unsigned long flags;
> + u32 val;
> +
> + /* disable legacy interrupts */
> + raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
> + val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
> + val |= PCIE_LEGACY_INT_ENABLE;
> + rockchip_pcie_writel_apb(rockchip, val,
> PCIE_CLIENT_INTR_MASK_LEGACY);
> + raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
> +};
> +
> +static void rockchip_intx_unmask(struct irq_data *data)
> +{
> + struct rockchip_pcie *rockchip = irq_data_get_irq_chip_data(data);
> + unsigned long flags;
> + u32 val;
> +
> + /* enable legacy interrupts */
> + raw_spin_lock_irqsave(&rockchip->irq_lock, flags);
> + val = HIWORD_UPDATE_BIT(PCIE_LEGACY_INT_ENABLE);
> + val &= ~PCIE_LEGACY_INT_ENABLE;
> + rockchip_pcie_writel_apb(rockchip, val,
> PCIE_CLIENT_INTR_MASK_LEGACY);
> + raw_spin_unlock_irqrestore(&rockchip->irq_lock, flags);
> +};
> +
> +static struct irq_chip rockchip_intx_irq_chip = {
> + .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
> + .irq_mask = rockchip_intx_mask,
> + .irq_unmask = rockchip_intx_unmask,
> + .name = "INTx",

For consistency, please place 'name' at the top, and 'flags' at the end.

> +};
> +
> +static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned
> int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_and_handler(irq, &rockchip_intx_irq_chip,
> handle_simple_irq);

Why isn't this a *level* handler, as per the PCI spec?

> + irq_set_chip_data(irq, domain->host_data);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops intx_domain_ops = {
> + .map = rockchip_pcie_intx_map,
> +};
> +
> +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie
> *rockchip)
> +{
> + struct device *dev = rockchip->pci.dev;
> + struct device_node *intc;
> +
> + raw_spin_lock_init(&rockchip->irq_lock);
> +
> + intc = of_get_child_by_name(dev->of_node,
> "legacy-interrupt-controller");
> + if (!intc) {
> + dev_err(dev, "missing child interrupt-controller node\n");
> + return -EINVAL;
> + }
> +
> + rockchip->irq_domain = irq_domain_add_linear(intc, PCI_NUM_INTX,
> + &intx_domain_ops, rockchip);
> + of_node_put(intc);
> + if (!rockchip->irq_domain) {
> + dev_err(dev, "failed to get a INTx IRQ domain\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
> {
> rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
> @@ -111,7 +218,19 @@ static int rockchip_pcie_host_init(struct
> pcie_port *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> + struct device *dev = rockchip->pci.dev;
> u32 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
> + int irq, ret;
> +
> + irq = of_irq_get_byname(dev->of_node, "legacy");
> + if (irq < 0)
> + return irq;
> +
> + irq_set_chained_handler_and_data(irq,
> rockchip_pcie_legacy_int_handler, rockchip);
> +

Installing the handler before the domain is instantiated is
unlikely to end well if you have a pending interrupt...

> + ret = rockchip_pcie_init_irq_domain(rockchip);
> + if (ret < 0)
> + dev_err(dev, "failed to init irq domain\n");
>
> /* LTSSM enable control mode */
> rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);

Thanks,

M.
--
Jazz is not dead. It just smells funny...