[PATCH 4/9] TWL4030: clear MADC interrupt status registers upon init

[PATCH 4/9] TWL4030: clear MADC interrupt status registers upon init

[Paul Walmsley]

twl_init_irq() does not clear MADC interrupt status registers upon init -
fix.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index bb0732c..9d93524 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -821,6 +821,20 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* MADC_ISR1 */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x61);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* MADC_ISR2 */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x63);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
 	/* key Pad */
 	/* KEYPAD - IMR1 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x12));

[PATCH 0/9] TWL4030 bugfixes and cleanups: version 2

[Paul Walmsley]

[ This is the second version of these patches - they now check the
SIH_CTRL.COR bit to determine whether to read or write to clear ]

At boot, twl_init_irq() tries to mask off all TWL4030 secondary
interrupts and clear them, since no secondary interrupt handlers are
registered yet (that happens later in the boot).

However, this code is flawed.  MADC, BCI, keypad, and GPIO ISRs are
either not cleared or are incompletely cleared.  Some BCI register
addresses are nonsensical.  And some code attempts to clear ISRs
before masking IMRs, which is race-prone (an interrupt could occur
after the ISR clear, but before the mask).

These problems can cause large numbers of boot messages similar to
this:

    TWL4030 module irq 368 is disabled but can't be masked!

and I2C transmit overflows, both observed during boot on the
BeagleBoard here.  The actual IRQ number sometimes varied depending on
which interrupt was unacknowledged.

This series fixes these issues and does some code cleanup, saving 272
bytes.

Thanks to Philip Balister <philip@balister.org>, Koen Kooi
<k.kooi@student.utwente.nl>, Måns Rullgård <mans@mansr.org>, Steve
Sakoman <sakoman@gmail.com> and others on #beagle for help tracing
these problems.

Thanks also to Peter De Schrijver <peter.de-schrijver@nokia.com> for
help with this second version, allowing several unnecessary I2C
transactions to be elided from twl_init_irq().

Tested on BeagleBoard rev B4 (3530ES2.2).


- Paul

---

size:
   text    data     bss     dec     hex filename
3076157  142840  104248 3323245  32b56d vmlinux.beagle.orig
3075885  142840  104248 3322973  32b45d vmlinux.beagle

diffstat:

 drivers/i2c/chips/twl4030-core.c   |  383 ++++++++++++++++++++++--------------
 drivers/i2c/chips/twl4030-gpio.c   |   48 -----
 drivers/i2c/chips/twl4030-pwrirq.c |   15 +
 include/linux/i2c/twl4030-gpio.h   |   76 +++++++
 include/linux/i2c/twl4030-pwrirq.h |   37 +++
 5 files changed, 359 insertions(+), 200 deletions(-)
 create mode 100644 include/linux/i2c/twl4030-gpio.h
 create mode 100644 include/linux/i2c/twl4030-pwrirq.h

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/9] TWL4030: remove superfluous PWR interrupt status clear before masking

[Paul Walmsley]

twl_irq_init() clears PWR interrupt status bits, then masks the interrupts
off, then clears the PWR interrupt status bits again.  The first clear
seems unnecessary, so, remove it.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index adc45d4..ff662bc 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -719,24 +719,6 @@ static void twl_init_irq(void)
 	char	*msg = "Unable to register interrupt subsystem";
 	unsigned int irq_num;
 
-	/*
-	 * We end up with interrupts from other modules before
-	 * they get a chance to handle them...
-	 */
-	/* PWR_ISR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x00);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
-
-	/* PWR_ISR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x02);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
-
 	/* PWR_IMR1 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x1);
 	if (res < 0) {

[PATCH 2/9] TWL4030: clear TWL GPIO interrupt status registers

[Paul Walmsley]

twl_init_irq() does not clear the TWL GPIO ISR registers, but the PIH
ISR thinks that it has.  This causes any previously-latched GPIO interrupts
to be stuck on until twl4030-gpio.c initializes, often drowning the console in

    TWL4030 module irq 368 is disabled but can't be masked!

messages.  This seems to be a particular problem when booting on Beagle.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |   42 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index ff662bc..dfc3805 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -857,6 +857,48 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* GPIO_ISR1A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x19);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* GPIO_ISR2A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1a);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* GPIO_ISR3A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1b);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* GPIO_ISR1B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1f);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* GPIO_ISR2B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x20);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* GPIO_ISR3B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x21);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
 	/* install an irq handler for each of the PIH modules */
 	for (i = TWL4030_IRQ_BASE; i < TWL4030_IRQ_END; i++) {
 		set_irq_chip(i, &twl4030_irq_chip);

[PATCH 3/9] TWL4030: use correct register addresses for BCI IMR registers

[Paul Walmsley]

The existing code to mask and clear BCI interrupts in twl_init_irq() is
wrong.  It uses the wrong register offsets, it does not mask all of the
BCI IMR registers, and it does not clear all of the BCI ISR registers.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |   40 ++++++++++++++++++++++++++++++++------
 1 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index dfc3805..bb0732c 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -750,29 +750,57 @@ static void twl_init_irq(void)
 	/* POWER HACK (END) */
 	/* Slave address 0x4A */
 
-	/* BCIIMR1_1 */
+	/* BCIIMR1A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x2);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* BCIIMR2A */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x3);
 	if (res < 0) {
 		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
 		return;
 	}
 
-	/* BCIIMR1_2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x4);
+	/* BCIIMR1B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x6);
 	if (res < 0) {
 		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
 		return;
 	}
 
-	/* BCIIMR2_1 */
+	/* BCIIMR2B */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x7);
 	if (res < 0) {
 		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
 		return;
 	}
 
-	/* BCIIMR2_2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x8);
+	/* BCIISR1A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x0);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* BCIISR2A */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x1);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* BCIISR1B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x4);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* BCIISR2B */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x5);
 	if (res < 0) {
 		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
 		return;

[PATCH 4/9] TWL4030: clear MADC interrupt status registers upon init

[Paul Walmsley]

twl_init_irq() does not clear MADC interrupt status registers upon init -
fix.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index bb0732c..9d93524 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -821,6 +821,20 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* MADC_ISR1 */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x61);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
+	/* MADC_ISR2 */
+	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x63);
+	if (res < 0) {
+		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
+		return;
+	}
+
 	/* key Pad */
 	/* KEYPAD - IMR1 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x12));

[PATCH 5/9] TWL4030: use *_SIH_CTRL.COR bit to determine whether to read or write ISR to clear

[Paul Walmsley]

TWL4030 interrupt status register bits can be cleared in one of two ways:
either by reading from the register, or by writing a 1 to the
appropriate bit(s) in the register.  This behavior can be altered at any
time by the <twlmodule>_SIH_CTRL.COR register bit ("clear-on-read").

The TWL4030 TRM is deeply confused as to whether COR=1 means that the
registers are cleared on reads, or cleared on writes.  Peter De
Schrijver <peter.de-schrijver> confirms that COR=1 means that the registers
are cleared on read.

So, for each TWL4030 SIH, check the value of the *_SIH_CTRL.COR bit, and if
it is 1, use reads to clear the ISRs; if it is 0, use writes.

Also, use WARN_ON() to warn if the read/write failed, and don't skip
the rest of the initialization on failure either.

Thanks to Peter for his help with this patch.


Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |  183 ++++++++++++++++++++++----------------
 1 files changed, 106 insertions(+), 77 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 9d93524..eae0634 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -133,6 +133,16 @@
 /* on I2C-1 for 2430SDP */
 #define CONFIG_I2C_TWL4030_ID		1
 
+/* SIH_CTRL registers */
+#define TWL4030_INT_PWR_SIH_CTRL	0x07
+#define TWL4030_INTERRUPTS_BCISIHCTRL	0x0d
+#define TWL4030_MADC_MADC_SIH_CTRL	0x67
+#define TWL4030_KEYPAD_KEYP_SIH_CTRL	0x17
+#define TWL4030_GPIO_GPIO_SIH_CTRL	0x2d
+
+#define TWL4030_SIH_CTRL_COR_MASK	(1 << 2)
+
+
 /* Helper functions */
 static int
 twl4030_detect_client(struct i2c_adapter *adapter, unsigned char sid);
@@ -712,13 +722,61 @@ static int power_companion_init(void)
 	return e;
 }
 
+/**
+ * twl4030_i2c_clear_isr - clear TWL4030 SIH ISR regs via read + write
+ * @mod_no: TWL4030 module number
+ * @reg: register index to clear
+ * @cor: value of the <module>_SIH_CTRL.COR bit (1 or 0)
+ *
+ * Either reads (cor == 1) or writes (cor == 0) to a TWL4030 interrupt
+ * status register to ensure that any prior interrupts are cleared.
+ * Returns the status from the I2C read operation.
+ */
+static int twl4030_i2c_clear_isr(u8 mod_no, u8 reg, u8 cor)
+{
+	u8 tmp;
+
+	return (cor) ? twl4030_i2c_read_u8(mod_no, &tmp, reg) :
+		twl4030_i2c_write_u8(mod_no, 0xff, reg);
+}
+
+/**
+ * twl4030_read_cor_bit - are TWL module ISRs cleared by reads or writes?
+ * @mod_no: TWL4030 module number
+ * @reg: register index to clear
+ *
+ * Returns 1 if the TWL4030 SIH interrupt status registers (ISRs) for
+ * the specified TWL module are cleared by reads, or 0 if cleared by
+ * writes.
+ */
+static int twl4030_read_cor_bit(u8 mod_no, u8 reg)
+{
+	u8 tmp = 0;
+
+	WARN_ON(twl4030_i2c_read_u8(mod_no, &tmp, reg) < 0);
+
+	tmp &= TWL4030_SIH_CTRL_COR_MASK;
+	tmp >>= __ffs(TWL4030_SIH_CTRL_COR_MASK);
+
+	return tmp;
+}
+
 static void twl_init_irq(void)
 {
 	int	i = 0;
 	int	res = 0;
+	int cor;
 	char	*msg = "Unable to register interrupt subsystem";
 	unsigned int irq_num;
 
+	/*
+	 * For each TWL4030 module with ISR/IMR registers, mask all
+	 * interrupts and then clear any existing interrupt status bits,
+	 * since we initially do not have any TWL4030 module interrupt
+	 * handlers present.
+	 */
+
+
 	/* PWR_IMR1 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x1);
 	if (res < 0) {
@@ -734,20 +792,18 @@ static void twl_init_irq(void)
 	}
 
 	/* Clear off any other pending interrupts on power */
+
+	/* Are PWR interrupt status bits cleared by reads or writes? */
+	cor = twl4030_read_cor_bit(TWL4030_MODULE_INT,
+				   TWL4030_INT_PWR_SIH_CTRL);
+	WARN_ON(cor < 0);
+
 	/* PWR_ISR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x00);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x00, cor) < 0);
 
 	/* PWR_ISR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x02);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
-	/* POWER HACK (END) */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x02, cor) < 0);
+
 	/* Slave address 0x4A */
 
 	/* BCIIMR1A */
@@ -778,33 +834,22 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* Are BCI interrupt status bits cleared by reads or writes? */
+	cor = twl4030_read_cor_bit(TWL4030_MODULE_INTERRUPTS,
+				   TWL4030_INTERRUPTS_BCISIHCTRL);
+	WARN_ON(cor < 0);
+
 	/* BCIISR1A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x0);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x0, cor) < 0);
 
 	/* BCIISR2A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x1);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x1, cor) < 0);
 
 	/* BCIISR1B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x4);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x4, cor) < 0);
 
 	/* BCIISR2B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x5);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x5, cor) < 0);
 
 	/* MAD C */
 	/* MADC_IMR1 */
@@ -821,19 +866,16 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* Are MADC interrupt status bits cleared by reads or writes? */
+	cor = twl4030_read_cor_bit(TWL4030_MODULE_MADC,
+				   TWL4030_MADC_MADC_SIH_CTRL);
+	WARN_ON(cor < 0);
+
 	/* MADC_ISR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x61);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x61, cor) < 0);
 
 	/* MADC_ISR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x63);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x63, cor) < 0);
 
 	/* key Pad */
 	/* KEYPAD - IMR1 */
@@ -842,12 +884,15 @@ static void twl_init_irq(void)
 		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
 		return;
 	}
-	{
-		u8 clear;
-		/* Clear ISR */
-		twl4030_i2c_read_u8(TWL4030_MODULE_KEYPAD, &clear, 0x11);
-		twl4030_i2c_read_u8(TWL4030_MODULE_KEYPAD, &clear, 0x11);
-	}
+
+	/* Are keypad interrupt status bits cleared by reads or writes? */
+	cor = twl4030_read_cor_bit(TWL4030_MODULE_KEYPAD,
+				   TWL4030_KEYPAD_KEYP_SIH_CTRL);
+	WARN_ON(cor < 0);
+
+	/* KEYPAD - ISR1 */
+	/* XXX does this still need to be done twice for some reason? */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x11, cor) < 0);
 
 	/* KEYPAD - IMR2 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x14));
@@ -856,6 +901,9 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* KEYPAD - ISR2 */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x13, cor) < 0);
+
 	/* Slave address 0x49 */
 	/* GPIO_IMR1A */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x1C));
@@ -899,47 +947,28 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* Are GPIO interrupt status bits cleared by reads or writes? */
+	cor = twl4030_read_cor_bit(TWL4030_MODULE_GPIO,
+				   TWL4030_GPIO_GPIO_SIH_CTRL);
+	WARN_ON(cor < 0);
+
 	/* GPIO_ISR1A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x19);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x19, cor) < 0);
 
 	/* GPIO_ISR2A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1a);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1a, cor) < 0);
 
 	/* GPIO_ISR3A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1b);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1b, cor) < 0);
 
 	/* GPIO_ISR1B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1f);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1f, cor) < 0);
 
 	/* GPIO_ISR2B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x20);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x20, cor) < 0);
 
 	/* GPIO_ISR3B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x21);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x21, cor) < 0);
 
 	/* install an irq handler for each of the PIH modules */
 	for (i = TWL4030_IRQ_BASE; i < TWL4030_IRQ_END; i++) {

[PATCH 6/9] TWL4030: change init-time IMR mask code to WARN if error

[Paul Walmsley]

twl_init_irq() prints error messages and returns if any interrupt mask
register writes fail.  Change this to generate a warning traceback and
to continue execution rather than skipping TWL init.  (These mask
writes should not fail at all unless either the I2C bus or the TWL4030
is somehow wedged.)

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |  100 +++++++-------------------------------
 1 files changed, 18 insertions(+), 82 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index eae0634..99cc143 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -778,18 +778,10 @@ static void twl_init_irq(void)
 
 
 	/* PWR_IMR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x1);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff, 0x1) < 0);
 
 	/* PWR_IMR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xFF, 0x3);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff, 0x3) < 0);
 
 	/* Clear off any other pending interrupts on power */
 
@@ -807,32 +799,16 @@ static void twl_init_irq(void)
 	/* Slave address 0x4A */
 
 	/* BCIIMR1A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x2);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x2) < 0);
 
-	/* BCIIMR2A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x3);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	/* BCIIMR2A  */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x3) < 0);
 
-	/* BCIIMR1B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x6);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	/* BCIIMR2A */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x6) < 0);
 
 	/* BCIIMR2B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xFF, 0x7);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x7) < 0);
 
 	/* Are BCI interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_INTERRUPTS,
@@ -853,18 +829,10 @@ static void twl_init_irq(void)
 
 	/* MAD C */
 	/* MADC_IMR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x62);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff, 0x62) < 0);
 
 	/* MADC_IMR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xFF, 0x64);
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff, 0x64) < 0);
 
 	/* Are MADC interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_MADC,
@@ -879,11 +847,7 @@ static void twl_init_irq(void)
 
 	/* key Pad */
 	/* KEYPAD - IMR1 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x12));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff, 0x12) < 0);
 
 	/* Are keypad interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_KEYPAD,
@@ -895,57 +859,29 @@ static void twl_init_irq(void)
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x11, cor) < 0);
 
 	/* KEYPAD - IMR2 */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x14));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff, 0x14) < 0);
 
 	/* KEYPAD - ISR2 */
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x13, cor) < 0);
 
 	/* Slave address 0x49 */
 	/* GPIO_IMR1A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x1C));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1c) < 0);
 
 	/* GPIO_IMR2A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x1D));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1d) < 0);
 
 	/* GPIO_IMR3A */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x1E));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1e) < 0);
 
 	/* GPIO_IMR1B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x22));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x22) < 0);
 
 	/* GPIO_IMR2B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x23));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x23) < 0);
 
 	/* GPIO_IMR3B */
-	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x24));
-	if (res < 0) {
-		pr_err("%s[%d][%d]\n", msg, res, __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x24) < 0);
 
 	/* Are GPIO interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_GPIO,

[PATCH 7/9] TWL4030: move TWL module register defs into separate include files

[Paul Walmsley]

twl_init_irq() uses "magic numbers" to access TWL module IMR and ISR
registers.  Symbolic constants are definitely preferred.

Rather than duplicating already existing symbolic constants in
twl4030-gpio.c and twl4030-pwrirq.c, move the existing constants out
into include files.  This patch should not change kernel behavior.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-gpio.c   |   48 -----------------------
 drivers/i2c/chips/twl4030-pwrirq.c |   15 +++----
 include/linux/i2c/twl4030-gpio.h   |   76 ++++++++++++++++++++++++++++++++++++
 include/linux/i2c/twl4030-pwrirq.h |   37 ++++++++++++++++++
 4 files changed, 121 insertions(+), 55 deletions(-)
 create mode 100644 include/linux/i2c/twl4030-gpio.h
 create mode 100644 include/linux/i2c/twl4030-pwrirq.h

diff --git a/drivers/i2c/chips/twl4030-gpio.c b/drivers/i2c/chips/twl4030-gpio.c
index f16a48b..9d17f45 100644
--- a/drivers/i2c/chips/twl4030-gpio.c
+++ b/drivers/i2c/chips/twl4030-gpio.c
@@ -38,6 +38,7 @@
 
 #include <linux/i2c.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/i2c/twl4030-gpio.h>
 #include <linux/slab.h>
 
 #include <asm/arch/irqs.h>
@@ -47,53 +48,6 @@
 
 #include <linux/device.h>
 
-/*
- * GPIO Block Register definitions
- */
-
-#define REG_GPIODATAIN1			0x0
-#define REG_GPIODATAIN2			0x1
-#define REG_GPIODATAIN3			0x2
-#define REG_GPIODATADIR1		0x3
-#define REG_GPIODATADIR2		0x4
-#define REG_GPIODATADIR3		0x5
-#define REG_GPIODATAOUT1		0x6
-#define REG_GPIODATAOUT2		0x7
-#define REG_GPIODATAOUT3		0x8
-#define REG_CLEARGPIODATAOUT1		0x9
-#define REG_CLEARGPIODATAOUT2		0xA
-#define REG_CLEARGPIODATAOUT3		0xB
-#define REG_SETGPIODATAOUT1		0xC
-#define REG_SETGPIODATAOUT2		0xD
-#define REG_SETGPIODATAOUT3		0xE
-#define REG_GPIO_DEBEN1			0xF
-#define REG_GPIO_DEBEN2			0x10
-#define REG_GPIO_DEBEN3			0x11
-#define REG_GPIO_CTRL			0x12
-#define REG_GPIOPUPDCTR1		0x13
-#define REG_GPIOPUPDCTR2		0x14
-#define REG_GPIOPUPDCTR3		0x15
-#define REG_GPIOPUPDCTR4		0x16
-#define REG_GPIOPUPDCTR5		0x17
-#define REG_GPIO_ISR1A			0x19
-#define REG_GPIO_ISR2A			0x1A
-#define REG_GPIO_ISR3A			0x1B
-#define REG_GPIO_IMR1A			0x1C
-#define REG_GPIO_IMR2A			0x1D
-#define REG_GPIO_IMR3A			0x1E
-#define REG_GPIO_ISR1B			0x1F
-#define REG_GPIO_ISR2B			0x20
-#define REG_GPIO_ISR3B			0x21
-#define REG_GPIO_IMR1B			0x22
-#define REG_GPIO_IMR2B			0x23
-#define REG_GPIO_IMR3B			0x24
-#define REG_GPIO_EDR1			0x28
-#define REG_GPIO_EDR2			0x29
-#define REG_GPIO_EDR3			0x2A
-#define REG_GPIO_EDR4			0x2B
-#define REG_GPIO_EDR5			0x2C
-#define REG_GPIO_SIH_CTRL		0x2D
-
 /* BitField Definitions */
 
 /* Data banks : 3 banks for 8 gpios each */
diff --git a/drivers/i2c/chips/twl4030-pwrirq.c b/drivers/i2c/chips/twl4030-pwrirq.c
index a4d2e92..1afdb65 100644
--- a/drivers/i2c/chips/twl4030-pwrirq.c
+++ b/drivers/i2c/chips/twl4030-pwrirq.c
@@ -27,10 +27,8 @@
 #include <linux/random.h>
 #include <linux/kthread.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/i2c/twl4030-pwrirq.h>
 
-#define PWR_ISR1 0
-#define PWR_IMR1 1
-#define PWR_SIH_CTRL 7
 #define PWR_SIH_CTRL_COR (1<<2)
 
 static u8 twl4030_pwrirq_mask;
@@ -93,7 +91,8 @@ static void do_twl4030_pwrmodule_irq(unsigned int irq, irq_desc_t *desc)
 			twl4030_pwrirq_mask |= 1 << (irq - TWL4030_PWR_IRQ_BASE);
 			local_irq_enable();
 			twl4030_i2c_write_u8(TWL4030_MODULE_INT,
-						twl4030_pwrirq_mask, PWR_IMR1);
+					     twl4030_pwrirq_mask,
+					     TWL4030_INT_PWR_IMR1);
 		}
 	}
 }
@@ -115,7 +114,7 @@ static void do_twl4030_pwrirq(unsigned int irq, irq_desc_t *desc)
 
 		local_irq_enable();
 		ret = twl4030_i2c_read_u8(TWL4030_MODULE_INT, &pwr_isr,
-						PWR_ISR1);
+					  TWL4030_INT_PWR_ISR1);
 		if (ret) {
 			printk(KERN_WARNING
 				"I2C error %d while reading TWL4030"
@@ -151,7 +150,7 @@ static int twl4030_pwrirq_thread(void *data)
 		twl4030_pwrirq_mask &= ~local_unmask;
 
 		twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
-					PWR_IMR1);
+				     TWL4030_INT_PWR_IMR1);
 
 		local_irq_disable();
 		if (!twl4030_pwrirq_pending_unmask)
@@ -172,14 +171,14 @@ static int __init twl4030_pwrirq_init(void)
 	twl4030_pwrirq_pending_unmask = 0;
 
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
-					PWR_IMR1);
+					TWL4030_INT_PWR_IMR1);
 	if (err)
 		return err;
 
 	/* Enable clear on read */
 
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, PWR_SIH_CTRL_COR,
-					PWR_SIH_CTRL);
+				   TWL4030_INT_PWR_SIH_CTRL);
 	if (err)
 		return err;
 
diff --git a/include/linux/i2c/twl4030-gpio.h b/include/linux/i2c/twl4030-gpio.h
new file mode 100644
index 0000000..7cbf610
--- /dev/null
+++ b/include/linux/i2c/twl4030-gpio.h
@@ -0,0 +1,76 @@
+/*
+ * twl4030-gpio.h - header for TWL4030 GPIO module
+ *
+ * Copyright (C) 2005-2006, 2008 Texas Instruments, Inc.
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Based on tlv320aic23.c:
+ * Copyright (c) by Kai Svahn <kai.svahn@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#ifndef __TWL4030_GPIO_H_
+#define __TWL4030_GPIO_H_
+
+/*
+ * GPIO Block Register definitions
+ */
+
+#define REG_GPIODATAIN1			0x0
+#define REG_GPIODATAIN2			0x1
+#define REG_GPIODATAIN3			0x2
+#define REG_GPIODATADIR1		0x3
+#define REG_GPIODATADIR2		0x4
+#define REG_GPIODATADIR3		0x5
+#define REG_GPIODATAOUT1		0x6
+#define REG_GPIODATAOUT2		0x7
+#define REG_GPIODATAOUT3		0x8
+#define REG_CLEARGPIODATAOUT1		0x9
+#define REG_CLEARGPIODATAOUT2		0xA
+#define REG_CLEARGPIODATAOUT3		0xB
+#define REG_SETGPIODATAOUT1		0xC
+#define REG_SETGPIODATAOUT2		0xD
+#define REG_SETGPIODATAOUT3		0xE
+#define REG_GPIO_DEBEN1			0xF
+#define REG_GPIO_DEBEN2			0x10
+#define REG_GPIO_DEBEN3			0x11
+#define REG_GPIO_CTRL			0x12
+#define REG_GPIOPUPDCTR1		0x13
+#define REG_GPIOPUPDCTR2		0x14
+#define REG_GPIOPUPDCTR3		0x15
+#define REG_GPIOPUPDCTR4		0x16
+#define REG_GPIOPUPDCTR5		0x17
+#define REG_GPIO_ISR1A			0x19
+#define REG_GPIO_ISR2A			0x1A
+#define REG_GPIO_ISR3A			0x1B
+#define REG_GPIO_IMR1A			0x1C
+#define REG_GPIO_IMR2A			0x1D
+#define REG_GPIO_IMR3A			0x1E
+#define REG_GPIO_ISR1B			0x1F
+#define REG_GPIO_ISR2B			0x20
+#define REG_GPIO_ISR3B			0x21
+#define REG_GPIO_IMR1B			0x22
+#define REG_GPIO_IMR2B			0x23
+#define REG_GPIO_IMR3B			0x24
+#define REG_GPIO_EDR1			0x28
+#define REG_GPIO_EDR2			0x29
+#define REG_GPIO_EDR3			0x2A
+#define REG_GPIO_EDR4			0x2B
+#define REG_GPIO_EDR5			0x2C
+#define REG_GPIO_SIH_CTRL		0x2D
+
+#endif /* End of __TWL4030_GPIO_H */
diff --git a/include/linux/i2c/twl4030-pwrirq.h b/include/linux/i2c/twl4030-pwrirq.h
new file mode 100644
index 0000000..7a13368
--- /dev/null
+++ b/include/linux/i2c/twl4030-pwrirq.h
@@ -0,0 +1,37 @@
+/*
+ * twl4030-gpio.h - header for TWL4030 GPIO module
+ *
+ * Copyright (C) 2008 Texas Instruments, Inc.
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#ifndef __TWL4030_PWRIRQ_H_
+#define __TWL4030_PWRIRQ_H_
+
+/*
+ * INT Module Register definitions
+ * (not all registers are defined below)
+ */
+
+#define TWL4030_INT_PWR_ISR1		0x0
+#define TWL4030_INT_PWR_IMR1		0x1
+#define TWL4030_INT_PWR_ISR2		0x2
+#define TWL4030_INT_PWR_IMR2		0x3
+#define TWL4030_INT_PWR_SIH_CTRL	0x7
+
+#endif /* End of __TWL4030_PWRIRQ_H */

[PATCH 8/9] TWL4030: use symbolic ISR/IMR register names during twl_init_irq()

[Paul Walmsley]

twl_init_irq() uses a bunch of magic numbers as register indices; this
has already led to several errors, fixed earlier in this patch series.
Now use descriptive macros instead of magic numbers.  This patch should
not change kernel behavior.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |  188 +++++++++++++++++++-------------------
 1 files changed, 96 insertions(+), 92 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 99cc143..38c227a 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -40,6 +40,9 @@
 
 #include <linux/i2c.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/i2c/twl4030-gpio.h>
+#include <linux/i2c/twl4030-madc.h>
+#include <linux/i2c/twl4030-pwrirq.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -114,6 +117,23 @@
 #define TWL4030_BASEADD_RTC		0x001C
 #define TWL4030_BASEADD_SECURED_REG	0x0000
 
+/* TWL4030 BCI registers */
+#define TWL4030_INTERRUPTS_BCIIMR1A	0x2
+#define TWL4030_INTERRUPTS_BCIIMR2A	0x3
+#define TWL4030_INTERRUPTS_BCIIMR1B	0x6
+#define TWL4030_INTERRUPTS_BCIIMR2B	0x7
+#define TWL4030_INTERRUPTS_BCIISR1A	0x0
+#define TWL4030_INTERRUPTS_BCIISR2A	0x1
+#define TWL4030_INTERRUPTS_BCIISR1B	0x4
+#define TWL4030_INTERRUPTS_BCIISR2B	0x5
+
+/* TWL4030 keypad registers */
+#define TWL4030_KEYPAD_KEYP_IMR1	0x12
+#define TWL4030_KEYPAD_KEYP_IMR2	0x14
+#define TWL4030_KEYPAD_KEYP_ISR1	0x11
+#define TWL4030_KEYPAD_KEYP_ISR2	0x13
+
+
 /* Triton Core internal information (END) */
 
 /* Few power values */
@@ -133,12 +153,10 @@
 /* on I2C-1 for 2430SDP */
 #define CONFIG_I2C_TWL4030_ID		1
 
-/* SIH_CTRL registers */
-#define TWL4030_INT_PWR_SIH_CTRL	0x07
+/* SIH_CTRL registers that aren't defined elsewhere */
 #define TWL4030_INTERRUPTS_BCISIHCTRL	0x0d
 #define TWL4030_MADC_MADC_SIH_CTRL	0x67
 #define TWL4030_KEYPAD_KEYP_SIH_CTRL	0x17
-#define TWL4030_GPIO_GPIO_SIH_CTRL	0x2d
 
 #define TWL4030_SIH_CTRL_COR_MASK	(1 << 2)
 
@@ -776,135 +794,121 @@ static void twl_init_irq(void)
 	 * handlers present.
 	 */
 
-
-	/* PWR_IMR1 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff, 0x1) < 0);
-
-	/* PWR_IMR2 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff, 0x3) < 0);
-
-	/* Clear off any other pending interrupts on power */
+	/* Mask INT (PWR) interrupts at TWL4030 */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff,
+				     TWL4030_INT_PWR_IMR1) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff,
+				     TWL4030_INT_PWR_IMR2) < 0);
 
 	/* Are PWR interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_INT,
 				   TWL4030_INT_PWR_SIH_CTRL);
 	WARN_ON(cor < 0);
 
-	/* PWR_ISR1 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x00, cor) < 0);
-
-	/* PWR_ISR2 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x02, cor) < 0);
+	/* Clear TWL4030 INT (PWR) ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
+				      TWL4030_INT_PWR_ISR1, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
+				      TWL4030_INT_PWR_ISR2, cor) < 0);
 
 	/* Slave address 0x4A */
 
-	/* BCIIMR1A */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x2) < 0);
-
-	/* BCIIMR2A  */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x3) < 0);
-
-	/* BCIIMR2A */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x6) < 0);
-
-	/* BCIIMR2B */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff, 0x7) < 0);
+	/* Mask BCI interrupts at TWL4030 */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
+				     TWL4030_INTERRUPTS_BCIIMR1A) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
+				     TWL4030_INTERRUPTS_BCIIMR2A) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
+				     TWL4030_INTERRUPTS_BCIIMR1B) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
+				     TWL4030_INTERRUPTS_BCIIMR2B) < 0);
 
 	/* Are BCI interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_INTERRUPTS,
 				   TWL4030_INTERRUPTS_BCISIHCTRL);
 	WARN_ON(cor < 0);
 
-	/* BCIISR1A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x0, cor) < 0);
-
-	/* BCIISR2A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x1, cor) < 0);
-
-	/* BCIISR1B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x4, cor) < 0);
-
-	/* BCIISR2B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x5, cor) < 0);
+	/* Clear TWL4030 BCI ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR1A, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR2A, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR1B, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR2B, cor) < 0);
 
 	/* MAD C */
-	/* MADC_IMR1 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff, 0x62) < 0);
-
-	/* MADC_IMR2 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff, 0x64) < 0);
+	/* Mask MADC interrupts at TWL4030 */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff,
+				     TWL4030_MADC_IMR1) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff,
+				     TWL4030_MADC_IMR2) < 0);
 
 	/* Are MADC interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_MADC,
 				   TWL4030_MADC_MADC_SIH_CTRL);
 	WARN_ON(cor < 0);
 
-	/* MADC_ISR1 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x61, cor) < 0);
-
-	/* MADC_ISR2 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x63, cor) < 0);
+	/* Clear TWL4030 MADC ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
+				      TWL4030_MADC_ISR1, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
+				      TWL4030_MADC_ISR2, cor) < 0);
 
 	/* key Pad */
-	/* KEYPAD - IMR1 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff, 0x12) < 0);
+	/* Mask keypad interrupts at TWL4030 */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff,
+				     TWL4030_KEYPAD_KEYP_IMR1) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff,
+				     TWL4030_KEYPAD_KEYP_IMR2) < 0);
 
 	/* Are keypad interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_KEYPAD,
 				   TWL4030_KEYPAD_KEYP_SIH_CTRL);
 	WARN_ON(cor < 0);
 
-	/* KEYPAD - ISR1 */
+	/* Clear TWL4030 keypad ISRs */
 	/* XXX does this still need to be done twice for some reason? */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x11, cor) < 0);
-
-	/* KEYPAD - IMR2 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff, 0x14) < 0);
-
-	/* KEYPAD - ISR2 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x13, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
+				      TWL4030_KEYPAD_KEYP_ISR1, cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
+				      TWL4030_KEYPAD_KEYP_ISR2, cor) < 0);
 
 	/* Slave address 0x49 */
-	/* GPIO_IMR1A */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1c) < 0);
-
-	/* GPIO_IMR2A */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1d) < 0);
-
-	/* GPIO_IMR3A */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x1e) < 0);
-
-	/* GPIO_IMR1B */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x22) < 0);
 
-	/* GPIO_IMR2B */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x23) < 0);
-
-	/* GPIO_IMR3B */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff, 0x24) < 0);
+	/* Mask GPIO interrupts at TWL4030 */
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR1A) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR2A) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR3A) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR1B) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR2B) < 0);
+	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
+				     REG_GPIO_IMR3B) < 0);
 
 	/* Are GPIO interrupt status bits cleared by reads or writes? */
 	cor = twl4030_read_cor_bit(TWL4030_MODULE_GPIO,
-				   TWL4030_GPIO_GPIO_SIH_CTRL);
+				   REG_GPIO_SIH_CTRL);
 	WARN_ON(cor < 0);
 
-	/* GPIO_ISR1A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x19, cor) < 0);
-
-	/* GPIO_ISR2A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1a, cor) < 0);
-
-	/* GPIO_ISR3A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1b, cor) < 0);
-
-	/* GPIO_ISR1B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1f, cor) < 0);
-
-	/* GPIO_ISR2B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x20, cor) < 0);
-
-	/* GPIO_ISR3B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x21, cor) < 0);
+	/* Clear TWL4030 GPIO ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1A,
+				      cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2A,
+				      cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3A,
+				      cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1B,
+				      cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2B,
+				      cor) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3B,
+				      cor) < 0);
 
 	/* install an irq handler for each of the PIH modules */
 	for (i = TWL4030_IRQ_BASE; i < TWL4030_IRQ_END; i++) {

[PATCH 9/9] TWL4030: convert early interrupt mask/clear funcs to use array

[Paul Walmsley]

Mask/clear TWL module IMRs/ISRs by iterating through arrays rather than
using a block of cut-and-pasted commands.  Removes 1056 bytes of bloat.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

 drivers/i2c/chips/twl4030-core.c |  302 +++++++++++++++++++++++---------------
 1 files changed, 180 insertions(+), 122 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 38c227a..3fd2b24 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -160,6 +160,136 @@
 
 #define TWL4030_SIH_CTRL_COR_MASK	(1 << 2)
 
+/**
+ * struct twl4030_mod_iregs - TWL module IMR/ISR regs to mask/clear at init
+ * @mod_no: TWL4030 module number (e.g., TWL4030_MODULE_GPIO)
+ * @sih_ctrl: address of module SIH_CTRL register
+ * @reg_cnt: number of IMR/ISR regs
+ * @imrs: pointer to array of TWL module interrupt mask register indices
+ * @isrs: pointer to array of TWL module interrupt status register indices
+ *
+ * Ties together TWL4030 modules and lists of IMR/ISR registers to mask/clear
+ * during twl_init_irq().
+ */
+struct twl4030_mod_iregs {
+	const u8 mod_no;
+	const u8 sih_ctrl;
+	const u8 reg_cnt;
+	const u8 *imrs;
+	const u8 *isrs;
+};
+
+/* TWL4030 INT module interrupt mask registers */
+static const u8 __initconst twl4030_int_imr_regs[] = {
+	TWL4030_INT_PWR_IMR1,
+	TWL4030_INT_PWR_IMR2,
+};
+
+/* TWL4030 INT module interrupt status registers */
+static const u8 __initconst twl4030_int_isr_regs[] = {
+	TWL4030_INT_PWR_ISR1,
+	TWL4030_INT_PWR_ISR2,
+};
+
+/* TWL4030 INTERRUPTS module interrupt mask registers */
+static const u8 __initconst twl4030_interrupts_imr_regs[] = {
+	TWL4030_INTERRUPTS_BCIIMR1A,
+	TWL4030_INTERRUPTS_BCIIMR1B,
+	TWL4030_INTERRUPTS_BCIIMR2A,
+	TWL4030_INTERRUPTS_BCIIMR2B,
+};
+
+/* TWL4030 INTERRUPTS module interrupt status registers */
+static const u8 __initconst twl4030_interrupts_isr_regs[] = {
+	TWL4030_INTERRUPTS_BCIISR1A,
+	TWL4030_INTERRUPTS_BCIISR1B,
+	TWL4030_INTERRUPTS_BCIISR2A,
+	TWL4030_INTERRUPTS_BCIISR2B,
+};
+
+/* TWL4030 MADC module interrupt mask registers */
+static const u8 __initconst twl4030_madc_imr_regs[] = {
+	TWL4030_MADC_IMR1,
+	TWL4030_MADC_IMR2,
+};
+
+/* TWL4030 MADC module interrupt status registers */
+static const u8 __initconst twl4030_madc_isr_regs[] = {
+	TWL4030_MADC_ISR1,
+	TWL4030_MADC_ISR2,
+};
+
+/* TWL4030 keypad module interrupt mask registers */
+static const u8 __initconst twl4030_keypad_imr_regs[] = {
+	TWL4030_KEYPAD_KEYP_IMR1,
+	TWL4030_KEYPAD_KEYP_IMR2,
+};
+
+/* TWL4030 keypad module interrupt status registers */
+static const u8 __initconst twl4030_keypad_isr_regs[] = {
+	TWL4030_KEYPAD_KEYP_ISR1,
+	TWL4030_KEYPAD_KEYP_ISR2,
+};
+
+/* TWL4030 GPIO module interrupt mask registers */
+static const u8 __initconst twl4030_gpio_imr_regs[] = {
+	REG_GPIO_IMR1A,
+	REG_GPIO_IMR1B,
+	REG_GPIO_IMR2A,
+	REG_GPIO_IMR2B,
+	REG_GPIO_IMR3A,
+	REG_GPIO_IMR3B,
+};
+
+/* TWL4030 GPIO module interrupt status registers */
+static const u8 __initconst twl4030_gpio_isr_regs[] = {
+	REG_GPIO_ISR1A,
+	REG_GPIO_ISR1B,
+	REG_GPIO_ISR2A,
+	REG_GPIO_ISR2B,
+	REG_GPIO_ISR3A,
+	REG_GPIO_ISR3B,
+};
+
+/* TWL4030 modules that have IMR/ISR registers that must be masked/cleared */
+static const struct twl4030_mod_iregs __initconst twl4030_mod_regs[] = {
+	{
+		.mod_no	  = TWL4030_MODULE_INT,
+		.sih_ctrl = TWL4030_INT_PWR_SIH_CTRL,
+		.reg_cnt  = ARRAY_SIZE(twl4030_int_imr_regs),
+		.imrs	  = twl4030_int_imr_regs,
+		.isrs	  = twl4030_int_isr_regs,
+	},
+	{
+		.mod_no	  = TWL4030_MODULE_INTERRUPTS,
+		.sih_ctrl = TWL4030_INTERRUPTS_BCISIHCTRL,
+		.reg_cnt  = ARRAY_SIZE(twl4030_interrupts_imr_regs),
+		.imrs	  = twl4030_interrupts_imr_regs,
+		.isrs	  = twl4030_interrupts_isr_regs,
+	},
+	{
+		.mod_no	  = TWL4030_MODULE_MADC,
+		.sih_ctrl = TWL4030_MADC_MADC_SIH_CTRL,
+		.reg_cnt  = ARRAY_SIZE(twl4030_madc_imr_regs),
+		.imrs	  = twl4030_madc_imr_regs,
+		.isrs	  = twl4030_madc_isr_regs,
+	},
+	{
+		.mod_no	  = TWL4030_MODULE_KEYPAD,
+		.sih_ctrl = TWL4030_KEYPAD_KEYP_SIH_CTRL,
+		.reg_cnt  = ARRAY_SIZE(twl4030_keypad_imr_regs),
+		.imrs	  = twl4030_keypad_imr_regs,
+		.isrs	  = twl4030_keypad_isr_regs,
+	},
+	{
+		.mod_no	  = TWL4030_MODULE_GPIO,
+		.sih_ctrl = REG_GPIO_SIH_CTRL,
+		.reg_cnt  = ARRAY_SIZE(twl4030_gpio_imr_regs),
+		.imrs	  = twl4030_gpio_imr_regs,
+		.isrs	  = twl4030_gpio_isr_regs,
+	},
+};
+
 
 /* Helper functions */
 static int
@@ -779,136 +909,64 @@ static int twl4030_read_cor_bit(u8 mod_no, u8 reg)
 	return tmp;
 }
 
+/**
+ * twl4030_mask_clear_intrs - mask and clear all TWL4030 interrupts
+ * @t: pointer to twl4030_mod_iregs array
+ * @t_sz: ARRAY_SIZE(t) (starting at 1)
+ *
+ * Mask all TWL4030 interrupt mask registers (IMRs) and clear all
+ * interrupt status registers (ISRs).  No return value, but will WARN if
+ * any I2C operations fail.
+ */
+static void __init twl4030_mask_clear_intrs(const struct twl4030_mod_iregs *t,
+					    const u8 t_sz)
+{
+	int i, j;
+
+	/*
+	 * N.B. - further efficiency is possible here.  Eight I2C
+	 * operations on BCI and GPIO modules are avoidable if I2C
+	 * burst read/write transactions were implemented.  Would
+	 * probably save about 1ms of boot time and a small amount of
+	 * power.
+	 */
+	for (i = 0; i < t_sz; i++) {
+		const struct twl4030_mod_iregs tmr = t[i];
+		int cor;
+
+		/* Are ISRs cleared by reads or writes? */
+		cor = twl4030_read_cor_bit(tmr.mod_no, tmr.sih_ctrl);
+		WARN_ON(cor < 0);
+
+		for (j = 0; j < tmr.reg_cnt; j++) {
+
+			/* Mask interrupts at the TWL4030 */
+			WARN_ON(twl4030_i2c_write_u8(tmr.mod_no, 0xff,
+						     tmr.imrs[j]) < 0);
+
+			/* Clear TWL4030 ISRs */
+			WARN_ON(twl4030_i2c_clear_isr(tmr.mod_no,
+						      tmr.isrs[j], cor) < 0);
+		}
+	}
+
+	return;
+}
+
+
 static void twl_init_irq(void)
 {
-	int	i = 0;
+	int	i;
 	int	res = 0;
-	int cor;
 	char	*msg = "Unable to register interrupt subsystem";
 	unsigned int irq_num;
 
 	/*
-	 * For each TWL4030 module with ISR/IMR registers, mask all
-	 * interrupts and then clear any existing interrupt status bits,
-	 * since we initially do not have any TWL4030 module interrupt
-	 * handlers present.
+	 * Mask and clear all TWL4030 interrupts since initially we do
+	 * not have any TWL4030 module interrupt handlers present
 	 */
-
-	/* Mask INT (PWR) interrupts at TWL4030 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff,
-				     TWL4030_INT_PWR_IMR1) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INT, 0xff,
-				     TWL4030_INT_PWR_IMR2) < 0);
-
-	/* Are PWR interrupt status bits cleared by reads or writes? */
-	cor = twl4030_read_cor_bit(TWL4030_MODULE_INT,
-				   TWL4030_INT_PWR_SIH_CTRL);
-	WARN_ON(cor < 0);
-
-	/* Clear TWL4030 INT (PWR) ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
-				      TWL4030_INT_PWR_ISR1, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
-				      TWL4030_INT_PWR_ISR2, cor) < 0);
-
-	/* Slave address 0x4A */
-
-	/* Mask BCI interrupts at TWL4030 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
-				     TWL4030_INTERRUPTS_BCIIMR1A) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
-				     TWL4030_INTERRUPTS_BCIIMR2A) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
-				     TWL4030_INTERRUPTS_BCIIMR1B) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
-				     TWL4030_INTERRUPTS_BCIIMR2B) < 0);
-
-	/* Are BCI interrupt status bits cleared by reads or writes? */
-	cor = twl4030_read_cor_bit(TWL4030_MODULE_INTERRUPTS,
-				   TWL4030_INTERRUPTS_BCISIHCTRL);
-	WARN_ON(cor < 0);
-
-	/* Clear TWL4030 BCI ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR1A, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR2A, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR1B, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR2B, cor) < 0);
-
-	/* MAD C */
-	/* Mask MADC interrupts at TWL4030 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff,
-				     TWL4030_MADC_IMR1) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_MADC, 0xff,
-				     TWL4030_MADC_IMR2) < 0);
-
-	/* Are MADC interrupt status bits cleared by reads or writes? */
-	cor = twl4030_read_cor_bit(TWL4030_MODULE_MADC,
-				   TWL4030_MADC_MADC_SIH_CTRL);
-	WARN_ON(cor < 0);
-
-	/* Clear TWL4030 MADC ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
-				      TWL4030_MADC_ISR1, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
-				      TWL4030_MADC_ISR2, cor) < 0);
-
-	/* key Pad */
-	/* Mask keypad interrupts at TWL4030 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff,
-				     TWL4030_KEYPAD_KEYP_IMR1) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff,
-				     TWL4030_KEYPAD_KEYP_IMR2) < 0);
-
-	/* Are keypad interrupt status bits cleared by reads or writes? */
-	cor = twl4030_read_cor_bit(TWL4030_MODULE_KEYPAD,
-				   TWL4030_KEYPAD_KEYP_SIH_CTRL);
-	WARN_ON(cor < 0);
-
-	/* Clear TWL4030 keypad ISRs */
-	/* XXX does this still need to be done twice for some reason? */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
-				      TWL4030_KEYPAD_KEYP_ISR1, cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
-				      TWL4030_KEYPAD_KEYP_ISR2, cor) < 0);
-
-	/* Slave address 0x49 */
-
-	/* Mask GPIO interrupts at TWL4030 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR1A) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR2A) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR3A) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR1B) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR2B) < 0);
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xff,
-				     REG_GPIO_IMR3B) < 0);
-
-	/* Are GPIO interrupt status bits cleared by reads or writes? */
-	cor = twl4030_read_cor_bit(TWL4030_MODULE_GPIO,
-				   REG_GPIO_SIH_CTRL);
-	WARN_ON(cor < 0);
-
-	/* Clear TWL4030 GPIO ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1A,
-				      cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2A,
-				      cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3A,
-				      cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1B,
-				      cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2B,
-				      cor) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3B,
-				      cor) < 0);
+	twl4030_mask_clear_intrs(twl4030_mod_regs,
+				 ARRAY_SIZE(twl4030_mod_regs));
 
 	/* install an irq handler for each of the PIH modules */
 	for (i = TWL4030_IRQ_BASE; i < TWL4030_IRQ_END; i++) {

Re: [PATCH 0/9] TWL4030 bugfixes and cleanups: version 2

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [080723 08:11]:
> [ This is the second version of these patches - they now check the
> SIH_CTRL.COR bit to determine whether to read or write to clear ]
> 
> At boot, twl_init_irq() tries to mask off all TWL4030 secondary
> interrupts and clear them, since no secondary interrupt handlers are
> registered yet (that happens later in the boot).
> 
> However, this code is flawed.  MADC, BCI, keypad, and GPIO ISRs are
> either not cleared or are incompletely cleared.  Some BCI register
> addresses are nonsensical.  And some code attempts to clear ISRs
> before masking IMRs, which is race-prone (an interrupt could occur
> after the ISR clear, but before the mask).
> 
> These problems can cause large numbers of boot messages similar to
> this:
> 
>     TWL4030 module irq 368 is disabled but can't be masked!
> 
> and I2C transmit overflows, both observed during boot on the
> BeagleBoard here.  The actual IRQ number sometimes varied depending on
> which interrupt was unacknowledged.
> 
> This series fixes these issues and does some code cleanup, saving 272
> bytes.
> 
> Thanks to Philip Balister <philip@balister.org>, Koen Kooi
> <k.kooi@student.utwente.nl>, Måns Rullgård <mans@mansr.org>, Steve
> Sakoman <sakoman@gmail.com> and others on #beagle for help tracing
> these problems.
> 
> Thanks also to Peter De Schrijver <peter.de-schrijver@nokia.com> for
> help with this second version, allowing several unnecessary I2C
> transactions to be elided from twl_init_irq().
> 
> Tested on BeagleBoard rev B4 (3530ES2.2).

Pushing this series today.

Tony

> 
> 
> - Paul
> 
> ---
> 
> size:
>    text    data     bss     dec     hex filename
> 3076157  142840  104248 3323245  32b56d vmlinux.beagle.orig
> 3075885  142840  104248 3322973  32b45d vmlinux.beagle
> 
> diffstat:
> 
>  drivers/i2c/chips/twl4030-core.c   |  383 ++++++++++++++++++++++--------------
>  drivers/i2c/chips/twl4030-gpio.c   |   48 -----
>  drivers/i2c/chips/twl4030-pwrirq.c |   15 +
>  include/linux/i2c/twl4030-gpio.h   |   76 +++++++
>  include/linux/i2c/twl4030-pwrirq.h |   37 +++
>  5 files changed, 359 insertions(+), 200 deletions(-)
>  create mode 100644 include/linux/i2c/twl4030-gpio.h
>  create mode 100644 include/linux/i2c/twl4030-pwrirq.h
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html