Skip to contentSkip to conversation

I2C driver for IMX

PATCH Superseded Darius 2 participants 2 messages Jul 8, 2008

Subsystems (5)

v8 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Darius

author
#0 lore raw

PATCH v8 1/1 I2C driver for IMX

Changes from V7 version:

- .remove function now is declarated with __devexit

---

arch/arm/mach-imx/mx1ads.c +21 -0
Index: linux-2.6.26-rc8/arch/arm/mach-imx/mx1ads.c
===================================================================
@@ -109,10 +109,31 @@ static struct platform_device imx_uart2_
 	}
 };
 
+static struct resource imx_i2c_resources[] = {
+	[0] = {
+		.start  = 0x00217000,
+		.end    = 0x00217010,
+		.flags  = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start  = I2C_INT,
+		.end    = I2C_INT,
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device imx_i2c_device = {
+	.name           = "imx-i2c",
+	.id             = 0,
+	.resource       = imx_i2c_resources,
+	.num_resources  = ARRAY_SIZE(imx_i2c_resources),
+};
+
 static struct platform_device *devices[] __initdata = {
 	&cs89x0_device,
 	&imx_uart1_device,
 	&imx_uart2_device,
+	&imx_i2c_device,
 };
 
 #ifdef CONFIG_MMC_IMX
drivers/i2c/busses/i2c-imx.c new file +629 -0

629 lines not on this page · raw message

drivers/i2c/busses/Kconfig +10 -0
Index: linux-2.6.26-rc8/drivers/i2c/busses/Kconfig
===================================================================
@@ -257,6 +257,16 @@ config I2C_IBM_IIC
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-ibm_iic.
 
+config I2C_IMX
+	tristate "IMX I2C interface"
+	depends on ARCH_IMX
+	help
+	  Say Y here if you want to use the IIC bus controller on
+	  the Freescale i.MXL and i.MX1 processors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-imx.
+
 config I2C_IOP3XX
 	tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
 	depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX || ARCH_IOP13XX
drivers/i2c/busses/Makefile +1 -0
Index: linux-2.6.26-rc8/drivers/i2c/busses/Makefile
===================================================================
@@ -18,6 +18,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
 obj-$(CONFIG_I2C_I810)		+= i2c-i810.o
 obj-$(CONFIG_I2C_IBM_IIC)	+= i2c-ibm_iic.o
+obj-$(CONFIG_I2C_IMX)		+= i2c-imx.o
 obj-$(CONFIG_I2C_IOP3XX)	+= i2c-iop3xx.o
 obj-$(CONFIG_I2C_IXP2000)	+= i2c-ixp2000.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o

-------------------------------------------------------------------

List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
maintainer
#1 lore raw

Hi Darius,

Quoted 71 lines · Darius

Changes from V7 version:

- .remove function now is declarated with __devexit

---

Index: linux-2.6.26-rc8/arch/arm/mach-imx/mx1ads.c
===================================================================

arch/arm/mach-imx/mx1ads.c +21 -0
@@ -109,10 +109,31 @@ static struct platform_device imx_uart2_
 	}
 };
 
+static struct resource imx_i2c_resources[] = {
+	[0] = {
+		.start  = 0x00217000,
+		.end    = 0x00217010,
+		.flags  = IORESOURCE_MEM,
+	},
+	[1] = {
+		.start  = I2C_INT,
+		.end    = I2C_INT,
+		.flags  = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device imx_i2c_device = {
+	.name           = "imx-i2c",
+	.id             = 0,
+	.resource       = imx_i2c_resources,
+	.num_resources  = ARRAY_SIZE(imx_i2c_resources),
+};
+
 static struct platform_device *devices[] __initdata = {
 	&cs89x0_device,
 	&imx_uart1_device,
 	&imx_uart2_device,
+	&imx_i2c_device,
 };
 
 #ifdef CONFIG_MMC_IMX

Index: linux-2.6.26-rc8/drivers/i2c/busses/i2c-imx.c
===================================================================

drivers/i2c/busses/i2c-imx.c new file +23 -0
@@ -0,0 +1,629 @@
+/*
+ *	Copyright (C) 2002 Motorola GSG-China
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version 2
+ *	of the License, or (at your option) any later version.
+ *
+ *	This program is distributed in the hope that it will be useful,
+ *	but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *	GNU General Public License for more details.
+ *
+ *	You should have received a copy of the GNU General Public License
+ *	along with this program; if not, write to the Free Software
+ *	Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307,
+ *	USA.
+ *
+ * Author:
+ *	Darius Augulis, Teltonika Inc.
+ *
+ * Desc.:
+ *	Implementation of I2C Adapter/Algorithm Driver

drivers/i2c/busses/i2c-imx.c line 33

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c new file +10 -0
+ *	Driver for I2C Bus integrated in i.MXL, i.MX1
+ *
+ *	module parameters:
+ *		- clkfreq:
+ *			Sets the desired clock rate
+ *			The default value is 100000
+ *			Max value is 400000
+ *		- imxslave:
+ *			IMX slave I2C address in decimal format
+ *			The default value is 0xAC in hex

Does this variable change anything? AFAIK Linux does not have I2C slave support, so what does this driver send if in slave mode?

Quoted 8 lines
drivers/i2c/busses/i2c-imx.c new file +8 -0
+ *
+ *	Derived from Motorola GSG China I2C example driver
+ *
+ *	Copyright (C) 2005 Torsten Koschorrek <koschorrek at synertronixx.de
+ *	Portions:
+ *	Copyright (C) 2005 Matthias Blaschke <blaschke at synertronixx.de
+ *	Copyright (C) 2007 RightHand Technologies, Inc. <adyeratrighthandtech.com>
+ *	Copyright (C) 2008 Darius Augulis <darius.augulis at teltonika.lt>

drivers/i2c/busses/i2c-imx.c line 51

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c new file +10 -0
+ *
+ */
+
+/** Includes *******************************************************************
+*******************************************************************************/
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/fs.h>

not needed

drivers/i2c/busses/i2c-imx.c line 52

Quoted 1 line
drivers/i2c/busses/i2c-imx.c new file +1 -0
+#include <linux/proc_fs.h>

ditto

Quoted 20 lines
drivers/i2c/busses/i2c-imx.c new file +20 -0
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <asm/arch/irqs.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/imx-regs.h>
+
+/** Defines ********************************************************************
+*******************************************************************************/
+
+/* This will be the driver name the kernel reports */
+#define DRIVER_NAME "imx-i2c"
+
+/* Default values of module parameters */
+#define IMX_I2C_SLAVE_ADDR	0xAC
+#define IMX_I2C_BIT_RATE	100000	/* 100kHz */
+

drivers/i2c/busses/i2c-imx.c line 82

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c new file +10 -0
+/* Timeouts */
+#define I2C_IMX_TIME_BUSY	2000	/* loop count */
+#define I2C_IMX_TIME_ACK	2000	/* loop count */
+#define I2C_IMX_TIME_TRX	5	/* seconds */
+
+/* Error numbers */
+#define I2C_IMX_ERR_BUSY	1
+#define I2C_IMX_ERR_TX_TIMEOUT	2
+#define I2C_IMX_ERR_RX_TIMEOUT	3
+#define I2C_IMX_ERR_RX_NO_ACK	4

You define your own error values here but never test for them, so this is quite unnessecary. Even worse, you pass the numbers to the I2C layer, so I2C_IMX_ERR_TX_TIMEOUT will translate to ENOENT which is surely not what you want.

<snip>

Quoted 9 lines
drivers/i2c/busses/i2c-imx.c new file +9 -0
+
+static int i2c_imx_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
+{
+	unsigned int i, temp;
+	int err = 0;
+	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(adapter);
+
+	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+

drivers/i2c/busses/i2c-imx.c line 402

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c new file +10 -0
+	/* Check or i2c bus is not busy */
+	err = i2c_imx_bus_busy(i2c_imx);
+	if (err)
+		goto fail0;
+
+	/* Enable i2c */
+	i2c_imx_enable(i2c_imx);
+
+	/* Start I2C transfer */
+	i2c_imx_start(i2c_imx);

These two functions are called only once and both modify the same register. Is it worth it to make them extra functions? Regarding the slave mode, you disable the whole I2C module outside the transfers, so there won't be any slave support at all.

Quoted 22 lines
drivers/i2c/busses/i2c-imx.c new file +22 -0
+
+	temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+
+	/* read/write data */
+	for (i = 0; i < num; i++) {
+		if (i) {
+			dev_dbg(&i2c_imx->adapter.dev,
+				"<%s> repeated start\n", __func__);
+			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+			temp |= I2CR_RSTA;
+			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+		}
+		dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> transfer message: %d\n", __func__, i);
+		/* write/read data */
+#ifdef CONFIG_I2C_DEBUG_BUS
+		temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+		dev_dbg(&i2c_imx->adapter.dev, "<%s> CONTROL: IEN=%d, IIEN=%d, "
+			"MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n", __func__,
+			(temp&I2CR_IEN ? 1 : 0), (temp&I2CR_IIEN ? 1 : 0),
+			(temp&I2CR_MSTA ? 1 : 0), (temp&I2CR_MTX ? 1 : 0),
+			(temp&I2CR_TXAK ? 1 : 0), (temp&I2CR_RSTA ? 1 : 0));

drivers/i2c/busses/i2c-imx.c line 434

Quoted 10 lines
drivers/i2c/busses/i2c-imx.c new file +10 -0
+		temp = readb(i2c_imx->base + IMX_I2C_I2SR);
+		dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, "
+			"IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
+			(temp&I2SR_ICF ? 1 : 0), (temp&I2SR_IAAS ? 1 : 0),
+			(temp&I2SR_IBB ? 1 : 0), (temp&I2SR_IAL ? 1 : 0),
+			(temp&I2SR_SRW ? 1 : 0), (temp&I2SR_IIF ? 1 : 0),
+			(temp&I2SR_RXAK ? 1 : 0));
+#endif
+		if (!(msgs[i].flags & I2C_M_RD))

positive logic would be easier to read.

Quoted 15 lines
drivers/i2c/busses/i2c-imx.c new file +15 -0
+			err = i2c_imx_write(i2c_imx, &msgs[i]);
+		else
+			err = i2c_imx_read(i2c_imx, &msgs[i]);
+	}
+
+fail0:
+	/* Stop bus */
+	i2c_imx_stop(i2c_imx);
+	/* disable i2c bus */
+	i2c_imx_disable(i2c_imx);
+	dev_dbg(&i2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
+		(err < 0) ? "error" : "success msg", (err < 0) ? err : num);
+	return (err < 0) ? err : num;
+}
+
Signature · 9 lines
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php