mfd: arizona: Fix lockdep recursion warning on set_irq_wake

Lockdep explicitly sets all the irq_desc locks as a single lock-class, which causes a "possible recursive locking detected" warning when we attempt to propagate the IRQ wake to our parent IRQ in arizona_irq_set_wake. Although this appears to be a false positive because an IRQ is unlikely to be its own parent, this was clearly intentionally prohibited.

To avoid this lockdep warning, take a cue from the regmap-irq system, and add bus lock callbacks on the IRQ chip and propagate the wake in the bus unlock which will happen after the desc lock has been released and thus avoid the issue.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-irq.c        | 35 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/arizona/core.h |  3 +++
 2 files changed, 37 insertions(+), 1 deletion(-)

drivers/mfd/arizona-irq.c +34 -1

index 5fef014..6497a65 100644
@@ -154,17 +154,48 @@ static void arizona_irq_disable(struct irq_data *data)
 {
 }
 
+static void arizona_irq_lock(struct irq_data *data)
+{
+	struct arizona *arizona = irq_data_get_irq_chip_data(data);
+
+	mutex_lock(&arizona->irq_lock);
+}
+
+static void arizona_irq_sync_unlock(struct irq_data *data)
+{
+	struct arizona *arizona = irq_data_get_irq_chip_data(data);
+	int i;
+
+	if (arizona->pending_wake_count < 0)
+		for (i = arizona->pending_wake_count; i < 0; i++)
+			irq_set_irq_wake(arizona->irq, 0);
+	else if (arizona->pending_wake_count > 0)
+		for (i = 0; i < arizona->pending_wake_count; i++)
+			irq_set_irq_wake(arizona->irq, 1);
+
+	arizona->pending_wake_count = 0;
+
+	mutex_unlock(&arizona->irq_lock);
+}
+
 static int arizona_irq_set_wake(struct irq_data *data, unsigned int on)
 {
 	struct arizona *arizona = irq_data_get_irq_chip_data(data);
 
-	return irq_set_irq_wake(arizona->irq, on);
+	if (on)
+		arizona->pending_wake_count++;
+	else
+		arizona->pending_wake_count--;
+
+	return 0;
 }
 
 static struct irq_chip arizona_irq_chip = {
 	.name			= "arizona",
 	.irq_disable		= arizona_irq_disable,
 	.irq_enable		= arizona_irq_enable,
+	.irq_bus_lock		= arizona_irq_lock,
+	.irq_bus_sync_unlock	= arizona_irq_sync_unlock,
 	.irq_set_wake		= arizona_irq_set_wake,
 };
 

@@ -193,6 +224,8 @@ int arizona_irq_init(struct arizona *arizona)
 	const struct regmap_irq_chip *aod, *irq;
 	struct irq_data *irq_data;
 
+	mutex_init(&arizona->irq_lock);
+
 	arizona->ctrlif_error = true;
 
 	switch (arizona->type) {

include/linux/mfd/arizona/core.h +3 -0

index d55a422..a0374ea 100644
@@ -148,6 +148,9 @@ struct arizona {
 	uint16_t dac_comp_coeff;
 	uint8_t dac_comp_enabled;
 	struct mutex dac_comp_lock;
+
+	int pending_wake_count;
+	struct mutex irq_lock;
 };
 
 int arizona_clk32k_enable(struct arizona *arizona);

FAO Thomas

Quoted 7 lines

Lockdep explicitly sets all the irq_desc locks as a single lock-class, which causes a "possible recursive locking detected" warning when we attempt to propagate the IRQ wake to our parent IRQ in arizona_irq_set_wake. Although this appears to be a false positive because an IRQ is unlikely to be its own parent, this was clearly intentionally prohibited.

Quoted 4 lines

To avoid this lockdep warning, take a cue from the regmap-irq system, and add bus lock callbacks on the IRQ chip and propagate the wake in the bus unlock which will happen after the desc lock has been released and thus avoid the issue.

This looks like a hack to me. I'd like Thomas (Cc'ed) to look it over.

Quoted 14 lines · Lee Jones

FAO Thomas

Quoted 11 lines

Lockdep explicitly sets all the irq_desc locks as a single lock-class, which causes a "possible recursive locking detected" warning when we attempt to propagate the IRQ wake to our parent IRQ in arizona_irq_set_wake. Although this appears to be a false positive because an IRQ is unlikely to be its own parent, this was clearly intentionally prohibited.

To avoid this lockdep warning, take a cue from the regmap-irq system, and add bus lock callbacks on the IRQ chip and propagate the wake in the bus unlock which will happen after the desc lock has been released and thus avoid the issue.

Quoted 1 line

This looks like a hack to me. I'd like Thomas (Cc'ed) to look it over.

irq_set_lockdep_class() exists for a reason. See kernel/irq/generic-chip.c or drivers/gpio/gpiolib.c for examples.