[PATCH v1 1/1] hsu_dma_pci: disable spurious interrupt

[PATCH v1 1/1] hsu_dma_pci: disable spurious interrupt

[Ferry Toth]

On Intel Tangier B0 and Anniedale the interrupt line, disregarding
to have different numbers, is shared between HSU DMA and UART IPs.
Thus on such SoCs we are expecting that IRQ handler is called in
UART driver only. hsu_pci_irq was handling the spurious interrupt
from HSU DMA by returning immediately. This wastes CPU time and
since HSU DMA and HSU UART interrupt occur simultaneously they race
to be handled causing delay to the HSU UART interrupt handling.
Fix this by disabling the interrupt entirely.

Fixes: 4831e0d9054c ("serial: 8250_mid: handle interrupt correctly in DMA case")
Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/dma/hsu/pci.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
index 07cc7320a614..9045a6f7f589 100644
--- a/drivers/dma/hsu/pci.c
+++ b/drivers/dma/hsu/pci.c
@@ -26,22 +26,12 @@
 static irqreturn_t hsu_pci_irq(int irq, void *dev)
 {
 	struct hsu_dma_chip *chip = dev;
-	struct pci_dev *pdev = to_pci_dev(chip->dev);
 	u32 dmaisr;
 	u32 status;
 	unsigned short i;
 	int ret = 0;
 	int err;
 
-	/*
-	 * On Intel Tangier B0 and Anniedale the interrupt line, disregarding
-	 * to have different numbers, is shared between HSU DMA and UART IPs.
-	 * Thus on such SoCs we are expecting that IRQ handler is called in
-	 * UART driver only.
-	 */
-	if (pdev->device == PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA)
-		return IRQ_HANDLED;
-
 	dmaisr = readl(chip->regs + HSU_PCI_DMAISR);
 	for (i = 0; i < chip->hsu->nr_channels; i++) {
 		if (dmaisr & 0x1) {
@@ -105,6 +95,17 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (ret)
 		goto err_register_irq;
 
+	/*
+	 * On Intel Tangier B0 and Anniedale the interrupt line, disregarding
+	 * to have different numbers, is shared between HSU DMA and UART IPs.
+	 * Thus on such SoCs we are expecting that IRQ handler is called in
+	 * UART driver only. Instead of handling the spurious interrupt
+	 * from HSU DMA here and waste CPU time and delay HSU UART interrupt
+	 * handling, disable the interrupt entirely.
+	 */
+	if (pdev->device == PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA)
+		disable_irq_nosync(chip->irq);
+
 	pci_set_drvdata(pdev, chip);
 
 	return 0;
-- 
2.27.0

Re: [PATCH v1 1/1] hsu_dma_pci: disable spurious interrupt

[Andy Shevchenko]

On Wed, Jan 13, 2021 at 5:23 AM Ferry Toth <ftoth@exalondelft.nl> wrote:
>
> On Intel Tangier B0 and Anniedale the interrupt line, disregarding
> to have different numbers, is shared between HSU DMA and UART IPs.
> Thus on such SoCs we are expecting that IRQ handler is called in
> UART driver only. hsu_pci_irq was handling the spurious interrupt

hsu_pci_irq()

> from HSU DMA by returning immediately. This wastes CPU time and
> since HSU DMA and HSU UART interrupt occur simultaneously they race
> to be handled causing delay to the HSU UART interrupt handling.
> Fix this by disabling the interrupt entirely.

Title should be "dmaengine: hsu: ..."

After addressing above
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Fixes: 4831e0d9054c ("serial: 8250_mid: handle interrupt correctly in DMA case")
> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
> ---
>  drivers/dma/hsu/pci.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
> index 07cc7320a614..9045a6f7f589 100644
> --- a/drivers/dma/hsu/pci.c
> +++ b/drivers/dma/hsu/pci.c
> @@ -26,22 +26,12 @@
>  static irqreturn_t hsu_pci_irq(int irq, void *dev)
>  {
>         struct hsu_dma_chip *chip = dev;
> -       struct pci_dev *pdev = to_pci_dev(chip->dev);
>         u32 dmaisr;
>         u32 status;
>         unsigned short i;
>         int ret = 0;
>         int err;
>
> -       /*
> -        * On Intel Tangier B0 and Anniedale the interrupt line, disregarding
> -        * to have different numbers, is shared between HSU DMA and UART IPs.
> -        * Thus on such SoCs we are expecting that IRQ handler is called in
> -        * UART driver only.
> -        */
> -       if (pdev->device == PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA)
> -               return IRQ_HANDLED;
> -
>         dmaisr = readl(chip->regs + HSU_PCI_DMAISR);
>         for (i = 0; i < chip->hsu->nr_channels; i++) {
>                 if (dmaisr & 0x1) {
> @@ -105,6 +95,17 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>         if (ret)
>                 goto err_register_irq;
>
> +       /*
> +        * On Intel Tangier B0 and Anniedale the interrupt line, disregarding
> +        * to have different numbers, is shared between HSU DMA and UART IPs.
> +        * Thus on such SoCs we are expecting that IRQ handler is called in
> +        * UART driver only. Instead of handling the spurious interrupt
> +        * from HSU DMA here and waste CPU time and delay HSU UART interrupt
> +        * handling, disable the interrupt entirely.
> +        */
> +       if (pdev->device == PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA)
> +               disable_irq_nosync(chip->irq);
> +
>         pci_set_drvdata(pdev, chip);
>
>         return 0;
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] hsu_dma_pci: disable spurious interrupt

[Vinod Koul]

On 13-01-21, 15:05, Andy Shevchenko wrote:
> On Wed, Jan 13, 2021 at 5:23 AM Ferry Toth <ftoth@exalondelft.nl> wrote:
> >
> > On Intel Tangier B0 and Anniedale the interrupt line, disregarding
> > to have different numbers, is shared between HSU DMA and UART IPs.
> > Thus on such SoCs we are expecting that IRQ handler is called in
> > UART driver only. hsu_pci_irq was handling the spurious interrupt
> 
> hsu_pci_irq()
> 
> > from HSU DMA by returning immediately. This wastes CPU time and
> > since HSU DMA and HSU UART interrupt occur simultaneously they race
> > to be handled causing delay to the HSU UART interrupt handling.
> > Fix this by disabling the interrupt entirely.
> 
> Title should be "dmaengine: hsu: ..."

Fixed that up while applying, so applied

-- 
~Vinod