[PATCH 0/2] twl4030-usb patches

[PATCH 0/2] twl4030-usb patches

[Felipe Balbi]

The following patches convert twl4030-usb to a platform_device
in order to set the usb mode via platform_data and get
rid of the Kconfig entry for that.

Later, we add a sysfs entry for reporting the vbus events
coming from triton, this let's us tell the userspace we
have a usb connection (vbus is on) and trigger an application
or something else.

Felipe Balbi (2):
  i2c: twl4030-usb: move to platform_device
  i2c: twl4030-usb: add 'vbus' sysfs file

 arch/arm/mach-omap2/usb-musb.c  |   19 ++
 drivers/i2c/chips/Kconfig       |   16 -
 drivers/i2c/chips/twl4030-usb.c |  593 +++++++++++++--------------------------
 include/linux/i2c/twl4030-usb.h |  256 +++++++++++++++++
 4 files changed, 467 insertions(+), 417 deletions(-)
 create mode 100644 include/linux/i2c/twl4030-usb.h

[PATCH 1/3] i2c: twl4030-usb: move to platform_device

[Felipe Balbi]

This patch moves twl4030-usb to platform_device. Later patches
will come adding proper notifications for twl usb irq events
via sysfs. The files will be placed under
/sys/devices/platform/twl4030-usb/

Also get rid of transceiver mode Kconfig selection, that will be
done by passing the desired mode via platform_data.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/mach-omap2/usb-musb.c  |   19 ++
 drivers/i2c/chips/Kconfig       |   16 --
 drivers/i2c/chips/twl4030-usb.c |  550 +++++++++++----------------------------
 include/linux/i2c/twl4030-usb.h |  257 ++++++++++++++++++
 4 files changed, 422 insertions(+), 420 deletions(-)
 create mode 100644 include/linux/i2c/twl4030-usb.h

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 3f90a93..afb377d 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -146,9 +146,28 @@ static struct platform_device musb_device = {
 };
 #endif
 
+#if defined(CONFIG_TWL4030_USB) || defined(CONFIG_TWL4030_USB_MODULE)
+static struct twl4030_usb_platform_data twl4030_usb_plat = {
+	.usb_mode	= T2_USB_MODE_ULPI,
+};
+
+static struct platform_device twl4030_usb_device = {
+	.name		= "twl4030-usb",
+	.id		= -1,
+	.dev		= {
+		.platform_data = &twl4030_usb_plat,
+	},
+};
+#endif
 
 void __init usb_musb_init(void)
 {
+#if defined(CONFIG_TWL4030_USB) || defined(CONFIG_TWL4030_USB_MODULE)
+	if (platform_device_register(&twl4030_usb_device) < 0) {
+		printk(KERN_ERR "Unable to register TWL4030 USB device\n");
+		return;
+	}
+#endif
 #ifdef CONFIG_USB_MUSB_SOC
 	if (platform_device_register(&musb_device) < 0) {
 		printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index e91be60..121aec9 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -186,22 +186,6 @@ config TWL4030_USB
 	tristate "TWL4030 USB Transceiver Driver"
 	depends on TWL4030_CORE
 
-choice
-	prompt "Transceiver mode"
-	depends on TWL4030_USB
-	help
-	  TWL4030 USB transceiver can operate in various
-	  mutually-exclusive modes. Select one of them.
-
-config TWL4030_USB_HS_ULPI
-	depends on TWL4030_USB
-	bool "High-speed ULPI"
-	help
-	  Say Y here if the TWL4030 is connected to high-speed USB
-	  controller through a ULPI interface.
-
-endchoice
-
 config TWL4030_PWRBUTTON
 	tristate "TWL4030 Power button Driver"
 	depends on TWL4030_CORE
diff --git a/drivers/i2c/chips/twl4030-usb.c b/drivers/i2c/chips/twl4030-usb.c
index 2906b82..0d62e65 100644
--- a/drivers/i2c/chips/twl4030-usb.c
+++ b/drivers/i2c/chips/twl4030-usb.c
@@ -2,6 +2,8 @@
  * twl4030_usb - TWL4030 USB transceiver, talking to OMAP OTG controller
  *
  * Copyright (C) 2004-2007 Texas Instruments
+ * Copyright (C) 2008 Nokia Corporation
+ * Contact: Felipe Balbi <felipe.balbi@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
@@ -22,268 +24,37 @@
  *	- 3-pin mode support may be added in future.
  */
 
-
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/time.h>
 #include <linux/interrupt.h>
+#include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/usb.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/i2c/twl4030-usb.h>
 #include <mach/usb.h>
 
-/* Register defines */
-
-#define VENDOR_ID_LO			0x00
-#define VENDOR_ID_HI			0x01
-#define PRODUCT_ID_LO			0x02
-#define PRODUCT_ID_HI			0x03
-
-#define FUNC_CTRL			0x04
-#define FUNC_CTRL_SET			0x05
-#define FUNC_CTRL_CLR			0x06
-#define FUNC_CTRL_SUSPENDM		(1 << 6)
-#define FUNC_CTRL_RESET			(1 << 5)
-#define FUNC_CTRL_OPMODE_MASK		(3 << 3) /* bits 3 and 4 */
-#define FUNC_CTRL_OPMODE_NORMAL		(0 << 3)
-#define FUNC_CTRL_OPMODE_NONDRIVING	(1 << 3)
-#define FUNC_CTRL_OPMODE_DISABLE_BIT_NRZI	(2 << 3)
-#define FUNC_CTRL_TERMSELECT		(1 << 2)
-#define FUNC_CTRL_XCVRSELECT_MASK	(3 << 0) /* bits 0 and 1 */
-#define FUNC_CTRL_XCVRSELECT_HS		(0 << 0)
-#define FUNC_CTRL_XCVRSELECT_FS		(1 << 0)
-#define FUNC_CTRL_XCVRSELECT_LS		(2 << 0)
-#define FUNC_CTRL_XCVRSELECT_FS4LS	(3 << 0)
-
-#define IFC_CTRL			0x07
-#define IFC_CTRL_SET			0x08
-#define IFC_CTRL_CLR			0x09
-#define IFC_CTRL_INTERFACE_PROTECT_DISABLE	(1 << 7)
-#define IFC_CTRL_AUTORESUME		(1 << 4)
-#define IFC_CTRL_CLOCKSUSPENDM		(1 << 3)
-#define IFC_CTRL_CARKITMODE		(1 << 2)
-#define IFC_CTRL_FSLSSERIALMODE_3PIN	(1 << 1)
-
-#define TWL4030_OTG_CTRL		0x0A
-#define TWL4030_OTG_CTRL_SET		0x0B
-#define TWL4030_OTG_CTRL_CLR		0x0C
-#define TWL4030_OTG_CTRL_DRVVBUS	(1 << 5)
-#define TWL4030_OTG_CTRL_CHRGVBUS	(1 << 4)
-#define TWL4030_OTG_CTRL_DISCHRGVBUS	(1 << 3)
-#define TWL4030_OTG_CTRL_DMPULLDOWN	(1 << 2)
-#define TWL4030_OTG_CTRL_DPPULLDOWN	(1 << 1)
-#define TWL4030_OTG_CTRL_IDPULLUP	(1 << 0)
-
-#define USB_INT_EN_RISE			0x0D
-#define USB_INT_EN_RISE_SET		0x0E
-#define USB_INT_EN_RISE_CLR		0x0F
-#define USB_INT_EN_FALL			0x10
-#define USB_INT_EN_FALL_SET		0x11
-#define USB_INT_EN_FALL_CLR		0x12
-#define USB_INT_STS			0x13
-#define USB_INT_LATCH			0x14
-#define USB_INT_IDGND			(1 << 4)
-#define USB_INT_SESSEND			(1 << 3)
-#define USB_INT_SESSVALID		(1 << 2)
-#define USB_INT_VBUSVALID		(1 << 1)
-#define USB_INT_HOSTDISCONNECT		(1 << 0)
-
-#define CARKIT_CTRL			0x19
-#define CARKIT_CTRL_SET			0x1A
-#define CARKIT_CTRL_CLR			0x1B
-#define CARKIT_CTRL_MICEN		(1 << 6)
-#define CARKIT_CTRL_SPKRIGHTEN		(1 << 5)
-#define CARKIT_CTRL_SPKLEFTEN		(1 << 4)
-#define CARKIT_CTRL_RXDEN		(1 << 3)
-#define CARKIT_CTRL_TXDEN		(1 << 2)
-#define CARKIT_CTRL_IDGNDDRV		(1 << 1)
-#define CARKIT_CTRL_CARKITPWR		(1 << 0)
-#define CARKIT_PLS_CTRL			0x22
-#define CARKIT_PLS_CTRL_SET		0x23
-#define CARKIT_PLS_CTRL_CLR		0x24
-#define CARKIT_PLS_CTRL_SPKRRIGHT_BIASEN	(1 << 3)
-#define CARKIT_PLS_CTRL_SPKRLEFT_BIASEN	(1 << 2)
-#define CARKIT_PLS_CTRL_RXPLSEN		(1 << 1)
-#define CARKIT_PLS_CTRL_TXPLSEN		(1 << 0)
-
-#define MCPC_CTRL			0x30
-#define MCPC_CTRL_SET			0x31
-#define MCPC_CTRL_CLR			0x32
-#define MCPC_CTRL_RTSOL			(1 << 7)
-#define MCPC_CTRL_EXTSWR		(1 << 6)
-#define MCPC_CTRL_EXTSWC		(1 << 5)
-#define MCPC_CTRL_VOICESW		(1 << 4)
-#define MCPC_CTRL_OUT64K		(1 << 3)
-#define MCPC_CTRL_RTSCTSSW		(1 << 2)
-#define MCPC_CTRL_HS_UART		(1 << 0)
-
-#define MCPC_IO_CTRL			0x33
-#define MCPC_IO_CTRL_SET		0x34
-#define MCPC_IO_CTRL_CLR		0x35
-#define MCPC_IO_CTRL_MICBIASEN		(1 << 5)
-#define MCPC_IO_CTRL_CTS_NPU		(1 << 4)
-#define MCPC_IO_CTRL_RXD_PU		(1 << 3)
-#define MCPC_IO_CTRL_TXDTYP		(1 << 2)
-#define MCPC_IO_CTRL_CTSTYP		(1 << 1)
-#define MCPC_IO_CTRL_RTSTYP		(1 << 0)
-
-#define MCPC_CTRL2			0x36
-#define MCPC_CTRL2_SET			0x37
-#define MCPC_CTRL2_CLR			0x38
-#define MCPC_CTRL2_MCPC_CK_EN		(1 << 0)
-
-#define OTHER_FUNC_CTRL			0x80
-#define OTHER_FUNC_CTRL_SET		0x81
-#define OTHER_FUNC_CTRL_CLR		0x82
-#define OTHER_FUNC_CTRL_BDIS_ACON_EN	(1 << 4)
-#define OTHER_FUNC_CTRL_FIVEWIRE_MODE	(1 << 2)
-
-#define OTHER_IFC_CTRL			0x83
-#define OTHER_IFC_CTRL_SET		0x84
-#define OTHER_IFC_CTRL_CLR		0x85
-#define OTHER_IFC_CTRL_OE_INT_EN	(1 << 6)
-#define OTHER_IFC_CTRL_CEA2011_MODE	(1 << 5)
-#define OTHER_IFC_CTRL_FSLSSERIALMODE_4PIN	(1 << 4)
-#define OTHER_IFC_CTRL_HIZ_ULPI_60MHZ_OUT	(1 << 3)
-#define OTHER_IFC_CTRL_HIZ_ULPI		(1 << 2)
-#define OTHER_IFC_CTRL_ALT_INT_REROUTE	(1 << 0)
-
-#define OTHER_INT_EN_RISE		0x86
-#define OTHER_INT_EN_RISE_SET		0x87
-#define OTHER_INT_EN_RISE_CLR		0x88
-#define OTHER_INT_EN_FALL		0x89
-#define OTHER_INT_EN_FALL_SET		0x8A
-#define OTHER_INT_EN_FALL_CLR		0x8B
-#define OTHER_INT_STS			0x8C
-#define OTHER_INT_LATCH			0x8D
-#define OTHER_INT_VB_SESS_VLD		(1 << 7)
-#define OTHER_INT_DM_HI			(1 << 6) /* not valid for "latch" reg */
-#define OTHER_INT_DP_HI			(1 << 5) /* not valid for "latch" reg */
-#define OTHER_INT_BDIS_ACON		(1 << 3) /* not valid for "fall" regs */
-#define OTHER_INT_MANU			(1 << 1)
-#define OTHER_INT_ABNORMAL_STRESS	(1 << 0)
-
-#define ID_STATUS			0x96
-#define ID_RES_FLOAT			(1 << 4)
-#define ID_RES_440K			(1 << 3)
-#define ID_RES_200K			(1 << 2)
-#define ID_RES_102K			(1 << 1)
-#define ID_RES_GND			(1 << 0)
-
-#define POWER_CTRL			0xAC
-#define POWER_CTRL_SET			0xAD
-#define POWER_CTRL_CLR			0xAE
-#define POWER_CTRL_OTG_ENAB		(1 << 5)
-
-#define OTHER_IFC_CTRL2			0xAF
-#define OTHER_IFC_CTRL2_SET		0xB0
-#define OTHER_IFC_CTRL2_CLR		0xB1
-#define OTHER_IFC_CTRL2_ULPI_STP_LOW	(1 << 4)
-#define OTHER_IFC_CTRL2_ULPI_TXEN_POL	(1 << 3)
-#define OTHER_IFC_CTRL2_ULPI_4PIN_2430	(1 << 2)
-#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_MASK	(3 << 0) /* bits 0 and 1 */
-#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_INT1N	(0 << 0)
-#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_INT2N	(1 << 0)
-
-#define REG_CTRL_EN			0xB2
-#define REG_CTRL_EN_SET			0xB3
-#define REG_CTRL_EN_CLR			0xB4
-#define REG_CTRL_ERROR			0xB5
-#define ULPI_I2C_CONFLICT_INTEN		(1 << 0)
-
-#define OTHER_FUNC_CTRL2		0xB8
-#define OTHER_FUNC_CTRL2_SET		0xB9
-#define OTHER_FUNC_CTRL2_CLR		0xBA
-#define OTHER_FUNC_CTRL2_VBAT_TIMER_EN	(1 << 0)
-
-/* following registers do not have separate _clr and _set registers */
-#define VBUS_DEBOUNCE			0xC0
-#define ID_DEBOUNCE			0xC1
-#define VBAT_TIMER			0xD3
-#define PHY_PWR_CTRL			0xFD
-#define PHY_PWR_PHYPWD			(1 << 0)
-#define PHY_CLK_CTRL			0xFE
-#define PHY_CLK_CTRL_CLOCKGATING_EN	(1 << 2)
-#define PHY_CLK_CTRL_CLK32K_EN		(1 << 1)
-#define REQ_PHY_DPLL_CLK		(1 << 0)
-#define PHY_CLK_CTRL_STS		0xFF
-#define PHY_DPLL_CLK			(1 << 0)
-
-/* In module TWL4030_MODULE_PM_MASTER */
-#define PROTECT_KEY			0x0E
-
-/* In module TWL4030_MODULE_PM_RECEIVER */
-#define VUSB_DEDICATED1			0x7D
-#define VUSB_DEDICATED2			0x7E
-#define VUSB1V5_DEV_GRP			0x71
-#define VUSB1V5_TYPE			0x72
-#define VUSB1V5_REMAP			0x73
-#define VUSB1V8_DEV_GRP			0x74
-#define VUSB1V8_TYPE			0x75
-#define VUSB1V8_REMAP			0x76
-#define VUSB3V1_DEV_GRP			0x77
-#define VUSB3V1_TYPE			0x78
-#define VUSB3V1_REMAP			0x79
-
-#define ID_STATUS			0x96
-#define ID_RES_FLOAT			(1 << 4) /* mini-B */
-#define ID_RES_440K			(1 << 3) /* type 2 charger */
-#define ID_RES_200K			(1 << 2) /* 5-wire carkit or
-						    type 1 charger */
-#define ID_RES_102K			(1 << 1) /* phone */
-#define ID_RES_GND			(1 << 0) /* mini-A */
-
-/* In module TWL4030_MODULE_INTBR */
-#define PMBR1				0x0D
-#define GPIO_USB_4PIN_ULPI_2430C	(3 << 0)
-
-/* In module TWL4030_MODULE_INT */
-#define REG_PWR_ISR1			0x00
-#define REG_PWR_IMR1			0x01
-#define USB_PRES			(1 << 2)
-#define REG_PWR_EDR1			0x05
-#define USB_PRES_FALLING		(1 << 4)
-#define USB_PRES_RISING			(1 << 5)
-#define REG_PWR_SIH_CTRL		0x07
-#define COR				(1 << 2)
-
-/* internal define on top of container_of */
-#define xceiv_to_twl(x)		container_of((x), struct twl4030_usb, otg);
-
-/* bits in OTG_CTRL */
-
-#define	OTG_XCEIV_OUTPUTS \
-	(OTG_ASESSVLD|OTG_BSESSEND|OTG_BSESSVLD|OTG_VBUSVLD|OTG_ID)
-#define	OTG_XCEIV_INPUTS \
-	(OTG_PULLDOWN|OTG_PULLUP|OTG_DRV_VBUS|OTG_PD_VBUS|OTG_PU_VBUS|OTG_PU_ID)
-#define	OTG_CTRL_BITS \
-	(OTG_A_BUSREQ|OTG_A_SETB_HNPEN|OTG_B_BUSREQ|OTG_B_HNPEN|OTG_BUSDROP)
-	/* and OTG_PULLUP is sometimes written */
-
-#define	OTG_CTRL_MASK	(OTG_DRIVER_SEL| \
-	OTG_XCEIV_OUTPUTS|OTG_XCEIV_INPUTS| \
-	OTG_CTRL_BITS)
-
-
-/*-------------------------------------------------------------------------*/
-
 struct twl4030_usb {
 	struct otg_transceiver	otg;
+	struct device		*dev;
+
+	/* pin configuration */
+	enum twl4030_usb_mode	usb_mode;
 	int			irq;
-	u8			usb_mode;	/* pin configuration */
-#define T2_USB_MODE_ULPI		1
-/* #define T2_USB_MODE_CEA2011_3PIN	2 */
 	u8			asleep;
 };
 
-static struct twl4030_usb *the_transceiver;
+/* internal define on top of container_of */
+#define xceiv_to_twl(x)		container_of((x), struct twl4030_usb, otg);
 
 /*-------------------------------------------------------------------------*/
 
-static int twl4030_i2c_write_u8_verify(u8 module, u8 data, u8 address)
+static int twl4030_i2c_write_u8_verify(struct twl4030_usb *twl,
+		u8 module, u8 data, u8 address)
 {
 	u8 check;
 
@@ -297,46 +68,51 @@ static int twl4030_i2c_write_u8_verify(u8 module, u8 data, u8 address)
 						(check == data))
 		return 0;
 	/* Failed again: Return error */
+
 	return -EBUSY;
 }
 
-#define twl4030_usb_write_verify(address, data)	\
-	twl4030_i2c_write_u8_verify(TWL4030_MODULE_USB, (data), (address))
+#define twl4030_usb_write_verify(twl, address, data)	\
+	twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_USB, (data), (address))
 
-static inline int twl4030_usb_write(u8 address, u8 data)
+static inline int twl4030_usb_write(struct twl4030_usb *twl,
+		u8 address, u8 data)
 {
 	int ret = 0;
+
 	ret = twl4030_i2c_write_u8(TWL4030_MODULE_USB, data, address);
 	if (ret >= 0) {
 #if 0	/* debug */
 		u8 data1;
 		if (twl4030_i2c_read_u8(TWL4030_MODULE_USB, &data1,
 					address) < 0)
-			printk(KERN_ERR "re-read failed\n");
+			dev_err(twl->dev, "re-read failed\n");
 		else
-			printk(KERN_INFO
+			dev_dbg(twl->dev,
 			       "Write %s wrote %x read %x from reg %x\n",
 			       (data1 == data) ? "succeed" : "mismatch",
 			       data, data1, address);
 #endif
 	} else {
-		printk(KERN_WARNING
+		dev_warn(twl->dev,
 			"TWL4030:USB:Write[0x%x] Error %d\n", address, ret);
 	}
+
 	return ret;
 }
 
-static inline int twl4030_usb_read(u8 address)
+static inline int twl4030_usb_read(struct twl4030_usb *twl, u8 address)
 {
 	u8 data;
 	int ret = 0;
+
 	ret = twl4030_i2c_read_u8(TWL4030_MODULE_USB, &data, address);
-	if (ret >= 0) {
+	if (ret >= 0)
 		ret = data;
-	} else {
-		printk(KERN_WARNING
+	else
+		dev_warn(twl->dev,
 			"TWL4030:USB:Read[0x%x] Error %d\n", address, ret);
-	}
+
 	return ret;
 }
 
@@ -345,14 +121,13 @@ static inline int twl4030_usb_read(u8 address)
 static inline int
 twl4030_usb_set_bits(struct twl4030_usb *twl, u8 reg, u8 bits)
 {
-	return twl4030_usb_write(reg + 1, bits);
+	return twl4030_usb_write(twl, reg + 1, bits);
 }
 
 static inline int
 twl4030_usb_clear_bits(struct twl4030_usb *twl, u8 reg, u8 bits)
 {
-	return twl4030_usb_write(reg + 2, bits);
-
+	return twl4030_usb_write(twl, reg + 2, bits);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -380,185 +155,130 @@ static void twl4030_usb_set_mode(struct twl4030_usb *twl, int mode)
 	};
 }
 
-#ifdef CONFIG_TWL4030_USB_HS_ULPI
-static void hs_usb_init(struct twl4030_usb *twl)
-{
-	twl->usb_mode = T2_USB_MODE_ULPI;
-	return;
-}
-
-#endif
-
-static void twl4030_i2c_access(int on)
+static void twl4030_i2c_access(struct twl4030_usb *twl, int on)
 {
 	unsigned long timeout;
-	int val = twl4030_usb_read(PHY_CLK_CTRL);
+	int val = twl4030_usb_read(twl, PHY_CLK_CTRL);
 
 	if (val >= 0) {
 		if (on) {
 			/* enable DPLL to access PHY registers over I2C */
 			val |= REQ_PHY_DPLL_CLK;
-			if (twl4030_usb_write_verify(PHY_CLK_CTRL,
-								(u8)val) < 0) {
-				printk(KERN_ERR "twl4030_usb: i2c write failed,"
-						" line %d\n", __LINE__);
-				return;
-			}
+			WARN_ON(twl4030_usb_write_verify(twl, PHY_CLK_CTRL,
+						(u8)val) < 0);
 
 			timeout = jiffies + HZ;
-			while (!(twl4030_usb_read(PHY_CLK_CTRL_STS) &
+			while (!(twl4030_usb_read(twl, PHY_CLK_CTRL_STS) &
 							PHY_DPLL_CLK)
 				&& time_before(jiffies, timeout))
 					udelay(10);
-			if (!(twl4030_usb_read(PHY_CLK_CTRL_STS) &
+			if (!(twl4030_usb_read(twl, PHY_CLK_CTRL_STS) &
 							PHY_DPLL_CLK))
-				printk(KERN_ERR "Timeout setting T2 HSUSB "
+				dev_err(twl->dev, "Timeout setting T2 HSUSB "
 						"PHY DPLL clock\n");
 		} else {
 			/* let ULPI control the DPLL clock */
 			val &= ~REQ_PHY_DPLL_CLK;
-			if (twl4030_usb_write_verify(PHY_CLK_CTRL,
-								(u8)val) < 0) {
-				printk(KERN_ERR "twl4030_usb: i2c write failed,"
-						" line %d\n", __LINE__);
-			}
+			WARN_ON(twl4030_usb_write_verify(twl, PHY_CLK_CTRL,
+						(u8)val) < 0);
 		}
 	}
-	return;
 }
 
-static void usb_irq_enable(int rising, int falling)
+static void usb_irq_enable(struct twl4030_usb *twl, int rising, int falling)
 {
 	u8 val;
 
 	/* edge setup */
-	if (twl4030_i2c_read_u8(TWL4030_MODULE_INT, &val, REG_PWR_EDR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c read failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&val, REG_PWR_EDR1) < 0);
+
 	val &= ~(USB_PRES_RISING | USB_PRES_FALLING);
 	if (rising)
 		val = val | USB_PRES_RISING;
 	if (falling)
 		val = val | USB_PRES_FALLING;
-	if (twl4030_i2c_write_u8_verify(TWL4030_MODULE_INT, val,
-							REG_PWR_EDR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c write failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
+				val, REG_PWR_EDR1) < 0);
 
 	/* un-mask interrupt */
-	if (twl4030_i2c_read_u8(TWL4030_MODULE_INT, &val, REG_PWR_IMR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c read failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&val, REG_PWR_IMR1) < 0);
+
 	val &= ~USB_PRES;
-	if (twl4030_i2c_write_u8_verify(TWL4030_MODULE_INT, val,
-							REG_PWR_IMR1) < 0)
-		printk(KERN_ERR "twl4030_usb: i2c write failed,"
-				" line %d\n", __LINE__);
 
-	return;
+	WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
+				val, REG_PWR_IMR1) < 0);
 }
 
-static void usb_irq_disable(void)
+static void usb_irq_disable(struct twl4030_usb *twl)
 {
 	u8 val;
 
 	/* undo edge setup */
-	if (twl4030_i2c_read_u8(TWL4030_MODULE_INT, &val, REG_PWR_EDR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c read failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&val, REG_PWR_EDR1) < 0);
 	val &= ~(USB_PRES_RISING | USB_PRES_FALLING);
-	if (twl4030_i2c_write_u8_verify(TWL4030_MODULE_INT, val,
-							REG_PWR_EDR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c write failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
+				val, REG_PWR_EDR1) < 0);
 
 	/* mask interrupt */
-	if (twl4030_i2c_read_u8(TWL4030_MODULE_INT, &val, REG_PWR_IMR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c read failed,"
-				" line %d\n", __LINE__);
-		return;
-	}
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&val, REG_PWR_IMR1) < 0);
 	val |= USB_PRES;
-	if (twl4030_i2c_write_u8_verify(TWL4030_MODULE_INT, val,
-							REG_PWR_IMR1) < 0)
-		printk(KERN_ERR "twl4030_usb: i2c write failed,"
-				" line %d\n", __LINE__);
 
-	return;
+	WARN_ON(twl4030_i2c_write_u8_verify(twl, TWL4030_MODULE_INT,
+				val, REG_PWR_IMR1) < 0);
 }
 
 static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 {
 	u8 pwr;
 
-	pwr = twl4030_usb_read(PHY_PWR_CTRL);
+	pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
 	if (on) {
 		pwr &= ~PHY_PWR_PHYPWD;
-		if (twl4030_usb_write_verify(PHY_PWR_CTRL, pwr) < 0) {
-			printk(KERN_ERR "twl4030_usb: i2c write failed,"
-					" line %d\n", __LINE__);
-			return;
-		}
-		twl4030_usb_write(PHY_CLK_CTRL,
-				  twl4030_usb_read(PHY_CLK_CTRL) |
+		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
+		twl4030_usb_write(twl, PHY_CLK_CTRL,
+				  twl4030_usb_read(twl, PHY_CLK_CTRL) |
 					(PHY_CLK_CTRL_CLOCKGATING_EN |
 						PHY_CLK_CTRL_CLK32K_EN));
 	} else  {
 		pwr |= PHY_PWR_PHYPWD;
-		if (twl4030_usb_write_verify(PHY_PWR_CTRL, pwr) < 0) {
-			printk(KERN_ERR "twl4030_usb: i2c write failed,"
-					" line %d\n", __LINE__);
-		}
+		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
 	}
-	return;
 }
 
-static void twl4030_phy_suspend(int controller_off)
+static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
 {
-	struct twl4030_usb *twl = the_transceiver;
-
 	if (controller_off)
-		usb_irq_disable();
+		usb_irq_disable(twl);
 
 	if (twl->asleep)
 		return;
 
 	if (!controller_off)
 		/* enable rising edge interrupt to detect cable attach */
-		usb_irq_enable(1, 0);
+		usb_irq_enable(twl, 1, 0);
 
 	twl4030_phy_power(twl, 0);
 	twl->asleep = 1;
-	return;
 }
 
-static void twl4030_phy_resume(void)
+static void twl4030_phy_resume(struct twl4030_usb *twl)
 {
-	struct twl4030_usb *twl = the_transceiver;
-
 	if (!twl->asleep)
 		return;
 
 	/* enable falling edge interrupt to detect cable detach */
-	usb_irq_enable(0, 1);
+	usb_irq_enable(twl, 0, 1);
 
 	twl4030_phy_power(twl, 1);
-	twl4030_i2c_access(1);
+	twl4030_i2c_access(twl, 1);
 	twl4030_usb_set_mode(twl, twl->usb_mode);
 	if (twl->usb_mode == T2_USB_MODE_ULPI)
-		twl4030_i2c_access(0);
+		twl4030_i2c_access(twl, 0);
 	twl->asleep = 0;
-	return;
 }
 
 static void twl4030_usb_ldo_init(struct twl4030_usb *twl)
@@ -591,59 +311,57 @@ static void twl4030_usb_ldo_init(struct twl4030_usb *twl)
 
 static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 {
-	int ret = IRQ_NONE;
+	struct twl4030_usb *twl = _twl;
 	u8 val;
 
 	/* action based on cable attach or detach */
-	if (twl4030_i2c_read_u8(TWL4030_MODULE_INT, &val, REG_PWR_EDR1) < 0) {
-		printk(KERN_ERR "twl4030_usb: i2c read failed,"
-				" line %d\n", __LINE__);
-		goto done;
-	}
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&val, REG_PWR_EDR1) < 0);
 
 	if (val & USB_PRES_RISING) {
-		twl4030_phy_resume();
+		twl4030_phy_resume(twl);
 		twl4030charger_usb_en(1);
 	} else {
 		twl4030charger_usb_en(0);
-		twl4030_phy_suspend(0);
+		twl4030_phy_suspend(twl, 0);
 	}
 
-	ret = IRQ_HANDLED;
-
-done:
-	return ret;
+	return IRQ_HANDLED;
 }
 
 static int twl4030_set_suspend(struct otg_transceiver *x, int suspend)
 {
+	struct twl4030_usb *twl = xceiv_to_twl(x);
+
 	if (suspend)
-		twl4030_phy_suspend(1);
+		twl4030_phy_suspend(twl, 1);
 	else
-		twl4030_phy_resume();
+		twl4030_phy_resume(twl);
 
 	return 0;
 }
 
-static int twl4030_set_peripheral(struct otg_transceiver *xceiv,
+static int twl4030_set_peripheral(struct otg_transceiver *x,
 		struct usb_gadget *gadget)
 {
+	struct twl4030_usb *twl;
 	u32 l;
-	struct twl4030_usb *twl = xceiv_to_twl(xceiv);
 
-	if (!xceiv)
+	if (!x)
 		return -ENODEV;
 
+	twl = xceiv_to_twl(x);
+
 	if (!gadget) {
 		omap_writew(0, OTG_IRQ_EN);
-		twl4030_phy_suspend(1);
+		twl4030_phy_suspend(twl, 1);
 		twl->otg.gadget = NULL;
 
 		return -ENODEV;
 	}
 
 	twl->otg.gadget = gadget;
-	twl4030_phy_resume();
+	twl4030_phy_resume(twl);
 
 	l = omap_readl(OTG_CTRL) & OTG_CTRL_MASK;
 	l &= ~(OTG_XCEIV_OUTPUTS|OTG_CTRL_BITS);
@@ -660,23 +378,25 @@ static int twl4030_set_peripheral(struct otg_transceiver *xceiv,
 	return 0;
 }
 
-static int twl4030_set_host(struct otg_transceiver *xceiv, struct usb_bus *host)
+static int twl4030_set_host(struct otg_transceiver *x, struct usb_bus *host)
 {
-	struct twl4030_usb *twl = xceiv_to_twl(xceiv);
+	struct twl4030_usb *twl;
 
-	if (!xceiv)
+	if (!x)
 		return -ENODEV;
 
+	twl = xceiv_to_twl(x);
+
 	if (!host) {
 		omap_writew(0, OTG_IRQ_EN);
-		twl4030_phy_suspend(1);
+		twl4030_phy_suspend(twl, 1);
 		twl->otg.host = NULL;
 
 		return -ENODEV;
 	}
 
 	twl->otg.host = host;
-	twl4030_phy_resume();
+	twl4030_phy_resume(twl);
 
 	twl4030_usb_set_bits(twl, TWL4030_OTG_CTRL,
 			TWL4030_OTG_CTRL_DMPULLDOWN
@@ -689,63 +409,65 @@ static int twl4030_set_host(struct otg_transceiver *xceiv, struct usb_bus *host)
 	return 0;
 }
 
-static int __init twl4030_usb_init(void)
+static int __init twl4030_usb_probe(struct platform_device *pdev)
 {
+	struct twl4030_usb_platform_data *pdata = pdev->dev.platform_data;
 	struct twl4030_usb	*twl;
 	int status;
 
-	if (the_transceiver)
-		return 0;
-
 	twl = kzalloc(sizeof *twl, GFP_KERNEL);
 	if (!twl)
-		return 0;
-
-	the_transceiver = twl;
+		return -ENOMEM;
 
+	twl->dev		= &pdev->dev;
 	twl->irq		= TWL4030_PWRIRQ_USB_PRES;
 	twl->otg.set_host	= twl4030_set_host;
 	twl->otg.set_peripheral	= twl4030_set_peripheral;
 	twl->otg.set_suspend	= twl4030_set_suspend;
 
-	usb_irq_disable();
+	if (!pdata) {
+		dev_info(&pdev->dev, "platform_data not available, defaulting"
+				" to ULPI mode\n");
+		twl->usb_mode		= T2_USB_MODE_ULPI;
+	} else {
+		twl->usb_mode	= pdata->usb_mode;
+	}
+
+	usb_irq_disable(twl);
 	status = request_irq(twl->irq, twl4030_usb_irq, 0, "twl4030_usb", twl);
 	if (status < 0) {
-		printk(KERN_DEBUG "can't get IRQ %d, err %d\n",
+		dev_dbg(&pdev->dev, "can't get IRQ %d, err %d\n",
 			twl->irq, status);
 		kfree(twl);
-		return -ENODEV;
+		return status;
 	}
 
-#if defined(CONFIG_TWL4030_USB_HS_ULPI)
-	hs_usb_init(twl);
-#endif
+
 	twl4030_usb_ldo_init(twl);
 	twl4030_phy_power(twl, 1);
-	twl4030_i2c_access(1);
+	twl4030_i2c_access(twl, 1);
 	twl4030_usb_set_mode(twl, twl->usb_mode);
-	if (twl->usb_mode == T2_USB_MODE_ULPI)
-		twl4030_i2c_access(0);
 
 	twl->asleep = 0;
 
-	if (twl->usb_mode == T2_USB_MODE_ULPI)
-		twl4030_phy_suspend(1);
+	if (twl->usb_mode == T2_USB_MODE_ULPI) {
+		twl4030_i2c_access(twl, 0);
+		twl4030_phy_suspend(twl, 0);
+	}
 
 	otg_set_transceiver(&twl->otg);
-
-	printk(KERN_INFO "Initialized TWL4030 USB module\n");
+	dev_set_drvdata(&pdev->dev, twl);
+	dev_info(&pdev->dev, "Initialized TWL4030 USB module\n");
 
 	return 0;
 }
 
-
-static void __exit twl4030_usb_exit(void)
+static int __exit twl4030_usb_remove(struct platform_device *pdev)
 {
-	struct twl4030_usb *twl = the_transceiver;
+	struct twl4030_usb *twl = dev_get_drvdata(&pdev->dev);
 	int val;
 
-	usb_irq_disable();
+	usb_irq_disable(twl);
 	free_irq(twl->irq, twl);
 
 	/* set transceiver mode to power on defaults */
@@ -755,11 +477,11 @@ static void __exit twl4030_usb_exit(void)
 	 * clear dpll clock request for i2c access,
 	 * disable 32KHz
 	 */
-	val = twl4030_usb_read(PHY_CLK_CTRL);
+	val = twl4030_usb_read(twl, PHY_CLK_CTRL);
 	if (val >= 0) {
 		val |= PHY_CLK_CTRL_CLOCKGATING_EN;
 		val &= ~(PHY_CLK_CTRL_CLK32K_EN | REQ_PHY_DPLL_CLK);
-		twl4030_usb_write(PHY_CLK_CTRL, (u8)val);
+		twl4030_usb_write(twl, PHY_CLK_CTRL, (u8)val);
 	}
 
 	/* disable complete OTG block */
@@ -768,12 +490,32 @@ static void __exit twl4030_usb_exit(void)
 	twl4030_phy_power(twl, 0);
 
 	kfree(twl);
+
+	return 0;
+}
+
+static struct platform_driver twl4030_driver = {
+	.probe		= twl4030_usb_probe,
+	.remove		= __exit_p(twl4030_remove),
+	.driver		= {
+		.name	= "twl4030-usb",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init twl4030_usb_init(void)
+{
+	return platform_driver_register(&twl4030_driver);
 }
+module_init(twl4030_usb_init);
 
-subsys_initcall(twl4030_usb_init);
+static void __exit twl4030_usb_exit(void)
+{
+	platform_driver_unregister(&twl4030_driver);
+}
 module_exit(twl4030_usb_exit);
 
-MODULE_ALIAS("i2c:twl4030-usb");
-MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_ALIAS("platform:twl4030-usb");
+MODULE_AUTHOR("Texas Instruments, Inc, Nokia Corporation");
 MODULE_DESCRIPTION("TWL4030 USB transceiver driver");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c/twl4030-usb.h b/include/linux/i2c/twl4030-usb.h
new file mode 100644
index 0000000..9cd2e50
--- /dev/null
+++ b/include/linux/i2c/twl4030-usb.h
@@ -0,0 +1,257 @@
+/*
+ * twl4030-usb.h - header for TWL4030 USB Transceiver
+ *
+ * Copyright (C) 2004-2007 Texas Instruments
+ * Copyright (C) 2008 Nokia Corporation
+ * Contact: Felipe Balbi <felipe.balbi@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_USB_H_
+#define __TWL4030_USB_H_
+
+/* Register defines */
+
+#define VENDOR_ID_LO			0x00
+#define VENDOR_ID_HI			0x01
+#define PRODUCT_ID_LO			0x02
+#define PRODUCT_ID_HI			0x03
+
+#define FUNC_CTRL			0x04
+#define FUNC_CTRL_SET			0x05
+#define FUNC_CTRL_CLR			0x06
+#define FUNC_CTRL_SUSPENDM		(1 << 6)
+#define FUNC_CTRL_RESET			(1 << 5)
+#define FUNC_CTRL_OPMODE_MASK		(3 << 3) /* bits 3 and 4 */
+#define FUNC_CTRL_OPMODE_NORMAL		(0 << 3)
+#define FUNC_CTRL_OPMODE_NONDRIVING	(1 << 3)
+#define FUNC_CTRL_OPMODE_DISABLE_BIT_NRZI	(2 << 3)
+#define FUNC_CTRL_TERMSELECT		(1 << 2)
+#define FUNC_CTRL_XCVRSELECT_MASK	(3 << 0) /* bits 0 and 1 */
+#define FUNC_CTRL_XCVRSELECT_HS		(0 << 0)
+#define FUNC_CTRL_XCVRSELECT_FS		(1 << 0)
+#define FUNC_CTRL_XCVRSELECT_LS		(2 << 0)
+#define FUNC_CTRL_XCVRSELECT_FS4LS	(3 << 0)
+
+#define IFC_CTRL			0x07
+#define IFC_CTRL_SET			0x08
+#define IFC_CTRL_CLR			0x09
+#define IFC_CTRL_INTERFACE_PROTECT_DISABLE	(1 << 7)
+#define IFC_CTRL_AUTORESUME		(1 << 4)
+#define IFC_CTRL_CLOCKSUSPENDM		(1 << 3)
+#define IFC_CTRL_CARKITMODE		(1 << 2)
+#define IFC_CTRL_FSLSSERIALMODE_3PIN	(1 << 1)
+
+#define TWL4030_OTG_CTRL		0x0A
+#define TWL4030_OTG_CTRL_SET		0x0B
+#define TWL4030_OTG_CTRL_CLR		0x0C
+#define TWL4030_OTG_CTRL_DRVVBUS	(1 << 5)
+#define TWL4030_OTG_CTRL_CHRGVBUS	(1 << 4)
+#define TWL4030_OTG_CTRL_DISCHRGVBUS	(1 << 3)
+#define TWL4030_OTG_CTRL_DMPULLDOWN	(1 << 2)
+#define TWL4030_OTG_CTRL_DPPULLDOWN	(1 << 1)
+#define TWL4030_OTG_CTRL_IDPULLUP	(1 << 0)
+
+#define USB_INT_EN_RISE			0x0D
+#define USB_INT_EN_RISE_SET		0x0E
+#define USB_INT_EN_RISE_CLR		0x0F
+#define USB_INT_EN_FALL			0x10
+#define USB_INT_EN_FALL_SET		0x11
+#define USB_INT_EN_FALL_CLR		0x12
+#define USB_INT_STS			0x13
+#define USB_INT_LATCH			0x14
+#define USB_INT_IDGND			(1 << 4)
+#define USB_INT_SESSEND			(1 << 3)
+#define USB_INT_SESSVALID		(1 << 2)
+#define USB_INT_VBUSVALID		(1 << 1)
+#define USB_INT_HOSTDISCONNECT		(1 << 0)
+
+#define CARKIT_CTRL			0x19
+#define CARKIT_CTRL_SET			0x1A
+#define CARKIT_CTRL_CLR			0x1B
+#define CARKIT_CTRL_MICEN		(1 << 6)
+#define CARKIT_CTRL_SPKRIGHTEN		(1 << 5)
+#define CARKIT_CTRL_SPKLEFTEN		(1 << 4)
+#define CARKIT_CTRL_RXDEN		(1 << 3)
+#define CARKIT_CTRL_TXDEN		(1 << 2)
+#define CARKIT_CTRL_IDGNDDRV		(1 << 1)
+#define CARKIT_CTRL_CARKITPWR		(1 << 0)
+#define CARKIT_PLS_CTRL			0x22
+#define CARKIT_PLS_CTRL_SET		0x23
+#define CARKIT_PLS_CTRL_CLR		0x24
+#define CARKIT_PLS_CTRL_SPKRRIGHT_BIASEN	(1 << 3)
+#define CARKIT_PLS_CTRL_SPKRLEFT_BIASEN	(1 << 2)
+#define CARKIT_PLS_CTRL_RXPLSEN		(1 << 1)
+#define CARKIT_PLS_CTRL_TXPLSEN		(1 << 0)
+
+#define MCPC_CTRL			0x30
+#define MCPC_CTRL_SET			0x31
+#define MCPC_CTRL_CLR			0x32
+#define MCPC_CTRL_RTSOL			(1 << 7)
+#define MCPC_CTRL_EXTSWR		(1 << 6)
+#define MCPC_CTRL_EXTSWC		(1 << 5)
+#define MCPC_CTRL_VOICESW		(1 << 4)
+#define MCPC_CTRL_OUT64K		(1 << 3)
+#define MCPC_CTRL_RTSCTSSW		(1 << 2)
+#define MCPC_CTRL_HS_UART		(1 << 0)
+
+#define MCPC_IO_CTRL			0x33
+#define MCPC_IO_CTRL_SET		0x34
+#define MCPC_IO_CTRL_CLR		0x35
+#define MCPC_IO_CTRL_MICBIASEN		(1 << 5)
+#define MCPC_IO_CTRL_CTS_NPU		(1 << 4)
+#define MCPC_IO_CTRL_RXD_PU		(1 << 3)
+#define MCPC_IO_CTRL_TXDTYP		(1 << 2)
+#define MCPC_IO_CTRL_CTSTYP		(1 << 1)
+#define MCPC_IO_CTRL_RTSTYP		(1 << 0)
+
+#define MCPC_CTRL2			0x36
+#define MCPC_CTRL2_SET			0x37
+#define MCPC_CTRL2_CLR			0x38
+#define MCPC_CTRL2_MCPC_CK_EN		(1 << 0)
+
+#define OTHER_FUNC_CTRL			0x80
+#define OTHER_FUNC_CTRL_SET		0x81
+#define OTHER_FUNC_CTRL_CLR		0x82
+#define OTHER_FUNC_CTRL_BDIS_ACON_EN	(1 << 4)
+#define OTHER_FUNC_CTRL_FIVEWIRE_MODE	(1 << 2)
+
+#define OTHER_IFC_CTRL			0x83
+#define OTHER_IFC_CTRL_SET		0x84
+#define OTHER_IFC_CTRL_CLR		0x85
+#define OTHER_IFC_CTRL_OE_INT_EN	(1 << 6)
+#define OTHER_IFC_CTRL_CEA2011_MODE	(1 << 5)
+#define OTHER_IFC_CTRL_FSLSSERIALMODE_4PIN	(1 << 4)
+#define OTHER_IFC_CTRL_HIZ_ULPI_60MHZ_OUT	(1 << 3)
+#define OTHER_IFC_CTRL_HIZ_ULPI		(1 << 2)
+#define OTHER_IFC_CTRL_ALT_INT_REROUTE	(1 << 0)
+
+#define OTHER_INT_EN_RISE		0x86
+#define OTHER_INT_EN_RISE_SET		0x87
+#define OTHER_INT_EN_RISE_CLR		0x88
+#define OTHER_INT_EN_FALL		0x89
+#define OTHER_INT_EN_FALL_SET		0x8A
+#define OTHER_INT_EN_FALL_CLR		0x8B
+#define OTHER_INT_STS			0x8C
+#define OTHER_INT_LATCH			0x8D
+#define OTHER_INT_VB_SESS_VLD		(1 << 7)
+#define OTHER_INT_DM_HI			(1 << 6) /* not valid for "latch" reg */
+#define OTHER_INT_DP_HI			(1 << 5) /* not valid for "latch" reg */
+#define OTHER_INT_BDIS_ACON		(1 << 3) /* not valid for "fall" regs */
+#define OTHER_INT_MANU			(1 << 1)
+#define OTHER_INT_ABNORMAL_STRESS	(1 << 0)
+
+#define ID_STATUS			0x96
+#define ID_RES_FLOAT			(1 << 4)
+#define ID_RES_440K			(1 << 3)
+#define ID_RES_200K			(1 << 2)
+#define ID_RES_102K			(1 << 1)
+#define ID_RES_GND			(1 << 0)
+
+#define POWER_CTRL			0xAC
+#define POWER_CTRL_SET			0xAD
+#define POWER_CTRL_CLR			0xAE
+#define POWER_CTRL_OTG_ENAB		(1 << 5)
+
+#define OTHER_IFC_CTRL2			0xAF
+#define OTHER_IFC_CTRL2_SET		0xB0
+#define OTHER_IFC_CTRL2_CLR		0xB1
+#define OTHER_IFC_CTRL2_ULPI_STP_LOW	(1 << 4)
+#define OTHER_IFC_CTRL2_ULPI_TXEN_POL	(1 << 3)
+#define OTHER_IFC_CTRL2_ULPI_4PIN_2430	(1 << 2)
+#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_MASK	(3 << 0) /* bits 0 and 1 */
+#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_INT1N	(0 << 0)
+#define OTHER_IFC_CTRL2_USB_INT_OUTSEL_INT2N	(1 << 0)
+
+#define REG_CTRL_EN			0xB2
+#define REG_CTRL_EN_SET			0xB3
+#define REG_CTRL_EN_CLR			0xB4
+#define REG_CTRL_ERROR			0xB5
+#define ULPI_I2C_CONFLICT_INTEN		(1 << 0)
+
+#define OTHER_FUNC_CTRL2		0xB8
+#define OTHER_FUNC_CTRL2_SET		0xB9
+#define OTHER_FUNC_CTRL2_CLR		0xBA
+#define OTHER_FUNC_CTRL2_VBAT_TIMER_EN	(1 << 0)
+
+/* following registers do not have separate _clr and _set registers */
+#define VBUS_DEBOUNCE			0xC0
+#define ID_DEBOUNCE			0xC1
+#define VBAT_TIMER			0xD3
+#define PHY_PWR_CTRL			0xFD
+#define PHY_PWR_PHYPWD			(1 << 0)
+#define PHY_CLK_CTRL			0xFE
+#define PHY_CLK_CTRL_CLOCKGATING_EN	(1 << 2)
+#define PHY_CLK_CTRL_CLK32K_EN		(1 << 1)
+#define REQ_PHY_DPLL_CLK		(1 << 0)
+#define PHY_CLK_CTRL_STS		0xFF
+#define PHY_DPLL_CLK			(1 << 0)
+
+/* In module TWL4030_MODULE_PM_MASTER */
+#define PROTECT_KEY			0x0E
+
+/* In module TWL4030_MODULE_PM_RECEIVER */
+#define VUSB_DEDICATED1			0x7D
+#define VUSB_DEDICATED2			0x7E
+#define VUSB1V5_DEV_GRP			0x71
+#define VUSB1V5_TYPE			0x72
+#define VUSB1V5_REMAP			0x73
+#define VUSB1V8_DEV_GRP			0x74
+#define VUSB1V8_TYPE			0x75
+#define VUSB1V8_REMAP			0x76
+#define VUSB3V1_DEV_GRP			0x77
+#define VUSB3V1_TYPE			0x78
+#define VUSB3V1_REMAP			0x79
+
+/* In module TWL4030_MODULE_INTBR */
+#define PMBR1				0x0D
+#define GPIO_USB_4PIN_ULPI_2430C	(3 << 0)
+
+/* In module TWL4030_MODULE_INT */
+#define REG_PWR_ISR1			0x00
+#define REG_PWR_IMR1			0x01
+#define USB_PRES			(1 << 2)
+#define REG_PWR_EDR1			0x05
+#define USB_PRES_FALLING		(1 << 4)
+#define USB_PRES_RISING			(1 << 5)
+#define REG_PWR_SIH_CTRL		0x07
+#define COR				(1 << 2)
+
+/* bits in OTG_CTRL */
+#define	OTG_XCEIV_OUTPUTS \
+	(OTG_ASESSVLD|OTG_BSESSEND|OTG_BSESSVLD|OTG_VBUSVLD|OTG_ID)
+#define	OTG_XCEIV_INPUTS \
+	(OTG_PULLDOWN|OTG_PULLUP|OTG_DRV_VBUS|OTG_PD_VBUS|OTG_PU_VBUS|OTG_PU_ID)
+#define	OTG_CTRL_BITS \
+	(OTG_A_BUSREQ|OTG_A_SETB_HNPEN|OTG_B_BUSREQ|OTG_B_HNPEN|OTG_BUSDROP)
+	/* and OTG_PULLUP is sometimes written */
+
+#define	OTG_CTRL_MASK	(OTG_DRIVER_SEL| \
+	OTG_XCEIV_OUTPUTS|OTG_XCEIV_INPUTS| \
+	OTG_CTRL_BITS)
+
+enum twl4030_usb_mode {
+	T2_USB_MODE_ULPI = 1,
+	T2_USB_MODE_CEA2011_3PIN = 2,
+};
+
+struct twl4030_usb_platform_data {
+	enum twl4030_usb_mode	usb_mode;
+};
+
+#endif /* End of __TWL4030_USB_H */
+
-- 
1.6.0.1.126.ga118

[PATCH 2/2] i2c: twl4030-usb: add 'vbus' sysfs file

[Felipe Balbi]

vbus sysfs file will report the state of vbus irq coming from
twl4030-usb.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 drivers/i2c/chips/twl4030-usb.c |   51 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-usb.c b/drivers/i2c/chips/twl4030-usb.c
index 0d62e65..1429557 100644
--- a/drivers/i2c/chips/twl4030-usb.c
+++ b/drivers/i2c/chips/twl4030-usb.c
@@ -29,6 +29,8 @@
 #include <linux/time.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
 #include <linux/io.h>
 #include <linux/usb.h>
 #include <linux/usb/ch9.h>
@@ -39,11 +41,17 @@
 #include <mach/usb.h>
 
 struct twl4030_usb {
+	struct work_struct	irq_work;
 	struct otg_transceiver	otg;
 	struct device		*dev;
 
+	/* for vbus reporting with irqs disabled */
+	spinlock_t		lock;
+
 	/* pin configuration */
 	enum twl4030_usb_mode	usb_mode;
+
+	unsigned		vbus:1;
 	int			irq;
 	u8			asleep;
 };
@@ -309,6 +317,29 @@ static void twl4030_usb_ldo_init(struct twl4030_usb *twl)
 	twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, 0, PROTECT_KEY);
 }
 
+static ssize_t twl4030_usb_vbus_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct twl4030_usb *twl = dev_get_drvdata(dev);
+	unsigned long flags;
+	int ret = -EINVAL;
+
+	spin_lock_irqsave(&twl->lock, flags);
+	ret = sprintf(buf, "%s\n", twl->vbus ? "on" : "off");
+	spin_unlock_irqrestore(&twl->lock, flags);
+
+	return ret;
+}
+static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
+
+static void twl4030_usb_irq_work(struct work_struct *work)
+{
+	struct twl4030_usb *twl = container_of(work,
+			struct twl4030_usb, irq_work);
+
+	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
+}
+
 static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 {
 	struct twl4030_usb *twl = _twl;
@@ -321,10 +352,13 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 	if (val & USB_PRES_RISING) {
 		twl4030_phy_resume(twl);
 		twl4030charger_usb_en(1);
+		twl->vbus = 1;
 	} else {
 		twl4030charger_usb_en(0);
+		twl->vbus = 0;
 		twl4030_phy_suspend(twl, 0);
 	}
+	schedule_work(&twl->irq_work);
 
 	return IRQ_HANDLED;
 }
@@ -414,16 +448,22 @@ static int __init twl4030_usb_probe(struct platform_device *pdev)
 	struct twl4030_usb_platform_data *pdata = pdev->dev.platform_data;
 	struct twl4030_usb	*twl;
 	int status;
+	u8			vbus;
 
 	twl = kzalloc(sizeof *twl, GFP_KERNEL);
 	if (!twl)
 		return -ENOMEM;
 
+	WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
+				&vbus, REG_PWR_EDR1) < 0);
+	vbus &= USB_PRES_RISING;
+
 	twl->dev		= &pdev->dev;
 	twl->irq		= TWL4030_PWRIRQ_USB_PRES;
 	twl->otg.set_host	= twl4030_set_host;
 	twl->otg.set_peripheral	= twl4030_set_peripheral;
 	twl->otg.set_suspend	= twl4030_set_suspend;
+	twl->vbus		= vbus ? 1 : 0;
 
 	if (!pdata) {
 		dev_info(&pdev->dev, "platform_data not available, defaulting"
@@ -433,6 +473,12 @@ static int __init twl4030_usb_probe(struct platform_device *pdev)
 		twl->usb_mode	= pdata->usb_mode;
 	}
 
+	/* init spinlock for workqueue */
+	spin_lock_init(&twl->lock);
+
+	/* init irq workqueue before request_irq */
+	INIT_WORK(&twl->irq_work, twl4030_usb_irq_work);
+
 	usb_irq_disable(twl);
 	status = request_irq(twl->irq, twl4030_usb_irq, 0, "twl4030_usb", twl);
 	if (status < 0) {
@@ -442,7 +488,6 @@ static int __init twl4030_usb_probe(struct platform_device *pdev)
 		return status;
 	}
 
-
 	twl4030_usb_ldo_init(twl);
 	twl4030_phy_power(twl, 1);
 	twl4030_i2c_access(twl, 1);
@@ -459,6 +504,9 @@ static int __init twl4030_usb_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, twl);
 	dev_info(&pdev->dev, "Initialized TWL4030 USB module\n");
 
+	if (device_create_file(&pdev->dev, &dev_attr_vbus))
+		dev_warn(&pdev->dev, "could not create sysfs file\n");
+
 	return 0;
 }
 
@@ -469,6 +517,7 @@ static int __exit twl4030_usb_remove(struct platform_device *pdev)
 
 	usb_irq_disable(twl);
 	free_irq(twl->irq, twl);
+	device_remove_file(twl->dev, &dev_attr_vbus);
 
 	/* set transceiver mode to power on defaults */
 	twl4030_usb_set_mode(twl, -1);
-- 
1.6.0.1.126.ga118

Re: [PATCH 0/2] twl4030-usb patches

[Felipe Balbi]

On Tue, Sep 02, 2008 at 03:31:50PM +0300, Felipe Balbi wrote:
> The following patches convert twl4030-usb to a platform_device
> in order to set the usb mode via platform_data and get
> rid of the Kconfig entry for that.
> 
> Later, we add a sysfs entry for reporting the vbus events
> coming from triton, this let's us tell the userspace we
> have a usb connection (vbus is on) and trigger an application
> or something else.

Just one comment here. I'll apply these to the big series of drivers
going to mainline.

TWL4030 driver won't be accepted the way it is now. Starting from the
fact that twl4030-core is an old style i2c driver and Jean Delvare
already stated his plans to drop support for old style i2c drivers.

twl4030-usb is already moved to platform_driver, twl4030_gpio should be
moved to gpiolib also before sending to mainline.

Comments are really welcome.

-- 
balbi

Re: [PATCH 0/2] twl4030-usb patches

[David Brownell]

On Tuesday 02 September 2008, Felipe Balbi wrote:
> 
> The following patches convert twl4030-usb to a platform_device
> in order to set the usb mode via platform_data and get
> rid of the Kconfig entry for that.

Shouldn't twl4030 first be converted to be a "new style"
I2C driver?  Then if this approach is to be used, its
probe() would create those platform devices as children
of the I2C device node, and with relevant platform_data.

The platform_data for all those child devices should be
provided as part of the platform_data for the "main" TWL
driver, so everything can be properly board-specific.

Also, since it seems twl4030 docs won't become public
but docs for its catalog versions[0] will, it'd be good
to update file headers and Kconfig to mention tps659x0
parts so that folk without twl4030 NDA's can eventually
work with this code...

- Dave


[0] http://focus.ti.com/docs/prod/folders/print/tps65950.html

Re: [PATCH 0/2] twl4030-usb patches

[David Brownell]

On Tuesday 02 September 2008, Felipe Balbi wrote:
> twl4030_gpio should be
> moved to gpiolib also before sending to mainline.

Snapshot of what's in my tree ... 


=========== CUT HERE
From: David Brownell <dbrownell@users.sourceforge.net>

Make the TWL4030 GPIOs hook up to gpiolib.

Start to phase out the older TWL-specific calls, starting with
not exporting them (except for the few declared in the public
header, and accessed from arch/arm/mach-omap2/hsmmc.c).

# the Kconfig dependency changes in 2.6.27-rc ...

# should have the base GPIO set up better; minimally
# in a system header, ideally platform data ...

# the whole twl driver should be "new style".

NYET-Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/i2c/chips/Kconfig        |    2 
 drivers/i2c/chips/twl4030-gpio.c |   81 +++++++++++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 17 deletions(-)

--- beagle.orig/drivers/i2c/chips/Kconfig	2008-08-21 16:11:04.000000000 -0700
+++ beagle/drivers/i2c/chips/Kconfig	2008-08-21 19:58:02.000000000 -0700
@@ -159,7 +159,7 @@ config TWL4030_CORE
 
 config TWL4030_GPIO
 	bool "TWL4030 GPIO Driver"
-	depends on TWL4030_CORE
+	depends on TWL4030_CORE && GPIOLIB
 
 config TWL4030_MADC
 	tristate "TWL4030 MADC Driver"
--- beagle.orig/drivers/i2c/chips/twl4030-gpio.c	2008-08-21 16:11:04.000000000 -0700
+++ beagle/drivers/i2c/chips/twl4030-gpio.c	2008-08-21 16:15:51.000000000 -0700
@@ -31,10 +31,10 @@
 #include <linux/init.h>
 #include <linux/time.h>
 #include <linux/interrupt.h>
-#include <linux/random.h>
-#include <linux/syscalls.h>
+#include <linux/device.h>
 #include <linux/kthread.h>
 #include <linux/irq.h>
+#include <linux/gpio.h>
 
 #include <linux/i2c.h>
 #include <linux/i2c/twl4030.h>
@@ -46,7 +46,9 @@
 #include <mach/gpio.h>
 #include <mach/mux.h>
 
-#include <linux/device.h>
+/* TEMPORARY HACK */
+#define	TWL4030_GPIO_BASE	OMAP_MAX_GPIO_LINES
+
 
 /* BitField Definitions */
 
@@ -242,7 +244,7 @@ static void twl4030_gpio_unmask_irqchip(
 }
 
 static struct irq_chip twl4030_gpio_irq_chip = {
-	.name	= "twl4030-gpio",
+	.name	= "twl4030",
 	.ack	= twl4030_gpio_mask_and_ack_irqchip,
 	.mask	= twl4030_gpio_mask_irqchip,
 	.unmask	= twl4030_gpio_unmask_irqchip,
@@ -297,23 +299,28 @@ int twl4030_request_gpio(int gpio)
 	if (unlikely(gpio >= TWL4030_GPIO_MAX))
 		return -EPERM;
 
+	ret = gpio_request(TWL4030_GPIO_BASE + gpio, NULL);
+	if (ret < 0)
+		return ret;
+
 	down(&gpio_sem);
 	if (gpio_usage_count & (0x1 << gpio))
 		ret = -EBUSY;
 	else {
 		u8 clear_pull[6] = { 0, 0, 0, 0, 0, 0 };
+
 		/* First time usage? - switch on GPIO module */
 		if (!gpio_usage_count) {
-			ret =
-			gpio_twl4030_write(REG_GPIO_CTRL,
+			ret = gpio_twl4030_write(REG_GPIO_CTRL,
 					MASK_GPIO_CTRL_GPIO_ON);
 			ret = gpio_twl4030_write(REG_GPIO_SIH_CTRL, 0x00);
 		}
 		if (!ret)
 			gpio_usage_count |= (0x1 << gpio);
+		else
+			gpio_free(TWL4030_GPIO_BASE + gpio);
 
-		ret =
-		twl4030_i2c_write(TWL4030_MODULE_GPIO, clear_pull,
+		ret = twl4030_i2c_write(TWL4030_MODULE_GPIO, clear_pull,
 				REG_GPIOPUPDCTR1, 5);
 	}
 	up(&gpio_sem);
@@ -335,8 +342,10 @@ int twl4030_free_gpio(int gpio)
 
 	if ((gpio_usage_count & (0x1 << gpio)) == 0)
 		ret = -EPERM;
-	else
+	else {
 		gpio_usage_count &= ~(0x1 << gpio);
+		gpio_free(TWL4030_GPIO_BASE + gpio);
+	}
 
 	/* Last time usage? - switch off GPIO module */
 	if (!gpio_usage_count)
@@ -350,7 +359,7 @@ EXPORT_SYMBOL(twl4030_free_gpio);
 /*
  * Set direction for TWL4030 GPIO
  */
-int twl4030_set_gpio_direction(int gpio, int is_input)
+static int twl4030_set_gpio_direction(int gpio, int is_input)
 {
 	u8 d_bnk = GET_GPIO_DATA_BANK(gpio);
 	u8 d_msk = MASK_GPIODATADIR_GPIOxDIR(GET_GPIO_DATA_OFF(gpio));
@@ -377,12 +386,11 @@ int twl4030_set_gpio_direction(int gpio,
 	up(&gpio_sem);
 	return ret;
 }
-EXPORT_SYMBOL(twl4030_set_gpio_direction);
 
 /*
  * To enable/disable GPIO pin on TWL4030
  */
-int twl4030_set_gpio_dataout(int gpio, int enable)
+static int twl4030_set_gpio_dataout(int gpio, int enable)
 {
 	u8 d_bnk = GET_GPIO_DATA_BANK(gpio);
 	u8 d_msk = MASK_GPIODATAOUT_GPIOxOUT(GET_GPIO_DATA_OFF(gpio));
@@ -403,7 +411,6 @@ int twl4030_set_gpio_dataout(int gpio, i
 	up(&gpio_sem);
 	return ret;
 }
-EXPORT_SYMBOL(twl4030_set_gpio_dataout);
 
 /*
  * To get the status of a GPIO pin on TWL4030
@@ -430,6 +437,7 @@ int twl4030_get_gpio_datain(int gpio)
 }
 EXPORT_SYMBOL(twl4030_get_gpio_datain);
 
+#if 0
 /*
  * Configure PULL type for a GPIO pin on TWL4030
  */
@@ -465,7 +473,7 @@ int twl4030_set_gpio_pull(int gpio, int 
 	up(&gpio_sem);
 	return ret;
 }
-EXPORT_SYMBOL(twl4030_set_gpio_pull);
+#endif
 
 /*
  * Configure Edge control for a GPIO pin on TWL4030
@@ -538,6 +546,7 @@ int twl4030_set_gpio_debounce(int gpio, 
 }
 EXPORT_SYMBOL(twl4030_set_gpio_debounce);
 
+#if 0
 /*
  * Configure Card detect for GPIO pin on TWL4030
  */
@@ -567,7 +576,7 @@ int twl4030_set_gpio_card_detect(int gpi
 	up(&gpio_sem);
 	return (ret);
 }
-EXPORT_SYMBOL(twl4030_set_gpio_card_detect);
+#endif
 
 /* MODULE FUNCTIONS */
 
@@ -703,12 +712,52 @@ static void do_twl4030_gpio_module_irq(u
 	}
 }
 
-/* TWL4030 Initialization module */
+static int twl_direction_in(struct gpio_chip *chip, unsigned offset)
+{
+	return twl4030_set_gpio_direction(offset, 1);
+}
+
+static int twl_get(struct gpio_chip *chip, unsigned offset)
+{
+	int status = twl4030_get_gpio_datain(offset);
+
+	return (status < 0) ? 0 : status;
+}
+
+static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int value)
+{
+	twl4030_set_gpio_dataout(offset, value);
+	return twl4030_set_gpio_direction(offset, 0);
+}
+
+static void twl_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	twl4030_set_gpio_dataout(offset, value);
+}
+
+static struct gpio_chip twl_gpiochip = {
+	.label			= "twl4030",
+	.owner			= THIS_MODULE,
+	.direction_input	= twl_direction_in,
+	.get			= twl_get,
+	.direction_output	= twl_direction_out,
+	.set			= twl_set,
+	.base			= TWL4030_GPIO_BASE,
+	.ngpio			= TWL4030_GPIO_MAX,
+	.can_sleep		= 1,
+};
+
+/* TWL4030 GPIO Initialization module */
 static int __init gpio_twl4030_init(void)
 {
 	int ret;
 	int irq = 0;
 
+	ret = gpiochip_add(&twl_gpiochip);
+	if (ret < 0)
+		pr_err("%s: could not register gpiochip, %d\n",
+				__func__, ret);
+
 	/* init the global locking sem */
 	sema_init(&gpio_sem, 1);
 

Re: [PATCH 0/2] twl4030-usb patches

[Felipe Balbi]

On Tue, Sep 02, 2008 at 08:43:56AM -0700, David Brownell wrote:
> Snapshot of what's in my tree ... 

Great, a few comments below.

> # should have the base GPIO set up better; minimally
> # in a system header, ideally platform data ...

Could you describe this a bit more ? What do you mean by "base GPIO" ?
If you mean the first gpio number for passing to struct gpio_chip we
could use a platform_data if we make this one also a platform_driver,
right ?

> 
> # the whole twl driver should be "new style".

maybe only twl4030-core.c should hold the whole i2c new style thingy,
all the other drivers (usb, vibrators, keypad, rtc, etc) should/could
be a platform_driver.

>  static struct irq_chip twl4030_gpio_irq_chip = {
> -	.name	= "twl4030-gpio",
> +	.name	= "twl4030",

twl4030-core.c already uses this name. Wouldn't it be odd when cat
/proc/interrupts ? I'd like to see which are gpio interrupts and which
aren't

>  	down(&gpio_sem);
>  	if (gpio_usage_count & (0x1 << gpio))
>  		ret = -EBUSY;

how about also putting the brackets for the if(), maybe in a cleanup
patch before this one ?

>  	if ((gpio_usage_count & (0x1 << gpio)) == 0)
>  		ret = -EPERM;

missing brackets.

-- 
balbi

Re: [PATCH 0/2] twl4030-usb patches

[Felipe Balbi]

On Tue, Sep 02, 2008 at 08:39:28AM -0700, David Brownell wrote:
> On Tuesday 02 September 2008, Felipe Balbi wrote:
> > 
> > The following patches convert twl4030-usb to a platform_device
> > in order to set the usb mode via platform_data and get
> > rid of the Kconfig entry for that.
> 
> Shouldn't twl4030 first be converted to be a "new style"
> I2C driver?  Then if this approach is to be used, its
> probe() would create those platform devices as children
> of the I2C device node, and with relevant platform_data.

In that case, how would handle the case where more than one twl client
sits in the same i2c address ?

USB is alone on one id, but vibrator, pwm and others (sorry, at home
without docs) shares the same id. How would you pass the proper
platform_data to different drivers ?

I'd say a good approach, would be to let twl4030-core.c handle all the i2c
communication and the other twl modules become a platform_device using
the read/write functions exported from twl4030-core.c

> The platform_data for all those child devices should be
> provided as part of the platform_data for the "main" TWL
> driver, so everything can be properly board-specific.

let me get it straight. You mean something like:

struct twl4030_core_platform_data {
	struct twl4030_usb_platform_data *usb_data;
	struct twl4030_rtc_platform_data *rtc_data;
	[...]
};

Hmm... yeah, maybe that could work. But I'd have to see how you're
thinking of letting twl4030-core.c create all the platform_devices for
all the other modules.

One thing we might say, Jean Delvare won't accept twl4030 the way it is
now. Old style i2c drivers will be dropped soon. We have to get twl4030
properly done otherwise it'll be a pain later.

> Also, since it seems twl4030 docs won't become public
> but docs for its catalog versions[0] will, it'd be good
> to update file headers and Kconfig to mention tps659x0
> parts so that folk without twl4030 NDA's can eventually
> work with this code...

Good idea. I'm sure we should do it at some point. Thanks for the link.

-- 
balbi

Re: [PATCH 0/2] twl4030-usb patches

[David Brownell]

On Tuesday 02 September 2008, Felipe Balbi wrote:
> On Tue, Sep 02, 2008 at 08:39:28AM -0700, David Brownell wrote:
> > On Tuesday 02 September 2008, Felipe Balbi wrote:
> > > 
> > > The following patches convert twl4030-usb to a platform_device
> > > in order to set the usb mode via platform_data and get
> > > rid of the Kconfig entry for that.
> > 
> > Shouldn't twl4030 first be converted to be a "new style"
> > I2C driver?  Then if this approach is to be used, its
> > probe() would create those platform devices as children
> > of the I2C device node, and with relevant platform_data.
> 
> In that case, how would handle the case where more than one twl client
> sits in the same i2c address ?

Same way it does now ...

What's BROKEN with the current scheme is that the driver
model tree looks something like

	/sys/devices/platform/omap_i2c.X/X-00{N0,N1,N2,N3}
	/sys/devices/platform/{twl-gpio,twl-rtc,twl-foo,...}

That is, these devices are in the wrong place.  Now, I don't
really care much if they're all parented by the N0 node;
that's basically just a strange artifact of that chip.  All
that I suggested was making sure those platform devices
are properly parented:  by N0/N1/whatever.


> USB is alone on one id, but vibrator, pwm and others (sorry, at home
> without docs) shares the same id. How would you pass the proper
> platform_data to different drivers ?

Pass it through twl4030 platform data.  It's all board-specific,
and if you don't pass platform data for e.g. vibrator or LEDs
then you'd know this board doesn't have that hardware ... so
don't create those platform devices (even if the driver does
happen to be configured into this multi-board kernel).


> I'd say a good approach, would be to let twl4030-core.c handle all the i2c
> communication and the other twl modules become a platform_device using
> the read/write functions exported from twl4030-core.c

How is that different from what I said?

I was only highlighting particular brokenness of how this
driver currently initializes, with a straightforward fix
that's fully attuned to what's done upstream.

 
> > The platform_data for all those child devices should be
> > provided as part of the platform_data for the "main" TWL
> > driver, so everything can be properly board-specific.
> 
> let me get it straight. You mean something like:
> 
> struct twl4030_core_platform_data {
> 	struct twl4030_usb_platform_data *usb_data;
> 	struct twl4030_rtc_platform_data *rtc_data;

... gpio_platform_data, LED config, etc ...

> 	[...]
> };

Basically.  It may be that some of that stuff doesn't need
to be board-specific though.  At any rate there should be
a pointer to a "struct twl4030_core_platform_data" holding
board-specific stuff in the "struct i2c_board_info" that
each board provides when it does I2C initialization.


> Hmm... yeah, maybe that could work. But I'd have to see how you're
> thinking of letting twl4030-core.c create all the platform_devices for
> all the other modules.

static int twl4030_probe(struct i2c_client *c, const struct i2c_device_id *id)
{
	struct twl4030_core_platform_data *pdata = c->platform_data;
	struct platform_device *pdev;
	int status;

	...
	if (have_twl_rtc_driver() && pdata->rtc_data) {
		pdev = platform_device_alloc("twl4030_rtc", -1);
		... nullcheck ...
		status = platform_add_data(pdev, pdata->rtc_data,
				sizeof(*pdata->rtc_data));
		... status check ...
		status = platform_device_add_resources(pdev, ... the irq ... );
		... status_check ...
		pdev->dev.parent = &c->dev;
		status = platform_device_add(pdev);
		... status check ...
	}
	... etc
}

Or I suppose you could statically allocate the device data; not
particularly encouraged where pdev->dev.parent is non-null though.

 
> One thing we might say, Jean Delvare won't accept twl4030 the way it is
> now. Old style i2c drivers will be dropped soon. We have to get twl4030
> properly done otherwise it'll be a pain later.

He's also not keen on growing drivers/i2c/chips ...
So this driver should probably move to drivers/mfd too.

(When it starts moving upstream, I suspect some folk will
ask why it doesn't support the drivers/regulator calls;
that's still new, I'd not worry much.)

- Dave

 
> > Also, since it seems twl4030 docs won't become public
> > but docs for its catalog versions[0] will, it'd be good
> > to update file headers and Kconfig to mention tps659x0
> > parts so that folk without twl4030 NDA's can eventually
> > work with this code...
> 
> Good idea. I'm sure we should do it at some point. Thanks for the link.
> 
> -- 
> balbi
> 

Re: [PATCH 0/2] twl4030-usb patches

[Felipe Balbi]

On Tue, Sep 02, 2008 at 12:37:56PM -0700, David Brownell wrote:
> Same way it does now ...
> 
> What's BROKEN with the current scheme is that the driver
> model tree looks something like
> 
> 	/sys/devices/platform/omap_i2c.X/X-00{N0,N1,N2,N3}
> 	/sys/devices/platform/{twl-gpio,twl-rtc,twl-foo,...}
> 
> That is, these devices are in the wrong place.  Now, I don't
> really care much if they're all parented by the N0 node;
> that's basically just a strange artifact of that chip.  All
> that I suggested was making sure those platform devices
> are properly parented:  by N0/N1/whatever.

Agreed... it sure looks odd.

> Pass it through twl4030 platform data.  It's all board-specific,
> and if you don't pass platform data for e.g. vibrator or LEDs
> then you'd know this board doesn't have that hardware ... so
> don't create those platform devices (even if the driver does
> happen to be configured into this multi-board kernel).

got it.

> static int twl4030_probe(struct i2c_client *c, const struct i2c_device_id *id)
> {
> 	struct twl4030_core_platform_data *pdata = c->platform_data;
> 	struct platform_device *pdev;
> 	int status;
> 
> 	...
> 	if (have_twl_rtc_driver() && pdata->rtc_data) {
> 		pdev = platform_device_alloc("twl4030_rtc", -1);
> 		... nullcheck ...
> 		status = platform_add_data(pdev, pdata->rtc_data,
> 				sizeof(*pdata->rtc_data));
> 		... status check ...
> 		status = platform_device_add_resources(pdev, ... the irq ... );
> 		... status_check ...
> 		pdev->dev.parent = &c->dev;
> 		status = platform_device_add(pdev);
> 		... status check ...
> 	}
> 	... etc
> }
> 
> Or I suppose you could statically allocate the device data; not
> particularly encouraged where pdev->dev.parent is non-null though.

hmm... that looks good. Gotta dig more into the platform_driver stuff.
Thanks for the point.

> > One thing we might say, Jean Delvare won't accept twl4030 the way it is
> > now. Old style i2c drivers will be dropped soon. We have to get twl4030
> > properly done otherwise it'll be a pain later.
> 
> He's also not keen on growing drivers/i2c/chips ...
> So this driver should probably move to drivers/mfd too.

Sure... it should really be there since it really is a multifunction
device. In that case it would be merged through Samuel Ortiz, right ?
Most likely Cc:ing i2c@lm-sensors.org so i2c people could comment on the
code as well.

> (When it starts moving upstream, I suspect some folk will
> ask why it doesn't support the drivers/regulator calls;
> that's still new, I'd not worry much.)

Well, but if we have proper APIs for doing stuff, we should really start
using them, don't you think ?

I'll postpone my twl4030-usb patch and try to work on twl4030-core.c.
It'll be a huge task moving all of that to new style i2c driver.

-- 
balbi

Re: [PATCH 0/2] twl4030-usb patches

[David Brownell]

On Tuesday 02 September 2008, Felipe Balbi wrote:
> On Tue, Sep 02, 2008 at 08:43:56AM -0700, David Brownell wrote:
> > Snapshot of what's in my tree ... 
> 
> Great, a few comments below.
> 
> > # should have the base GPIO set up better; minimally
> > # in a system header, ideally platform data ...
> 
> Could you describe this a bit more ? What do you mean by "base GPIO" ?
> If you mean the first gpio number for passing to struct gpio_chip we
> could use a platform_data if we make this one also a platform_driver,
> right ?

Right.  See for example <linux/i2c/pca953x.h> which holds just
such data.  Note the setup()/teardown() stuff, which various OMAP
boards would be using to pass GPIO numbers to the HSMMC code ...
ensuring that the HSMMC driver isn't set up before its card detect
and writeprotect logic can work.

(Once the HSMMC board-specific init starts to look sane that is...)


 
> > # the whole twl driver should be "new style".
> 
> maybe only twl4030-core.c should hold the whole i2c new style thingy,
> all the other drivers (usb, vibrators, keypad, rtc, etc) should/could
> be a platform_driver.

That's "whole"!  :)

Thing is, this is a Multi-Function Device (MFD) and whoever did
this code decided to factor it as drivers around a core.  I'm
not proposing that be changed ... just that the parts should be
glued together properly.  Where "properly" includes not just
getting the driver model tree correct, but also passing in the
board-specific data from the board-*.c files instead of hard
wiring everything to mach the first available hardware.  (Today,
it's all hard-wired.)

 
> >  static struct irq_chip twl4030_gpio_irq_chip = {
> > -	.name	= "twl4030-gpio",
> > +	.name	= "twl4030",
> 
> twl4030-core.c already uses this name. Wouldn't it be odd when cat
> /proc/interrupts ? I'd like to see which are gpio interrupts and which
> aren't

They're all from the TWL4030 though.  :)

/proc/interrupts is misformatted with the long name though...

 
> >  	down(&gpio_sem);
> >  	if (gpio_usage_count & (0x1 << gpio))
> >  		ret = -EBUSY;
> 
> how about also putting the brackets for the if(), maybe in a cleanup
> patch before this one ?
> 
> >  	if ((gpio_usage_count & (0x1 << gpio)) == 0)
> >  		ret = -EPERM;
> 
> missing brackets.

Fixed. 

Re: [PATCH 0/2] twl4030-usb patches

[David Brownell]

On Tuesday 02 September 2008, Felipe Balbi wrote:
> > > One thing we might say, Jean Delvare won't accept twl4030 the way it is
> > > now. Old style i2c drivers will be dropped soon. We have to get twl4030
> > > properly done otherwise it'll be a pain later.
> > 
> > He's also not keen on growing drivers/i2c/chips ...
> > So this driver should probably move to drivers/mfd too.
> 
> Sure... it should really be there since it really is a multifunction
> device. In that case it would be merged through Samuel Ortiz, right ?
> Most likely Cc:ing i2c@lm-sensors.org so i2c people could comment on the
> code as well.

Yes.  Though considering it's an irq_chip that dispatches
through I2C ... maybe an LKML review too, for the core.

 
> > (When it starts moving upstream, I suspect some folk will
> > ask why it doesn't support the drivers/regulator calls;
> > that's still new, I'd not worry much.)
> 
> Well, but if we have proper APIs for doing stuff, we should really start
> using them, don't you think ?

That's why I mentioned that.  My priority would be getting
the existing core code fixed/merged.  Then other bits as
needed to get widely-available boards (Beagle, soon Overo)
working in mainline.  (Then the rest.)

Being the first "external" user of a new API framework is
something to take slowly... IMO it's not worth holding up
twl4030 support to do that.

 
> I'll postpone my twl4030-usb patch and try to work on twl4030-core.c.
> It'll be a huge task moving all of that to new style i2c driver.

Well, it's got obvious increments that are simple.  Core first;
then the other bits, incrementally.  Maybe the core could be
pushed upstream without a lot of the other functions exposed.

Note that the LED support isn't written yet, so there'd be
nothing to convert.  :)

- Dave