[PATCH 0/9] TWL4030 bugfixes and cleanups

[PATCH 0/9] TWL4030 bugfixes and cleanups

[Paul Walmsley]

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!

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 336
bytes.  The __init changes cause an extra 4KB page to be freed after
kernel init on Beagle.

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.

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

- Paul

---

Paul Walmsley (9):
      TWL4030: convert early interrupt mask/clear funcs to use array
      TWL4030: use symbolic ISR/IMR register names during twl_init_irq()
      TWL4030: move TWL module register defs into separate include files
      TWL4030: change init-time IMR mask code to WARN if error
      TWL4030: read and write module ISRs to clear them at init
      TWL4030: clear MADC interrupt status registers upon init
      TWL4030: use correct register addresses for BCI IMR registers
      TWL4030: clear TWL GPIO interrupt status registers
      TWL4030: remove superfluous PWR interrupt status clear before masking


 drivers/i2c/chips/twl4030-core.c   |  335 ++++++++++++++++++++----------------
 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, 303 insertions(+), 208 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: read and write module ISRs to clear them at init

[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").

twl4030-core.c does not touch these *_SIH_CTRL registers during boot,
and the TWL4030 TRM is deeply confused as to whether COR=1 means that
the registers are cleared on reads, or cleared on writes.

So, take the cautious way out and both read from and write to the TWL4030
module ISRs to clear them at startup.  Also, use WARN_ON() to warn if the
read/write failed, and don't skip the rest of the initialization on failure
either.

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

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

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 9d93524..615fb84 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -712,6 +712,28 @@ 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
+ *
+ * Reads, then writes 0xff to a TWL4030 interrupt status register to ensure
+ * that interrupts are cleared.  The read + write is necessary since we
+ * don't know whether the COR bit is set in <module>_SIH_CTRL.  Returns
+ * the status from the I2C read operation.
+ */
+static int twl4030_i2c_clear_isr(u8 mod_no, u8 reg)
+{
+	int res;
+	u8 tmp;
+
+	res = twl4030_i2c_read_u8(mod_no, &tmp, reg);
+	if (res < 0)
+		return res;
+
+	return twl4030_i2c_write_u8(mod_no, 0xff, reg);
+}
+
 static void twl_init_irq(void)
 {
 	int	i = 0;
@@ -719,6 +741,13 @@ static void twl_init_irq(void)
 	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) {
@@ -735,19 +764,11 @@ static void twl_init_irq(void)
 
 	/* Clear off any other pending interrupts on power */
 	/* 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) < 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) < 0);
+
 	/* Slave address 0x4A */
 
 	/* BCIIMR1A */
@@ -779,32 +800,16 @@ static void twl_init_irq(void)
 	}
 
 	/* 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) < 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) < 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) < 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) < 0);
 
 	/* MAD C */
 	/* MADC_IMR1 */
@@ -822,18 +827,10 @@ static void twl_init_irq(void)
 	}
 
 	/* 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) < 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) < 0);
 
 	/* key Pad */
 	/* KEYPAD - IMR1 */
@@ -842,12 +839,10 @@ 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);
-	}
+
+	/* KEYPAD - ISR1 */
+	/* XXX does this still need to be done twice for some reason? */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x11) < 0);
 
 	/* KEYPAD - IMR2 */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xFF, (0x14));
@@ -856,6 +851,9 @@ static void twl_init_irq(void)
 		return;
 	}
 
+	/* KEYPAD - ISR2 */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x13) < 0);
+
 	/* Slave address 0x49 */
 	/* GPIO_IMR1A */
 	res = twl4030_i2c_write_u8(TWL4030_MODULE_GPIO, 0xFF, (0x1C));
@@ -900,46 +898,22 @@ static void twl_init_irq(void)
 	}
 
 	/* 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) < 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) < 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) < 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) < 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) < 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) < 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 615fb84..1906635 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -749,18 +749,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 */
 	/* PWR_ISR1 */
@@ -772,32 +764,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);
 
 	/* BCIISR1A */
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x0) < 0);
@@ -813,18 +789,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);
 
 	/* MADC_ISR1 */
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x61) < 0);
@@ -834,68 +802,36 @@ 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);
 
 	/* KEYPAD - ISR1 */
 	/* XXX does this still need to be done twice for some reason? */
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD, 0x11) < 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) < 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);
 
 	/* GPIO_ISR1A */
 	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x19) < 0);

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

[Paul Walmsley]

twl_init_irq() accesses TWL module IMR and ISR registers.  Currently, it
uses "magic numbers" for these register indices, but 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 |  181 +++++++++++++++++++-------------------
 1 files changed, 89 insertions(+), 92 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 1906635..5855f5f 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 */
@@ -748,108 +768,85 @@ 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 */
-	/* PWR_ISR1 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x00) < 0);
-
-	/* PWR_ISR2 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT, 0x02) < 0);
+	/* 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);
+	/* Clear TWL4030 INT (PWR) ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
+				      TWL4030_INT_PWR_ISR1) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
+				      TWL4030_INT_PWR_ISR2) < 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);
-
-	/* BCIISR1A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x0) < 0);
-
-	/* BCIISR2A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x1) < 0);
-
-	/* BCIISR1B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x4) < 0);
-
-	/* BCIISR2B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS, 0x5) < 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);
+	/* Clear TWL4030 BCI ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR1A) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR2A) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR1B) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
+				      TWL4030_INTERRUPTS_BCIISR2B) < 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);
-
-	/* MADC_ISR1 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x61) < 0);
-
-	/* MADC_ISR2 */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC, 0x63) < 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);
+	/* Clear TWL4030 MADC ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
+				      TWL4030_MADC_ISR1) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
+				      TWL4030_MADC_ISR2) < 0);
 
 	/* key Pad */
-	/* KEYPAD - IMR1 */
-	WARN_ON(twl4030_i2c_write_u8(TWL4030_MODULE_KEYPAD, 0xff, 0x12) < 0);
-
-	/* KEYPAD - ISR1 */
+	/* 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);
+	/* 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) < 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) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
+				      TWL4030_KEYPAD_KEYP_ISR1) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
+				      TWL4030_KEYPAD_KEYP_ISR2) < 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);
-
-	/* GPIO_ISR1A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x19) < 0);
-
-	/* GPIO_ISR2A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1a) < 0);
-
-	/* GPIO_ISR3A */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1b) < 0);
-
-	/* GPIO_ISR1B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x1f) < 0);
-
-	/* GPIO_ISR2B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x20) < 0);
-
-	/* GPIO_ISR3B */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, 0x21) < 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);
+
+	/* Clear TWL4030 GPIO ISRs */
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1A) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2A) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3A) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1B) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2B) < 0);
+	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3B) < 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 632 bytes of bloat.

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

 drivers/i2c/chips/twl4030-core.c |  218 ++++++++++++++++++++++++--------------
 1 files changed, 137 insertions(+), 81 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-core.c b/drivers/i2c/chips/twl4030-core.c
index 5855f5f..47d547d 100644
--- a/drivers/i2c/chips/twl4030-core.c
+++ b/drivers/i2c/chips/twl4030-core.c
@@ -153,6 +153,130 @@
 /* on I2C-1 for 2430SDP */
 #define CONFIG_I2C_TWL4030_ID		1
 
+/**
+ * struct twl4030_mod_iregs - TWL module IMR/ISR regs to mask/clear at init
+ * @mod_no: TWL4030 module number (e.g., TWL4030_MODULE_GPIO)
+ * @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 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,
+		.reg_cnt = ARRAY_SIZE(twl4030_int_imr_regs),
+		.imrs = twl4030_int_imr_regs,
+		.isrs = twl4030_int_isr_regs,
+	},
+	{
+		.mod_no = TWL4030_MODULE_INTERRUPTS,
+		.reg_cnt = ARRAY_SIZE(twl4030_interrupts_imr_regs),
+		.imrs = twl4030_interrupts_imr_regs,
+		.isrs = twl4030_interrupts_isr_regs,
+	},
+	{
+		.mod_no = TWL4030_MODULE_MADC,
+		.reg_cnt = ARRAY_SIZE(twl4030_madc_imr_regs),
+		.imrs = twl4030_madc_imr_regs,
+		.isrs = twl4030_madc_isr_regs,
+	},
+	{
+		.mod_no = TWL4030_MODULE_KEYPAD,
+		.reg_cnt = ARRAY_SIZE(twl4030_keypad_imr_regs),
+		.imrs = twl4030_keypad_imr_regs,
+		.isrs = twl4030_keypad_isr_regs,
+	},
+	{
+		.mod_no = TWL4030_MODULE_GPIO,
+		.reg_cnt = ARRAY_SIZE(twl4030_gpio_imr_regs),
+		.imrs = twl4030_gpio_imr_regs,
+		.isrs = twl4030_gpio_isr_regs,
+	},
+};
+
+
 /* Helper functions */
 static int
 twl4030_detect_client(struct i2c_adapter *adapter, unsigned char sid);
@@ -756,7 +880,7 @@ static int twl4030_i2c_clear_isr(u8 mod_no, u8 reg)
 
 static void twl_init_irq(void)
 {
-	int	i = 0;
+	int	i, j;
 	int	res = 0;
 	char	*msg = "Unable to register interrupt subsystem";
 	unsigned int irq_num;
@@ -767,86 +891,18 @@ static void twl_init_irq(void)
 	 * since we initially 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);
-	/* Clear TWL4030 INT (PWR) ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
-				      TWL4030_INT_PWR_ISR1) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INT,
-				      TWL4030_INT_PWR_ISR2) < 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);
-	/* Clear TWL4030 BCI ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR1A) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR2A) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR1B) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_INTERRUPTS,
-				      TWL4030_INTERRUPTS_BCIISR2B) < 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);
-	/* Clear TWL4030 MADC ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
-				      TWL4030_MADC_ISR1) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_MADC,
-				      TWL4030_MADC_ISR2) < 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);
-	/* 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) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_KEYPAD,
-				      TWL4030_KEYPAD_KEYP_ISR2) < 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);
-
-	/* Clear TWL4030 GPIO ISRs */
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1A) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2A) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3A) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR1B) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR2B) < 0);
-	WARN_ON(twl4030_i2c_clear_isr(TWL4030_MODULE_GPIO, REG_GPIO_ISR3B) < 0);
+	for (i = 0; i < ARRAY_SIZE(twl4030_mod_regs); i++) {
+		const struct twl4030_mod_iregs tmr = twl4030_mod_regs[i];
+
+		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]) < 0);
+		}
+	}
 
 	/* install an irq handler for each of the PIH modules */
 	for (i = TWL4030_IRQ_BASE; i < TWL4030_IRQ_END; i++) {

Re: [PATCH 5/9] TWL4030: read and write module ISRs to clear them at init

[Peter 'p2' De Schrijver]

On Thu, Jul 17, 2008 at 07:34:52PM -0600, ext Paul Walmsley wrote:
> 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").
> 
> twl4030-core.c does not touch these *_SIH_CTRL registers during boot,
> and the TWL4030 TRM is deeply confused as to whether COR=1 means that
> the registers are cleared on reads, or cleared on writes.
> 

That's true. But reality is fortunately not so confused :) COR=1 means
all IRQs are acknowledged when reading the corresponding ISR. COR=0
means you need to write 1 to the bits in the ISR for interrupts you want to
acknowledge.

Hope this helps,

Cheers,

Peter.

-- 
goa is a state of mind

Re: [PATCH 5/9] TWL4030: read and write module ISRs to clear them at init

[Paul Walmsley]

Hello Peter,

On Fri, 18 Jul 2008, Peter 'p2' De Schrijver wrote:

> On Thu, Jul 17, 2008 at 07:34:52PM -0600, ext Paul Walmsley wrote:
> > 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").
> > 
> > twl4030-core.c does not touch these *_SIH_CTRL registers during boot,
> > and the TWL4030 TRM is deeply confused as to whether COR=1 means that
> > the registers are cleared on reads, or cleared on writes.
> > 
> 
> That's true. But reality is fortunately not so confused :) COR=1 means
> all IRQs are acknowledged when reading the corresponding ISR. COR=0
> means you need to write 1 to the bits in the ISR for interrupts you want to
> acknowledge.
> 
> Hope this helps,

thanks very much, it does help.  Ought to save a little time on boot.  
Will post an updated set tomorrow...
  

- Paul