[PATH 0/3] isp1301 changes

[PATH 0/3] isp1301 changes

[Felipe Balbi]

Hi Jean,

I have a new approach for the isp1301 driver, took your idea
of adding a new irq_flags member to i2c_boardinfo and i2c_client.

If it's ok for you, i'll also send a patch putting linux-omap and
linux-mailine isp1301 in sync.

Comments are welcome as usual.

regads,

	-- Balbi



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH] I2C: Introduce irq_flags in i2c_boardinfo and i2c_client

[Felipe Balbi]

Drivers like isp1301_omap.c are used in several different
boards and thus we need different irq_flags. Instead of
keeping conditional code in the driver only because of
irq_flags we can get it from i2c_boardinfo.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 drivers/i2c/i2c-core.c |    1 +
 include/linux/i2c.h    |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index fd84b2a..5a290f6 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -254,6 +254,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->flags = info->flags & ~I2C_CLIENT_WAKE;
 	client->addr = info->addr;
 	client->irq = info->irq;
+	client->irq_flags = info->irq_flags;
 
 	strlcpy(client->driver_name, info->driver_name,
 		sizeof(client->driver_name));
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 365e0df..fe715f6 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -175,6 +175,7 @@ struct i2c_client {
 	struct i2c_driver *driver;	/* and our access routines	*/
 	struct device dev;		/* the device structure		*/
 	int irq;			/* irq issued by device (or -1) */
+	int irq_flags;			/* flags for irq		*/
 	char driver_name[KOBJ_NAME_LEN];
 	struct list_head list;		/* DEPRECATED */
 	struct completion released;
@@ -227,6 +228,7 @@ struct i2c_board_info {
 	unsigned short	addr;
 	void		*platform_data;
 	int		irq;
+	int		irq_flags;
 };
 
 /**
-- 
1.5.4.3.447.gc95b3


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 1

[Felipe Balbi]

Based on David Brownell's patch for tps65010, this patch
starts converting isp1301_omap.c to new-style i2c driver.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 drivers/i2c/chips/isp1301_omap.c |   61 +++++++++++++++++++------------------
 1 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index 2a31601..d094b99 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -49,7 +49,8 @@ MODULE_LICENSE("GPL");
 
 struct isp1301 {
 	struct otg_transceiver	otg;
-	struct i2c_client	client;
+	struct i2c_client	*client;
+	struct i2c_client	c;
 	void			(*i2c_release)(struct device *dev);
 
 	int			irq;
@@ -153,25 +154,25 @@ static struct i2c_driver isp1301_driver;
 static inline u8
 isp1301_get_u8(struct isp1301 *isp, u8 reg)
 {
-	return i2c_smbus_read_byte_data(&isp->client, reg + 0);
+	return i2c_smbus_read_byte_data(isp->client, reg + 0);
 }
 
 static inline int
 isp1301_get_u16(struct isp1301 *isp, u8 reg)
 {
-	return i2c_smbus_read_word_data(&isp->client, reg);
+	return i2c_smbus_read_word_data(isp->client, reg);
 }
 
 static inline int
 isp1301_set_bits(struct isp1301 *isp, u8 reg, u8 bits)
 {
-	return i2c_smbus_write_byte_data(&isp->client, reg + 0, bits);
+	return i2c_smbus_write_byte_data(isp->client, reg + 0, bits);
 }
 
 static inline int
 isp1301_clear_bits(struct isp1301 *isp, u8 reg, u8 bits)
 {
-	return i2c_smbus_write_byte_data(&isp->client, reg + 1, bits);
+	return i2c_smbus_write_byte_data(isp->client, reg + 1, bits);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -349,10 +350,10 @@ isp1301_defer_work(struct isp1301 *isp, int work)
 	int status;
 
 	if (isp && !test_and_set_bit(work, &isp->todo)) {
-		(void) get_device(&isp->client.dev);
+		(void) get_device(&isp->client->dev);
 		status = schedule_work(&isp->work);
 		if (!status && !isp->working)
-			dev_vdbg(&isp->client.dev,
+			dev_vdbg(&isp->client->dev,
 				"work item %d may be lost\n", work);
 	}
 }
@@ -1107,7 +1108,7 @@ isp1301_work(struct work_struct *work)
 		/* transfer state from otg engine to isp1301 */
 		if (test_and_clear_bit(WORK_UPDATE_ISP, &isp->todo)) {
 			otg_update_isp(isp);
-			put_device(&isp->client.dev);
+			put_device(&isp->client->dev);
 		}
 #endif
 		/* transfer state from isp1301 to otg engine */
@@ -1115,7 +1116,7 @@ isp1301_work(struct work_struct *work)
 			u8		stat = isp1301_clear_latch(isp);
 
 			isp_update_otg(isp, stat);
-			put_device(&isp->client.dev);
+			put_device(&isp->client->dev);
 		}
 
 		if (test_and_clear_bit(WORK_HOST_RESUME, &isp->todo)) {
@@ -1150,7 +1151,7 @@ isp1301_work(struct work_struct *work)
 			}
 			host_resume(isp);
 			// mdelay(10);
-			put_device(&isp->client.dev);
+			put_device(&isp->client->dev);
 		}
 
 		if (test_and_clear_bit(WORK_TIMER, &isp->todo)) {
@@ -1159,15 +1160,15 @@ isp1301_work(struct work_struct *work)
 			if (!stop)
 				mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
 #endif
-			put_device(&isp->client.dev);
+			put_device(&isp->client->dev);
 		}
 
 		if (isp->todo)
-			dev_vdbg(&isp->client.dev,
+			dev_vdbg(&isp->client->dev,
 				"work done, todo = 0x%lx\n",
 				isp->todo);
 		if (stop) {
-			dev_dbg(&isp->client.dev, "stop\n");
+			dev_dbg(&isp->client->dev, "stop\n");
 			break;
 		}
 	} while (isp->todo);
@@ -1191,7 +1192,7 @@ static void isp1301_release(struct device *dev)
 {
 	struct isp1301	*isp;
 
-	isp = container_of(dev, struct isp1301, client.dev);
+	isp = container_of(dev, struct isp1301, c.dev);
 
 	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
 	if (isp->i2c_release)
@@ -1205,7 +1206,7 @@ static int isp1301_detach_client(struct i2c_client *i2c)
 {
 	struct isp1301	*isp;
 
-	isp = container_of(i2c, struct isp1301, client);
+	isp = container_of(i2c, struct isp1301, c);
 
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
@@ -1257,7 +1258,7 @@ static int isp1301_otg_enable(struct isp1301 *isp)
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_FALLING,
 		INTR_VBUS_VLD | INTR_SESS_VLD | INTR_ID_GND);
 
-	dev_info(&isp->client.dev, "ready for dual-role USB ...\n");
+	dev_info(&isp->client->dev, "ready for dual-role USB ...\n");
 
 	return 0;
 }
@@ -1282,7 +1283,7 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 
 #ifdef	CONFIG_USB_OTG
 	isp->otg.host = host;
-	dev_dbg(&isp->client.dev, "registered host\n");
+	dev_dbg(&isp->client->dev, "registered host\n");
 	host_suspend(isp);
 	if (isp->otg.gadget)
 		return isp1301_otg_enable(isp);
@@ -1297,7 +1298,7 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 	if (machine_is_omap_h2())
 		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
 
-	dev_info(&isp->client.dev, "A-Host sessions ok\n");
+	dev_info(&isp->client->dev, "A-Host sessions ok\n");
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
 		INTR_ID_GND);
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_FALLING,
@@ -1315,7 +1316,7 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 	return 0;
 
 #else
-	dev_dbg(&isp->client.dev, "host sessions not allowed\n");
+	dev_dbg(&isp->client->dev, "host sessions not allowed\n");
 	return -EINVAL;
 #endif
 
@@ -1341,7 +1342,7 @@ isp1301_set_peripheral(struct otg_transceiver *otg, struct usb_gadget *gadget)
 
 #ifdef	CONFIG_USB_OTG
 	isp->otg.gadget = gadget;
-	dev_dbg(&isp->client.dev, "registered gadget\n");
+	dev_dbg(&isp->client->dev, "registered gadget\n");
 	/* gadget driver may be suspended until vbus_connect () */
 	if (isp->otg.host)
 		return isp1301_otg_enable(isp);
@@ -1364,8 +1365,8 @@ isp1301_set_peripheral(struct otg_transceiver *otg, struct usb_gadget *gadget)
 		INTR_SESS_VLD);
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_FALLING,
 		INTR_VBUS_VLD);
-	dev_info(&isp->client.dev, "B-Peripheral sessions ok\n");
-	dump_regs(isp, __FUNCTION__);
+	dev_info(&isp->client->dev, "B-Peripheral sessions ok\n");
+	dump_regs(isp, __func__);
 
 	/* If this has a Mini-AB connector, this mode is highly
 	 * nonstandard ... but can be handy for testing, so long
@@ -1377,7 +1378,7 @@ isp1301_set_peripheral(struct otg_transceiver *otg, struct usb_gadget *gadget)
 	return 0;
 
 #else
-	dev_dbg(&isp->client.dev, "peripheral sessions not allowed\n");
+	dev_dbg(&isp->client->dev, "peripheral sessions not allowed\n");
 	return -EINVAL;
 #endif
 }
@@ -1493,12 +1494,12 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
 	isp->timer.data = (unsigned long) isp;
 
 	isp->irq = -1;
-	isp->client.addr = address;
-	i2c_set_clientdata(&isp->client, isp);
-	isp->client.adapter = bus;
-	isp->client.driver = &isp1301_driver;
-	strlcpy(isp->client.name, DRIVER_NAME, I2C_NAME_SIZE);
-	i2c = &isp->client;
+	isp->c.addr = address;
+	i2c_set_clientdata(&isp->c, isp);
+	isp->c.adapter = bus;
+	isp->c.driver = &isp1301_driver;
+	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
+	isp->client = i2c = &isp->c;
 
 	/* if this is a true probe, verify the chip ... */
 	if (kind < 0) {
@@ -1583,7 +1584,7 @@ fail2:
 		goto fail1;
 	}
 
-	isp->otg.dev = &isp->client.dev;
+	isp->otg.dev = &isp->client->dev;
 	isp->otg.label = DRIVER_NAME;
 
 	isp->otg.set_host = isp1301_set_host,
-- 
1.5.4.3.447.gc95b3


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

Based on David Brownell's patch for tps65010, this patch
finish conversting isp1301_omap.c to new-style i2c driver.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 arch/arm/mach-omap1/board-h2.c   |    1 +
 arch/arm/mach-omap1/board-h3.c   |    6 +
 arch/arm/mach-omap2/board-h4.c   |   12 +++
 drivers/i2c/chips/isp1301_omap.c |  198 +++++++------------------------------
 include/linux/i2c/isp1301_omap.h |   92 ++++++++++++++++++
 5 files changed, 149 insertions(+), 160 deletions(-)
 create mode 100644 include/linux/i2c/isp1301_omap.h

diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 5079877..81a8c87 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -357,6 +357,7 @@ static struct i2c_board_info __initdata h2_i2c_board_info[] = {
 		I2C_BOARD_INFO("isp1301_omap", 0x2d),
 		.type		= "isp1301_omap",
 		.irq		= OMAP_GPIO_IRQ(2),
+		.irq_flags	= IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING,
 	},
 };
 
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index c3ef1ee..474921f 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -477,6 +477,12 @@ static struct i2c_board_info __initdata h3_i2c_board_info[] = {
                .type           = "tps65013",
                /* .irq         = OMAP_GPIO_IRQ(??), */
        },
+       {
+               I2C_BOARD_INFO("isp1301_omap", 0x2d),
+               .type		= "isp1301_omap",
+               .irq		= OMAP_GPIO_IRQ(14),
+	       .irq_flags	= IRQF_SAMPLE_RANDOM,
+       },
 };
 
 static struct omap_gpio_switch h3_gpio_switches[] __initdata = {
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index f125f43..6e57021 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -301,6 +301,15 @@ static struct omap_board_config_kernel h4_config[] = {
 	{ OMAP_TAG_LCD,		&h4_lcd_config },
 };
 
+static struct i2c_board_info __initdata h4_i2c_board_info[] = {
+	{
+		I2C_BOARD_INFO("isp1301_omap", 0x2d),
+		.type		= "isp1301_omap",
+		.irq		= OMAP_GPIO_IRQ(125),
+		.irq_flags	= IRQF_SAMPLE_RANDOM,
+	},
+};
+
 static void __init omap_h4_init(void)
 {
 	/*
@@ -321,6 +330,9 @@ static void __init omap_h4_init(void)
 	}
 #endif
 
+	i2c_register_board_info(1, h4_i2c_board_info,
+			ARRAY_SIZE(h4_i2c_board_info));
+
 	platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
 	omap_board_config = h4_config;
 	omap_board_config_size = ARRAY_SIZE(h4_config);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index d094b99..f88f9ae 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -25,14 +25,9 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
-#include <linux/usb/ch9.h>
-#include <linux/usb/gadget.h>
-#include <linux/usb.h>
-#include <linux/usb/otg.h>
 #include <linux/i2c.h>
+#include <linux/i2c/isp1301_omap.h>
 #include <linux/workqueue.h>
-
-#include <asm/irq.h>
 #include <asm/arch/usb.h>
 
 
@@ -50,12 +45,8 @@ MODULE_LICENSE("GPL");
 struct isp1301 {
 	struct otg_transceiver	otg;
 	struct i2c_client	*client;
-	struct i2c_client	c;
 	void			(*i2c_release)(struct device *dev);
 
-	int			irq;
-	int			irq_type;
-
 	u32			last_otg_ctrl;
 	unsigned		working:1;
 
@@ -140,13 +131,6 @@ static inline void notresponding(struct isp1301 *isp)
 /*-------------------------------------------------------------------------*/
 
 /* only two addresses possible */
-#define	ISP_BASE		0x2c
-static unsigned short normal_i2c[] = {
-	ISP_BASE, ISP_BASE + 1,
-	I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
 static struct i2c_driver isp1301_driver;
 
 /* smbus apis are used for portability */
@@ -177,64 +161,6 @@ isp1301_clear_bits(struct isp1301 *isp, u8 reg, u8 bits)
 
 /*-------------------------------------------------------------------------*/
 
-/* identification */
-#define	ISP1301_VENDOR_ID		0x00	/* u16 read */
-#define	ISP1301_PRODUCT_ID		0x02	/* u16 read */
-#define	ISP1301_BCD_DEVICE		0x14	/* u16 read */
-
-#define	I2C_VENDOR_ID_PHILIPS		0x04cc
-#define	I2C_PRODUCT_ID_PHILIPS_1301	0x1301
-
-/* operational registers */
-#define	ISP1301_MODE_CONTROL_1		0x04	/* u8 read, set, +1 clear */
-#	define	MC1_SPEED_REG		(1 << 0)
-#	define	MC1_SUSPEND_REG		(1 << 1)
-#	define	MC1_DAT_SE0		(1 << 2)
-#	define	MC1_TRANSPARENT		(1 << 3)
-#	define	MC1_BDIS_ACON_EN	(1 << 4)
-#	define	MC1_OE_INT_EN		(1 << 5)
-#	define	MC1_UART_EN		(1 << 6)
-#	define	MC1_MASK		0x7f
-#define	ISP1301_MODE_CONTROL_2		0x12	/* u8 read, set, +1 clear */
-#	define	MC2_GLOBAL_PWR_DN	(1 << 0)
-#	define	MC2_SPD_SUSP_CTRL	(1 << 1)
-#	define	MC2_BI_DI		(1 << 2)
-#	define	MC2_TRANSP_BDIR0	(1 << 3)
-#	define	MC2_TRANSP_BDIR1	(1 << 4)
-#	define	MC2_AUDIO_EN		(1 << 5)
-#	define	MC2_PSW_EN		(1 << 6)
-#	define	MC2_EN2V7		(1 << 7)
-#define	ISP1301_OTG_CONTROL_1		0x06	/* u8 read, set, +1 clear */
-#	define	OTG1_DP_PULLUP		(1 << 0)
-#	define	OTG1_DM_PULLUP		(1 << 1)
-#	define	OTG1_DP_PULLDOWN	(1 << 2)
-#	define	OTG1_DM_PULLDOWN	(1 << 3)
-#	define	OTG1_ID_PULLDOWN	(1 << 4)
-#	define	OTG1_VBUS_DRV		(1 << 5)
-#	define	OTG1_VBUS_DISCHRG	(1 << 6)
-#	define	OTG1_VBUS_CHRG		(1 << 7)
-#define	ISP1301_OTG_STATUS		0x10	/* u8 readonly */
-#	define	OTG_B_SESS_END		(1 << 6)
-#	define	OTG_B_SESS_VLD		(1 << 7)
-
-#define	ISP1301_INTERRUPT_SOURCE	0x08	/* u8 read */
-#define	ISP1301_INTERRUPT_LATCH		0x0A	/* u8 read, set, +1 clear */
-
-#define	ISP1301_INTERRUPT_FALLING	0x0C	/* u8 read, set, +1 clear */
-#define	ISP1301_INTERRUPT_RISING	0x0E	/* u8 read, set, +1 clear */
-
-/* same bitfields in all interrupt registers */
-#	define	INTR_VBUS_VLD		(1 << 0)
-#	define	INTR_SESS_VLD		(1 << 1)
-#	define	INTR_DP_HI		(1 << 2)
-#	define	INTR_ID_GND		(1 << 3)
-#	define	INTR_DM_HI		(1 << 4)
-#	define	INTR_ID_FLOAT		(1 << 5)
-#	define	INTR_BDIS_ACON		(1 << 6)
-#	define	INTR_CR_INT		(1 << 7)
-
-/*-------------------------------------------------------------------------*/
-
 static const char *state_string(enum usb_otg_state state)
 {
 	switch (state) {
@@ -1190,9 +1116,7 @@ static void isp1301_timer(unsigned long _isp)
 
 static void isp1301_release(struct device *dev)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(dev, struct isp1301, c.dev);
+	struct isp1301		*isp = dev_get_drvdata(dev);
 
 	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
 	if (isp->i2c_release)
@@ -1202,30 +1126,27 @@ static void isp1301_release(struct device *dev)
 
 static struct isp1301 *the_transceiver;
 
-static int isp1301_detach_client(struct i2c_client *i2c)
+static int __exit isp1301_remove(struct i2c_client *client)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(i2c, struct isp1301, c);
+	struct isp1301	*isp = i2c_get_clientdata(client);
 
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
-	free_irq(isp->irq, isp);
+
+	if (client->irq > 0)
+		free_irq(client->irq, isp);
 #ifdef	CONFIG_USB_OTG
 	otg_unbind(isp);
 #endif
-	if (machine_is_omap_h2())
-		omap_free_gpio(2);
-
 	isp->timer.data = 0;
 	set_bit(WORK_STOP, &isp->todo);
 	del_timer_sync(&isp->timer);
 	flush_scheduled_work();
 
-	put_device(&i2c->dev);
+	put_device(&client->dev);
 	the_transceiver = 0;
 
-	return i2c_detach_client(i2c);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1295,9 +1216,6 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 
 	power_up(isp);
 
-	if (machine_is_omap_h2())
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
-
 	dev_info(&isp->client->dev, "A-Host sessions ok\n");
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
 		INTR_ID_GND);
@@ -1475,11 +1393,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
 /*-------------------------------------------------------------------------*/
 
 /* no error returns, they'd just make bus scanning stop */
-static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
+static int __init isp1301_probe(struct i2c_client *client)
 {
 	int			status;
 	struct isp1301		*isp;
-	struct i2c_client	*i2c;
 
 	if (the_transceiver)
 		return 0;
@@ -1492,45 +1409,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
 	init_timer(&isp->timer);
 	isp->timer.function = isp1301_timer;
 	isp->timer.data = (unsigned long) isp;
-
-	isp->irq = -1;
-	isp->c.addr = address;
-	i2c_set_clientdata(&isp->c, isp);
-	isp->c.adapter = bus;
-	isp->c.driver = &isp1301_driver;
-	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
-	isp->client = i2c = &isp->c;
+	isp->client = client;
 
 	/* if this is a true probe, verify the chip ... */
-	if (kind < 0) {
-		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
-		if (status != I2C_VENDOR_ID_PHILIPS) {
-			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
-				address, status);
-			goto fail1;
-		}
-		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
-		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
-			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
-				address, status);
-			goto fail1;
-		}
+	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
+	if (status != I2C_VENDOR_ID_PHILIPS) {
+		dev_dbg(&client->dev, "not philips id: %d\n", status);
+		goto fail1;
+	}
+	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
+	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
+		dev_dbg(&client->dev, "not isp1301, %d\n", status);
+		goto fail1;
 	}
 
-	status = i2c_attach_client(i2c);
-	if (status < 0) {
-		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
-				DRIVER_NAME, address, status);
 fail1:
 		kfree(isp);
 		return 0;
-	}
-	isp->i2c_release = i2c->dev.release;
-	i2c->dev.release = isp1301_release;
+
+	isp->i2c_release = client->dev.release = isp1301_release;
 
 	/* initial development used chiprev 2.00 */
-	status = i2c_smbus_read_word_data(i2c, ISP1301_BCD_DEVICE);
-	dev_info(&i2c->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
+	status = i2c_smbus_read_word_data(client, ISP1301_BCD_DEVICE);
+	dev_info(&client->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
 		status >> 8, status & 0xff);
 
 	/* make like power-on reset */
@@ -1551,40 +1452,25 @@ fail1:
 #ifdef	CONFIG_USB_OTG
 	status = otg_bind(isp);
 	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't bind OTG\n");
+		dev_dbg(&client->dev, "can't bind OTG\n");
 		goto fail2;
 	}
 #endif
 
-	if (machine_is_omap_h2()) {
-		/* full speed signaling by default */
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
-			MC1_SPEED_REG);
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
-			MC2_SPD_SUSP_CTRL);
-
-		/* IRQ wired at M14 */
-		omap_cfg_reg(M14_1510_GPIO2);
-		isp->irq = OMAP_GPIO_IRQ(2);
-		if (gpio_request(2, "isp1301") == 0)
-			gpio_direction_input(2);
-		isp->irq_type = IRQF_TRIGGER_FALLING;
-	}
-
-	isp->irq_type |= IRQF_SAMPLE_RANDOM;
-	status = request_irq(isp->irq, isp1301_irq,
-			isp->irq_type, DRIVER_NAME, isp);
-	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
-				isp->irq, status);
+	if (client->irq > 0) {
+		status = request_irq(client->irq, isp1301_irq,
+				client->irq_flags, DRIVER_NAME, isp);
+		if (status < 0) {
+			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
+					client->irq, status);
 #ifdef	CONFIG_USB_OTG
 fail2:
 #endif
-		i2c_detach_client(i2c);
-		goto fail1;
+			goto fail1;
+		}
 	}
 
-	isp->otg.dev = &isp->client->dev;
+	isp->otg.dev = &client->dev;
 	isp->otg.label = DRIVER_NAME;
 
 	isp->otg.set_host = isp1301_set_host,
@@ -1606,31 +1492,23 @@ fail2:
 
 #ifdef	VERBOSE
 	mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
-	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
+	dev_dbg(&client->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
 #endif
 
 	status = otg_set_transceiver(&isp->otg);
 	if (status < 0)
-		dev_err(&i2c->dev, "can't register transceiver, %d\n",
+		dev_err(&client->dev, "can't register transceiver, %d\n",
 			status);
 
 	return 0;
 }
 
-static int isp1301_scan_bus(struct i2c_adapter *bus)
-{
-	if (!i2c_check_functionality(bus, I2C_FUNC_SMBUS_BYTE_DATA
-			| I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EINVAL;
-	return i2c_probe(bus, &addr_data, isp1301_probe);
-}
-
 static struct i2c_driver isp1301_driver = {
 	.driver = {
 		.name	= "isp1301_omap",
 	},
-	.attach_adapter	= isp1301_scan_bus,
-	.detach_client	= isp1301_detach_client,
+	.probe	= isp1301_probe,
+	.remove	= __exit_p(isp1301_remove),
 };
 
 /*-------------------------------------------------------------------------*/
diff --git a/include/linux/i2c/isp1301_omap.h b/include/linux/i2c/isp1301_omap.h
new file mode 100644
index 0000000..de47460
--- /dev/null
+++ b/include/linux/i2c/isp1301_omap.h
@@ -0,0 +1,92 @@
+/* include/linux/i2c/isp1301_omap.h
+ *
+ * Copyright (C) 2008 Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
+ *
+ * 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 SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
+ * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * 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.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef __LINUX_I2C_ISP1301_OMAP_H
+#define __LINUX_I2C_ISP1301_OMAP_H
+
+/* identification */
+#define	ISP1301_VENDOR_ID		0x00	/* u16 read */
+#define	ISP1301_PRODUCT_ID		0x02	/* u16 read */
+#define	ISP1301_BCD_DEVICE		0x14	/* u16 read */
+
+#define	I2C_VENDOR_ID_PHILIPS		0x04cc
+#define	I2C_PRODUCT_ID_PHILIPS_1301	0x1301
+
+/* operational registers */
+#define	ISP1301_MODE_CONTROL_1		0x04	/* u8 read, set, +1 clear */
+#	define	MC1_SPEED_REG		(1 << 0)
+#	define	MC1_SUSPEND_REG		(1 << 1)
+#	define	MC1_DAT_SE0		(1 << 2)
+#	define	MC1_TRANSPARENT		(1 << 3)
+#	define	MC1_BDIS_ACON_EN	(1 << 4)
+#	define	MC1_OE_INT_EN		(1 << 5)
+#	define	MC1_UART_EN		(1 << 6)
+#	define	MC1_MASK		0x7f
+#define	ISP1301_MODE_CONTROL_2		0x12	/* u8 read, set, +1 clear */
+#	define	MC2_GLOBAL_PWR_DN	(1 << 0)
+#	define	MC2_SPD_SUSP_CTRL	(1 << 1)
+#	define	MC2_BI_DI		(1 << 2)
+#	define	MC2_TRANSP_BDIR0	(1 << 3)
+#	define	MC2_TRANSP_BDIR1	(1 << 4)
+#	define	MC2_AUDIO_EN		(1 << 5)
+#	define	MC2_PSW_EN		(1 << 6)
+#	define	MC2_EN2V7		(1 << 7)
+#define	ISP1301_OTG_CONTROL_1		0x06	/* u8 read, set, +1 clear */
+#	define	OTG1_DP_PULLUP		(1 << 0)
+#	define	OTG1_DM_PULLUP		(1 << 1)
+#	define	OTG1_DP_PULLDOWN	(1 << 2)
+#	define	OTG1_DM_PULLDOWN	(1 << 3)
+#	define	OTG1_ID_PULLDOWN	(1 << 4)
+#	define	OTG1_VBUS_DRV		(1 << 5)
+#	define	OTG1_VBUS_DISCHRG	(1 << 6)
+#	define	OTG1_VBUS_CHRG		(1 << 7)
+#define	ISP1301_OTG_STATUS		0x10	/* u8 readonly */
+#	define	OTG_B_SESS_END		(1 << 6)
+#	define	OTG_B_SESS_VLD		(1 << 7)
+
+#define	ISP1301_INTERRUPT_SOURCE	0x08	/* u8 read */
+#define	ISP1301_INTERRUPT_LATCH		0x0A	/* u8 read, set, +1 clear */
+
+#define	ISP1301_INTERRUPT_FALLING	0x0C	/* u8 read, set, +1 clear */
+#define	ISP1301_INTERRUPT_RISING	0x0E	/* u8 read, set, +1 clear */
+
+/* same bitfields in all interrupt registers */
+#	define	INTR_VBUS_VLD		(1 << 0)
+#	define	INTR_SESS_VLD		(1 << 1)
+#	define	INTR_DP_HI		(1 << 2)
+#	define	INTR_ID_GND		(1 << 3)
+#	define	INTR_DM_HI		(1 << 4)
+#	define	INTR_ID_FLOAT		(1 << 5)
+#	define	INTR_BDIS_ACON		(1 << 6)
+#	define	INTR_CR_INT		(1 << 7)
+
+struct isp1301_platform_data {
+	unsigned	irqflags;
+	unsigned	bit;
+
+};
+
+#endif /*  __LINUX_I2C_ISP1301_OMAP_H */
+
-- 
1.5.4.3.447.gc95b3


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

On Sat, Mar 15, 2008 at 02:47:17PM +0200, Felipe Balbi wrote:
> Based on David Brownell's patch for tps65010, this patch
> finish conversting isp1301_omap.c to new-style i2c driver.
> 
> Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> ---
>  arch/arm/mach-omap1/board-h2.c   |    1 +
>  arch/arm/mach-omap1/board-h3.c   |    6 +
>  arch/arm/mach-omap2/board-h4.c   |   12 +++
>  drivers/i2c/chips/isp1301_omap.c |  198 +++++++------------------------------
>  include/linux/i2c/isp1301_omap.h |   92 ++++++++++++++++++
>  5 files changed, 149 insertions(+), 160 deletions(-)
>  create mode 100644 include/linux/i2c/isp1301_omap.h
> 
> diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
> index 5079877..81a8c87 100644
> --- a/arch/arm/mach-omap1/board-h2.c
> +++ b/arch/arm/mach-omap1/board-h2.c
> @@ -357,6 +357,7 @@ static struct i2c_board_info __initdata h2_i2c_board_info[] = {
>  		I2C_BOARD_INFO("isp1301_omap", 0x2d),
>  		.type		= "isp1301_omap",
>  		.irq		= OMAP_GPIO_IRQ(2),
> +		.irq_flags	= IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING,
>  	},
>  };
>  
> diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
> index c3ef1ee..474921f 100644
> --- a/arch/arm/mach-omap1/board-h3.c
> +++ b/arch/arm/mach-omap1/board-h3.c
> @@ -477,6 +477,12 @@ static struct i2c_board_info __initdata h3_i2c_board_info[] = {
>                 .type           = "tps65013",
>                 /* .irq         = OMAP_GPIO_IRQ(??), */
>         },
> +       {
> +               I2C_BOARD_INFO("isp1301_omap", 0x2d),
> +               .type		= "isp1301_omap",
> +               .irq		= OMAP_GPIO_IRQ(14),
> +	       .irq_flags	= IRQF_SAMPLE_RANDOM,
> +       },
>  };
>  
>  static struct omap_gpio_switch h3_gpio_switches[] __initdata = {
> diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
> index f125f43..6e57021 100644
> --- a/arch/arm/mach-omap2/board-h4.c
> +++ b/arch/arm/mach-omap2/board-h4.c
> @@ -301,6 +301,15 @@ static struct omap_board_config_kernel h4_config[] = {
>  	{ OMAP_TAG_LCD,		&h4_lcd_config },
>  };
>  
> +static struct i2c_board_info __initdata h4_i2c_board_info[] = {
> +	{
> +		I2C_BOARD_INFO("isp1301_omap", 0x2d),
> +		.type		= "isp1301_omap",
> +		.irq		= OMAP_GPIO_IRQ(125),
> +		.irq_flags	= IRQF_SAMPLE_RANDOM,
> +	},
> +};
> +
>  static void __init omap_h4_init(void)
>  {
>  	/*
> @@ -321,6 +330,9 @@ static void __init omap_h4_init(void)
>  	}
>  #endif
>  
> +	i2c_register_board_info(1, h4_i2c_board_info,
> +			ARRAY_SIZE(h4_i2c_board_info));
> +
>  	platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
>  	omap_board_config = h4_config;
>  	omap_board_config_size = ARRAY_SIZE(h4_config);
> diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
> index d094b99..f88f9ae 100644
> --- a/drivers/i2c/chips/isp1301_omap.c
> +++ b/drivers/i2c/chips/isp1301_omap.c
> @@ -25,14 +25,9 @@
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> -#include <linux/usb/ch9.h>
> -#include <linux/usb/gadget.h>
> -#include <linux/usb.h>
> -#include <linux/usb/otg.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c/isp1301_omap.h>

This include is wrong... sorry

>  #include <linux/workqueue.h>
> -
> -#include <asm/irq.h>
>  #include <asm/arch/usb.h>
>  
>  
> @@ -50,12 +45,8 @@ MODULE_LICENSE("GPL");
>  struct isp1301 {
>  	struct otg_transceiver	otg;
>  	struct i2c_client	*client;
> -	struct i2c_client	c;
>  	void			(*i2c_release)(struct device *dev);
>  
> -	int			irq;
> -	int			irq_type;
> -
>  	u32			last_otg_ctrl;
>  	unsigned		working:1;
>  
> @@ -140,13 +131,6 @@ static inline void notresponding(struct isp1301 *isp)
>  /*-------------------------------------------------------------------------*/
>  
>  /* only two addresses possible */
> -#define	ISP_BASE		0x2c
> -static unsigned short normal_i2c[] = {
> -	ISP_BASE, ISP_BASE + 1,
> -	I2C_CLIENT_END };
> -
> -I2C_CLIENT_INSMOD;
> -
>  static struct i2c_driver isp1301_driver;
>  
>  /* smbus apis are used for portability */
> @@ -177,64 +161,6 @@ isp1301_clear_bits(struct isp1301 *isp, u8 reg, u8 bits)
>  
>  /*-------------------------------------------------------------------------*/
>  
> -/* identification */
> -#define	ISP1301_VENDOR_ID		0x00	/* u16 read */
> -#define	ISP1301_PRODUCT_ID		0x02	/* u16 read */
> -#define	ISP1301_BCD_DEVICE		0x14	/* u16 read */
> -
> -#define	I2C_VENDOR_ID_PHILIPS		0x04cc
> -#define	I2C_PRODUCT_ID_PHILIPS_1301	0x1301
> -
> -/* operational registers */
> -#define	ISP1301_MODE_CONTROL_1		0x04	/* u8 read, set, +1 clear */
> -#	define	MC1_SPEED_REG		(1 << 0)
> -#	define	MC1_SUSPEND_REG		(1 << 1)
> -#	define	MC1_DAT_SE0		(1 << 2)
> -#	define	MC1_TRANSPARENT		(1 << 3)
> -#	define	MC1_BDIS_ACON_EN	(1 << 4)
> -#	define	MC1_OE_INT_EN		(1 << 5)
> -#	define	MC1_UART_EN		(1 << 6)
> -#	define	MC1_MASK		0x7f
> -#define	ISP1301_MODE_CONTROL_2		0x12	/* u8 read, set, +1 clear */
> -#	define	MC2_GLOBAL_PWR_DN	(1 << 0)
> -#	define	MC2_SPD_SUSP_CTRL	(1 << 1)
> -#	define	MC2_BI_DI		(1 << 2)
> -#	define	MC2_TRANSP_BDIR0	(1 << 3)
> -#	define	MC2_TRANSP_BDIR1	(1 << 4)
> -#	define	MC2_AUDIO_EN		(1 << 5)
> -#	define	MC2_PSW_EN		(1 << 6)
> -#	define	MC2_EN2V7		(1 << 7)
> -#define	ISP1301_OTG_CONTROL_1		0x06	/* u8 read, set, +1 clear */
> -#	define	OTG1_DP_PULLUP		(1 << 0)
> -#	define	OTG1_DM_PULLUP		(1 << 1)
> -#	define	OTG1_DP_PULLDOWN	(1 << 2)
> -#	define	OTG1_DM_PULLDOWN	(1 << 3)
> -#	define	OTG1_ID_PULLDOWN	(1 << 4)
> -#	define	OTG1_VBUS_DRV		(1 << 5)
> -#	define	OTG1_VBUS_DISCHRG	(1 << 6)
> -#	define	OTG1_VBUS_CHRG		(1 << 7)
> -#define	ISP1301_OTG_STATUS		0x10	/* u8 readonly */
> -#	define	OTG_B_SESS_END		(1 << 6)
> -#	define	OTG_B_SESS_VLD		(1 << 7)
> -
> -#define	ISP1301_INTERRUPT_SOURCE	0x08	/* u8 read */
> -#define	ISP1301_INTERRUPT_LATCH		0x0A	/* u8 read, set, +1 clear */
> -
> -#define	ISP1301_INTERRUPT_FALLING	0x0C	/* u8 read, set, +1 clear */
> -#define	ISP1301_INTERRUPT_RISING	0x0E	/* u8 read, set, +1 clear */
> -
> -/* same bitfields in all interrupt registers */
> -#	define	INTR_VBUS_VLD		(1 << 0)
> -#	define	INTR_SESS_VLD		(1 << 1)
> -#	define	INTR_DP_HI		(1 << 2)
> -#	define	INTR_ID_GND		(1 << 3)
> -#	define	INTR_DM_HI		(1 << 4)
> -#	define	INTR_ID_FLOAT		(1 << 5)
> -#	define	INTR_BDIS_ACON		(1 << 6)
> -#	define	INTR_CR_INT		(1 << 7)
> -
> -/*-------------------------------------------------------------------------*/
> -
>  static const char *state_string(enum usb_otg_state state)
>  {
>  	switch (state) {
> @@ -1190,9 +1116,7 @@ static void isp1301_timer(unsigned long _isp)
>  
>  static void isp1301_release(struct device *dev)
>  {
> -	struct isp1301	*isp;
> -
> -	isp = container_of(dev, struct isp1301, c.dev);
> +	struct isp1301		*isp = dev_get_drvdata(dev);
>  
>  	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
>  	if (isp->i2c_release)
> @@ -1202,30 +1126,27 @@ static void isp1301_release(struct device *dev)
>  
>  static struct isp1301 *the_transceiver;
>  
> -static int isp1301_detach_client(struct i2c_client *i2c)
> +static int __exit isp1301_remove(struct i2c_client *client)
>  {
> -	struct isp1301	*isp;
> -
> -	isp = container_of(i2c, struct isp1301, c);
> +	struct isp1301	*isp = i2c_get_clientdata(client);
>  
>  	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
>  	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
> -	free_irq(isp->irq, isp);
> +
> +	if (client->irq > 0)
> +		free_irq(client->irq, isp);
>  #ifdef	CONFIG_USB_OTG
>  	otg_unbind(isp);
>  #endif
> -	if (machine_is_omap_h2())
> -		omap_free_gpio(2);
> -
>  	isp->timer.data = 0;
>  	set_bit(WORK_STOP, &isp->todo);
>  	del_timer_sync(&isp->timer);
>  	flush_scheduled_work();
>  
> -	put_device(&i2c->dev);
> +	put_device(&client->dev);
>  	the_transceiver = 0;
>  
> -	return i2c_detach_client(i2c);
> +	return 0;
>  }
>  
>  /*-------------------------------------------------------------------------*/
> @@ -1295,9 +1216,6 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
>  
>  	power_up(isp);
>  
> -	if (machine_is_omap_h2())
> -		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
> -
>  	dev_info(&isp->client->dev, "A-Host sessions ok\n");
>  	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
>  		INTR_ID_GND);
> @@ -1475,11 +1393,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
>  /*-------------------------------------------------------------------------*/
>  
>  /* no error returns, they'd just make bus scanning stop */
> -static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
> +static int __init isp1301_probe(struct i2c_client *client)
>  {
>  	int			status;
>  	struct isp1301		*isp;
> -	struct i2c_client	*i2c;
>  
>  	if (the_transceiver)
>  		return 0;
> @@ -1492,45 +1409,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
>  	init_timer(&isp->timer);
>  	isp->timer.function = isp1301_timer;
>  	isp->timer.data = (unsigned long) isp;
> -
> -	isp->irq = -1;
> -	isp->c.addr = address;
> -	i2c_set_clientdata(&isp->c, isp);
> -	isp->c.adapter = bus;
> -	isp->c.driver = &isp1301_driver;
> -	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
> -	isp->client = i2c = &isp->c;
> +	isp->client = client;
>  
>  	/* if this is a true probe, verify the chip ... */
> -	if (kind < 0) {
> -		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
> -		if (status != I2C_VENDOR_ID_PHILIPS) {
> -			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
> -				address, status);
> -			goto fail1;
> -		}
> -		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
> -		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
> -			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
> -				address, status);
> -			goto fail1;
> -		}
> +	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
> +	if (status != I2C_VENDOR_ID_PHILIPS) {
> +		dev_dbg(&client->dev, "not philips id: %d\n", status);
> +		goto fail1;
> +	}
> +	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
> +	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
> +		dev_dbg(&client->dev, "not isp1301, %d\n", status);
> +		goto fail1;
>  	}
>  
> -	status = i2c_attach_client(i2c);
> -	if (status < 0) {
> -		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
> -				DRIVER_NAME, address, status);
>  fail1:
>  		kfree(isp);
>  		return 0;
> -	}
> -	isp->i2c_release = i2c->dev.release;
> -	i2c->dev.release = isp1301_release;
> +
> +	isp->i2c_release = client->dev.release = isp1301_release;
>  
>  	/* initial development used chiprev 2.00 */
> -	status = i2c_smbus_read_word_data(i2c, ISP1301_BCD_DEVICE);
> -	dev_info(&i2c->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
> +	status = i2c_smbus_read_word_data(client, ISP1301_BCD_DEVICE);
> +	dev_info(&client->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
>  		status >> 8, status & 0xff);
>  
>  	/* make like power-on reset */
> @@ -1551,40 +1452,25 @@ fail1:
>  #ifdef	CONFIG_USB_OTG
>  	status = otg_bind(isp);
>  	if (status < 0) {
> -		dev_dbg(&i2c->dev, "can't bind OTG\n");
> +		dev_dbg(&client->dev, "can't bind OTG\n");
>  		goto fail2;
>  	}
>  #endif
>  
> -	if (machine_is_omap_h2()) {
> -		/* full speed signaling by default */
> -		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
> -			MC1_SPEED_REG);
> -		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
> -			MC2_SPD_SUSP_CTRL);
> -
> -		/* IRQ wired at M14 */
> -		omap_cfg_reg(M14_1510_GPIO2);
> -		isp->irq = OMAP_GPIO_IRQ(2);
> -		if (gpio_request(2, "isp1301") == 0)
> -			gpio_direction_input(2);
> -		isp->irq_type = IRQF_TRIGGER_FALLING;
> -	}
> -
> -	isp->irq_type |= IRQF_SAMPLE_RANDOM;
> -	status = request_irq(isp->irq, isp1301_irq,
> -			isp->irq_type, DRIVER_NAME, isp);
> -	if (status < 0) {
> -		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
> -				isp->irq, status);
> +	if (client->irq > 0) {
> +		status = request_irq(client->irq, isp1301_irq,
> +				client->irq_flags, DRIVER_NAME, isp);
> +		if (status < 0) {
> +			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
> +					client->irq, status);
>  #ifdef	CONFIG_USB_OTG
>  fail2:
>  #endif
> -		i2c_detach_client(i2c);
> -		goto fail1;
> +			goto fail1;
> +		}
>  	}
>  
> -	isp->otg.dev = &isp->client->dev;
> +	isp->otg.dev = &client->dev;
>  	isp->otg.label = DRIVER_NAME;
>  
>  	isp->otg.set_host = isp1301_set_host,
> @@ -1606,31 +1492,23 @@ fail2:
>  
>  #ifdef	VERBOSE
>  	mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
> -	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
> +	dev_dbg(&client->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
>  #endif
>  
>  	status = otg_set_transceiver(&isp->otg);
>  	if (status < 0)
> -		dev_err(&i2c->dev, "can't register transceiver, %d\n",
> +		dev_err(&client->dev, "can't register transceiver, %d\n",
>  			status);
>  
>  	return 0;
>  }
>  
> -static int isp1301_scan_bus(struct i2c_adapter *bus)
> -{
> -	if (!i2c_check_functionality(bus, I2C_FUNC_SMBUS_BYTE_DATA
> -			| I2C_FUNC_SMBUS_READ_WORD_DATA))
> -		return -EINVAL;
> -	return i2c_probe(bus, &addr_data, isp1301_probe);
> -}
> -
>  static struct i2c_driver isp1301_driver = {
>  	.driver = {
>  		.name	= "isp1301_omap",
>  	},
> -	.attach_adapter	= isp1301_scan_bus,
> -	.detach_client	= isp1301_detach_client,
> +	.probe	= isp1301_probe,
> +	.remove	= __exit_p(isp1301_remove),
>  };
>  
>  /*-------------------------------------------------------------------------*/
> diff --git a/include/linux/i2c/isp1301_omap.h b/include/linux/i2c/isp1301_omap.h
> new file mode 100644
> index 0000000..de47460
> --- /dev/null
> +++ b/include/linux/i2c/isp1301_omap.h
> @@ -0,0 +1,92 @@
> +/* include/linux/i2c/isp1301_omap.h
> + *
> + * Copyright (C) 2008 Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> + *
> + * 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 SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * 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.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#ifndef __LINUX_I2C_ISP1301_OMAP_H
> +#define __LINUX_I2C_ISP1301_OMAP_H
> +
> +/* identification */
> +#define	ISP1301_VENDOR_ID		0x00	/* u16 read */
> +#define	ISP1301_PRODUCT_ID		0x02	/* u16 read */
> +#define	ISP1301_BCD_DEVICE		0x14	/* u16 read */
> +
> +#define	I2C_VENDOR_ID_PHILIPS		0x04cc
> +#define	I2C_PRODUCT_ID_PHILIPS_1301	0x1301
> +
> +/* operational registers */
> +#define	ISP1301_MODE_CONTROL_1		0x04	/* u8 read, set, +1 clear */
> +#	define	MC1_SPEED_REG		(1 << 0)
> +#	define	MC1_SUSPEND_REG		(1 << 1)
> +#	define	MC1_DAT_SE0		(1 << 2)
> +#	define	MC1_TRANSPARENT		(1 << 3)
> +#	define	MC1_BDIS_ACON_EN	(1 << 4)
> +#	define	MC1_OE_INT_EN		(1 << 5)
> +#	define	MC1_UART_EN		(1 << 6)
> +#	define	MC1_MASK		0x7f
> +#define	ISP1301_MODE_CONTROL_2		0x12	/* u8 read, set, +1 clear */
> +#	define	MC2_GLOBAL_PWR_DN	(1 << 0)
> +#	define	MC2_SPD_SUSP_CTRL	(1 << 1)
> +#	define	MC2_BI_DI		(1 << 2)
> +#	define	MC2_TRANSP_BDIR0	(1 << 3)
> +#	define	MC2_TRANSP_BDIR1	(1 << 4)
> +#	define	MC2_AUDIO_EN		(1 << 5)
> +#	define	MC2_PSW_EN		(1 << 6)
> +#	define	MC2_EN2V7		(1 << 7)
> +#define	ISP1301_OTG_CONTROL_1		0x06	/* u8 read, set, +1 clear */
> +#	define	OTG1_DP_PULLUP		(1 << 0)
> +#	define	OTG1_DM_PULLUP		(1 << 1)
> +#	define	OTG1_DP_PULLDOWN	(1 << 2)
> +#	define	OTG1_DM_PULLDOWN	(1 << 3)
> +#	define	OTG1_ID_PULLDOWN	(1 << 4)
> +#	define	OTG1_VBUS_DRV		(1 << 5)
> +#	define	OTG1_VBUS_DISCHRG	(1 << 6)
> +#	define	OTG1_VBUS_CHRG		(1 << 7)
> +#define	ISP1301_OTG_STATUS		0x10	/* u8 readonly */
> +#	define	OTG_B_SESS_END		(1 << 6)
> +#	define	OTG_B_SESS_VLD		(1 << 7)
> +
> +#define	ISP1301_INTERRUPT_SOURCE	0x08	/* u8 read */
> +#define	ISP1301_INTERRUPT_LATCH		0x0A	/* u8 read, set, +1 clear */
> +
> +#define	ISP1301_INTERRUPT_FALLING	0x0C	/* u8 read, set, +1 clear */
> +#define	ISP1301_INTERRUPT_RISING	0x0E	/* u8 read, set, +1 clear */
> +
> +/* same bitfields in all interrupt registers */
> +#	define	INTR_VBUS_VLD		(1 << 0)
> +#	define	INTR_SESS_VLD		(1 << 1)
> +#	define	INTR_DP_HI		(1 << 2)
> +#	define	INTR_ID_GND		(1 << 3)
> +#	define	INTR_DM_HI		(1 << 4)
> +#	define	INTR_ID_FLOAT		(1 << 5)
> +#	define	INTR_BDIS_ACON		(1 << 6)
> +#	define	INTR_CR_INT		(1 << 7)
> +
> +struct isp1301_platform_data {
> +	unsigned	irqflags;
> +	unsigned	bit;
> +
> +};
> +
> +#endif /*  __LINUX_I2C_ISP1301_OMAP_H */
> +
> -- 
> 1.5.4.3.447.gc95b3
> 

-- 
Best Regards,

Felipe Balbi
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org
http://blog.felipebalbi.com

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

Now it's ok.
patch inline.

======================== CUT HERE =====================

>From 779861bf9c1efdf7eec7da541ce0d2771be6e620 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
Date: Tue, 1 Jan 2008 23:00:18 -0500
Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

Based on David Brownell's patch for tps65010, this patch
finish conversting isp1301_omap.c to new-style i2c driver.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 arch/arm/mach-omap1/board-h2.c   |    1 +
 arch/arm/mach-omap1/board-h3.c   |    6 ++
 arch/arm/mach-omap2/board-h4.c   |   12 +++
 drivers/i2c/chips/isp1301_omap.c |  140 ++++++++++----------------------------
 4 files changed, 56 insertions(+), 103 deletions(-)

diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 5079877..81a8c87 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -357,6 +357,7 @@ static struct i2c_board_info __initdata h2_i2c_board_info[] = {
 		I2C_BOARD_INFO("isp1301_omap", 0x2d),
 		.type		= "isp1301_omap",
 		.irq		= OMAP_GPIO_IRQ(2),
+		.irq_flags	= IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING,
 	},
 };
 
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index c3ef1ee..474921f 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -477,6 +477,12 @@ static struct i2c_board_info __initdata h3_i2c_board_info[] = {
                .type           = "tps65013",
                /* .irq         = OMAP_GPIO_IRQ(??), */
        },
+       {
+               I2C_BOARD_INFO("isp1301_omap", 0x2d),
+               .type		= "isp1301_omap",
+               .irq		= OMAP_GPIO_IRQ(14),
+	       .irq_flags	= IRQF_SAMPLE_RANDOM,
+       },
 };
 
 static struct omap_gpio_switch h3_gpio_switches[] __initdata = {
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index f125f43..6e57021 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -301,6 +301,15 @@ static struct omap_board_config_kernel h4_config[] = {
 	{ OMAP_TAG_LCD,		&h4_lcd_config },
 };
 
+static struct i2c_board_info __initdata h4_i2c_board_info[] = {
+	{
+		I2C_BOARD_INFO("isp1301_omap", 0x2d),
+		.type		= "isp1301_omap",
+		.irq		= OMAP_GPIO_IRQ(125),
+		.irq_flags	= IRQF_SAMPLE_RANDOM,
+	},
+};
+
 static void __init omap_h4_init(void)
 {
 	/*
@@ -321,6 +330,9 @@ static void __init omap_h4_init(void)
 	}
 #endif
 
+	i2c_register_board_info(1, h4_i2c_board_info,
+			ARRAY_SIZE(h4_i2c_board_info));
+
 	platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
 	omap_board_config = h4_config;
 	omap_board_config_size = ARRAY_SIZE(h4_config);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index d094b99..0f5a2ae 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -25,17 +25,10 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
-#include <linux/usb/ch9.h>
-#include <linux/usb/gadget.h>
-#include <linux/usb.h>
-#include <linux/usb/otg.h>
 #include <linux/i2c.h>
 #include <linux/workqueue.h>
-
-#include <asm/irq.h>
 #include <asm/arch/usb.h>
 
-
 #ifndef	DEBUG
 #undef	VERBOSE
 #endif
@@ -50,12 +43,8 @@ MODULE_LICENSE("GPL");
 struct isp1301 {
 	struct otg_transceiver	otg;
 	struct i2c_client	*client;
-	struct i2c_client	c;
 	void			(*i2c_release)(struct device *dev);
 
-	int			irq;
-	int			irq_type;
-
 	u32			last_otg_ctrl;
 	unsigned		working:1;
 
@@ -140,13 +129,6 @@ static inline void notresponding(struct isp1301 *isp)
 /*-------------------------------------------------------------------------*/
 
 /* only two addresses possible */
-#define	ISP_BASE		0x2c
-static unsigned short normal_i2c[] = {
-	ISP_BASE, ISP_BASE + 1,
-	I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
 static struct i2c_driver isp1301_driver;
 
 /* smbus apis are used for portability */
@@ -1190,9 +1172,7 @@ static void isp1301_timer(unsigned long _isp)
 
 static void isp1301_release(struct device *dev)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(dev, struct isp1301, c.dev);
+	struct isp1301		*isp = dev_get_drvdata(dev);
 
 	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
 	if (isp->i2c_release)
@@ -1202,30 +1182,27 @@ static void isp1301_release(struct device *dev)
 
 static struct isp1301 *the_transceiver;
 
-static int isp1301_detach_client(struct i2c_client *i2c)
+static int __exit isp1301_remove(struct i2c_client *client)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(i2c, struct isp1301, c);
+	struct isp1301	*isp = i2c_get_clientdata(client);
 
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
-	free_irq(isp->irq, isp);
+
+	if (client->irq > 0)
+		free_irq(client->irq, isp);
 #ifdef	CONFIG_USB_OTG
 	otg_unbind(isp);
 #endif
-	if (machine_is_omap_h2())
-		omap_free_gpio(2);
-
 	isp->timer.data = 0;
 	set_bit(WORK_STOP, &isp->todo);
 	del_timer_sync(&isp->timer);
 	flush_scheduled_work();
 
-	put_device(&i2c->dev);
+	put_device(&client->dev);
 	the_transceiver = 0;
 
-	return i2c_detach_client(i2c);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1295,9 +1272,6 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 
 	power_up(isp);
 
-	if (machine_is_omap_h2())
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
-
 	dev_info(&isp->client->dev, "A-Host sessions ok\n");
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
 		INTR_ID_GND);
@@ -1475,11 +1449,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
 /*-------------------------------------------------------------------------*/
 
 /* no error returns, they'd just make bus scanning stop */
-static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
+static int __init isp1301_probe(struct i2c_client *client)
 {
 	int			status;
 	struct isp1301		*isp;
-	struct i2c_client	*i2c;
 
 	if (the_transceiver)
 		return 0;
@@ -1492,45 +1465,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
 	init_timer(&isp->timer);
 	isp->timer.function = isp1301_timer;
 	isp->timer.data = (unsigned long) isp;
-
-	isp->irq = -1;
-	isp->c.addr = address;
-	i2c_set_clientdata(&isp->c, isp);
-	isp->c.adapter = bus;
-	isp->c.driver = &isp1301_driver;
-	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
-	isp->client = i2c = &isp->c;
+	isp->client = client;
 
 	/* if this is a true probe, verify the chip ... */
-	if (kind < 0) {
-		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
-		if (status != I2C_VENDOR_ID_PHILIPS) {
-			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
-				address, status);
-			goto fail1;
-		}
-		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
-		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
-			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
-				address, status);
-			goto fail1;
-		}
+	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
+	if (status != I2C_VENDOR_ID_PHILIPS) {
+		dev_dbg(&client->dev, "not philips id: %d\n", status);
+		goto fail1;
+	}
+	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
+	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
+		dev_dbg(&client->dev, "not isp1301, %d\n", status);
+		goto fail1;
 	}
 
-	status = i2c_attach_client(i2c);
-	if (status < 0) {
-		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
-				DRIVER_NAME, address, status);
 fail1:
 		kfree(isp);
 		return 0;
-	}
-	isp->i2c_release = i2c->dev.release;
-	i2c->dev.release = isp1301_release;
+
+	isp->i2c_release = client->dev.release = isp1301_release;
 
 	/* initial development used chiprev 2.00 */
-	status = i2c_smbus_read_word_data(i2c, ISP1301_BCD_DEVICE);
-	dev_info(&i2c->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
+	status = i2c_smbus_read_word_data(client, ISP1301_BCD_DEVICE);
+	dev_info(&client->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
 		status >> 8, status & 0xff);
 
 	/* make like power-on reset */
@@ -1551,40 +1508,25 @@ fail1:
 #ifdef	CONFIG_USB_OTG
 	status = otg_bind(isp);
 	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't bind OTG\n");
+		dev_dbg(&client->dev, "can't bind OTG\n");
 		goto fail2;
 	}
 #endif
 
-	if (machine_is_omap_h2()) {
-		/* full speed signaling by default */
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
-			MC1_SPEED_REG);
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
-			MC2_SPD_SUSP_CTRL);
-
-		/* IRQ wired at M14 */
-		omap_cfg_reg(M14_1510_GPIO2);
-		isp->irq = OMAP_GPIO_IRQ(2);
-		if (gpio_request(2, "isp1301") == 0)
-			gpio_direction_input(2);
-		isp->irq_type = IRQF_TRIGGER_FALLING;
-	}
-
-	isp->irq_type |= IRQF_SAMPLE_RANDOM;
-	status = request_irq(isp->irq, isp1301_irq,
-			isp->irq_type, DRIVER_NAME, isp);
-	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
-				isp->irq, status);
+	if (client->irq > 0) {
+		status = request_irq(client->irq, isp1301_irq,
+				client->irq_flags, DRIVER_NAME, isp);
+		if (status < 0) {
+			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
+					client->irq, status);
 #ifdef	CONFIG_USB_OTG
 fail2:
 #endif
-		i2c_detach_client(i2c);
-		goto fail1;
+			goto fail1;
+		}
 	}
 
-	isp->otg.dev = &isp->client->dev;
+	isp->otg.dev = &client->dev;
 	isp->otg.label = DRIVER_NAME;
 
 	isp->otg.set_host = isp1301_set_host,
@@ -1606,31 +1548,23 @@ fail2:
 
 #ifdef	VERBOSE
 	mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
-	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
+	dev_dbg(&client->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
 #endif
 
 	status = otg_set_transceiver(&isp->otg);
 	if (status < 0)
-		dev_err(&i2c->dev, "can't register transceiver, %d\n",
+		dev_err(&client->dev, "can't register transceiver, %d\n",
 			status);
 
 	return 0;
 }
 
-static int isp1301_scan_bus(struct i2c_adapter *bus)
-{
-	if (!i2c_check_functionality(bus, I2C_FUNC_SMBUS_BYTE_DATA
-			| I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EINVAL;
-	return i2c_probe(bus, &addr_data, isp1301_probe);
-}
-
 static struct i2c_driver isp1301_driver = {
 	.driver = {
 		.name	= "isp1301_omap",
 	},
-	.attach_adapter	= isp1301_scan_bus,
-	.detach_client	= isp1301_detach_client,
+	.probe	= isp1301_probe,
+	.remove	= __exit_p(isp1301_remove),
 };
 
 /*-------------------------------------------------------------------------*/
-- 
1.5.4.3.447.gc95b3

-- 
Best Regards,

Felipe Balbi
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org
http://blog.felipebalbi.com

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

On Sat, Mar 15, 2008 at 02:54:59PM +0200, Felipe Balbi wrote:
> Now it's ok.
> patch inline.

Ugh, missing interrupt.h

============== CUT HERE ==============

>From 6018acbb287b6f72874bfe6588541eba3b1f919b Mon Sep 17 00:00:00 2001
From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
Date: Tue, 1 Jan 2008 23:00:18 -0500
Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

Based on David Brownell's patch for tps65010, this patch
finish conversting isp1301_omap.c to new-style i2c driver.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 arch/arm/mach-omap1/board-h2.c   |    2 +
 arch/arm/mach-omap1/board-h3.c   |    7 ++
 arch/arm/mach-omap2/board-h4.c   |   13 ++++
 drivers/i2c/chips/isp1301_omap.c |  135 ++++++++++---------------------------
 4 files changed, 59 insertions(+), 98 deletions(-)

diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 5079877..7dec922 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
@@ -357,6 +358,7 @@ static struct i2c_board_info __initdata h2_i2c_board_info[] = {
 		I2C_BOARD_INFO("isp1301_omap", 0x2d),
 		.type		= "isp1301_omap",
 		.irq		= OMAP_GPIO_IRQ(2),
+		.irq_flags	= IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING,
 	},
 };
 
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index c3ef1ee..6b785b8 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -19,6 +19,7 @@
 #include <linux/major.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/interrupt.h>
 #include <linux/errno.h>
 #include <linux/workqueue.h>
 #include <linux/i2c.h>
@@ -477,6 +478,12 @@ static struct i2c_board_info __initdata h3_i2c_board_info[] = {
                .type           = "tps65013",
                /* .irq         = OMAP_GPIO_IRQ(??), */
        },
+       {
+               I2C_BOARD_INFO("isp1301_omap", 0x2d),
+               .type		= "isp1301_omap",
+               .irq		= OMAP_GPIO_IRQ(14),
+	       .irq_flags	= IRQF_SAMPLE_RANDOM,
+       },
 };
 
 static struct omap_gpio_switch h3_gpio_switches[] __initdata = {
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index f125f43..b43347c 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/interrupt.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/delay.h>
@@ -301,6 +302,15 @@ static struct omap_board_config_kernel h4_config[] = {
 	{ OMAP_TAG_LCD,		&h4_lcd_config },
 };
 
+static struct i2c_board_info __initdata h4_i2c_board_info[] = {
+	{
+		I2C_BOARD_INFO("isp1301_omap", 0x2d),
+		.type		= "isp1301_omap",
+		.irq		= OMAP_GPIO_IRQ(125),
+		.irq_flags	= IRQF_SAMPLE_RANDOM,
+	},
+};
+
 static void __init omap_h4_init(void)
 {
 	/*
@@ -321,6 +331,9 @@ static void __init omap_h4_init(void)
 	}
 #endif
 
+	i2c_register_board_info(1, h4_i2c_board_info,
+			ARRAY_SIZE(h4_i2c_board_info));
+
 	platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
 	omap_board_config = h4_config;
 	omap_board_config_size = ARRAY_SIZE(h4_config);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index d094b99..f79e7de 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -31,8 +31,6 @@
 #include <linux/usb/otg.h>
 #include <linux/i2c.h>
 #include <linux/workqueue.h>
-
-#include <asm/irq.h>
 #include <asm/arch/usb.h>
 
 
@@ -50,12 +48,8 @@ MODULE_LICENSE("GPL");
 struct isp1301 {
 	struct otg_transceiver	otg;
 	struct i2c_client	*client;
-	struct i2c_client	c;
 	void			(*i2c_release)(struct device *dev);
 
-	int			irq;
-	int			irq_type;
-
 	u32			last_otg_ctrl;
 	unsigned		working:1;
 
@@ -140,13 +134,6 @@ static inline void notresponding(struct isp1301 *isp)
 /*-------------------------------------------------------------------------*/
 
 /* only two addresses possible */
-#define	ISP_BASE		0x2c
-static unsigned short normal_i2c[] = {
-	ISP_BASE, ISP_BASE + 1,
-	I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
 static struct i2c_driver isp1301_driver;
 
 /* smbus apis are used for portability */
@@ -1190,9 +1177,7 @@ static void isp1301_timer(unsigned long _isp)
 
 static void isp1301_release(struct device *dev)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(dev, struct isp1301, c.dev);
+	struct isp1301		*isp = dev_get_drvdata(dev);
 
 	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
 	if (isp->i2c_release)
@@ -1202,30 +1187,27 @@ static void isp1301_release(struct device *dev)
 
 static struct isp1301 *the_transceiver;
 
-static int isp1301_detach_client(struct i2c_client *i2c)
+static int __exit isp1301_remove(struct i2c_client *client)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(i2c, struct isp1301, c);
+	struct isp1301	*isp = i2c_get_clientdata(client);
 
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
-	free_irq(isp->irq, isp);
+
+	if (client->irq > 0)
+		free_irq(client->irq, isp);
 #ifdef	CONFIG_USB_OTG
 	otg_unbind(isp);
 #endif
-	if (machine_is_omap_h2())
-		omap_free_gpio(2);
-
 	isp->timer.data = 0;
 	set_bit(WORK_STOP, &isp->todo);
 	del_timer_sync(&isp->timer);
 	flush_scheduled_work();
 
-	put_device(&i2c->dev);
+	put_device(&client->dev);
 	the_transceiver = 0;
 
-	return i2c_detach_client(i2c);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1295,9 +1277,6 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 
 	power_up(isp);
 
-	if (machine_is_omap_h2())
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
-
 	dev_info(&isp->client->dev, "A-Host sessions ok\n");
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
 		INTR_ID_GND);
@@ -1475,11 +1454,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
 /*-------------------------------------------------------------------------*/
 
 /* no error returns, they'd just make bus scanning stop */
-static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
+static int __init isp1301_probe(struct i2c_client *client)
 {
 	int			status;
 	struct isp1301		*isp;
-	struct i2c_client	*i2c;
 
 	if (the_transceiver)
 		return 0;
@@ -1492,45 +1470,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
 	init_timer(&isp->timer);
 	isp->timer.function = isp1301_timer;
 	isp->timer.data = (unsigned long) isp;
-
-	isp->irq = -1;
-	isp->c.addr = address;
-	i2c_set_clientdata(&isp->c, isp);
-	isp->c.adapter = bus;
-	isp->c.driver = &isp1301_driver;
-	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
-	isp->client = i2c = &isp->c;
+	isp->client = client;
 
 	/* if this is a true probe, verify the chip ... */
-	if (kind < 0) {
-		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
-		if (status != I2C_VENDOR_ID_PHILIPS) {
-			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
-				address, status);
-			goto fail1;
-		}
-		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
-		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
-			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
-				address, status);
-			goto fail1;
-		}
+	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
+	if (status != I2C_VENDOR_ID_PHILIPS) {
+		dev_dbg(&client->dev, "not philips id: %d\n", status);
+		goto fail1;
+	}
+	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
+	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
+		dev_dbg(&client->dev, "not isp1301, %d\n", status);
+		goto fail1;
 	}
 
-	status = i2c_attach_client(i2c);
-	if (status < 0) {
-		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
-				DRIVER_NAME, address, status);
 fail1:
 		kfree(isp);
 		return 0;
-	}
-	isp->i2c_release = i2c->dev.release;
-	i2c->dev.release = isp1301_release;
+
+	isp->i2c_release = client->dev.release = isp1301_release;
 
 	/* initial development used chiprev 2.00 */
-	status = i2c_smbus_read_word_data(i2c, ISP1301_BCD_DEVICE);
-	dev_info(&i2c->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
+	status = i2c_smbus_read_word_data(client, ISP1301_BCD_DEVICE);
+	dev_info(&client->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
 		status >> 8, status & 0xff);
 
 	/* make like power-on reset */
@@ -1551,40 +1513,25 @@ fail1:
 #ifdef	CONFIG_USB_OTG
 	status = otg_bind(isp);
 	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't bind OTG\n");
+		dev_dbg(&client->dev, "can't bind OTG\n");
 		goto fail2;
 	}
 #endif
 
-	if (machine_is_omap_h2()) {
-		/* full speed signaling by default */
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
-			MC1_SPEED_REG);
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
-			MC2_SPD_SUSP_CTRL);
-
-		/* IRQ wired at M14 */
-		omap_cfg_reg(M14_1510_GPIO2);
-		isp->irq = OMAP_GPIO_IRQ(2);
-		if (gpio_request(2, "isp1301") == 0)
-			gpio_direction_input(2);
-		isp->irq_type = IRQF_TRIGGER_FALLING;
-	}
-
-	isp->irq_type |= IRQF_SAMPLE_RANDOM;
-	status = request_irq(isp->irq, isp1301_irq,
-			isp->irq_type, DRIVER_NAME, isp);
-	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
-				isp->irq, status);
+	if (client->irq > 0) {
+		status = request_irq(client->irq, isp1301_irq,
+				client->irq_flags, DRIVER_NAME, isp);
+		if (status < 0) {
+			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
+					client->irq, status);
 #ifdef	CONFIG_USB_OTG
 fail2:
 #endif
-		i2c_detach_client(i2c);
-		goto fail1;
+			goto fail1;
+		}
 	}
 
-	isp->otg.dev = &isp->client->dev;
+	isp->otg.dev = &client->dev;
 	isp->otg.label = DRIVER_NAME;
 
 	isp->otg.set_host = isp1301_set_host,
@@ -1606,31 +1553,23 @@ fail2:
 
 #ifdef	VERBOSE
 	mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
-	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
+	dev_dbg(&client->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
 #endif
 
 	status = otg_set_transceiver(&isp->otg);
 	if (status < 0)
-		dev_err(&i2c->dev, "can't register transceiver, %d\n",
+		dev_err(&client->dev, "can't register transceiver, %d\n",
 			status);
 
 	return 0;
 }
 
-static int isp1301_scan_bus(struct i2c_adapter *bus)
-{
-	if (!i2c_check_functionality(bus, I2C_FUNC_SMBUS_BYTE_DATA
-			| I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EINVAL;
-	return i2c_probe(bus, &addr_data, isp1301_probe);
-}
-
 static struct i2c_driver isp1301_driver = {
 	.driver = {
 		.name	= "isp1301_omap",
 	},
-	.attach_adapter	= isp1301_scan_bus,
-	.detach_client	= isp1301_detach_client,
+	.probe	= isp1301_probe,
+	.remove	= __exit_p(isp1301_remove),
 };
 
 /*-------------------------------------------------------------------------*/
-- 
1.5.4.3.447.gc95b3




-- 
Best Regards,

Felipe Balbi
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org
http://blog.felipebalbi.com

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[Jean Delvare]

On Sat, 15 Mar 2008 14:47:14 +0200, Felipe Balbi wrote:
> Hi Jean,
> 
> I have a new approach for the isp1301 driver, took your idea
> of adding a new irq_flags member to i2c_boardinfo and i2c_client.

I said we could do this _if_ many drivers needed this field. I didn't
mean to do it right now... So far I only know of the isp1301 driver
which would need this. And at any rate I'd like to hear David's opinion
before adding a new field to the core i2c structures.

> If it's ok for you, i'll also send a patch putting linux-omap and
> linux-mailine isp1301 in sync.

This would be great, yes. Divergence between trees tend to confuse
developers, often I receive patches which do not apply to my tree
because of this.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[Felipe Balbi]

On Sat, Mar 15, 2008 at 5:28 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> On Sat, 15 Mar 2008 14:47:14 +0200, Felipe Balbi wrote:
>  > Hi Jean,
>  >
>  > I have a new approach for the isp1301 driver, took your idea
>  > of adding a new irq_flags member to i2c_boardinfo and i2c_client.
>
>  I said we could do this _if_ many drivers needed this field. I didn't
>  mean to do it right now... So far I only know of the isp1301 driver
>  which would need this. And at any rate I'd like to hear David's opinion
>  before adding a new field to the core i2c structures.

Ok, so I'll prepare other patch based on addind include/i2c/isp1301_omap.h with
struct isp1301_platform_data.

But still, it makes really much sense adding this field cuz the
probability of having different irq flags on different boards is quite
high. Also, a driver shouldn't be conditional to an architecture. I
mean, isp1301 can be used with any arch, not only with omap. So as
much as we can make it platform independent as better driver we get.

>  > If it's ok for you, i'll also send a patch putting linux-omap and
>  > linux-mailine isp1301 in sync.
>
>  This would be great, yes. Divergence between trees tend to confuse
>  developers, often I receive patches which do not apply to my tree
>  because of this.

I already have such patch for both linux-omap and linux-mainline, but
it's based on adding irq_flags to i2c_boardinfo and i2c_client. So
let's wait Dave's comments ;-)

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[Felipe Balbi]

Fixing Dave's email. It was mis-spelled earlier.

On Sat, Mar 15, 2008 at 5:38 PM, Felipe Balbi
<felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> wrote:
> On Sat, Mar 15, 2008 at 5:28 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
>  > On Sat, 15 Mar 2008 14:47:14 +0200, Felipe Balbi wrote:
>  >  > Hi Jean,
>  >  >
>  >  > I have a new approach for the isp1301 driver, took your idea
>  >  > of adding a new irq_flags member to i2c_boardinfo and i2c_client.
>  >
>  >  I said we could do this _if_ many drivers needed this field. I didn't
>  >  mean to do it right now... So far I only know of the isp1301 driver
>  >  which would need this. And at any rate I'd like to hear David's opinion
>  >  before adding a new field to the core i2c structures.
>
>  Ok, so I'll prepare other patch based on addind include/i2c/isp1301_omap.h with
>  struct isp1301_platform_data.
>
>  But still, it makes really much sense adding this field cuz the
>  probability of having different irq flags on different boards is quite
>  high. Also, a driver shouldn't be conditional to an architecture. I
>  mean, isp1301 can be used with any arch, not only with omap. So as
>  much as we can make it platform independent as better driver we get.
>
>
>  >  > If it's ok for you, i'll also send a patch putting linux-omap and
>  >  > linux-mailine isp1301 in sync.
>  >
>  >  This would be great, yes. Divergence between trees tend to confuse
>  >  developers, often I receive patches which do not apply to my tree
>  >  because of this.
>
>  I already have such patch for both linux-omap and linux-mainline, but
>  it's based on adding irq_flags to i2c_boardinfo and i2c_client. So
>  let's wait Dave's comments ;-)
>
>  --
>  Best Regards,
>
>  Felipe Balbi
>  felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>



-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[Jean Delvare]

On Sat, Mar 15, 2008 at 5:38 PM, Felipe Balbi
<felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> wrote:
> On Sat, Mar 15, 2008 at 5:28 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
>  > On Sat, 15 Mar 2008 14:47:14 +0200, Felipe Balbi wrote:
>  >  > Hi Jean,
>  >  >
>  >  > I have a new approach for the isp1301 driver, took your idea
>  >  > of adding a new irq_flags member to i2c_boardinfo and i2c_client.
>  >
>  >  I said we could do this _if_ many drivers needed this field. I didn't
>  >  mean to do it right now... So far I only know of the isp1301 driver
>  >  which would need this. And at any rate I'd like to hear David's opinion
>  >  before adding a new field to the core i2c structures.
>
>  Ok, so I'll prepare other patch based on addind include/i2c/isp1301_omap.h with
>  struct isp1301_platform_data.

Would be better, yes. If we finally decide to add an irq_flags field to
the i2c core structures, we'll update the isp1301 driver accordingly.

>  But still, it makes really much sense adding this field cuz the
>  probability of having different irq flags on different boards is quite
>  high. Also, a driver shouldn't be conditional to an architecture. I
>  mean, isp1301 can be used with any arch, not only with omap. So as
>  much as we can make it platform independent as better driver we get.

I agree that it would be great if the isp1301 driver was arch-neutral.
But I fail to see how this is related to adding an irq_flags field to
struct i2c_board_info and struct i2c_client or not.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[Felipe Balbi]

On Sat, Mar 15, 2008 at 6:47 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> On Sat, Mar 15, 2008 at 5:38 PM, Felipe Balbi
>  <felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> wrote:
>  > On Sat, Mar 15, 2008 at 5:28 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
>  >  > On Sat, 15 Mar 2008 14:47:14 +0200, Felipe Balbi wrote:
>  >  >  > Hi Jean,
>  >  >  >
>  >  >  > I have a new approach for the isp1301 driver, took your idea
>  >  >  > of adding a new irq_flags member to i2c_boardinfo and i2c_client.
>  >  >
>  >  >  I said we could do this _if_ many drivers needed this field. I didn't
>  >  >  mean to do it right now... So far I only know of the isp1301 driver
>  >  >  which would need this. And at any rate I'd like to hear David's opinion
>  >  >  before adding a new field to the core i2c structures.
>  >
>  >  Ok, so I'll prepare other patch based on addind include/i2c/isp1301_omap.h with
>  >  struct isp1301_platform_data.
>
>  Would be better, yes. If we finally decide to add an irq_flags field to
>  the i2c core structures, we'll update the isp1301 driver accordingly.

Ok, tomorrow probably i won't be able to work on that, but I'll update
asap and keep the other patches in a separate branch ;-)

>  >  But still, it makes really much sense adding this field cuz the
>  >  probability of having different irq flags on different boards is quite
>  >  high. Also, a driver shouldn't be conditional to an architecture. I
>  >  mean, isp1301 can be used with any arch, not only with omap. So as
>  >  much as we can make it platform independent as better driver we get.
>
>  I agree that it would be great if the isp1301 driver was arch-neutral.
>  But I fail to see how this is related to adding an irq_flags field to
>  struct i2c_board_info and struct i2c_client or not.

We're gonna have to keep adding arch-conditional code on that, if the
code were arch neutral a simple i2c_boardinfo and
i2c_register_boardinfo(); would be enough for the driver to work on
the new arch (possibly); otherwise we still have to keep "hacking"
isp1301 driver to add more arch-conditional code mostly cuz of
irq_flags (and some other bits we have to set on different arch, but
that makes sense going on a platform_data as well).

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATH 0/3] isp1301 changes

[David Brownell]

On Saturday 15 March 2008, Felipe Balbi wrote:
> >  high. Also, a driver shouldn't be conditional to an architecture. I
> >  mean, isp1301 can be used with any arch, not only with omap. So as
> >  much as we can make it platform independent as better driver we get.

Actually, *this* driver is specific to OMAP since it's got to
coordinate between an OMAP-specific OTG controller and the
more generic isp1301 chip.  So that's not a concern here.

It would be nice of course to have a clean split between the
isp1301 code and the OTG controller ... but I'm not sure how
useful that would be.  The ISP1301 may have effectively become
a CEA standard (yes?) for external full speed OTG transceivers,
but it's not like there have been a flood of Linux platforms
supporting it.

All the new platforms are going for high speed OTG, not full
speed ...

- Dave




_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

This patch uses set_irq_type as suggested by Dave.

============= CUT HERE =========

>From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
Date: Tue, 1 Jan 2008 23:00:18 -0500
Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

Based on David Brownell's patch for tps65010, this patch
finish conversting isp1301_omap.c to new-style i2c driver.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 arch/arm/mach-omap1/board-h2.c   |    4 +
 arch/arm/mach-omap1/board-h3.c   |    8 ++
 arch/arm/mach-omap2/board-h4.c   |   14 ++++
 drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
 4 files changed, 62 insertions(+), 98 deletions(-)

diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 5079877..fd9567c 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -22,6 +22,8 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
@@ -366,6 +368,8 @@ static void __init h2_init_irq(void)
 	omap_init_irq();
 	omap_gpio_init();
 	h2_init_smc91x();
+	set_irq_type(OMAP_GPIO_IRQ(2),
+			IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING);
 }
 
 static struct omap_usb_config h2_usb_config __initdata = {
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index c3ef1ee..c1180b6 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -19,6 +19,8 @@
 #include <linux/major.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/errno.h>
 #include <linux/workqueue.h>
 #include <linux/i2c.h>
@@ -477,6 +479,11 @@ static struct i2c_board_info __initdata h3_i2c_board_info[] = {
                .type           = "tps65013",
                /* .irq         = OMAP_GPIO_IRQ(??), */
        },
+       {
+               I2C_BOARD_INFO("isp1301_omap", 0x2d),
+               .type		= "isp1301_omap",
+               .irq		= OMAP_GPIO_IRQ(14),
+       },
 };
 
 static struct omap_gpio_switch h3_gpio_switches[] __initdata = {
@@ -546,6 +553,7 @@ static void __init h3_init_irq(void)
 	omap_init_irq();
 	omap_gpio_init();
 	h3_init_smc91x();
+	set_irq_type(OMAP_GPIO_IRQ(14), IRQF_SAMPLE_RANDOM);
 }
 
 static void __init h3_map_io(void)
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index f125f43..cc815e6 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -14,6 +14,8 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/delay.h>
@@ -275,6 +277,7 @@ static void __init omap_h4_init_irq(void)
 	omap2_init_common_hw();
 	omap_init_irq();
 	omap_gpio_init();
+	set_irq_type(OMAP_GPIO_IRQ(125), IRQF_SAMPLE_RANDOM);
 }
 
 static struct omap_uart_config h4_uart_config __initdata = {
@@ -301,6 +304,14 @@ static struct omap_board_config_kernel h4_config[] = {
 	{ OMAP_TAG_LCD,		&h4_lcd_config },
 };
 
+static struct i2c_board_info __initdata h4_i2c_board_info[] = {
+	{
+		I2C_BOARD_INFO("isp1301_omap", 0x2d),
+		.type		= "isp1301_omap",
+		.irq		= OMAP_GPIO_IRQ(125),
+	},
+};
+
 static void __init omap_h4_init(void)
 {
 	/*
@@ -321,6 +332,9 @@ static void __init omap_h4_init(void)
 	}
 #endif
 
+	i2c_register_board_info(1, h4_i2c_board_info,
+			ARRAY_SIZE(h4_i2c_board_info));
+
 	platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
 	omap_board_config = h4_config;
 	omap_board_config_size = ARRAY_SIZE(h4_config);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index d094b99..9f2ccaf 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -31,8 +31,6 @@
 #include <linux/usb/otg.h>
 #include <linux/i2c.h>
 #include <linux/workqueue.h>
-
-#include <asm/irq.h>
 #include <asm/arch/usb.h>
 
 
@@ -50,12 +48,8 @@ MODULE_LICENSE("GPL");
 struct isp1301 {
 	struct otg_transceiver	otg;
 	struct i2c_client	*client;
-	struct i2c_client	c;
 	void			(*i2c_release)(struct device *dev);
 
-	int			irq;
-	int			irq_type;
-
 	u32			last_otg_ctrl;
 	unsigned		working:1;
 
@@ -140,13 +134,6 @@ static inline void notresponding(struct isp1301 *isp)
 /*-------------------------------------------------------------------------*/
 
 /* only two addresses possible */
-#define	ISP_BASE		0x2c
-static unsigned short normal_i2c[] = {
-	ISP_BASE, ISP_BASE + 1,
-	I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
 static struct i2c_driver isp1301_driver;
 
 /* smbus apis are used for portability */
@@ -1190,9 +1177,7 @@ static void isp1301_timer(unsigned long _isp)
 
 static void isp1301_release(struct device *dev)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(dev, struct isp1301, c.dev);
+	struct isp1301		*isp = dev_get_drvdata(dev);
 
 	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
 	if (isp->i2c_release)
@@ -1202,30 +1187,27 @@ static void isp1301_release(struct device *dev)
 
 static struct isp1301 *the_transceiver;
 
-static int isp1301_detach_client(struct i2c_client *i2c)
+static int __exit isp1301_remove(struct i2c_client *client)
 {
-	struct isp1301	*isp;
-
-	isp = container_of(i2c, struct isp1301, c);
+	struct isp1301	*isp = i2c_get_clientdata(client);
 
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
 	isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
-	free_irq(isp->irq, isp);
+
+	if (client->irq > 0)
+		free_irq(client->irq, isp);
 #ifdef	CONFIG_USB_OTG
 	otg_unbind(isp);
 #endif
-	if (machine_is_omap_h2())
-		omap_free_gpio(2);
-
 	isp->timer.data = 0;
 	set_bit(WORK_STOP, &isp->todo);
 	del_timer_sync(&isp->timer);
 	flush_scheduled_work();
 
-	put_device(&i2c->dev);
+	put_device(&client->dev);
 	the_transceiver = 0;
 
-	return i2c_detach_client(i2c);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1295,9 +1277,6 @@ isp1301_set_host(struct otg_transceiver *otg, struct usb_bus *host)
 
 	power_up(isp);
 
-	if (machine_is_omap_h2())
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
-
 	dev_info(&isp->client->dev, "A-Host sessions ok\n");
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
 		INTR_ID_GND);
@@ -1475,11 +1454,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
 /*-------------------------------------------------------------------------*/
 
 /* no error returns, they'd just make bus scanning stop */
-static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
+static int __init isp1301_probe(struct i2c_client *client)
 {
 	int			status;
 	struct isp1301		*isp;
-	struct i2c_client	*i2c;
 
 	if (the_transceiver)
 		return 0;
@@ -1492,45 +1470,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
 	init_timer(&isp->timer);
 	isp->timer.function = isp1301_timer;
 	isp->timer.data = (unsigned long) isp;
-
-	isp->irq = -1;
-	isp->c.addr = address;
-	i2c_set_clientdata(&isp->c, isp);
-	isp->c.adapter = bus;
-	isp->c.driver = &isp1301_driver;
-	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
-	isp->client = i2c = &isp->c;
+	isp->client = client;
 
 	/* if this is a true probe, verify the chip ... */
-	if (kind < 0) {
-		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
-		if (status != I2C_VENDOR_ID_PHILIPS) {
-			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
-				address, status);
-			goto fail1;
-		}
-		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
-		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
-			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
-				address, status);
-			goto fail1;
-		}
+	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
+	if (status != I2C_VENDOR_ID_PHILIPS) {
+		dev_dbg(&client->dev, "not philips id: %d\n", status);
+		goto fail1;
+	}
+	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
+	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
+		dev_dbg(&client->dev, "not isp1301, %d\n", status);
+		goto fail1;
 	}
 
-	status = i2c_attach_client(i2c);
-	if (status < 0) {
-		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
-				DRIVER_NAME, address, status);
 fail1:
 		kfree(isp);
 		return 0;
-	}
-	isp->i2c_release = i2c->dev.release;
-	i2c->dev.release = isp1301_release;
+
+	isp->i2c_release = client->dev.release = isp1301_release;
 
 	/* initial development used chiprev 2.00 */
-	status = i2c_smbus_read_word_data(i2c, ISP1301_BCD_DEVICE);
-	dev_info(&i2c->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
+	status = i2c_smbus_read_word_data(client, ISP1301_BCD_DEVICE);
+	dev_info(&client->dev, "chiprev %x.%02x, driver " DRIVER_VERSION "\n",
 		status >> 8, status & 0xff);
 
 	/* make like power-on reset */
@@ -1551,40 +1513,24 @@ fail1:
 #ifdef	CONFIG_USB_OTG
 	status = otg_bind(isp);
 	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't bind OTG\n");
+		dev_dbg(&client->dev, "can't bind OTG\n");
 		goto fail2;
 	}
 #endif
 
-	if (machine_is_omap_h2()) {
-		/* full speed signaling by default */
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
-			MC1_SPEED_REG);
-		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
-			MC2_SPD_SUSP_CTRL);
-
-		/* IRQ wired at M14 */
-		omap_cfg_reg(M14_1510_GPIO2);
-		isp->irq = OMAP_GPIO_IRQ(2);
-		if (gpio_request(2, "isp1301") == 0)
-			gpio_direction_input(2);
-		isp->irq_type = IRQF_TRIGGER_FALLING;
-	}
-
-	isp->irq_type |= IRQF_SAMPLE_RANDOM;
-	status = request_irq(isp->irq, isp1301_irq,
-			isp->irq_type, DRIVER_NAME, isp);
-	if (status < 0) {
-		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
-				isp->irq, status);
+	if (client->irq > 0) {
+		status = request_irq(client->irq, isp1301_irq, 0, DRIVER_NAME, isp);
+		if (status < 0) {
+			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
+					client->irq, status);
 #ifdef	CONFIG_USB_OTG
 fail2:
 #endif
-		i2c_detach_client(i2c);
-		goto fail1;
+			goto fail1;
+		}
 	}
 
-	isp->otg.dev = &isp->client->dev;
+	isp->otg.dev = &client->dev;
 	isp->otg.label = DRIVER_NAME;
 
 	isp->otg.set_host = isp1301_set_host,
@@ -1606,31 +1552,23 @@ fail2:
 
 #ifdef	VERBOSE
 	mod_timer(&isp->timer, jiffies + TIMER_JIFFIES);
-	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
+	dev_dbg(&client->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
 #endif
 
 	status = otg_set_transceiver(&isp->otg);
 	if (status < 0)
-		dev_err(&i2c->dev, "can't register transceiver, %d\n",
+		dev_err(&client->dev, "can't register transceiver, %d\n",
 			status);
 
 	return 0;
 }
 
-static int isp1301_scan_bus(struct i2c_adapter *bus)
-{
-	if (!i2c_check_functionality(bus, I2C_FUNC_SMBUS_BYTE_DATA
-			| I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EINVAL;
-	return i2c_probe(bus, &addr_data, isp1301_probe);
-}
-
 static struct i2c_driver isp1301_driver = {
 	.driver = {
 		.name	= "isp1301_omap",
 	},
-	.attach_adapter	= isp1301_scan_bus,
-	.detach_client	= isp1301_detach_client,
+	.probe	= isp1301_probe,
+	.remove	= __exit_p(isp1301_remove),
 };
 
 /*-------------------------------------------------------------------------*/
-- 
1.5.4.3.447.gc95b3

-- 
Best Regards,

Felipe Balbi
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org
http://blog.felipebalbi.com

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Felipe Balbi]

There are some changes on linux-omap's isp1301_omap.c
that could already be in linux-mainline after
omap1/2 got merged.

This patch sync such changes from linux-omap into linux-mailine.

Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
---
 drivers/i2c/chips/isp1301_omap.c |   40 +++++++++++++++++++++++++++++--------
 1 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index 9f2ccaf..9e3a6f3 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -84,7 +84,8 @@ struct isp1301 {
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef	CONFIG_MACH_OMAP_H2
+#if	defined(CONFIG_MACH_OMAP_H2) || \
+	defined(CONFIG_MACH_OMAP_H3)
 
 /* board-specific PM hooks */
 
@@ -122,17 +123,30 @@ static void enable_vbus_source(struct isp1301 *isp)
 }
 
 
-/* products will deliver OTG messages with LEDs, GUI, etc */
-static inline void notresponding(struct isp1301 *isp)
+#else
+
+static void enable_vbus_draw(struct isp1301 *isp, unsigned mA)
 {
-	printk(KERN_NOTICE "OTG device not responding.\n");
+	pr_debug("%s UNIMPL\n", __FUNCTION__);
 }
 
+static void enable_vbus_source(struct isp1301 *isp)
+{
+	pr_debug("%s UNIMPL\n", __FUNCTION__);
+}
 
 #endif
 
 /*-------------------------------------------------------------------------*/
 
+/* products will deliver OTG messages with LEDs, GUI, etc */
+static inline void notresponding(struct isp1301 *isp)
+{
+	printk(KERN_NOTICE "OTG device not responding.\n");
+}
+
+/*-------------------------------------------------------------------------*/
+
 /* only two addresses possible */
 static struct i2c_driver isp1301_driver;
 
@@ -495,6 +509,7 @@ static inline void check_state(struct isp1301 *isp, const char *tag) { }
 static void update_otg1(struct isp1301 *isp, u8 int_src)
 {
 	u32	otg_ctrl;
+	u8      int_id;
 
 	otg_ctrl = OTG_CTRL_REG
 			& OTG_CTRL_MASK
@@ -508,7 +523,10 @@ static void update_otg1(struct isp1301 *isp, u8 int_src)
 	}
 	if (int_src & INTR_VBUS_VLD)
 		otg_ctrl |= OTG_VBUSVLD;
-	if (int_src & INTR_ID_GND) {		/* default-A */
+
+	int_id = isp1301_get_u8(isp, ISP1301_INTERRUPT_SOURCE);
+
+	if (int_id & INTR_ID_GND) {		/* default-A */
 		if (isp->otg.state == OTG_STATE_B_IDLE
 				|| isp->otg.state == OTG_STATE_UNDEFINED) {
 			a_idle(isp, "init");
@@ -1063,7 +1081,7 @@ static void isp_update_otg(struct isp1301 *isp, u8 stat)
 	/* update the OTG controller state to match the isp1301; may
 	 * trigger OPRT_CHG irqs for changes going to the isp1301.
 	 */
-	update_otg1(isp, isp_stat);
+	update_otg1(isp, isp_stat); /* pass the actual interrupt latch status */
 	update_otg2(isp, isp_bstat);
 	check_state(isp, __FUNCTION__);
 #endif
@@ -1337,13 +1355,13 @@ isp1301_set_peripheral(struct otg_transceiver *otg, struct usb_gadget *gadget)
 	power_up(isp);
 	isp->otg.state = OTG_STATE_B_IDLE;
 
-	if (machine_is_omap_h2())
+	if (machine_is_omap_h2() || machine_is_omap_h3())
 		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
 
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
-		INTR_SESS_VLD);
+		INTR_SESS_VLD | INTR_VBUS_VLD);
 	isp1301_set_bits(isp, ISP1301_INTERRUPT_FALLING,
-		INTR_VBUS_VLD);
+		INTR_VBUS_VLD | INTR_SESS_VLD);
 	dev_info(&isp->client->dev, "B-Peripheral sessions ok\n");
 	dump_regs(isp, __func__);
 
@@ -1420,6 +1438,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
 	 * So do this part as early as possible...
 	 */
 	switch (isp->otg.state) {
+	case OTG_STATE_B_PERIPHERAL:
+		isp->otg.state = OTG_STATE_B_WAIT_ACON;
+		isp1301_defer_work(isp, WORK_UPDATE_ISP);
+		break;
 	case OTG_STATE_B_HOST:
 		isp->otg.state = OTG_STATE_B_PERIPHERAL;
 		/* caller will suspend next */
-- 
1.5.4.3.447.gc95b3

-- 
Best Regards,

Felipe Balbi
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org
http://blog.felipebalbi.com

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[David Brownell]

On Sunday 16 March 2008, Felipe Balbi wrote:
> +       set_irq_type(OMAP_GPIO_IRQ(14), IRQF_SAMPLE_RANDOM);

You should provide *only* IRQF_TRIGGER_* bits when
you call set_irq_type() ... while the docs are weak
here (too), notice how kernel/irq/manage.c masks
out all except the four bits in IRQF_TRIGGER_MASK.

No irq_chip should pay attention to other bits in
that call.  Other flags, like IRQF_{DISABLED,SHARED},
are driver-specific.

Plus, the whole point is to specify the trigger mode,
so calling this the equivalent of "use the default"
(IRQF_TRIGGER_NONE == 0) is pointless.  :)

Also, I'm sure someone will be tempted to remove the
IRQF_SAMPLE_RANDOM here, and it's something of a fair
argument for externally controlled signals.  It's not
random when an external agent can control delivery of
signals at the millisecond level... hence it's not a
crypto-strong source of randomness.  Luckily most
OMAPs include a hardware RNG (except the 35xx series,
for some odd reason?) so that's not a real issue.

- Dave



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[David Brownell]

On Sunday 16 March 2008, Felipe Balbi wrote:
> There are some changes on linux-omap's isp1301_omap.c
> that could already be in linux-mainline after
> omap1/2 got merged.
> 
> This patch sync such changes from linux-omap into linux-mailine.

Did you re-test this on H2?

The reason the state machine changes haven't gone upstream
is because, last I checked, they broke OTG on H2 ... the
controller on H3 had some incompatible changes.

- Dave




_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Felipe Balbi]

On Sun, 16 Mar 2008 09:59:51 -0800, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
wrote:
> On Sunday 16 March 2008, Felipe Balbi wrote:
>> There are some changes on linux-omap's isp1301_omap.c
>> that could already be in linux-mainline after
>> omap1/2 got merged.
>>
>> This patch sync such changes from linux-omap into linux-mailine.
> 
> Did you re-test this on H2?
> 
> The reason the state machine changes haven't gone upstream
> is because, last I checked, they broke OTG on H2 ... the
> controller on H3 had some incompatible changes.

Like i told you before,  dont have access to h2 anymore.
only omap3 :s

if you could try whenever you have time... otherwise ill revert

> 
> - Dave
-- 
Best Regards,

Felipe Balbi
http://felipebalbi.com
me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Jean Delvare]

Hi Felipe,

On Sun, 16 Mar 2008 12:56:18 +0200, Felipe Balbi wrote:
> This patch uses set_irq_type as suggested by Dave.
> 
> ============= CUT HERE =========
> 
> From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
> From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> Date: Tue, 1 Jan 2008 23:00:18 -0500
> Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
> 
> Based on David Brownell's patch for tps65010, this patch
> finish conversting isp1301_omap.c to new-style i2c driver.
> 
> Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> ---
>  arch/arm/mach-omap1/board-h2.c   |    4 +
>  arch/arm/mach-omap1/board-h3.c   |    8 ++
>  arch/arm/mach-omap2/board-h4.c   |   14 ++++
>  drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
>  4 files changed, 62 insertions(+), 98 deletions(-)

OK, I've applied this version of the patch. Note though that I am still
worried that it removed platform-specific code (in
isp1301_detach_client/isp1301_remove, isp1301_set_host and
isp1301_probe). The IRQ mode setting has moved to the board code, but
the rest has simply disappeared. I really would like you to confirm
that it is OK, or to send an updated patch if not.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Jean Delvare]

Hi Felipe,

On Sun, 16 Mar 2008 13:00:35 +0200, Felipe Balbi wrote:
> There are some changes on linux-omap's isp1301_omap.c
> that could already be in linux-mainline after
> omap1/2 got merged.
> 
> This patch sync such changes from linux-omap into linux-mailine.

I fear I will need a better description than this. Please explain what
the changes are and why they are needed. "sync with an external tree"
isn't sufficient.

> 
> Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> ---
>  drivers/i2c/chips/isp1301_omap.c |   40 +++++++++++++++++++++++++++++--------
>  1 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
> index 9f2ccaf..9e3a6f3 100644
> --- a/drivers/i2c/chips/isp1301_omap.c
> +++ b/drivers/i2c/chips/isp1301_omap.c
> @@ -84,7 +84,8 @@ struct isp1301 {
>  
>  /*-------------------------------------------------------------------------*/
>  
> -#ifdef	CONFIG_MACH_OMAP_H2
> +#if	defined(CONFIG_MACH_OMAP_H2) || \
> +	defined(CONFIG_MACH_OMAP_H3)
>  
>  /* board-specific PM hooks */
>  
> @@ -122,17 +123,30 @@ static void enable_vbus_source(struct isp1301 *isp)
>  }
>  
>  
> -/* products will deliver OTG messages with LEDs, GUI, etc */
> -static inline void notresponding(struct isp1301 *isp)
> +#else
> +
> +static void enable_vbus_draw(struct isp1301 *isp, unsigned mA)
>  {
> -	printk(KERN_NOTICE "OTG device not responding.\n");
> +	pr_debug("%s UNIMPL\n", __FUNCTION__);
>  }
>  
> +static void enable_vbus_source(struct isp1301 *isp)
> +{
> +	pr_debug("%s UNIMPL\n", __FUNCTION__);
> +}
>  
>  #endif
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/* products will deliver OTG messages with LEDs, GUI, etc */
> +static inline void notresponding(struct isp1301 *isp)
> +{
> +	printk(KERN_NOTICE "OTG device not responding.\n");
> +}
> +
> +/*-------------------------------------------------------------------------*/
> +
>  /* only two addresses possible */
>  static struct i2c_driver isp1301_driver;
>  
> @@ -495,6 +509,7 @@ static inline void check_state(struct isp1301 *isp, const char *tag) { }
>  static void update_otg1(struct isp1301 *isp, u8 int_src)
>  {
>  	u32	otg_ctrl;
> +	u8      int_id;
>  
>  	otg_ctrl = OTG_CTRL_REG
>  			& OTG_CTRL_MASK
> @@ -508,7 +523,10 @@ static void update_otg1(struct isp1301 *isp, u8 int_src)
>  	}
>  	if (int_src & INTR_VBUS_VLD)
>  		otg_ctrl |= OTG_VBUSVLD;
> -	if (int_src & INTR_ID_GND) {		/* default-A */
> +
> +	int_id = isp1301_get_u8(isp, ISP1301_INTERRUPT_SOURCE);
> +
> +	if (int_id & INTR_ID_GND) {		/* default-A */
>  		if (isp->otg.state == OTG_STATE_B_IDLE
>  				|| isp->otg.state == OTG_STATE_UNDEFINED) {
>  			a_idle(isp, "init");
> @@ -1063,7 +1081,7 @@ static void isp_update_otg(struct isp1301 *isp, u8 stat)
>  	/* update the OTG controller state to match the isp1301; may
>  	 * trigger OPRT_CHG irqs for changes going to the isp1301.
>  	 */
> -	update_otg1(isp, isp_stat);
> +	update_otg1(isp, isp_stat); /* pass the actual interrupt latch status */
>  	update_otg2(isp, isp_bstat);
>  	check_state(isp, __FUNCTION__);
>  #endif
> @@ -1337,13 +1355,13 @@ isp1301_set_peripheral(struct otg_transceiver *otg, struct usb_gadget *gadget)
>  	power_up(isp);
>  	isp->otg.state = OTG_STATE_B_IDLE;
>  
> -	if (machine_is_omap_h2())
> +	if (machine_is_omap_h2() || machine_is_omap_h3())
>  		isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1, MC1_DAT_SE0);
>  
>  	isp1301_set_bits(isp, ISP1301_INTERRUPT_RISING,
> -		INTR_SESS_VLD);
> +		INTR_SESS_VLD | INTR_VBUS_VLD);
>  	isp1301_set_bits(isp, ISP1301_INTERRUPT_FALLING,
> -		INTR_VBUS_VLD);
> +		INTR_VBUS_VLD | INTR_SESS_VLD);
>  	dev_info(&isp->client->dev, "B-Peripheral sessions ok\n");
>  	dump_regs(isp, __func__);
>  
> @@ -1420,6 +1438,10 @@ isp1301_start_hnp(struct otg_transceiver *dev)
>  	 * So do this part as early as possible...
>  	 */
>  	switch (isp->otg.state) {
> +	case OTG_STATE_B_PERIPHERAL:
> +		isp->otg.state = OTG_STATE_B_WAIT_ACON;
> +		isp1301_defer_work(isp, WORK_UPDATE_ISP);
> +		break;
>  	case OTG_STATE_B_HOST:
>  		isp->otg.state = OTG_STATE_B_PERIPHERAL;
>  		/* caller will suspend next */


-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

hi,

On Tue, Mar 18, 2008 at 2:42 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Felipe,
>
>
>  On Sun, 16 Mar 2008 12:56:18 +0200, Felipe Balbi wrote:
>  > This patch uses set_irq_type as suggested by Dave.
>  >
>  > ============= CUT HERE =========
>  >
>  > From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
>  > From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
>  > Date: Tue, 1 Jan 2008 23:00:18 -0500
>  > Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
>  >
>  > Based on David Brownell's patch for tps65010, this patch
>  > finish conversting isp1301_omap.c to new-style i2c driver.
>  >
>  > Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
>  > ---
>  >  arch/arm/mach-omap1/board-h2.c   |    4 +
>  >  arch/arm/mach-omap1/board-h3.c   |    8 ++
>  >  arch/arm/mach-omap2/board-h4.c   |   14 ++++
>  >  drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
>  >  4 files changed, 62 insertions(+), 98 deletions(-)
>
>  OK, I've applied this version of the patch. Note though that I am still
>  worried that it removed platform-specific code (in
>  isp1301_detach_client/isp1301_remove, isp1301_set_host and
>  isp1301_probe). The IRQ mode setting has moved to the board code, but
>  the rest has simply disappeared. I really would like you to confirm
>  that it is OK, or to send an updated patch if not.

The board specific code was regarding gpio interrupts, and we now get
the correct gpio irq
from i2c_board_info so when I free_irq() i'm already freeing the gpio, right ?

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Felipe Balbi]

On Tue, Mar 18, 2008 at 2:46 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Felipe,
>
>
>  On Sun, 16 Mar 2008 13:00:35 +0200, Felipe Balbi wrote:
>  > There are some changes on linux-omap's isp1301_omap.c
>  > that could already be in linux-mainline after
>  > omap1/2 got merged.
>  >
>  > This patch sync such changes from linux-omap into linux-mailine.
>
>  I fear I will need a better description than this. Please explain what
>  the changes are and why they are needed. "sync with an external tree"
>  isn't sufficient.

Here's a better description:

Now that omap1/2 were merged on mainline we can bring the isp1301_omap.c
changes that were done for board-h3 and keep the code in sync. This will allow
us to decrese the diff between linux-omap and mainline and avoid merge conflicts
later.


Is this good enough ?
I can think in something else if you need a better description ;-)

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Jean Delvare]

Huuuuu, I'm just see this:

On Sun, 16 Mar 2008 12:56:18 +0200, Felipe Balbi wrote:
> -static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
> +static int __init isp1301_probe(struct i2c_client *client)
>  {
>  	int			status;
>  	struct isp1301		*isp;
> -	struct i2c_client	*i2c;
>  
>  	if (the_transceiver)
>  		return 0;
> @@ -1492,45 +1470,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
>  	init_timer(&isp->timer);
>  	isp->timer.function = isp1301_timer;
>  	isp->timer.data = (unsigned long) isp;
> -
> -	isp->irq = -1;
> -	isp->c.addr = address;
> -	i2c_set_clientdata(&isp->c, isp);
> -	isp->c.adapter = bus;
> -	isp->c.driver = &isp1301_driver;
> -	strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
> -	isp->client = i2c = &isp->c;
> +	isp->client = client;
>  
>  	/* if this is a true probe, verify the chip ... */
> -	if (kind < 0) {
> -		status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
> -		if (status != I2C_VENDOR_ID_PHILIPS) {
> -			dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
> -				address, status);
> -			goto fail1;
> -		}
> -		status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
> -		if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
> -			dev_dbg(&bus->dev, "%d not isp1301, %d\n",
> -				address, status);
> -			goto fail1;
> -		}
> +	status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
> +	if (status != I2C_VENDOR_ID_PHILIPS) {
> +		dev_dbg(&client->dev, "not philips id: %d\n", status);
> +		goto fail1;
> +	}
> +	status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
> +	if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
> +		dev_dbg(&client->dev, "not isp1301, %d\n", status);
> +		goto fail1;
>  	}
>  
> -	status = i2c_attach_client(i2c);
> -	if (status < 0) {
> -		dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
> -				DRIVER_NAME, address, status);
>  fail1:
>  		kfree(isp);
>  		return 0;
> -	}

This is an unconditional failure, isn't it? So if I may ask: how did
you test this patch to not notice this?

> -	isp->i2c_release = i2c->dev.release;
> -	i2c->dev.release = isp1301_release;
> +
> +	isp->i2c_release = client->dev.release = isp1301_release;

And this is a functional change which I really doubt was made on
purpose. It will make isp1301_release loop forever.

Don't know about you, but I am seriously tired by this patch. We're
working on it for more than 2 months, it must have seen half a dozen
revisions by now, and it's still broken enough for me to be sure that
it wasn't even tested.

The obvious reason is that this patch does several things at once. It's
not just converting the driver to a new-style i2c driver, it is also
moving the board-specific code out of the driver (or plain killing it
without explanations), and on top of this you add unrelated code
cleanups (or breakage). Patches are supposed to be split into
functional changes that are easy to review and test, exactly to avoid
this situation.

So I'll just drop this patch for now. I can't afford pushing broken
stuff into -mm. Let me know when you have something better and well
tested.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Jean Delvare]

On Tue, 18 Mar 2008 15:09:43 +0200, Felipe Balbi wrote:
> On Tue, Mar 18, 2008 at 2:46 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> > Hi Felipe,
> >
> >
> >  On Sun, 16 Mar 2008 13:00:35 +0200, Felipe Balbi wrote:
> >  > There are some changes on linux-omap's isp1301_omap.c
> >  > that could already be in linux-mainline after
> >  > omap1/2 got merged.
> >  >
> >  > This patch sync such changes from linux-omap into linux-mailine.
> >
> >  I fear I will need a better description than this. Please explain what
> >  the changes are and why they are needed. "sync with an external tree"
> >  isn't sufficient.
> 
> Here's a better description:
> 
> Now that omap1/2 were merged on mainline we can bring the isp1301_omap.c
> changes that were done for board-h3 and keep the code in sync. This will allow
> us to decrese the diff between linux-omap and mainline and avoid merge conflicts
> later.
> 
> 
> Is this good enough ?

No. You are just paraphrasing, this doesn't help. I need an explanation
of the _functional_ changes implemented by the patch. I don't care much
that these changes come from the omap tree, I don't know anything about
this tree. What I need to know is why the changes are needed by
themselves. What problem(s) does this patch address?

> I can think in something else if you need a better description ;-)

Thanks,
-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Felipe Balbi]

Hmm...

On Tue, Mar 18, 2008 at 3:15 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Huuuuu, I'm just see this:
>
>
>  On Sun, 16 Mar 2008 12:56:18 +0200, Felipe Balbi wrote:
>
>
> > -static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
>  > +static int __init isp1301_probe(struct i2c_client *client)
>  >  {
>  >       int                     status;
>  >       struct isp1301          *isp;
>  > -     struct i2c_client       *i2c;
>  >
>  >       if (the_transceiver)
>  >               return 0;
>  > @@ -1492,45 +1470,29 @@ static int isp1301_probe(struct i2c_adapter *bus, int address, int kind)
>  >       init_timer(&isp->timer);
>  >       isp->timer.function = isp1301_timer;
>  >       isp->timer.data = (unsigned long) isp;
>  > -
>  > -     isp->irq = -1;
>  > -     isp->c.addr = address;
>  > -     i2c_set_clientdata(&isp->c, isp);
>  > -     isp->c.adapter = bus;
>  > -     isp->c.driver = &isp1301_driver;
>  > -     strlcpy(isp->c.name, DRIVER_NAME, I2C_NAME_SIZE);
>  > -     isp->client = i2c = &isp->c;
>  > +     isp->client = client;
>  >
>  >       /* if this is a true probe, verify the chip ... */
>  > -     if (kind < 0) {
>  > -             status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
>  > -             if (status != I2C_VENDOR_ID_PHILIPS) {
>  > -                     dev_dbg(&bus->dev, "addr %d not philips id: %d\n",
>  > -                             address, status);
>  > -                     goto fail1;
>  > -             }
>  > -             status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
>  > -             if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
>  > -                     dev_dbg(&bus->dev, "%d not isp1301, %d\n",
>  > -                             address, status);
>  > -                     goto fail1;
>  > -             }
>  > +     status = isp1301_get_u16(isp, ISP1301_VENDOR_ID);
>  > +     if (status != I2C_VENDOR_ID_PHILIPS) {
>  > +             dev_dbg(&client->dev, "not philips id: %d\n", status);
>  > +             goto fail1;
>  > +     }
>  > +     status = isp1301_get_u16(isp, ISP1301_PRODUCT_ID);
>  > +     if (status != I2C_PRODUCT_ID_PHILIPS_1301) {
>  > +             dev_dbg(&client->dev, "not isp1301, %d\n", status);
>  > +             goto fail1;
>  >       }
>  >
>  > -     status = i2c_attach_client(i2c);
>  > -     if (status < 0) {
>  > -             dev_dbg(&bus->dev, "can't attach %s to device %d, err %d\n",
>  > -                             DRIVER_NAME, address, status);
>  >  fail1:
>  >               kfree(isp);
>  >               return 0;
>  > -     }
>
>  This is an unconditional failure, isn't it? So if I may ask: how did
>  you test this patch to not notice this?

I'll fix this one...

>  > -     isp->i2c_release = i2c->dev.release;
>  > -     i2c->dev.release = isp1301_release;
>  > +
>  > +     isp->i2c_release = client->dev.release = isp1301_release;
>
>  And this is a functional change which I really doubt was made on
>  purpose. It will make isp1301_release loop forever.

I can't see why holding the function in struct isp1301_omap will make
it loop forever. There's actually a comment on the code that says:

static void isp1301_release(struct device *dev)
{
	struct isp1301		*isp = dev_get_drvdata(dev);

	/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
	if (isp->i2c_release)
		isp->i2c_release(dev);
	kfree (isp);
}

>  Don't know about you, but I am seriously tired by this patch. We're
>  working on it for more than 2 months, it must have seen half a dozen
>  revisions by now, and it's still broken enough for me to be sure that
>  it wasn't even tested.

Like i told before, I don't have access anymore to omap1 or omap2
boards. I asked Dave to test when he had anytime but it looks like
he hadn't any :-s

>  The obvious reason is that this patch does several things at once. It's
>  not just converting the driver to a new-style i2c driver, it is also
>  moving the board-specific code out of the driver (or plain killing it
>  without explanations), and on top of this you add unrelated code
>  cleanups (or breakage). Patches are supposed to be split into
>  functional changes that are easy to review and test, exactly to avoid
>  this situation.

Looks like you're right here.

>  So I'll just drop this patch for now. I can't afford pushing broken
>  stuff into -mm. Let me know when you have something better and well
>  tested.

Fair enough, but i won't have a way to test this patch. So better
letting someone
else play with it.

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Jean Delvare]

On Tue, 18 Mar 2008 15:06:38 +0200, Felipe Balbi wrote:
> hi,
> 
> On Tue, Mar 18, 2008 at 2:42 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> > Hi Felipe,
> >
> >
> >  On Sun, 16 Mar 2008 12:56:18 +0200, Felipe Balbi wrote:
> >  > This patch uses set_irq_type as suggested by Dave.
> >  >
> >  > ============= CUT HERE =========
> >  >
> >  > From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
> >  > From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> >  > Date: Tue, 1 Jan 2008 23:00:18 -0500
> >  > Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
> >  >
> >  > Based on David Brownell's patch for tps65010, this patch
> >  > finish conversting isp1301_omap.c to new-style i2c driver.
> >  >
> >  > Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> >  > ---
> >  >  arch/arm/mach-omap1/board-h2.c   |    4 +
> >  >  arch/arm/mach-omap1/board-h3.c   |    8 ++
> >  >  arch/arm/mach-omap2/board-h4.c   |   14 ++++
> >  >  drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
> >  >  4 files changed, 62 insertions(+), 98 deletions(-)
> >
> >  OK, I've applied this version of the patch. Note though that I am still
> >  worried that it removed platform-specific code (in
> >  isp1301_detach_client/isp1301_remove, isp1301_set_host and
> >  isp1301_probe). The IRQ mode setting has moved to the board code, but
> >  the rest has simply disappeared. I really would like you to confirm
> >  that it is OK, or to send an updated patch if not.
> 
> The board specific code was regarding gpio interrupts, and we now get
> the correct gpio irq
> from i2c_board_info so when I free_irq() i'm already freeing the gpio, right ?

You aren't asking me, are you? I don't have any idea. I don't know
anything about embedded stuff, let alone omap in particular. Someone
else will have to answer this question.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[Felipe Balbi]

On Tue, Mar 18, 2008 at 3:23 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> On Tue, 18 Mar 2008 15:09:43 +0200, Felipe Balbi wrote:
>  > On Tue, Mar 18, 2008 at 2:46 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
>  > > Hi Felipe,
>  > >
>  > >
>  > >  On Sun, 16 Mar 2008 13:00:35 +0200, Felipe Balbi wrote:
>  > >  > There are some changes on linux-omap's isp1301_omap.c
>  > >  > that could already be in linux-mainline after
>  > >  > omap1/2 got merged.
>  > >  >
>  > >  > This patch sync such changes from linux-omap into linux-mailine.
>  > >
>  > >  I fear I will need a better description than this. Please explain what
>  > >  the changes are and why they are needed. "sync with an external tree"
>  > >  isn't sufficient.
>  >
>  > Here's a better description:
>  >
>  > Now that omap1/2 were merged on mainline we can bring the isp1301_omap.c
>  > changes that were done for board-h3 and keep the code in sync. This will allow
>  > us to decrese the diff between linux-omap and mainline and avoid merge conflicts
>  > later.
>  >
>  >
>  > Is this good enough ?
>
>  No. You are just paraphrasing, this doesn't help. I need an explanation
>  of the _functional_ changes implemented by the patch. I don't care much
>  that these changes come from the omap tree, I don't know anything about
>  this tree. What I need to know is why the changes are needed by
>  themselves. What problem(s) does this patch address?

otherwise the driver won't work on board-h3, simple enough

-- 
Best Regards,

Felipe Balbi
felipebalbi-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301: Sync with linux-omap

[David Brownell]

On Tuesday 18 March 2008, Felipe Balbi wrote:
> >
> >  No. You are just paraphrasing, this doesn't help. I need an explanation
> >  of the _functional_ changes implemented by the patch. I don't care much
> >  that these changes come from the omap tree, I don't know anything about
> >  this tree. What I need to know is why the changes are needed by
> >  themselves. What problem(s) does this patch address?
> 
> otherwise the driver won't work on board-h3, simple enough

But the reason those changes were never pushed upstream is that
they were known to break board-h2 !!

I was never keen on merging them into the OMAP tree in the first
place, specifically for that reason ... the original idea was
to merge patches that worked on *both* chip revisions.  Failing
that, to follow up the H3 patches with bugfixes; those fixes
never got submitted.

So for what it's worth, I'll NAK those state machine updates.
On the grounds that they *add* bugs.  H3 doesn't work in the
mainline tree yet, so there's no way such a patch could ever
be viewed as an improving anything whatsoever for mainline.





_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[David Brownell]

> > The board specific code was regarding gpio interrupts, and we now get
> > the correct gpio irq from i2c_board_info so when I free_irq() i'm
> > already freeing the gpio, right ? 

Freeing an IRQ is not the same as freeing a GPIO; of course not.

I'd have to grab the latest version of this patch to see if this
all makes sense.

- Dave

 
> You aren't asking me, are you? I don't have any idea. I don't know
> anything about embedded stuff, let alone omap in particular. Someone
> else will have to answer this question.



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[David Brownell]

On Sunday 16 March 2008, Felipe Balbi wrote:
> -       if (machine_is_omap_h2()) {
> -               /* full speed signaling by default */
> -               isp1301_set_bits(isp, ISP1301_MODE_CONTROL_1,
> -                       MC1_SPEED_REG);
> -               isp1301_set_bits(isp, ISP1301_MODE_CONTROL_2,
> -                       MC2_SPD_SUSP_CTRL);

Where did this code end up?  ISTR it was required since the
transceiver's reset mode didn't match how this board was wired.

The details of how the transceiver is wired are something that
should go into platform_data for each board, unless there's code
to pull it out of the transceiver configuration register and
then interpret it here (yeech).  This would be at the level of
whether it uses 6-wire, 4-wire, or 3-wire signaling, and a few
more related points...

ISTR the original patch I sent didn't try to do so much, and
left some of the board-specific code.


> -
> -               /* IRQ wired at M14 */
> -               omap_cfg_reg(M14_1510_GPIO2);

I don't recall seeing this pinmu directive move into
the H2 board setup ... or the gpio_request, below.
Only the IRQ setup moved; but IRQ setup presumes the
pin has already been properly set up as a gpio input.

The result would be that this couldn't possibly work.


> -               isp->irq = OMAP_GPIO_IRQ(2);
> -               if (gpio_request(2, "isp1301") == 0)
> -                       gpio_direction_input(2);
> -               isp->irq_type = IRQF_TRIGGER_FALLING;
> -       }



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Ben Dooks]

On Sun, Mar 16, 2008 at 12:56:18PM +0200, Felipe Balbi wrote:
> This patch uses set_irq_type as suggested by Dave.
> 
> ============= CUT HERE =========
> 
> >From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
> From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> Date: Tue, 1 Jan 2008 23:00:18 -0500
> Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
> 
> Based on David Brownell's patch for tps65010, this patch
> finish conversting isp1301_omap.c to new-style i2c driver.
> 
> Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> ---
>  arch/arm/mach-omap1/board-h2.c   |    4 +
>  arch/arm/mach-omap1/board-h3.c   |    8 ++
>  arch/arm/mach-omap2/board-h4.c   |   14 ++++
>  drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
>  4 files changed, 62 insertions(+), 98 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
> index 5079877..fd9567c 100644
> --- a/arch/arm/mach-omap1/board-h2.c
> +++ b/arch/arm/mach-omap1/board-h2.c
> @@ -22,6 +22,8 @@
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
>  #include <linux/i2c.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
> @@ -366,6 +368,8 @@ static void __init h2_init_irq(void)
>  	omap_init_irq();
>  	omap_gpio_init();
>  	h2_init_smc91x();
> +	set_irq_type(OMAP_GPIO_IRQ(2),
> +			IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING);

Calling set_irq_type directly is not the recommended way of doing
this. Use the correct trigger flags to request_irq().


-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Jean Delvare]

Hi Ben,

On Mon, 31 Mar 2008 18:37:49 +0100, Ben Dooks wrote:
> On Sun, Mar 16, 2008 at 12:56:18PM +0200, Felipe Balbi wrote:
> > This patch uses set_irq_type as suggested by Dave.
> > 
> > ============= CUT HERE =========
> > 
> > >From 293a73277794a1ca0f2307e7fe9d5ecd76d3ad06 Mon Sep 17 00:00:00 2001
> > From: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> > Date: Tue, 1 Jan 2008 23:00:18 -0500
> > Subject: [PATCH] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
> > 
> > Based on David Brownell's patch for tps65010, this patch
> > finish conversting isp1301_omap.c to new-style i2c driver.
> > 
> > Signed-off-by: Felipe Balbi <me-uiRdBs8odbtmTBlB0Cgj/Q@public.gmane.org>
> > ---
> >  arch/arm/mach-omap1/board-h2.c   |    4 +
> >  arch/arm/mach-omap1/board-h3.c   |    8 ++
> >  arch/arm/mach-omap2/board-h4.c   |   14 ++++
> >  drivers/i2c/chips/isp1301_omap.c |  134 ++++++++++---------------------------
> >  4 files changed, 62 insertions(+), 98 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
> > index 5079877..fd9567c 100644
> > --- a/arch/arm/mach-omap1/board-h2.c
> > +++ b/arch/arm/mach-omap1/board-h2.c
> > @@ -22,6 +22,8 @@
> >  #include <linux/kernel.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/delay.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> >  #include <linux/i2c.h>
> >  #include <linux/mtd/mtd.h>
> >  #include <linux/mtd/nand.h>
> > @@ -366,6 +368,8 @@ static void __init h2_init_irq(void)
> >  	omap_init_irq();
> >  	omap_gpio_init();
> >  	h2_init_smc91x();
> > +	set_irq_type(OMAP_GPIO_IRQ(2),
> > +			IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING);
> 
> Calling set_irq_type directly is not the recommended way of doing
> this. Use the correct trigger flags to request_irq().

Why that? set_irq_type() can be called from platform code, while
request_irq() is called from device drivers. When different platforms
use the same driver and need different IRQ trigger flags, using
set_irq_type() saves the need to carry the information from each
platform to the device driver. What's wrong with this approach?

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c