Discussion:
[PATCH fixes v2] mfd: max77693: Fix always masked MUIC interrupts
Krzysztof Kozlowski
2014-10-10 10:48:35 UTC
Permalink
All interrupts coming from MUIC were ignored because interrupt source
register was masked.

The Maxim 77693 has a "interrupt source" - a separate register and interrupts
which give information about PMIC block triggering the individual
interrupt (charger, topsys, MUIC, flash LED).

By default bootloader could initialize this register to "mask all"
value. In such case (observed on Trats2 board) MUIC interrupts won't be
generated regardless of their mask status. Regmap irq chip was unmasking
individual MUIC interrupts but the source was masked

Before introducing regmap irq chip this interrupt source was unmasked,
read and acked. Reading and acking is not necessary but unmasking is.

Signed-off-by: Krzysztof Kozlowski <***@samsung.com>
Cc: <***@vger.kernel.org>
Fixes: 342d669c1ee4 ("mfd: max77693: Handle IRQs using regmap")

---

Changes since v1:
1. Do the unmasking in main MFD driver. Additionally unmask all
interrupts, not only MUIC. Suggested by Chanwoo Choi.
---
drivers/mfd/max77693.c | 12 ++++++++++++
include/linux/mfd/max77693-private.h | 7 +++++++
2 files changed, 19 insertions(+)

diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 4b54da1ff7ab..4e4e6a1a6301 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -253,6 +253,17 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
goto err_irq_muic;
}

+ /* Unmask interrupts from all blocks in interrupt source register */
+ ret = regmap_update_bits(max77693->regmap,
+ MAX77693_PMIC_REG_INTSRC_MASK,
+ SRC_IRQ_ALL, ~SRC_IRQ_ALL);
+ if (ret < 0) {
+ dev_err(max77693->dev,
+ "Could not unmask interrupts in INTSRC: %d\n",
+ ret);
+ goto err_intsrc;
+ }
+
pm_runtime_set_active(max77693->dev);

ret = mfd_add_devices(max77693->dev, -1, max77693_devs,
@@ -264,6 +275,7 @@ static int max77693_i2c_probe(struct i2c_client *i2c,

err_mfd:
mfd_remove_devices(max77693->dev);
+err_intsrc:
regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
err_irq_muic:
regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
index e1b2b61285b9..15fed5365a20 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -438,6 +438,13 @@ enum max77693_irq_source {
MAX77693_IRQ_GROUP_NR,
};

+#define SRC_IRQ_CHARGER BIT(0)
+#define SRC_IRQ_TOP BIT(1)
+#define SRC_IRQ_FLASH BIT(2)
+#define SRC_IRQ_MUIC BIT(3)
+#define SRC_IRQ_ALL (SRC_IRQ_CHARGER | SRC_IRQ_TOP \
+ | SRC_IRQ_FLASH | SRC_IRQ_MUIC)
+
#define LED_IRQ_FLED2_OPEN BIT(0)
#define LED_IRQ_FLED2_SHORT BIT(1)
#define LED_IRQ_FLED1_OPEN BIT(2)
--
1.9.1
Lee Jones
2014-10-10 10:56:00 UTC
Permalink
Post by Krzysztof Kozlowski
All interrupts coming from MUIC were ignored because interrupt source
register was masked.
=20
The Maxim 77693 has a "interrupt source" - a separate register and in=
terrupts
Post by Krzysztof Kozlowski
which give information about PMIC block triggering the individual
interrupt (charger, topsys, MUIC, flash LED).
=20
By default bootloader could initialize this register to "mask all"
value. In such case (observed on Trats2 board) MUIC interrupts won't =
be
Post by Krzysztof Kozlowski
generated regardless of their mask status. Regmap irq chip was unmask=
ing
Post by Krzysztof Kozlowski
individual MUIC interrupts but the source was masked
=20
Before introducing regmap irq chip this interrupt source was unmasked=
,
Post by Krzysztof Kozlowski
read and acked. Reading and acking is not necessary but unmasking is.
=20
Fixes: 342d669c1ee4 ("mfd: max77693: Handle IRQs using regmap")
Applied to -fixes until someone complains.
Post by Krzysztof Kozlowski
---
=20
1. Do the unmasking in main MFD driver. Additionally unmask all
interrupts, not only MUIC. Suggested by Chanwoo Choi.
---
drivers/mfd/max77693.c | 12 ++++++++++++
include/linux/mfd/max77693-private.h | 7 +++++++
2 files changed, 19 insertions(+)
=20
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 4b54da1ff7ab..4e4e6a1a6301 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -253,6 +253,17 @@ static int max77693_i2c_probe(struct i2c_client =
*i2c,
Post by Krzysztof Kozlowski
goto err_irq_muic;
}
=20
+ /* Unmask interrupts from all blocks in interrupt source register *=
/
Post by Krzysztof Kozlowski
+ ret =3D regmap_update_bits(max77693->regmap,
+ MAX77693_PMIC_REG_INTSRC_MASK,
+ SRC_IRQ_ALL, ~SRC_IRQ_ALL);
+ if (ret < 0) {
+ dev_err(max77693->dev,
+ "Could not unmask interrupts in INTSRC: %d\n",
+ ret);
+ goto err_intsrc;
+ }
+
pm_runtime_set_active(max77693->dev);
=20
ret =3D mfd_add_devices(max77693->dev, -1, max77693_devs,
@@ -264,6 +275,7 @@ static int max77693_i2c_probe(struct i2c_client *=
i2c,
Post by Krzysztof Kozlowski
=20
mfd_remove_devices(max77693->dev);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd=
/max77693-private.h
Post by Krzysztof Kozlowski
index e1b2b61285b9..15fed5365a20 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -438,6 +438,13 @@ enum max77693_irq_source {
MAX77693_IRQ_GROUP_NR,
};
=20
+#define SRC_IRQ_CHARGER BIT(0)
+#define SRC_IRQ_TOP BIT(1)
+#define SRC_IRQ_FLASH BIT(2)
+#define SRC_IRQ_MUIC BIT(3)
+#define SRC_IRQ_ALL (SRC_IRQ_CHARGER | SRC_IRQ_TOP \
+ | SRC_IRQ_FLASH | SRC_IRQ_MUIC)
+
#define LED_IRQ_FLED2_OPEN BIT(0)
#define LED_IRQ_FLED2_SHORT BIT(1)
#define LED_IRQ_FLED1_OPEN BIT(2)
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog
Krzysztof Kozlowski
2014-10-22 09:28:43 UTC
Permalink
Post by Lee Jones
=20
All interrupts coming from MUIC were ignored because interrupt sour=
ce
Post by Lee Jones
register was masked.
=20
The Maxim 77693 has a "interrupt source" - a separate register and =
interrupts
Post by Lee Jones
which give information about PMIC block triggering the individual
interrupt (charger, topsys, MUIC, flash LED).
=20
By default bootloader could initialize this register to "mask all"
value. In such case (observed on Trats2 board) MUIC interrupts won'=
t be
Post by Lee Jones
generated regardless of their mask status. Regmap irq chip was unma=
sking
Post by Lee Jones
individual MUIC interrupts but the source was masked
=20
Before introducing regmap irq chip this interrupt source was unmask=
ed,
Post by Lee Jones
read and acked. Reading and acking is not necessary but unmasking i=
s.
Post by Lee Jones
=20
Fixes: 342d669c1ee4 ("mfd: max77693: Handle IRQs using regmap")
=20
Applied to -fixes until someone complains.
Hi Lee,

I cannot find this patch on gitweb (MFD tree), have you applied it?

If not then could you squash it with:
mfd: max77693: Fix a truncate warning
https://lkml.org/lkml/2014/10/14/161

If you wish I could resend it.

Best regards,
Krzysztof
Post by Lee Jones
=20
---
=20
1. Do the unmasking in main MFD driver. Additionally unmask all
interrupts, not only MUIC. Suggested by Chanwoo Choi.
---
drivers/mfd/max77693.c | 12 ++++++++++++
include/linux/mfd/max77693-private.h | 7 +++++++
2 files changed, 19 insertions(+)
=20
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 4b54da1ff7ab..4e4e6a1a6301 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -253,6 +253,17 @@ static int max77693_i2c_probe(struct i2c_clien=
t *i2c,
Post by Lee Jones
goto err_irq_muic;
}
=20
+ /* Unmask interrupts from all blocks in interrupt source register=
*/
Post by Lee Jones
+ ret =3D regmap_update_bits(max77693->regmap,
+ MAX77693_PMIC_REG_INTSRC_MASK,
+ SRC_IRQ_ALL, ~SRC_IRQ_ALL);
+ if (ret < 0) {
+ dev_err(max77693->dev,
+ "Could not unmask interrupts in INTSRC: %d\n",
+ ret);
+ goto err_intsrc;
+ }
+
pm_runtime_set_active(max77693->dev);
=20
ret =3D mfd_add_devices(max77693->dev, -1, max77693_devs,
@@ -264,6 +275,7 @@ static int max77693_i2c_probe(struct i2c_client=
*i2c,
Post by Lee Jones
=20
mfd_remove_devices(max77693->dev);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/m=
fd/max77693-private.h
Post by Lee Jones
index e1b2b61285b9..15fed5365a20 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -438,6 +438,13 @@ enum max77693_irq_source {
MAX77693_IRQ_GROUP_NR,
};
=20
+#define SRC_IRQ_CHARGER BIT(0)
+#define SRC_IRQ_TOP BIT(1)
+#define SRC_IRQ_FLASH BIT(2)
+#define SRC_IRQ_MUIC BIT(3)
+#define SRC_IRQ_ALL (SRC_IRQ_CHARGER | SRC_IRQ_TOP \
+ | SRC_IRQ_FLASH | SRC_IRQ_MUIC)
+
#define LED_IRQ_FLED2_OPEN BIT(0)
#define LED_IRQ_FLED2_SHORT BIT(1)
#define LED_IRQ_FLED1_OPEN BIT(2)
=20
Lee Jones
2014-10-22 11:37:27 UTC
Permalink
Post by Krzysztof Kozlowski
Post by Lee Jones
=20
All interrupts coming from MUIC were ignored because interrupt so=
urce
Post by Krzysztof Kozlowski
Post by Lee Jones
register was masked.
=20
The Maxim 77693 has a "interrupt source" - a separate register an=
d interrupts
Post by Krzysztof Kozlowski
Post by Lee Jones
which give information about PMIC block triggering the individual
interrupt (charger, topsys, MUIC, flash LED).
=20
By default bootloader could initialize this register to "mask all=
"
Post by Krzysztof Kozlowski
Post by Lee Jones
value. In such case (observed on Trats2 board) MUIC interrupts wo=
n't be
Post by Krzysztof Kozlowski
Post by Lee Jones
generated regardless of their mask status. Regmap irq chip was un=
masking
Post by Krzysztof Kozlowski
Post by Lee Jones
individual MUIC interrupts but the source was masked
=20
Before introducing regmap irq chip this interrupt source was unma=
sked,
Post by Krzysztof Kozlowski
Post by Lee Jones
read and acked. Reading and acking is not necessary but unmasking=
is.
Post by Krzysztof Kozlowski
Post by Lee Jones
=20
Fixes: 342d669c1ee4 ("mfd: max77693: Handle IRQs using regmap")
=20
Applied to -fixes until someone complains.
=20
Hi Lee,
=20
I cannot find this patch on gitweb (MFD tree), have you applied it?
=20
mfd: max77693: Fix a truncate warning
https://lkml.org/lkml/2014/10/14/161
=20
If you wish I could resend it.
It as applied, just not pushed.

I can squash that commit into it yes.
Post by Krzysztof Kozlowski
Post by Lee Jones
---
=20
1. Do the unmasking in main MFD driver. Additionally unmask all
interrupts, not only MUIC. Suggested by Chanwoo Choi.
---
drivers/mfd/max77693.c | 12 ++++++++++++
include/linux/mfd/max77693-private.h | 7 +++++++
2 files changed, 19 insertions(+)
=20
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 4b54da1ff7ab..4e4e6a1a6301 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -253,6 +253,17 @@ static int max77693_i2c_probe(struct i2c_cli=
ent *i2c,
Post by Krzysztof Kozlowski
Post by Lee Jones
goto err_irq_muic;
}
=20
+ /* Unmask interrupts from all blocks in interrupt source regist=
er */
Post by Krzysztof Kozlowski
Post by Lee Jones
+ ret =3D regmap_update_bits(max77693->regmap,
+ MAX77693_PMIC_REG_INTSRC_MASK,
+ SRC_IRQ_ALL, ~SRC_IRQ_ALL);
+ if (ret < 0) {
+ dev_err(max77693->dev,
+ "Could not unmask interrupts in INTSRC: %d\n",
+ ret);
+ goto err_intsrc;
+ }
+
pm_runtime_set_active(max77693->dev);
=20
ret =3D mfd_add_devices(max77693->dev, -1, max77693_devs,
@@ -264,6 +275,7 @@ static int max77693_i2c_probe(struct i2c_clie=
nt *i2c,
Post by Krzysztof Kozlowski
Post by Lee Jones
=20
mfd_remove_devices(max77693->dev);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
diff --git a/include/linux/mfd/max77693-private.h b/include/linux=
/mfd/max77693-private.h
Post by Krzysztof Kozlowski
Post by Lee Jones
index e1b2b61285b9..15fed5365a20 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -438,6 +438,13 @@ enum max77693_irq_source {
MAX77693_IRQ_GROUP_NR,
};
=20
+#define SRC_IRQ_CHARGER BIT(0)
+#define SRC_IRQ_TOP BIT(1)
+#define SRC_IRQ_FLASH BIT(2)
+#define SRC_IRQ_MUIC BIT(3)
+#define SRC_IRQ_ALL (SRC_IRQ_CHARGER | SRC_IRQ_TOP \
+ | SRC_IRQ_FLASH | SRC_IRQ_MUIC)
+
#define LED_IRQ_FLED2_OPEN BIT(0)
#define LED_IRQ_FLED2_SHORT BIT(1)
#define LED_IRQ_FLED1_OPEN BIT(2)
=20
=20
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog

Chanwoo Choi
2014-10-10 10:56:42 UTC
Permalink
Post by Krzysztof Kozlowski
All interrupts coming from MUIC were ignored because interrupt source
register was masked.
The Maxim 77693 has a "interrupt source" - a separate register and interrupts
which give information about PMIC block triggering the individual
interrupt (charger, topsys, MUIC, flash LED).
By default bootloader could initialize this register to "mask all"
value. In such case (observed on Trats2 board) MUIC interrupts won't be
generated regardless of their mask status. Regmap irq chip was unmasking
individual MUIC interrupts but the source was masked
Before introducing regmap irq chip this interrupt source was unmasked,
read and acked. Reading and acking is not necessary but unmasking is.
Fixes: 342d669c1ee4 ("mfd: max77693: Handle IRQs using regmap")
---
1. Do the unmasking in main MFD driver. Additionally unmask all
interrupts, not only MUIC. Suggested by Chanwoo Choi.
---
drivers/mfd/max77693.c | 12 ++++++++++++
include/linux/mfd/max77693-private.h | 7 +++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 4b54da1ff7ab..4e4e6a1a6301 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -253,6 +253,17 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
goto err_irq_muic;
}
+ /* Unmask interrupts from all blocks in interrupt source register */
+ ret = regmap_update_bits(max77693->regmap,
+ MAX77693_PMIC_REG_INTSRC_MASK,
+ SRC_IRQ_ALL, ~SRC_IRQ_ALL);
+ if (ret < 0) {
+ dev_err(max77693->dev,
+ "Could not unmask interrupts in INTSRC: %d\n",
+ ret);
+ goto err_intsrc;
+ }
+
pm_runtime_set_active(max77693->dev);
ret = mfd_add_devices(max77693->dev, -1, max77693_devs,
@@ -264,6 +275,7 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
mfd_remove_devices(max77693->dev);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
index e1b2b61285b9..15fed5365a20 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -438,6 +438,13 @@ enum max77693_irq_source {
MAX77693_IRQ_GROUP_NR,
};
+#define SRC_IRQ_CHARGER BIT(0)
+#define SRC_IRQ_TOP BIT(1)
+#define SRC_IRQ_FLASH BIT(2)
+#define SRC_IRQ_MUIC BIT(3)
+#define SRC_IRQ_ALL (SRC_IRQ_CHARGER | SRC_IRQ_TOP \
+ | SRC_IRQ_FLASH | SRC_IRQ_MUIC)
+
#define LED_IRQ_FLED2_OPEN BIT(0)
#define LED_IRQ_FLED2_SHORT BIT(1)
#define LED_IRQ_FLED1_OPEN BIT(2)
Reviewed-by: Chanwoo Choi <***@samsung.com>

Thanks,
Chanwoo Choi
Loading...