Re: [PATCH 2/2] mfd: max77620: Add pre/post irq handler before/after servicing interrupt

From: Laxman Dewangan
Date: Wed Jun 08 2016 - 11:55:25 EST


Hi Lee,

On Wednesday 08 June 2016 08:11 PM, Lee Jones wrote:
On Fri, 20 May 2016, Laxman Dewangan wrote:

+ * MAX77620 and MAX20024 has the following steps of the interrupt handling
+ * for TOP interrupts:
+ * 1. When interrupt occurs from PMIC, mask the PMIC interrupt by setting GLBLM.
+ * 2. Read IRQTOP and service the interrupt.
+ * 3. Once all interrupts has been checked and serviced, the interrupt service
+ * routine un-masks the hardware interrupt line by clearing GLBLM.
+ */
+static int max77620_top_irq_chip_pre_irq_handler(void *irq_drv_data)
+{
+ struct max77620_chip *chip = irq_drv_data;
+ int ret;
+
+ ret = regmap_update_bits(chip->rmap, MAX77620_REG_INTENLBT,
+ MAX77620_GLBLM_MASK, MAX77620_GLBLM_MASK);
+ if (ret < 0)
+ dev_err(chip->dev, "Failed to set GLBLM: %d\n", ret);
+
+ return ret;
+}
+
+static int max77620_top_irq_chip_post_irq_handler(void *irq_drv_data)
+{
+ struct max77620_chip *chip = irq_drv_data;
+ int ret;
+
+ ret = regmap_update_bits(chip->rmap, MAX77620_REG_INTENLBT,
+ MAX77620_GLBLM_MASK, 0);
+ if (ret < 0)
+ dev_err(chip->dev, "Failed to reset GLBLM: %d\n", ret);
+
+ return ret;
+}
This seems massively over compacted. All you're effectively doing
here is masking and unmasking the IRQs, which we do almost
ubiquitously with interrupt controllers. Can't you just call the
functions "max77629_{un}mask_irqs()"?



Actually, per PMIC HW design, we need to toggle this bit on ISRs. Before reading the status, need to set 1 and then after handling it need to set 0. This cannot be done by any other bit toggling or masking/unmasking interrupt controller interrupt.

This is hard requirement from the PMIC chip.