Skip to contentSkip to conversation

mfd: arizona: Check if AOD interrupts are pending before dispatching

PATCH Superseded Richard Fitzgerald 2 participants 2 messages Apr 14, 2016 – Apr 25, 2016

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mfd: arizona: Check if AOD interrupts are pending before dispatching

Previously the arizona_irq_thread implementation would call handle_nested_irqs() to handle AOD interrupts without checking if any were actually pending. The kernel will see these as spurious IRQs and will eventually disable the IRQ.

This patch ensures we only launch the nested handler if there are AOD interrupts pending in the codec.

---
 drivers/mfd/arizona-irq.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/mfd/arizona-irq.c +18 -3
index edeb495..edaf592 100644
@@ -97,7 +97,7 @@ static irqreturn_t arizona_irq_thread(int irq, void *data)
 {
 	struct arizona *arizona = data;
 	bool poll;
-	unsigned int val;
+	unsigned int val, nest_irq;
 	int ret;
 
 	ret = pm_runtime_get_sync(arizona->dev);
@@ -109,8 +109,23 @@ static irqreturn_t arizona_irq_thread(int irq, void *data)
 	do {
 		poll = false;
 
-		if (arizona->aod_irq_chip)
-			handle_nested_irq(irq_find_mapping(arizona->virq, 0));
+		if (arizona->aod_irq_chip) {
+			/*
+			 * Check the AOD status register to determine whether
+			 * the nested IRQ handler should be called.
+			 */
+			ret = regmap_read(arizona->regmap,
+					  ARIZONA_AOD_IRQ1,
+					  &val);
+			if (ret == 0 && val != 0) {
+				nest_irq = irq_find_mapping(arizona->virq, 0);
+				handle_nested_irq(nest_irq);
+			} else if (ret != 0) {
+				dev_err(arizona->dev,
+					"Failed to read AOD IRQ1 %d\n",
+					ret);
+			}
+		}
 
 		/*
 		 * Check if one of the main interrupts is asserted and only
Signature · 1 line
1.9.1
#1 lore raw
Quoted 31 lines · Richard Fitzgerald

Previously the arizona_irq_thread implementation would call handle_nested_irqs() to handle AOD interrupts without checking if any were actually pending. The kernel will see these as spurious IRQs and will eventually disable the IRQ.

This patch ensures we only launch the nested handler if there are AOD interrupts pending in the codec.

---
 drivers/mfd/arizona-irq.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/mfd/arizona-irq.c +1 -1
index edeb495..edaf592 100644
@@ -97,7 +97,7 @@ static irqreturn_t arizona_irq_thread(int irq, void *data)
 {
 	struct arizona *arizona = data;
 	bool poll;
-	unsigned int val;
+	unsigned int val, nest_irq;
 	int ret;
 
 	ret = pm_runtime_get_sync(arizona->dev);
@@ -109,8 +109,23 @@ static irqreturn_t arizona_irq_thread(int irq, void *data)
 	do {
 		poll = false;
 

drivers/mfd/arizona-irq.c static irqreturn_t arizona_irq_thread(int irq, void *data) · line 119

Quoted 10 lines
drivers/mfd/arizona-irq.c +8 -2
-		if (arizona->aod_irq_chip)
-			handle_nested_irq(irq_find_mapping(arizona->virq, 0));
+		if (arizona->aod_irq_chip) {
+			/*
+			 * Check the AOD status register to determine whether
+			 * the nested IRQ handler should be called.
+			 */
+			ret = regmap_read(arizona->regmap,
+					  ARIZONA_AOD_IRQ1,
+					  &val);

Nit: Pop this on the line above.

drivers/mfd/arizona-irq.c static irqreturn_t arizona_irq_thread(int irq, void *data) · line 124

Quoted 5 lines
drivers/mfd/arizona-irq.c +5 -0
+			if (ret == 0 && val != 0) {
+				nest_irq = irq_find_mapping(arizona->virq, 0);
+				handle_nested_irq(nest_irq);
+			} else if (ret != 0) {
+				dev_err(arizona->dev,

It's only a valid 'error' if you're return an error condition. AFAICT, you're not doing that, so use dev_warn() instead.

drivers/mfd/arizona-irq.c static irqreturn_t arizona_irq_thread(int irq, void *data) · line 126

Quoted 2 lines
drivers/mfd/arizona-irq.c +2 -0
+					"Failed to read AOD IRQ1 %d\n",
+					ret);

Nit: Pop this on the line above.

drivers/mfd/arizona-irq.c static irqreturn_t arizona_irq_thread(int irq, void *data) · line 128

Quoted 2 lines
drivers/mfd/arizona-irq.c +2 -0
+			}
+		}

I think the whole thing would be better written like:

		if (arizona->aod_irq_chip) {
			/*
			 * Check the AOD status register to determine whether
			 * the nested IRQ handler should be called.
			 */
			ret = regmap_read(arizona->regmap,
					  ARIZONA_AOD_IRQ1, &val);
			if (ret)
				dev_warn(arizona->dev,
					"Failed to read AOD IRQ1 %d\n",	ret);
			else if (val)
				handle_nested_irq(
					irq_find_mapping(arizona->virq, 0));
		}
Quoted 3 lines
drivers/mfd/arizona-irq.c +0 -0
 		/*
 		 * Check if one of the main interrupts is asserted and only
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog