Patchwork [v2,2/9] PCI: keystone: Modify legacy_irq_handler to check the IRQ_STATUS of INTA/B/C/D

login
register
mail settings
Submitter Kishon Vijay Abraham I
Date Feb. 7, 2019, 11:09 a.m.
Message ID <20190207110924.30716-3-kishon@ti.com>
Download mbox | patch
Permalink /patch/720303/
State New
Headers show

Comments

Kishon Vijay Abraham I - Feb. 7, 2019, 11:09 a.m.
The legacy interrupt handler directly checks the IRQ_STATUS register
corresponding to a interrupt line inorder to invoke generic_handle_irq.

While this is okay for K2G platform which has separate interrupt line for
each of the 4 legacy interrupts, AM654 which uses the same PCIe wrapper
has a single interrupt line for all the legacy interrupts. So for AM654
the interrupt handler won't be able to directly check the IRQ_STATUS
register corresponding to the interrupt line.

Also the legacy interrupt handler uses 'virq' obtained from
irq_of_parse_and_map to find the correct interrupt line which raised the
interrupt. There is no guarantee that virq assigned for contiguous hardware
irq will be contiguous and the interrupt handler might end up checking
the wrong IRQ_STATUS register.

In order to overcome the above issues, read the IRQ_STATUS register of
all the 4 legacy interrupts to determine which interrupt was raised.

Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
Lorenzo Pieralisi - Feb. 7, 2019, 4:15 p.m.
On Thu, Feb 07, 2019 at 04:39:17PM +0530, Kishon Vijay Abraham I wrote:
> The legacy interrupt handler directly checks the IRQ_STATUS register
> corresponding to a interrupt line inorder to invoke generic_handle_irq.
> 
> While this is okay for K2G platform which has separate interrupt line for
> each of the 4 legacy interrupts, AM654 which uses the same PCIe wrapper
> has a single interrupt line for all the legacy interrupts. So for AM654
> the interrupt handler won't be able to directly check the IRQ_STATUS
> register corresponding to the interrupt line.
> 
> Also the legacy interrupt handler uses 'virq' obtained from
> irq_of_parse_and_map to find the correct interrupt line which raised the
> interrupt. There is no guarantee that virq assigned for contiguous hardware
> irq will be contiguous and the interrupt handler might end up checking
> the wrong IRQ_STATUS register.
> 
> In order to overcome the above issues, read the IRQ_STATUS register of
> all the 4 legacy interrupts to determine which interrupt was raised.
> 
> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 5286a480f76b..4cf9849d5a1d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -214,16 +214,11 @@ static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
>  {
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 pending;
>  	int virq;
>  
> -	pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
> -
> -	if (BIT(0) & pending) {
> -		virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
> -		dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
> -		generic_handle_irq(virq);
> -	}
> +	virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
> +	dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
> +	generic_handle_irq(virq);
>  
>  	/* EOI the INTx interrupt */
>  	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
> @@ -607,8 +602,9 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>  	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned int irq_no;
> +	u32 reg;
>  
>  	dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>  
> @@ -618,7 +614,13 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>  	 * ack operation.
>  	 */
>  	chained_irq_enter(chip, desc);
> -	ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
> +	for (irq_no = 0; irq_no < PCI_NUM_INTX; irq_no++) {

I understand the aim of this code but now on platforms where there
is a 1:1 relationship between Linux IRQ and INTX this loop has
steps carried out for nothing.

If I understand the code correctly what this code does is force
looping over INTX status regs regardless of what linux IRQ number was
actually active.

You could do something faster by creating a matrix LinuxIRQ x INTx to
detect what INTx status register should actually be checked.

This seems overkill to me but it is not that complicated to implement
and may clarify the code (and avoid reading up to three registers for
nothing on the IRQ code path, which can make things faster too).

Lorenzo

> +		reg = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(irq_no));
> +		if (!(reg & INTx_EN))
> +			continue;
> +		ks_pcie_handle_legacy_irq(ks_pcie, irq_no);
> +	}
> +
>  	chained_irq_exit(chip, desc);
>  }
>  
> -- 
> 2.17.1
>
Bjorn Helgaas - Feb. 7, 2019, 8:52 p.m.
In the subject, "legacy_irq_handler" looks like it's supposed to be a
function name, but it's not.  Maybe something like:

  PCI: keystone: Check INTA/B/C/D IRQ_STATUS in ks_pcie_legacy_irq_handler()

On Thu, Feb 07, 2019 at 04:39:17PM +0530, Kishon Vijay Abraham I wrote:
> The legacy interrupt handler directly checks the IRQ_STATUS register
> corresponding to a interrupt line inorder to invoke generic_handle_irq.

s/The legacy interrupt handler/ks_pcie_handle_legacy_irq()/ ?
s/to a/to an/
s/inorder/in order/
s/generic_handle_irq/generic_handle_irq()/

> While this is okay for K2G platform which has separate interrupt line for
> each of the 4 legacy interrupts, AM654 which uses the same PCIe wrapper
> has a single interrupt line for all the legacy interrupts. So for AM654
> the interrupt handler won't be able to directly check the IRQ_STATUS
> register corresponding to the interrupt line.

s/platform which/platform, which/
s/separate interrupt line/separate interrupt lines/
s/AM654 which/AM654, which/
s/PCIe wrapper/PCIe wrapper,/
s/interrupt line for all/interrupt line shared by all/


> Also the legacy interrupt handler uses 'virq' obtained from
> irq_of_parse_and_map to find the correct interrupt line which raised the
> interrupt. There is no guarantee that virq assigned for contiguous hardware
> irq will be contiguous and the interrupt handler might end up checking
> the wrong IRQ_STATUS register.

s/irq_of_parse_and_map/irq_of_parse_and_map()
s/irq will/IRQ will/

> In order to overcome the above issues, read the IRQ_STATUS register of
> all the 4 legacy interrupts to determine which interrupt was raised.
> 
> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 5286a480f76b..4cf9849d5a1d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -214,16 +214,11 @@ static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
>  {
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 pending;
>  	int virq;
>  
> -	pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
> -
> -	if (BIT(0) & pending) {
> -		virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
> -		dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
> -		generic_handle_irq(virq);
> -	}
> +	virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
> +	dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
> +	generic_handle_irq(virq);
>  
>  	/* EOI the INTx interrupt */
>  	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
> @@ -607,8 +602,9 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>  	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned int irq_no;
> +	u32 reg;
>  
>  	dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>  
> @@ -618,7 +614,13 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>  	 * ack operation.
>  	 */
>  	chained_irq_enter(chip, desc);
> -	ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
> +	for (irq_no = 0; irq_no < PCI_NUM_INTX; irq_no++) {
> +		reg = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(irq_no));
> +		if (!(reg & INTx_EN))
> +			continue;
> +		ks_pcie_handle_legacy_irq(ks_pcie, irq_no);

It's too bad that reading IRQ_STATUS and writing IRQ_EOI are now in
separate functions.  It's nice to have them together for code auditing
purposes.

Could maybe accumulate a mask of which INTx bits are set and call
ks_pcie_handle_legacy_irq() only once with that mask?  Of course, then
you'd need another loop in ks_pcie_handle_legacy_irq().

> +	}
> +
>  	chained_irq_exit(chip, desc);
>  }
>  
> -- 
> 2.17.1
>
Kishon Vijay Abraham I - Feb. 8, 2019, 4:32 a.m.
Hi Lorenzo,

On 07/02/19 9:45 PM, Lorenzo Pieralisi wrote:
> On Thu, Feb 07, 2019 at 04:39:17PM +0530, Kishon Vijay Abraham I wrote:
>> The legacy interrupt handler directly checks the IRQ_STATUS register
>> corresponding to a interrupt line inorder to invoke generic_handle_irq.
>>
>> While this is okay for K2G platform which has separate interrupt line for
>> each of the 4 legacy interrupts, AM654 which uses the same PCIe wrapper
>> has a single interrupt line for all the legacy interrupts. So for AM654
>> the interrupt handler won't be able to directly check the IRQ_STATUS
>> register corresponding to the interrupt line.
>>
>> Also the legacy interrupt handler uses 'virq' obtained from
>> irq_of_parse_and_map to find the correct interrupt line which raised the
>> interrupt. There is no guarantee that virq assigned for contiguous hardware
>> irq will be contiguous and the interrupt handler might end up checking
>> the wrong IRQ_STATUS register.
>>
>> In order to overcome the above issues, read the IRQ_STATUS register of
>> all the 4 legacy interrupts to determine which interrupt was raised.
>>
>> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
>> index 5286a480f76b..4cf9849d5a1d 100644
>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>> @@ -214,16 +214,11 @@ static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
>>  {
>>  	struct dw_pcie *pci = ks_pcie->pci;
>>  	struct device *dev = pci->dev;
>> -	u32 pending;
>>  	int virq;
>>  
>> -	pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
>> -
>> -	if (BIT(0) & pending) {
>> -		virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
>> -		dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
>> -		generic_handle_irq(virq);
>> -	}
>> +	virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
>> +	dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
>> +	generic_handle_irq(virq);
>>  
>>  	/* EOI the INTx interrupt */
>>  	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
>> @@ -607,8 +602,9 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>>  	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>>  	struct dw_pcie *pci = ks_pcie->pci;
>>  	struct device *dev = pci->dev;
>> -	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>>  	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	unsigned int irq_no;
>> +	u32 reg;
>>  
>>  	dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>>  
>> @@ -618,7 +614,13 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>>  	 * ack operation.
>>  	 */
>>  	chained_irq_enter(chip, desc);
>> -	ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
>> +	for (irq_no = 0; irq_no < PCI_NUM_INTX; irq_no++) {
> 
> I understand the aim of this code but now on platforms where there
> is a 1:1 relationship between Linux IRQ and INTX this loop has
> steps carried out for nothing.
> 
> If I understand the code correctly what this code does is force
> looping over INTX status regs regardless of what linux IRQ number was
> actually active.

right. This driver is used by 2 platforms K2G and AM654 (The patches are there
on the list). K2G has 4 interrupt lines for each of the 4 legacy interrups
while AM654 has a single interrupt line. One of the purpose of this patch is to
have a single legacy interrupt handler for both K2G and AM654.
> 
> You could do something faster by creating a matrix LinuxIRQ x INTx to
> detect what INTx status register should actually be checked.
> 
> This seems overkill to me but it is not that complicated to implement
> and may clarify the code (and avoid reading up to three registers for
> nothing on the IRQ code path, which can make things faster too).

Agreed. But that's not possible for AM654 which has a single interrupt line and
all the registers has to be read to identify the interrupt source.

Thanks
Kishon
Kishon Vijay Abraham I - Feb. 8, 2019, 11:09 a.m.
Hi Bjorn,

On 08/02/19 2:22 AM, Bjorn Helgaas wrote:
> In the subject, "legacy_irq_handler" looks like it's supposed to be a
> function name, but it's not.  Maybe something like:
> 
>   PCI: keystone: Check INTA/B/C/D IRQ_STATUS in ks_pcie_legacy_irq_handler()
> 
> On Thu, Feb 07, 2019 at 04:39:17PM +0530, Kishon Vijay Abraham I wrote:
>> The legacy interrupt handler directly checks the IRQ_STATUS register
>> corresponding to a interrupt line inorder to invoke generic_handle_irq.
> 
> s/The legacy interrupt handler/ks_pcie_handle_legacy_irq()/ ?
> s/to a/to an/
> s/inorder/in order/
> s/generic_handle_irq/generic_handle_irq()/
> 
>> While this is okay for K2G platform which has separate interrupt line for
>> each of the 4 legacy interrupts, AM654 which uses the same PCIe wrapper
>> has a single interrupt line for all the legacy interrupts. So for AM654
>> the interrupt handler won't be able to directly check the IRQ_STATUS
>> register corresponding to the interrupt line.
> 
> s/platform which/platform, which/
> s/separate interrupt line/separate interrupt lines/
> s/AM654 which/AM654, which/
> s/PCIe wrapper/PCIe wrapper,/
> s/interrupt line for all/interrupt line shared by all/
> 
> 
>> Also the legacy interrupt handler uses 'virq' obtained from
>> irq_of_parse_and_map to find the correct interrupt line which raised the
>> interrupt. There is no guarantee that virq assigned for contiguous hardware
>> irq will be contiguous and the interrupt handler might end up checking
>> the wrong IRQ_STATUS register.
> 
> s/irq_of_parse_and_map/irq_of_parse_and_map()
> s/irq will/IRQ will/
> 
>> In order to overcome the above issues, read the IRQ_STATUS register of
>> all the 4 legacy interrupts to determine which interrupt was raised.
>>
>> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
>> index 5286a480f76b..4cf9849d5a1d 100644
>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>> @@ -214,16 +214,11 @@ static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
>>  {
>>  	struct dw_pcie *pci = ks_pcie->pci;
>>  	struct device *dev = pci->dev;
>> -	u32 pending;
>>  	int virq;
>>  
>> -	pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
>> -
>> -	if (BIT(0) & pending) {
>> -		virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
>> -		dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
>> -		generic_handle_irq(virq);
>> -	}
>> +	virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
>> +	dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
>> +	generic_handle_irq(virq);
>>  
>>  	/* EOI the INTx interrupt */
>>  	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
>> @@ -607,8 +602,9 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>>  	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>>  	struct dw_pcie *pci = ks_pcie->pci;
>>  	struct device *dev = pci->dev;
>> -	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>>  	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	unsigned int irq_no;
>> +	u32 reg;
>>  
>>  	dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>>  
>> @@ -618,7 +614,13 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>>  	 * ack operation.
>>  	 */
>>  	chained_irq_enter(chip, desc);
>> -	ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
>> +	for (irq_no = 0; irq_no < PCI_NUM_INTX; irq_no++) {
>> +		reg = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(irq_no));
>> +		if (!(reg & INTx_EN))
>> +			continue;
>> +		ks_pcie_handle_legacy_irq(ks_pcie, irq_no);
> 
> It's too bad that reading IRQ_STATUS and writing IRQ_EOI are now in
> separate functions.  It's nice to have them together for code auditing
> purposes.
> 
> Could maybe accumulate a mask of which INTx bits are set and call
> ks_pcie_handle_legacy_irq() only once with that mask?  Of course, then
> you'd need another loop in ks_pcie_handle_legacy_irq().

Patch "5" of this series "PCI: keystone: Cleanup ks_pcie_msi_irq_handler and
ks_pcie_legacy_irq_handler" does more cleanup in irq handler and there
ks_pcie_handle_legacy_irq is removed and everything is done in a single function.

I have to anyway revisit legacy irq handler and have some of the register
writes move to proper irq_chip callbacks as Lorenzo commented in [1]

[1] -> https://lkml.org/lkml/2019/1/24/333

Thanks
Kishon

Patch

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 5286a480f76b..4cf9849d5a1d 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -214,16 +214,11 @@  static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
 {
 	struct dw_pcie *pci = ks_pcie->pci;
 	struct device *dev = pci->dev;
-	u32 pending;
 	int virq;
 
-	pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
-
-	if (BIT(0) & pending) {
-		virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
-		dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
-		generic_handle_irq(virq);
-	}
+	virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
+	dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
+	generic_handle_irq(virq);
 
 	/* EOI the INTx interrupt */
 	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
@@ -607,8 +602,9 @@  static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
 	struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
 	struct dw_pcie *pci = ks_pcie->pci;
 	struct device *dev = pci->dev;
-	u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
 	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned int irq_no;
+	u32 reg;
 
 	dev_dbg(dev, ": Handling legacy irq %d\n", irq);
 
@@ -618,7 +614,13 @@  static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
 	 * ack operation.
 	 */
 	chained_irq_enter(chip, desc);
-	ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
+	for (irq_no = 0; irq_no < PCI_NUM_INTX; irq_no++) {
+		reg = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(irq_no));
+		if (!(reg & INTx_EN))
+			continue;
+		ks_pcie_handle_legacy_irq(ks_pcie, irq_no);
+	}
+
 	chained_irq_exit(chip, desc);
 }