[U-Boot-Users] [PATCH v2 3/7] add an i2c driver for mx31

From: Sascha Hauer s.hauer@pengutronix.de
This patch adds an i2c driver for Freescale i.MX processors
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Guennadi Liakhovetski lg@denx.de
---
No changes wrt v1
drivers/i2c/Makefile | 1 + drivers/i2c/mxc_i2c.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 0 deletions(-) create mode 100644 drivers/i2c/mxc_i2c.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 29d6c03..071ef00 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -29,6 +29,7 @@ COBJS-y += fsl_i2c.o COBJS-y += omap1510_i2c.o COBJS-y += omap24xx_i2c.o COBJS-y += tsi108_i2c.o +COBJS-y += mxc_i2c.o
COBJS := $(COBJS-y) SRCS := $(COBJS:.o=.c) diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c new file mode 100644 index 0000000..a218329 --- /dev/null +++ b/drivers/i2c/mxc_i2c.c @@ -0,0 +1,202 @@ +/* + * i2c driver for Freescale mx31 + * + * (c) 2007 Pengutronix, Sascha Hauer s.hauer@pengutronix.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * 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 + */ + +#include <common.h> + +#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC) + +#include <asm/arch/mx31.h> +#include <asm/arch/mx31-regs.h> + +#define IADR 0x00 +#define IFDR 0x04 +#define I2CR 0x08 +#define I2SR 0x0c +#define I2DR 0x10 + +#define I2CR_IEN (1 << 7) +#define I2CR_IIEN (1 << 6) +#define I2CR_MSTA (1 << 5) +#define I2CR_MTX (1 << 4) +#define I2CR_TX_NO_AK (1 << 3) +#define I2CR_RSTA (1 << 2) + +#define I2SR_ICF (1 << 7) +#define I2SR_IBB (1 << 5) +#define I2SR_IIF (1 << 1) +#define I2SR_RX_NO_AK (1 << 0) + +#ifdef CFG_I2C_MX31_PORT1 +#define I2C_BASE 0x43f80000 +#elif defined (CFG_I2C_MX31_PORT2) +#define I2C_BASE 0x43f98000 +#elif defined (CFG_I2C_MX31_PORT3) +#define I2C_BASE 0x43f84000 +#else +#error "define CFG_I2C_MX31_PORTx to use the mx31 I2C driver" +#endif + +#ifdef DEBUG +#define DPRINTF(args...) printf(args) +#else +#define DPRINTF(args...) +#endif + +static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144, + 160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960, + 1152, 1280, 1536, 1920, 2304, 2560, 3072, 3840}; + +void i2c_init(int speed, int unused) +{ + int freq = mx31_get_ipg_clk(); + int i; + + for (i = 0; i < 0x1f; i++) + if (freq / div[i] <= speed) + break; + + DPRINTF("%s: speed: %d\n",__FUNCTION__, speed); + + __REG16(I2C_BASE + I2CR) = 0; /* Reset module */ + __REG16(I2C_BASE + IFDR) = i; + __REG16(I2C_BASE + I2CR) = I2CR_IEN; + __REG16(I2C_BASE + I2SR) = 0; +} + +static int wait_busy(void) +{ + int timeout = 10000; + + while (!(__REG16(I2C_BASE + I2SR) & I2SR_IIF) && --timeout) + udelay(1); + __REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */ + + return timeout; +} + +static int tx_byte(u8 byte) +{ + __REG16(I2C_BASE + I2DR) = byte; + + if (!wait_busy() || __REG16(I2C_BASE + I2SR) & I2SR_RX_NO_AK) + return -1; + return 0; +} + +static int rx_byte(void) +{ + if (!wait_busy()) + return -1; + + return __REG16(I2C_BASE + I2DR); +} + +int i2c_probe(uchar chip) +{ + int ret; + + __REG16(I2C_BASE + I2CR) = 0; /* Reset module */ + __REG16(I2C_BASE + I2CR) = I2CR_IEN; + + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MSTA | I2CR_MTX; + ret = tx_byte(chip << 1); + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MTX; + + return ret; +} + +static int i2c_addr(uchar chip, uint addr, int alen) +{ + __REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */ + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MSTA | I2CR_MTX; + + if (tx_byte(chip << 1)) + return -1; + + while (alen--) + if (tx_byte((addr >> (alen * 8)) & 0xff)) + return -1; + return 0; +} + +int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) +{ + int timeout = 10000; + int ret; + + DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len); + + if (i2c_addr(chip, addr, alen)) { + printf("i2c_addr failed\n"); + return -1; + } + + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MSTA | I2CR_MTX | I2CR_RSTA; + + if (tx_byte(chip << 1 | 1)) + return -1; + + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MSTA | ((len == 1) ? I2CR_TX_NO_AK : 0); + + ret = __REG16(I2C_BASE + I2DR); + + while (len--) { + if ((ret = rx_byte()) < 0) + return -1; + *buf++ = ret; + if (len <= 1) + __REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MSTA | I2CR_TX_NO_AK; + } + + wait_busy(); + + __REG16(I2C_BASE + I2CR) = I2CR_IEN; + + while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout) + udelay(1); + + return 0; +} + +int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) +{ + int timeout = 10000; + DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len); + + if (i2c_addr(chip, addr, alen)) + return -1; + + while (len--) + if (tx_byte(*buf++)) + return -1; + + __REG16(I2C_BASE + I2CR) = I2CR_IEN; + + while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout) + udelay(1); + + return 0; +} + +#endif /* CONFIG_HARD_I2C */

In message Pine.LNX.4.64.0803261812210.5050@axis700.grange you wrote:
From: Sascha Hauer s.hauer@pengutronix.de
This patch adds an i2c driver for Freescale i.MX processors
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Guennadi Liakhovetski lg@denx.de
Pulled in directly, thanks.
Best regards,
Wolfgang Denk

On 20:40 Wed 26 Mar , Guennadi Liakhovetski wrote:
From: Sascha Hauer s.hauer@pengutronix.de
This patch adds an i2c driver for Freescale i.MX processors
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Guennadi Liakhovetski lg@denx.de
No changes wrt v1
drivers/i2c/Makefile | 1 + drivers/i2c/mxc_i2c.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 0 deletions(-) create mode 100644 drivers/i2c/mxc_i2c.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 29d6c03..071ef00 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -29,6 +29,7 @@ COBJS-y += fsl_i2c.o COBJS-y += omap1510_i2c.o COBJS-y += omap24xx_i2c.o COBJS-y += tsi108_i2c.o +COBJS-y += mxc_i2c.o
COBJS := $(COBJS-y) SRCS := $(COBJS:.o=.c) diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c new file mode 100644 index 0000000..a218329 --- /dev/null +++ b/drivers/i2c/mxc_i2c.c @@ -0,0 +1,202 @@ +/*
- i2c driver for Freescale mx31
- (c) 2007 Pengutronix, Sascha Hauer s.hauer@pengutronix.de
- See file CREDITS for list of people who contributed to this
- project.
- 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
- */
+#include <common.h>
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Wolfgang, Is it possible to NACK all patch that add new driver that do this
Best Regards, J.

On Apr 14, 2008, at 1:46 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 20:40 Wed 26 Mar , Guennadi Liakhovetski wrote:
From: Sascha Hauer s.hauer@pengutronix.de
This patch adds an i2c driver for Freescale i.MX processors
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Guennadi Liakhovetski lg@denx.de
No changes wrt v1
drivers/i2c/Makefile | 1 + drivers/i2c/mxc_i2c.c | 202 +++++++++++++++++++++++++++++++++++++++ ++++++++++ 2 files changed, 203 insertions(+), 0 deletions(-) create mode 100644 drivers/i2c/mxc_i2c.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 29d6c03..071ef00 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -29,6 +29,7 @@ COBJS-y += fsl_i2c.o COBJS-y += omap1510_i2c.o COBJS-y += omap24xx_i2c.o COBJS-y += tsi108_i2c.o +COBJS-y += mxc_i2c.o
COBJS := $(COBJS-y) SRCS := $(COBJS:.o=.c) diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c new file mode 100644 index 0000000..a218329 --- /dev/null +++ b/drivers/i2c/mxc_i2c.c @@ -0,0 +1,202 @@ +/*
- i2c driver for Freescale mx31
- (c) 2007 Pengutronix, Sascha Hauer s.hauer@pengutronix.de
- See file CREDITS for list of people who contributed to this
- project.
- 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
- */
+#include <common.h>
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Wolfgang, Is it possible to NACK all patch that add new driver that do this
Also, I believe the imx I2C is the same as the PPC I2C HW. I havent verified this but the register set looks oddly identical.
- k

In message 20080414064659.GB17663@game.jcrosoft.org you wrote:
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Hm... While I agree with the simple standard case of a single "#ifdef", I'm not so 100% sure it is a good thing to add such complex expressions to the Makefile.
Aren't we just moving the #ifdef hell from one place to another?
Wolfgang, Is it possible to NACK all patch that add new driver that do this
You mean, automatically? I would not know how to do that.
Best regards,
Wolfgang Denk

On Mon, 14 Apr 2008, Wolfgang Denk wrote:
In message 20080414064659.GB17663@game.jcrosoft.org you wrote:
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Hm... While I agree with the simple standard case of a single "#ifdef", I'm not so 100% sure it is a good thing to add such complex expressions to the Makefile.
Aren't we just moving the #ifdef hell from one place to another?
Wouldn't it be logical to assume, that if CONFIG_I2C_MXC is defined, CONFIG_HARD_I2C is meant too? So, we could just put in i2c.h
#ifdef CONFIG_I2C_MXC #define CONFIG_HARD_I2C #endif
And then use the simple
OBJC-$(CONFIG_I2C_MXC) += ...
Incremental patch?
in the Makefile? I personally do prefer when unneeded .c files do not get compiled at all, rather than compiled to "0"-byte big objects. Makes the build process and the resulting tree look cleaner, and the image a bit smaller.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de

On Mon, Apr 14, 2008 at 5:28 PM, Guennadi Liakhovetski lg@denx.de wrote:
On Mon, 14 Apr 2008, Wolfgang Denk wrote:
In message 20080414064659.GB17663@game.jcrosoft.org you wrote:
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Hm... While I agree with the simple standard case of a single "#ifdef", I'm not so 100% sure it is a good thing to add such complex expressions to the Makefile.
Aren't we just moving the #ifdef hell from one place to another?
Wouldn't it be logical to assume, that if CONFIG_I2C_MXC is defined, CONFIG_HARD_I2C is meant too? So, we could just put in i2c.h
#ifdef CONFIG_I2C_MXC #define CONFIG_HARD_I2C #endif
And then use the simple
OBJC-$(CONFIG_I2C_MXC) += ...
Yes, this is the way I'd solve the problem.
Cheers, g.

On 23:21 Mon 14 Apr , Grant Likely wrote:
On Mon, Apr 14, 2008 at 5:28 PM, Guennadi Liakhovetski lg@denx.de wrote:
On Mon, 14 Apr 2008, Wolfgang Denk wrote:
In message 20080414064659.GB17663@game.jcrosoft.org you wrote:
+#if defined(CONFIG_HARD_I2C) && defined (CONFIG_I2C_MXC)
Please stop to define this here, move it the Makefile
Hm... While I agree with the simple standard case of a single "#ifdef", I'm not so 100% sure it is a good thing to add such complex expressions to the Makefile.
Aren't we just moving the #ifdef hell from one place to another?
Wouldn't it be logical to assume, that if CONFIG_I2C_MXC is defined, CONFIG_HARD_I2C is meant too? So, we could just put in i2c.h
#ifdef CONFIG_I2C_MXC #define CONFIG_HARD_I2C #endif
And then use the simple
OBJC-$(CONFIG_I2C_MXC) += ...
Yes, this is the way I'd solve the problem.
I dont't like the idean to modify the i2c.h for each i2c drivers implemetation. I prefer to keep it into the config file and move it to the Kconfig. I hope that I've time the next merge window to send it.
Best Regards, J.

In message Pine.LNX.4.64.0804150024500.5332@axis700.grange you wrote:
Aren't we just moving the #ifdef hell from one place to another?
Wouldn't it be logical to assume, that if CONFIG_I2C_MXC is defined, CONFIG_HARD_I2C is meant too? So, we could just put in i2c.h
Well, what exactly is CONFIG_I2C_MXC supposed to mean? Sorry for asking, but I cannot find it documented anywhere (hint! hint! This needs to be fixed!). I tend to believe that in it's current form it's redundant, meaning that we have both CONFIG_CMD_I2C and CONFIG_MX31.
#ifdef CONFIG_I2C_MXC #define CONFIG_HARD_I2C #endif
Depending on the intended meaning of CONFIG_I2C_MXC there could also be someone who wants to use soft-I2C on that processors.
in the Makefile? I personally do prefer when unneeded .c files do not get compiled at all, rather than compiled to "0"-byte big objects. Makes the build process and the resulting tree look cleaner, and the image a bit smaller.
Me too.
But then, I definitely want to keep the Makefiles simple and clean, too.
Best regards,
Wolfgang Denk

On Tue, 15 Apr 2008, Wolfgang Denk wrote:
In message Pine.LNX.4.64.0804150024500.5332@axis700.grange you wrote:
Aren't we just moving the #ifdef hell from one place to another?
Wouldn't it be logical to assume, that if CONFIG_I2C_MXC is defined, CONFIG_HARD_I2C is meant too? So, we could just put in i2c.h
Well, what exactly is CONFIG_I2C_MXC supposed to mean? Sorry for asking, but I cannot find it documented anywhere (hint! hint! This needs to be fixed!). I tend to believe that in it's current form it's redundant, meaning that we have both CONFIG_CMD_I2C and CONFIG_MX31.
Ok, looking at examples, e.g., at cpu/mpc824x/drivers/i2c/i2c.c, it looks like u-boot presumes, that a system may only want to use one (hardware) i2c driver. I don't understand why this restriction is made, but if we want to keep it, we could just drop CONFIG_I2C_MXC and just use CONFIG_HARD_I2C. As for redundancy - that's exactly the reason why I don't think it is redundant. I can imagine an i.MX31 based system with another hardware i2c controller, wanting to use the external one and not needing the built-in one.
My preference would be to let CONFIG_I2C_MXC mean "use the mxc_i2c.c driver for I2C controllers like those on i.MX* / MXC SoCs from Freescale," similar to how i2c host drivers under drivers/i2c use their (also not always documented) config options:
CONFIG_TSI108_I2C, CONFIG_DRIVER_OMAP24XX_I2C, CONFIG_DRIVER_OMAP1510_I2C
and they just define both the hartdware-specific config and CONFIG_HARD_I2C in their *_config.h, but check only for the specific one in the .c file. So, shall I just remove the ifdef from .c, add
OBJS-$(CONFIG_I2C_MXC) += ...
to the Makefile and add it to the README?
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de

On Tue, Apr 15, 2008 at 12:28:21AM +0200, Guennadi Liakhovetski wrote:
I personally do prefer when unneeded .c files do not get compiled at all, rather than compiled to "0"-byte big objects. Makes the build process and the resulting tree look cleaner, and the image a bit smaller.
That's how it is done in u-boot-v2, btw.
Robert
participants (6)
-
Grant Likely
-
Guennadi Liakhovetski
-
Jean-Christophe PLAGNIOL-VILLARD
-
Kumar Gala
-
Robert Schwebel
-
Wolfgang Denk