Skip to contentSkip to conversation

idma64: Don't try to serve interrupts when device is powered off

PATCH Superseded Andy Shevchenko 2 participants 4 messages Mar 20, 2024 – Mar 21, 2024

Subsystems (2)
dmaengine Intel LPSS (Low Power SubSystem) drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Andy Shevchenko

author reviewer
#0 lore raw

PATCH 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] open here
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/dma/idma64.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/dma/idma64.c +4 -0
index 78a938969d7d..1b60e73d9322 100644
@@ -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;
Signature · 1 line
2.43.0.rc1.1.gbec44491f096
author reviewer
#1 lore raw
Quoted 4 lines · Andy Shevchenko

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.

...

Quoted 1 line

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...

Quoted 7 lines
drivers/dma/idma64.c +4 -0
+	/* 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;
Signature · 2 lines
With Best Regards,
Andy Shevchenko
Tested-by on v1 1/1
#2 lore raw
In reply to Andy Shevchenko
Quoted 6 lines · Andy Shevchenko
drivers/dma/idma64.c +0 -0
index 78a938969d7d..1b60e73d9322 100644
@@ -173,6 +173,10 @@ static irqreturn_t idma64_irq(int irq, void *dev)

drivers/dma/idma64.c static irqreturn_t idma64_irq(int irq, void *dev)

Quoted 10 lines
drivers/dma/idma64.c +4 -1
 	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]>
author reviewer
#3 lore raw
Quoted 1 line · Heiner Kallweit

On 20.03.2024 17:32, Andy Shevchenko wrote:

...

Quoted 12 lines

drivers/dma/idma64.c static irqreturn_t idma64_irq(int irq, void *dev)

Quoted 10 lines
drivers/dma/idma64.c +4 -1
 	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.

Signature · 2 lines
With Best Regards,
Andy Shevchenko