[v1,1/4] mfd: qcom-pm8008: Fix swapped mask/unmask in irq chip

Message ID 20230216222214.138671-2-aidanmacdonald.0x0@gmail.com
State New
Headers
Series regmap-irq fixes for qcom-pm8008 |

Commit Message

Aidan MacDonald Feb. 16, 2023, 10:22 p.m. UTC
  The usual behavior of mask registers is writing a '1' bit to
disable (mask) an interrupt; similarly, writing a '1' bit to
an unmask register enables (unmasks) an interrupt.

Due to a longstanding issue in regmap-irq, mask and unmask
registers were inverted when both kinds of registers were
present on the same chip, ie. regmap-irq actually wrote '1's
to the mask register to enable an IRQ and '1's to the unmask
register to disable an IRQ.

This was fixed by commit e8ffb12e7f06 ("regmap-irq: Fix
inverted handling of unmask registers") but the fix is opt-in
via mask_unmask_non_inverted = true because it requires manual
changes for each affected driver. The new behavior will become
the default once all drivers have been updated.

The PM8008 appears to rely on the inverted behavior. It has
separate set & clear registers for a register called INT_EN,
which presumably enables interrupts by writing '1's. Opt in
to the new non-inverted behavior & swap mask_base/unmask_base.

Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
 drivers/mfd/qcom-pm8008.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Lee Jones March 3, 2023, 10:43 a.m. UTC | #1
On Thu, 16 Feb 2023, Aidan MacDonald wrote:

> The usual behavior of mask registers is writing a '1' bit to
> disable (mask) an interrupt; similarly, writing a '1' bit to
> an unmask register enables (unmasks) an interrupt.
> 
> Due to a longstanding issue in regmap-irq, mask and unmask
> registers were inverted when both kinds of registers were
> present on the same chip, ie. regmap-irq actually wrote '1's
> to the mask register to enable an IRQ and '1's to the unmask
> register to disable an IRQ.
> 
> This was fixed by commit e8ffb12e7f06 ("regmap-irq: Fix
> inverted handling of unmask registers") but the fix is opt-in
> via mask_unmask_non_inverted = true because it requires manual
> changes for each affected driver. The new behavior will become
> the default once all drivers have been updated.
> 
> The PM8008 appears to rely on the inverted behavior. It has
> separate set & clear registers for a register called INT_EN,
> which presumably enables interrupts by writing '1's. Opt in
> to the new non-inverted behavior & swap mask_base/unmask_base.
> 
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
>  drivers/mfd/qcom-pm8008.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Applied, thanks
  

Patch

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index 9f3c4a01b4c1..39fd2a792e73 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -45,8 +45,8 @@  enum {
 #define PM8008_GPIO2_ADDR	PM8008_PERIPH_3_BASE
 
 #define PM8008_STATUS_BASE	(PM8008_PERIPH_0_BASE | INT_LATCHED_STS_OFFSET)
-#define PM8008_MASK_BASE	(PM8008_PERIPH_0_BASE | INT_EN_SET_OFFSET)
-#define PM8008_UNMASK_BASE	(PM8008_PERIPH_0_BASE | INT_EN_CLR_OFFSET)
+#define PM8008_MASK_BASE	(PM8008_PERIPH_0_BASE | INT_EN_CLR_OFFSET)
+#define PM8008_UNMASK_BASE	(PM8008_PERIPH_0_BASE | INT_EN_SET_OFFSET)
 #define PM8008_TYPE_BASE	(PM8008_PERIPH_0_BASE | INT_SET_TYPE_OFFSET)
 #define PM8008_ACK_BASE		(PM8008_PERIPH_0_BASE | INT_LATCHED_CLR_OFFSET)
 #define PM8008_POLARITY_HI_BASE	(PM8008_PERIPH_0_BASE | INT_POL_HIGH_OFFSET)
@@ -131,6 +131,7 @@  static struct regmap_irq_chip pm8008_irq_chip = {
 	.status_base		= PM8008_STATUS_BASE,
 	.mask_base		= PM8008_MASK_BASE,
 	.unmask_base		= PM8008_UNMASK_BASE,
+	.mask_unmask_non_inverted = true,
 	.type_base		= PM8008_TYPE_BASE,
 	.ack_base		= PM8008_ACK_BASE,
 	.virt_reg_base		= pm8008_virt_regs,