Skip to contentSkip to conversation

twl4030-core: move to drivers/mfd

PATCH In review David Brownell 4 participants 10 messages Oct 2, 2008 – Oct 4, 2008

Subsystems (2)
Multifunction devices (MFD) OMAP2+ support
lore
Share & tools
lore

David Brownell

author
#0 lore raw

PATCH 1/2 twl4030-core: move to drivers/mfd

Signed-off-by David Brownell <[email protected]>

Move twl4030-core to drivers/mfd from drivers/i2c/chips, along with its Kconfig and Makefile help.

The intent here is: no functional change, match the patch sent to LKML this morning (except, fix that spelling error in the new Kconfig text).

Signed-off-by David Brownell <[email protected]>

---
NOTE: minor weirdness. The twl403-pwrirq initializes earlier (?) than before ... but "i2c_omap i2c_omap.1: controller timed out" is the response to its first I2C request. So it bails, causing twl4030_usb and then rtc_twl4030 request_irq() calls to fail, and those drivers probe() calls fail.

But, for the first time in a long while, the USB host now enumerates my Ethernet adapter!! So it's a net win in terms of overall behavior, since I can now get the time from NTP (loss of RTC doesn't hurt).

Make the pwrirq code retry that first write (setting the mask to the value the core already set it) ... and pwrirq comes up, RTC, the transceiver glue. And USB can't enumerate that adapter. :(

 drivers/i2c/chips/Kconfig        |    6 
 drivers/i2c/chips/Makefile       |    2 
 drivers/i2c/chips/twl4030-core.c | 1255 -------------------------------------
 drivers/mfd/Kconfig              |   14 
 drivers/mfd/Makefile             |    2 
 drivers/mfd/twl4030-core.c       | 1255 +++++++++++++++++++++++++++++++++++++
 6 files changed, 1272 insertions(+), 1262 deletions(-)
drivers/i2c/chips/Kconfig +0 -6
@@ -151,12 +151,6 @@ config GPIOEXPANDER_OMAP
 	  If you say yes here you get support for I/O expander calls
 	  to configure IrDA, Camera and audio devices.
 
-config TWL4030_CORE
-	bool "TI's TWL4030 companion chip Core Driver Support"
-	depends on I2C=y && (ARCH_OMAP24XX || ARCH_OMAP34XX)
-	help
-	  Say yes here if you have TWL4030 chip on your board
-
 config TWL4030_MADC
 	tristate "TWL4030 MADC Driver"
 	depends on TWL4030_CORE
drivers/i2c/chips/Makefile +1 -1
@@ -24,7 +24,7 @@ obj-$(CONFIG_GPIOEXPANDER_OMAP)	+= gpio_
 obj-$(CONFIG_MENELAUS)		+= menelaus.o
 obj-$(CONFIG_SENSORS_TSL2550)	+= tsl2550.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
-obj-$(CONFIG_TWL4030_CORE)	+= twl4030-core.o twl4030-pwrirq.o twl4030-power.o
+obj-$(CONFIG_TWL4030_CORE)	+= twl4030-pwrirq.o twl4030-power.o
 obj-$(CONFIG_TWL4030_USB)	+= twl4030-usb.o
 obj-$(CONFIG_TWL4030_POWEROFF)	+= twl4030-poweroff.o
 obj-$(CONFIG_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
drivers/i2c/chips/twl4030-core.c deleted +0 -1255

1,255 lines not on this page · raw message

drivers/mfd/Kconfig +14 -0
@@ -50,6 +50,20 @@ config HTC_PASIC3
 	  HTC Magician devices, respectively. Actual functionality is
 	  handled by the leds-pasic3 and ds1wm drivers.
 
+config TWL4030_CORE
+	bool "Texas Instruments TWL4030/TPS659x0 Support"
+	depends on I2C=y && GENERIC_HARDIRQS && (ARCH_OMAP2 || ARCH_OMAP3)
+	help
+	  Say yes here if you have TWL4030 family chip on your board.
+	  This core driver provides register access and IRQ handling
+	  facilities, and registers devices for the various functions
+	  so that function-specific drivers can bind to them.
+
+	  These multi-function chips are found on many OMAP2 and OMAP3
+	  boards, providing power management, RTC, GPIO, keypad, a
+	  high speed USB OTG transceiver, an audio codec (on most
+	  versions) and many other features.
+
 config MFD_TMIO
 	bool
 	default n
drivers/mfd/Makefile +2 -0
@@ -12,6 +12,8 @@ obj-$(CONFIG_MFD_T7L66XB)	+= t7l66xb.o
 obj-$(CONFIG_MFD_TC6387XB)	+= tc6387xb.o
 obj-$(CONFIG_MFD_TC6393XB)	+= tc6393xb.o
 
+obj-$(CONFIG_TWL4030_CORE)	+= twl4030-core.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_MCP)		+= mcp-core.o
drivers/mfd/twl4030-core.c new file +1255 -0

1,255 lines not on this page · raw message

#1 lore raw
Quoted 1 line · David Brownell
Signed-off-by David Brownell <[email protected]>

I suppose this first one should be From:

Quoted 23 lines
drivers/i2c/chips/twl4030-core.c deleted +0 -23
-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 */

drivers/i2c/chips/twl4030-core.c line 1097

Quoted 10 lines
drivers/i2c/chips/twl4030-core.c deleted +0 -10
-			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;

this return can be removed.

Quoted 23 lines
drivers/mfd/twl4030-core.c new file +23 -0
+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 */

drivers/mfd/twl4030-core.c line 1097

Quoted 10 lines
drivers/mfd/twl4030-core.c new file +10 -0
+			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;

also this one

Signature · 1 line
balbi
author
#2 lore raw
Quoted 4 lines · Felipe Balbi
Quoted 1 line · David Brownell
Signed-off-by David Brownell <[email protected]>

I suppose this first one should be From:

Yes.

Quoted 3 lines

drivers/mfd/twl4030-core.c line 1097

Quoted 1 line
drivers/mfd/twl4030-core.c new file +1 -0
+	return;

also this one

I've got a "lots of twl-core cleanups" patch; I'll include this change there. This was *just* a "move".

#3 lore raw
Quoted 6 lines · David Brownell
Quoted 4 lines · Felipe Balbi
Quoted 1 line · David Brownell
Signed-off-by David Brownell <[email protected]>

I suppose this first one should be From:

Quoted 8 lines

Yes.

Quoted 3 lines

drivers/mfd/twl4030-core.c line 1097

Quoted 1 line
drivers/mfd/twl4030-core.c new file +1 -0
+	return;

also this one

I've got a "lots of twl-core cleanups" patch; I'll include this change there. This was *just* a "move".

Sounds good :-)

Signature · 1 line
balbi
maintainer
#4 lore raw
Quoted 9 lines · Felipe Balbi
Quoted 8 lines · David Brownell
Quoted 4 lines · Felipe Balbi
Quoted 1 line · David Brownell
Signed-off-by David Brownell <[email protected]>

I suppose this first one should be From:

Yes.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

drivers/mfd/twl4030-core.c line 1097

Quoted 1 line
drivers/mfd/twl4030-core.c new file +1 -0
+	return;

also this one

I've got a "lots of twl-core cleanups" patch; I'll include this change there. This was *just* a "move".

Sounds good :-)

Pushed.

Tony

#5 lore raw
Quoted 1 line · Tony Lindgren

Pushed.

After pulling this one (twl4030-core: move to drivers/mfd), TWL4030 doesn't initialize the same way on my BeagleBoard.

Before (708244b3f3d5dad958059729c384a068ecd46161):
twl4030_usb twl4030_usb: Initialized TWL4030 USB module
input: triton2-pwrbutton as /class/input/input0 triton2 power button driver initialized

After (56d33dd99b13c4f18df838e766c595299f7e1e0f):
can't get IRQ 378, err -22
twl4030_usb: probe of twl4030_usb failed with error -22 Unable to allocate IRQ for power button

This is a stock linux-omap with no patches.

- Nathan

author
#6 lore raw
Quoted 5 lines · Nathan Monson

can't get IRQ 378, err -22
twl4030_usb: probe of twl4030_usb failed with error -22 Unable to allocate IRQ for power button

This is a stock linux-omap with no patches.

Did you see "i2c_omap i2c_omap.1: controller timed out" earlier in the boot sequence? It would have this symptom, among others... Earlier in this thread you'll find:

http://marc.info/?l=linux-omap&m=122297833314280&w=2

commenting on the issue I noted when I posted this patch; showing how I work around that specific I2C error. It'd likely work for you too.

I've seen this i2c adapter hit such failures in other places too, but that location is more reproducible. And it has relatively *visible* nasty side effects...

- Dave

p.s. Shades of OMAP1 I2C problems! I recall similar
cases of the I2C adapter driver stumbling during boot and thus making registrations fail. The same workaround (retry) solved things then too...

#7 lore raw
Quoted 9 lines · David Brownell
Quoted 3 lines · Nathan Monson

can't get IRQ 378, err -22
twl4030_usb: probe of twl4030_usb failed with error -22 Unable to allocate IRQ for power button

Did you see "i2c_omap i2c_omap.1: controller timed out" earlier in the boot sequence? It would have this symptom, among others... Earlier in this thread you'll find:

Quoted 5 lines

http://marc.info/?l=linux-omap&m=122297833314280&w=2

commenting on the issue I noted when I posted this patch; showing how I work around that specific I2C error. It'd likely work for you too.

Thanks for pointing this out. The retry patch earlier in the thread fixes the problem.

On 5 test boots I received the "i2c_omap i2c_omap.1: controller timed out" message all 5 times, but it recovers and seems to work fine.

- Nathan

author
#8 lore raw
Quoted 11 lines · Nathan Monson
Quoted 5 lines

 http://marc.info/?l=linux-omap&m=122297833314280&w=2

commenting on the issue I noted when I posted this patch; showing how I work around that specific I2C error.  It'd likely work for you too.

Thanks for pointing this out.  The retry patch earlier in the thread fixes the problem.

On 5 test boots I received the "i2c_omap i2c_omap.1: controller timed out" message all 5 times, but it recovers and seems to work fine.

That's good news.

Now, I'll hope someone more in touch with the I2C hardware can take a look at drivers/i2c/busses/i2c-omap.c and suggest some way to stop getting this error ... nice that it's rather reproducible, if one were to debug it!

As I noted in the message above, I see this error in other cases too. All of them "of course" should be made to handle such faults better. But virtually nothing in the I2C stack does sane fault handling, so we also need the i2c-omap code to stop misbehaving.

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

author
#9 lore raw
In reply to David Brownell
Quoted 2 lines · David Brownell

NOTE: minor weirdness.  The twl403-pwrirq initializes earlier (?) than before ... but "i2c_omap i2c_omap.1: controller timed out"

I'm seeing a bunch of these ... not clear why. Usually while booting ... but regardless, lack of fault recovery makes for lots of trouble.

Quoted 11 lines

is the response to its first I2C request.  So it bails, causing twl4030_usb and then rtc_twl4030 request_irq() calls to fail, and those drivers probe() calls fail.

But, for the first time in a long while, the USB host now enumerates my Ethernet adapter!!  So it's a net win in terms of overall behavior, since I can now get the time from NTP (loss of RTC doesn't hurt).

Make the pwrirq code retry that first write (setting the mask to the value the core already set it) ... and pwrirq comes up, RTC, the transceiver glue.  And USB can't enumerate that adapter. :(

My retry hack is appended. If anyone has an idea what's up with the I2C driver, it'd be good to fix ... although I'm happy to see what fault handling bugs these errors turn up.

More info on the USB thing: the issue is that if the external hub is connected when MUSB comes up, it misbehaves. If it's plugged in later, it works. The misbehavior may involve not seeing the external hub; or it may instead be errors in some operations talking to a devices hooked up to it.

- Dave

Usually a single retry lets this chip behave.

---
 drivers/i2c/chips/twl4030-pwrirq.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/i2c/chips/twl4030-pwrirq.c +10 -0
@@ -161,6 +161,8 @@ static int twl4030_pwrirq_thread(void *d
 	return 0;
 }
 
+#include <linux/delay.h>
+
 static int __init twl4030_pwrirq_init(void)
 {
 	int i, err;

@@ -168,8 +170,16 @@ static int __init twl4030_pwrirq_init(vo
 	twl4030_pwrirq_mask = 0xff;
 	twl4030_pwrirq_pending_unmask = 0;
 
+/* HEY:  core already did this.
+ * But that's surely not why we
+ * sometimes see timeouts here ...
+ */
+for (i = 0; i < 5; i++) {
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
 					TWL4030_INT_PWR_IMR1);
+if (!err) break;
+msleep(10);
+}
 	if (err)
 		return err;

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