2024-03-20 16:32:24

by Andy Shevchenko

Subject: [PATCH v1 1/1] idma64: Don't try to serve interrupts when device is powered off

When iDMA 64-bit device is powered off, the IRQ status register
is all 1:s. This is never happen in real case and signalling that
the device is simply powered off. Don't try to serve interrupts
that are not ours.

Fixes: 667dfed98615 ("dmaengine: add a driver for Intel integrated DMA 64-bit")
Reported-by: Heiner Kallweit <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/dma/idma64.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/dma/idma64.c b/drivers/dma/idma64.c
index 78a938969d7d..1b60e73d9322 100644
--- a/drivers/dma/idma64.c
+++ b/drivers/dma/idma64.c
@@ -173,6 +173,10 @@ static irqreturn_t idma64_irq(int irq, void *dev)

dev_vdbg(idma64->dma.dev, "%s: status=%#x\n", __func__, status);

+ /* Since IRQ may be shared, check if DMA controller is powered on */
+ if (status == GENMASK(31, 0))
+ return IRQ_NONE;
+
/* Check if we have any interrupt from the DMA controller */
if (!status)
return IRQ_NONE;
--
2.43.0.rc1.1.gbec44491f096

2024-03-20 16:36:24

by Andy Shevchenko

Subject: Re: [PATCH v1 1/1] idma64: Don't try to serve interrupts when device is powered off

On Wed, Mar 20, 2024 at 06:32:10PM +0200, Andy Shevchenko wrote:
> When iDMA 64-bit device is powered off, the IRQ status register
> is all 1:s. This is never happen in real case and signalling that
> the device is simply powered off. Don't try to serve interrupts
> that are not ours.

..

> dev_vdbg(idma64->dma.dev, "%s: status=%#x\n", __func__, status);

Dunno if I actually also need to move this message to after the checks...

> + /* Since IRQ may be shared, check if DMA controller is powered on */
> + if (status == GENMASK(31, 0))
> + return IRQ_NONE;
> +
> /* Check if we have any interrupt from the DMA controller */
> if (!status)
> return IRQ_NONE;

--
With Best Regards,
Andy Shevchenko

2024-03-21 11:49:34

by Heiner Kallweit

Subject: Re: [PATCH v1 1/1] idma64: Don't try to serve interrupts when device is powered off

On 20.03.2024 17:32, Andy Shevchenko wrote:
> diff --git a/drivers/dma/idma64.c b/drivers/dma/idma64.c
> index 78a938969d7d..1b60e73d9322 100644
> --- a/drivers/dma/idma64.c
> +++ b/drivers/dma/idma64.c
> @@ -173,6 +173,10 @@ static irqreturn_t idma64_irq(int irq, void *dev)
>
> dev_vdbg(idma64->dma.dev, "%s: status=%#x\n", __func__, status);
>
> + /* Since IRQ may be shared, check if DMA controller is powered on */
> + if (status == GENMASK(31, 0))
> + return IRQ_NONE;
> +
> /* Check if we have any interrupt from the DMA controller */
> if (!status)
> return IRQ_NONE;
> -- 2.43.0.rc1.1.gbec44491f096

Tested-by: Heiner Kallweit <[email protected]>

2024-03-21 12:02:45

by Andy Shevchenko

Subject: Re: [PATCH v1 1/1] idma64: Don't try to serve interrupts when device is powered off

On Thu, Mar 21, 2024 at 12:49:20PM +0100, Heiner Kallweit wrote:
> On 20.03.2024 17:32, Andy Shevchenko wrote:

..

> > dev_vdbg(idma64->dma.dev, "%s: status=%#x\n", __func__, status);
> >
> > + /* Since IRQ may be shared, check if DMA controller is powered on */
> > + if (status == GENMASK(31, 0))
> > + return IRQ_NONE;
> > +
> > /* Check if we have any interrupt from the DMA controller */
> > if (!status)
> > return IRQ_NONE;
> > -- 2.43.0.rc1.1.gbec44491f096
>
> Tested-by: Heiner Kallweit <[email protected]>

Thank you! I think I'll move the test to be before the debug message as it
makes no sense to print when we have powered off device.
Nevertheless, I'll take your tag if no objections for v2.

--
With Best Regards,
Andy Shevchenko