[U-Boot] [PATCH V3 0/4] Improve I2C for orion5x, kirkwood and ED Mini V2

[U-Boot] [PATCH V3 0/4] Improve I2C for orion5x, kirkwood and ED Mini V2

[Albert Aribaud]

SUMMARY:

This patchset improves the driver for the Marvell TWSI interface found
on orion5x and kirkwood SoCs and adds I2C support to the orion5x-based
ED Mini V2 board. The mvtwsi driver is a complete rewrite, 50% shorter
in source code lines, 25% smaller in object size, much simpler and way
faster than the kirkwood_i2c driver it replaces.

PATCH SET:

Patch 1 removes dependencies on kirwkood_i2c from suen3, the only
board that mentioned it. Patch 2 renames the driver from kirkwood_i2c
to mvtwsi. Patch 3 replaces the old code with the new one. Patch 4
adds support for I2C in orion5x-based ED Mini V2.

TESTS:

This driver has been tested on an ED Mini V2 with basic u-boot i2c
commands, on an 5C372a RTC and an HT24LC08 1 KB eeprom (read+write).

HISTORY:

V1:	Initial submission as an addition rather than a replacement.

V2:	Fixed copyright line.
	Made mvtwsi a replacement for kirkwood_i2c.
	Made patches checkpatch-clean: 0 errors, 0 warnings.
	Various cosmetic changes.
	Removed useless i2c_end() function.

V3:	Reduced line lengths below 78 characters.
	Removed blank line after function description block comment.

Albert Aribaud (4):
  suen3: remove CONFIG_HARD_I2C and related defines
  i2c: rename kirkwood_i2c to mvtwsi
  i2c: rewrite mvtwsi, support orion5x and kirkwood
  edminiv2: add I2C support using mvtwsi driver

 drivers/i2c/Makefile       |    2 +-
 drivers/i2c/kirkwood_i2c.c |  496 --------------------------------------------
 drivers/i2c/mvtwsi.c       |  422 +++++++++++++++++++++++++++++++++++++
 include/configs/edminiv2.h |    9 +
 include/configs/km_arm.h   |    8 -
 5 files changed, 432 insertions(+), 505 deletions(-)
 delete mode 100644 drivers/i2c/kirkwood_i2c.c
 create mode 100644 drivers/i2c/mvtwsi.c

[U-Boot] [PATCH V3 1/4] suen3: remove CONFIG_HARD_I2C and related defines

[Albert Aribaud]

These are not used on this board, which uses soft I2C instead.

Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
---
 include/configs/km_arm.h |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/include/configs/km_arm.h b/include/configs/km_arm.h
index 6519c90..1617e69 100644
--- a/include/configs/km_arm.h
+++ b/include/configs/km_arm.h
@@ -142,16 +142,8 @@
 /*
  * I2C related stuff
  */
-#undef	CONFIG_HARD_I2C		/* I2C with hardware support */
 #define	CONFIG_SOFT_I2C		/* I2C bit-banged	*/
 
-#if defined(CONFIG_HARD_I2C)
-#define	CONFIG_I2C_KIRKWOOD
-#define	CONFIG_I2C_KW_REG_BASE		KW_TWSI_BASE
-#define	CONFIG_SYS_I2C_SLAVE		0x0
-#define	CONFIG_SYS_I2C_SPEED		100000
-#endif
-
 #define	CONFIG_KIRKWOOD_GPIO		/* Enable GPIO Support */
 #if defined(CONFIG_SOFT_I2C)
 #ifndef __ASSEMBLY__
-- 
1.7.1

[U-Boot] [PATCH V3 2/4] i2c: rename kirkwood_i2c to mvtwsi

[Albert Aribaud]

This driver is not kirkwood-specific and can also be used
e.g. by orion5x. Rename to a SoC-neutral name.

Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
---
 drivers/i2c/Makefile                     |    2 +-
 drivers/i2c/{kirkwood_i2c.c => mvtwsi.c} |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename drivers/i2c/{kirkwood_i2c.c => mvtwsi.c} (100%)

diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index d2c2515..ae04d53 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -28,7 +28,7 @@ LIB	:= $(obj)libi2c.a
 COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
 COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
 COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
-COBJS-$(CONFIG_I2C_KIRKWOOD) += kirkwood_i2c.o
+COBJS-$(CONFIG_I2C_DRIVER_MVTWSI) += mvtwsi.o
 COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
 COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
 COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
diff --git a/drivers/i2c/kirkwood_i2c.c b/drivers/i2c/mvtwsi.c
similarity index 100%
rename from drivers/i2c/kirkwood_i2c.c
rename to drivers/i2c/mvtwsi.c
-- 
1.7.1

[U-Boot] [PATCH V3 2/4] i2c: rename kirkwood_i2c to mvtwsi

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> Sent: Friday, August 27, 2010 11:42 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH V3 2/4] i2c: rename kirkwood_i2c to mvtwsi
> 
> This driver is not kirkwood-specific and can also be used
> e.g. by orion5x. Rename to a SoC-neutral name.
> 
> Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
> ---
>  drivers/i2c/Makefile                     |    2 +-
>  drivers/i2c/{kirkwood_i2c.c => mvtwsi.c} |    0
>  2 files changed, 1 insertions(+), 1 deletions(-)
>  rename drivers/i2c/{kirkwood_i2c.c => mvtwsi.c} (100%)
> 
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index d2c2515..ae04d53 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -28,7 +28,7 @@ LIB	:= $(obj)libi2c.a
>  COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
> -COBJS-$(CONFIG_I2C_KIRKWOOD) += kirkwood_i2c.o
> +COBJS-$(CONFIG_I2C_DRIVER_MVTWSI) += mvtwsi.o

This makefile needs separate spin for cleanup
But I think _DRIVER_ can be removed here to make it short at this moment

Regards..
Prafulla ..
 
>  COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
>  COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
>  COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
> diff --git a/drivers/i2c/kirkwood_i2c.c b/drivers/i2c/mvtwsi.c
> similarity index 100%
> rename from drivers/i2c/kirkwood_i2c.c
> rename to drivers/i2c/mvtwsi.c
> -- 
> 1.7.1
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support orion5x and kirkwood

[Albert Aribaud]

This rewrite of the mvtwsi driver is 25% smaller and much
faster and simpler than the previous code.

Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
---
 drivers/i2c/mvtwsi.c |  742 +++++++++++++++++++++++---------------------------
 1 files changed, 334 insertions(+), 408 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index a4409be..e7f08ac 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -1,10 +1,7 @@
 /*
- * Driver for the i2c controller on the Marvell line of host bridges
- * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, Orion SoC family),
- * and Kirkwood family.
+ * Driver for the TWSI (i2c) controller on the Marvell orion5x
  *
- * Based on:
- * Author: Mark A. Greer <mgreer@mvista.com>
+ * Author: Albert Aribaud <albert.aribaud@free.fr>
  * 2005 (c) MontaVista, Software, Inc.
  *
  * See file CREDITS for list of people who contributed to this
@@ -24,473 +21,402 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
  * MA 02110-1301 USA
- *
- * ported from Linux to u-boot
- * (C) Copyright 2009
- * Heiko Schocher, DENX Software Engineering, hs at denx.de.
  */
+
 #include <common.h>
 #include <i2c.h>
-#include <asm/arch/kirkwood.h>
 #include <asm/errno.h>
 #include <asm/io.h>
 
-DECLARE_GLOBAL_DATA_PTR;
+/*
+ * include a file that will provide CONFIG_I2C_MVTWSI_BASE
+ * and possibly other settings
+ */
 
-static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = 0;
-#if defined(CONFIG_I2C_MUX)
-static unsigned int i2c_bus_num_mux __attribute__ ((section ("data"))) = 0;
+#if defined(CONFIG_ORION5X)
+#include <asm/arch/orion5x.h>
+#else
+#error Driver mvtwsi not supported by SoC or board
 #endif
 
-/* Register defines */
-#define	KW_I2C_REG_SLAVE_ADDR			0x00
-#define	KW_I2C_REG_DATA				0x04
-#define	KW_I2C_REG_CONTROL			0x08
-#define	KW_I2C_REG_STATUS			0x0c
-#define	KW_I2C_REG_BAUD				0x0c
-#define	KW_I2C_REG_EXT_SLAVE_ADDR		0x10
-#define	KW_I2C_REG_SOFT_RESET			0x1c
-
-#define	KW_I2C_REG_CONTROL_ACK			0x00000004
-#define	KW_I2C_REG_CONTROL_IFLG			0x00000008
-#define	KW_I2C_REG_CONTROL_STOP			0x00000010
-#define	KW_I2C_REG_CONTROL_START		0x00000020
-#define	KW_I2C_REG_CONTROL_TWSIEN		0x00000040
-#define	KW_I2C_REG_CONTROL_INTEN		0x00000080
-
-/* Ctlr status values */
-#define	KW_I2C_STATUS_BUS_ERR			0x00
-#define	KW_I2C_STATUS_MAST_START		0x08
-#define	KW_I2C_STATUS_MAST_REPEAT_START		0x10
-#define	KW_I2C_STATUS_MAST_WR_ADDR_ACK		0x18
-#define	KW_I2C_STATUS_MAST_WR_ADDR_NO_ACK	0x20
-#define	KW_I2C_STATUS_MAST_WR_ACK		0x28
-#define	KW_I2C_STATUS_MAST_WR_NO_ACK		0x30
-#define	KW_I2C_STATUS_MAST_LOST_ARB		0x38
-#define	KW_I2C_STATUS_MAST_RD_ADDR_ACK		0x40
-#define	KW_I2C_STATUS_MAST_RD_ADDR_NO_ACK	0x48
-#define	KW_I2C_STATUS_MAST_RD_DATA_ACK		0x50
-#define	KW_I2C_STATUS_MAST_RD_DATA_NO_ACK	0x58
-#define	KW_I2C_STATUS_MAST_WR_ADDR_2_ACK	0xd0
-#define	KW_I2C_STATUS_MAST_WR_ADDR_2_NO_ACK	0xd8
-#define	KW_I2C_STATUS_MAST_RD_ADDR_2_ACK	0xe0
-#define	KW_I2C_STATUS_MAST_RD_ADDR_2_NO_ACK	0xe8
-#define	KW_I2C_STATUS_NO_STATUS			0xf8
-
-/* Driver states */
-enum {
-	KW_I2C_STATE_INVALID,
-	KW_I2C_STATE_IDLE,
-	KW_I2C_STATE_WAITING_FOR_START_COND,
-	KW_I2C_STATE_WAITING_FOR_ADDR_1_ACK,
-	KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK,
-	KW_I2C_STATE_WAITING_FOR_SLAVE_ACK,
-	KW_I2C_STATE_WAITING_FOR_SLAVE_DATA,
-};
+/*
+ * TWSI register structure
+ */
 
-/* Driver actions */
-enum {
-	KW_I2C_ACTION_INVALID,
-	KW_I2C_ACTION_CONTINUE,
-	KW_I2C_ACTION_SEND_START,
-	KW_I2C_ACTION_SEND_ADDR_1,
-	KW_I2C_ACTION_SEND_ADDR_2,
-	KW_I2C_ACTION_SEND_DATA,
-	KW_I2C_ACTION_RCV_DATA,
-	KW_I2C_ACTION_RCV_DATA_STOP,
-	KW_I2C_ACTION_SEND_STOP,
+struct  mvtwsi_registers {
+	u32 slave_address;
+	u32 data;
+	u32 control;
+	union {
+		u32 status;	/* when reading */
+		u32 baudrate;	/* when writing */
+	};
+	u32 extended_slave_address;
+	u32 reserved[2];
+	u32 soft_reset;
 };
 
-/* defines to get compatible with Linux driver */
-#define IRQ_NONE	0x0
-#define IRQ_HANDLED	0x01
+/*
+ * Control register fields
+ */
 
-#define I2C_M_TEN	0x01
-#define I2C_M_RD	0x02
-#define	I2C_M_REV_DIR_ADDR	0x04;
+#define	MVTWSI_CONTROL_ACK	0x00000004
+#define	MVTWSI_CONTROL_IFLG	0x00000008
+#define	MVTWSI_CONTROL_STOP	0x00000010
+#define	MVTWSI_CONTROL_START	0x00000020
+#define	MVTWSI_CONTROL_TWSIEN	0x00000040
+#define	MVTWSI_CONTROL_INTEN	0x00000080
 
-struct i2c_msg {
-	u32	addr;
-	u32	flags;
-	u8	*buf;
-	u32	len;
-};
+/*
+ * Status register values -- only those expected in normal master
+ * operation on non-10-bit-address devices; whatever status we don't
+ * expect in nominal conditions (bus errors, arbitration losses,
+ * missing ACKs...) we just pass back to the caller as an error
+ * code.
+ */
 
-struct kirkwood_i2c_data {
-	int			irq;
-	u32			state;
-	u32			action;
-	u32			aborting;
-	u32			cntl_bits;
-	void			*reg_base;
-	u32			reg_base_p;
-	u32			reg_size;
-	u32			addr1;
-	u32			addr2;
-	u32			bytes_left;
-	u32			byte_posn;
-	u32			block;
-	int			rc;
-	u32			freq_m;
-	u32			freq_n;
-	struct i2c_msg		*msg;
-};
+#define	MVTWSI_STATUS_START		0x08
+#define	MVTWSI_STATUS_REPEATED_START	0x10
+#define	MVTWSI_STATUS_ADDR_W_ACK	0x18
+#define	MVTWSI_STATUS_DATA_W_ACK	0x28
+#define	MVTWSI_STATUS_ADDR_R_ACK	0x40
+#define	MVTWSI_STATUS_ADDR_R_NAK	0x48
+#define	MVTWSI_STATUS_DATA_R_ACK	0x50
+#define	MVTWSI_STATUS_DATA_R_NAK	0x58
+#define	MVTWSI_STATUS_IDLE		0xF8
+
+/*
+ * The single instance of the controller we'll be dealing with
+ */
 
-static struct kirkwood_i2c_data __drv_data __attribute__ ((section (".data")));
-static struct kirkwood_i2c_data *drv_data = &__drv_data;
-static struct i2c_msg __i2c_msg __attribute__ ((section (".data")));
-static struct i2c_msg *kirkwood_i2c_msg = &__i2c_msg;
+static struct  mvtwsi_registers *twsi =
+	(struct  mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE;
 
 /*
- *****************************************************************************
- *
- *	Finite State Machine & Interrupt Routines
- *
- *****************************************************************************
+ * Returned statuses are 0 for success and nonzero otherwise.
+ * Currently, cmd_i2c and cmd_eeprom do not interpret an error status.
+ * Thus to ease debugging, the return status contains some debug info:
+ * - bits 31..24 are error class: 1 is timeout, 2 is 'status mismatch'.
+ * - bits 23..16 are the last value of the control register.
+ * - bits 15..8 are the last value of the status register.
+ * - bits 7..0 are the expected value of the status register.
  */
 
-static inline int abs(int n)
-{
-	 if(n >= 0)
-		return n;
-	else
-		return n * -1;
-}
+#define MVTWSI_ERROR_WRONG_STATUS	0x01
+#define MVTWSI_ERROR_TIMEOUT		0x02
 
-static void kirkwood_calculate_speed(int speed)
-{
-	int	calcspeed;
-	int	diff;
-	int	best_diff = CONFIG_SYS_TCLK;
-	int	best_speed = 0;
-	int	m, n;
-	int	tmp[8] = {2, 4, 8, 16, 32, 64, 128, 256};
+#define MVTWSI_ERROR(ec, lc, ls, es) (((ec << 24) & 0xFF000000) | \
+	((lc << 16) & 0x00FF0000) | ((ls<<8) & 0x0000FF00) | (es & 0xFF))
 
-	for (n = 0; n < 8; n++) {
-		for (m = 0; m < 16; m++) {
-			calcspeed = CONFIG_SYS_TCLK / (10 * (m + 1) * tmp[n]);
-			diff = abs((speed - calcspeed));
-			if ( diff < best_diff) {
-				best_diff = diff;
-				best_speed = calcspeed;
-				drv_data->freq_m = m;
-				drv_data->freq_n = n;
-			}
+/*
+ * Wait for IFLG to raise, or return 'timeout'; then if status is as expected,
+ * return 0 (ok) or return 'wrong status'.
+ */
+static int twsi_wait(int expected_status)
+{
+	int control, status;
+	int timeout = 1000;
+
+	do {
+		control = readl(&twsi->control);
+		if (control & MVTWSI_CONTROL_IFLG) {
+			status = readl(&twsi->status);
+			if (status == expected_status)
+				return 0;
+			else
+				return MVTWSI_ERROR(
+					MVTWSI_ERROR_WRONG_STATUS,
+					control, status, expected_status);
 		}
-	}
+		udelay(10); /* one clock cycle at 100 kHz */
+	} while (timeout--);
+	status = readl(&twsi->status);
+	return MVTWSI_ERROR(
+		MVTWSI_ERROR_TIMEOUT, control, status, expected_status);
 }
 
-/* Reset hardware and initialize FSM */
-static void
-kirkwood_i2c_hw_init(int speed, int slaveadd)
+/*
+ * These flags are ORed to any write to the control register
+ * They allow global setting of TWSIEN and ACK.
+ * By default none are set.
+ * twsi_start() sets TWSIEN (in case the controller was disabled)
+ * twsi_recv() sets ACK or resets it depending on expected status.
+ */
+static u8 twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
+
+/*
+ * Assert the START condition, either in a single I2C transaction
+ * or inside back-to-back ones (repeated starts).
+ */
+static int twsi_start(int expected_status)
 {
-	drv_data->state = KW_I2C_STATE_IDLE;
-
-	kirkwood_calculate_speed(speed);
-	writel(0, CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_SOFT_RESET);
-	writel((((drv_data->freq_m & 0xf) << 3) | (drv_data->freq_n & 0x7)),
-		CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_BAUD);
-	writel(slaveadd, CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_SLAVE_ADDR);
-	writel(0, CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_EXT_SLAVE_ADDR);
-	writel(KW_I2C_REG_CONTROL_TWSIEN | KW_I2C_REG_CONTROL_STOP,
-		CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
+	/* globally set TWSIEN in case it was not */
+	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
+	/* assert START */
+	writel(twsi_control_flags | MVTWSI_CONTROL_START, &twsi->control);
+	/* wait for controller to process START */
+	return twsi_wait(expected_status);
 }
 
-static void
-kirkwood_i2c_fsm(u32 status)
+/*
+ * Send a byte (i2c address or data).
+ */
+static int twsi_send(u8 byte, int expected_status)
 {
-	/*
-	 * If state is idle, then this is likely the remnants of an old
-	 * operation that driver has given up on or the user has killed.
-	 * If so, issue the stop condition and go to idle.
-	 */
-	if (drv_data->state == KW_I2C_STATE_IDLE) {
-		drv_data->action = KW_I2C_ACTION_SEND_STOP;
-		return;
-	}
-
-	/* The status from the ctlr [mostly] tells us what to do next */
-	switch (status) {
-	/* Start condition interrupt */
-	case KW_I2C_STATUS_MAST_START: /* 0x08 */
-	case KW_I2C_STATUS_MAST_REPEAT_START: /* 0x10 */
-		drv_data->action = KW_I2C_ACTION_SEND_ADDR_1;
-		drv_data->state = KW_I2C_STATE_WAITING_FOR_ADDR_1_ACK;
-		break;
-
-	/* Performing a write */
-	case KW_I2C_STATUS_MAST_WR_ADDR_ACK: /* 0x18 */
-		if (drv_data->msg->flags & I2C_M_TEN) {
-			drv_data->action = KW_I2C_ACTION_SEND_ADDR_2;
-			drv_data->state =
-				KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK;
-			break;
-		}
-		/* FALLTHRU */
-	case KW_I2C_STATUS_MAST_WR_ADDR_2_ACK: /* 0xd0 */
-	case KW_I2C_STATUS_MAST_WR_ACK: /* 0x28 */
-		if ((drv_data->bytes_left == 0)
-				|| (drv_data->aborting
-					&& (drv_data->byte_posn != 0))) {
-			drv_data->action = KW_I2C_ACTION_SEND_STOP;
-			drv_data->state = KW_I2C_STATE_IDLE;
-		} else {
-			drv_data->action = KW_I2C_ACTION_SEND_DATA;
-			drv_data->state =
-				KW_I2C_STATE_WAITING_FOR_SLAVE_ACK;
-			drv_data->bytes_left--;
-		}
-		break;
-
-	/* Performing a read */
-	case KW_I2C_STATUS_MAST_RD_ADDR_ACK: /* 40 */
-		if (drv_data->msg->flags & I2C_M_TEN) {
-			drv_data->action = KW_I2C_ACTION_SEND_ADDR_2;
-			drv_data->state =
-				KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK;
-			break;
-		}
-		/* FALLTHRU */
-	case KW_I2C_STATUS_MAST_RD_ADDR_2_ACK: /* 0xe0 */
-		if (drv_data->bytes_left == 0) {
-			drv_data->action = KW_I2C_ACTION_SEND_STOP;
-			drv_data->state = KW_I2C_STATE_IDLE;
-			break;
-		}
-		/* FALLTHRU */
-	case KW_I2C_STATUS_MAST_RD_DATA_ACK: /* 0x50 */
-		if (status != KW_I2C_STATUS_MAST_RD_DATA_ACK)
-			drv_data->action = KW_I2C_ACTION_CONTINUE;
-		else {
-			drv_data->action = KW_I2C_ACTION_RCV_DATA;
-			drv_data->bytes_left--;
-		}
-		drv_data->state = KW_I2C_STATE_WAITING_FOR_SLAVE_DATA;
-
-		if ((drv_data->bytes_left == 1) || drv_data->aborting)
-			drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_ACK;
-		break;
-
-	case KW_I2C_STATUS_MAST_RD_DATA_NO_ACK: /* 0x58 */
-		drv_data->action = KW_I2C_ACTION_RCV_DATA_STOP;
-		drv_data->state = KW_I2C_STATE_IDLE;
-		break;
-
-	case KW_I2C_STATUS_MAST_WR_ADDR_NO_ACK: /* 0x20 */
-	case KW_I2C_STATUS_MAST_WR_NO_ACK: /* 30 */
-	case KW_I2C_STATUS_MAST_RD_ADDR_NO_ACK: /* 48 */
-		/* Doesn't seem to be a device at other end */
-		drv_data->action = KW_I2C_ACTION_SEND_STOP;
-		drv_data->state = KW_I2C_STATE_IDLE;
-		drv_data->rc = -ENODEV;
-		break;
-
-	default:
-		printf("kirkwood_i2c_fsm: Ctlr Error -- state: 0x%x, "
-			"status: 0x%x, addr: 0x%x, flags: 0x%x\n",
-			 drv_data->state, status, drv_data->msg->addr,
-			 drv_data->msg->flags);
-		drv_data->action = KW_I2C_ACTION_SEND_STOP;
-		kirkwood_i2c_hw_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
-		drv_data->rc = -EIO;
-	}
+	/* put byte in data register for sending */
+	writel(byte, &twsi->data);
+	/* clear any pending interrupt -- that'll cause sending */
+	writel(twsi_control_flags, &twsi->control);
+	/* wait for controller to receive byte and check ACK */
+	return twsi_wait(expected_status);
 }
 
-static void
-kirkwood_i2c_do_action(void)
+/*
+ * Receive a byte.
+ * Global mvtwsi_control_flags variable says if we should ack or nak.
+ */
+static int twsi_recv(u8 *byte)
 {
-	switch(drv_data->action) {
-	case KW_I2C_ACTION_CONTINUE:
-		writel(drv_data->cntl_bits,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_SEND_START:
-		writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_START,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_SEND_ADDR_1:
-		writel(drv_data->addr1,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
-		writel(drv_data->cntl_bits,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_SEND_ADDR_2:
-		writel(drv_data->addr2,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
-		writel(drv_data->cntl_bits,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_SEND_DATA:
-		writel(drv_data->msg->buf[drv_data->byte_posn++],
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
-		writel(drv_data->cntl_bits,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_RCV_DATA:
-		drv_data->msg->buf[drv_data->byte_posn++] =
-			readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
-		writel(drv_data->cntl_bits,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		break;
-
-	case KW_I2C_ACTION_RCV_DATA_STOP:
-		drv_data->msg->buf[drv_data->byte_posn++] =
-			readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
-		drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_INTEN;
-		writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_STOP,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		drv_data->block = 0;
-		break;
-
-	case KW_I2C_ACTION_INVALID:
-	default:
-		printf("kirkwood_i2c_do_action: Invalid action: %d\n",
-			drv_data->action);
-		drv_data->rc = -EIO;
-		/* FALLTHRU */
-	case KW_I2C_ACTION_SEND_STOP:
-		drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_INTEN;
-		writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_STOP,
-			CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		drv_data->block = 0;
-		break;
-	}
+	int expected_status, status;
+
+	/* compute expected status based on ACK bit in global control flags */
+	if (twsi_control_flags & MVTWSI_CONTROL_ACK)
+		expected_status = MVTWSI_STATUS_DATA_R_ACK;
+	else
+		expected_status = MVTWSI_STATUS_DATA_R_NAK;
+	/* acknowledge *previous state* and launch receive */
+	writel(twsi_control_flags, &twsi->control);
+	/* wait for controller to receive byte and assert ACK or NAK */
+	status = twsi_wait(expected_status);
+	/* if we did receive expected byte then store it */
+	if (status == 0)
+		*byte = readl(&twsi->data);
+	/* return status */
+	return status;
 }
 
-static	int
-kirkwood_i2c_intr(void)
+/*
+ * Assert the STOP condition.
+ * This is also used to force the bus back in idle (SDA=SCL=1).
+ */
+static int twsi_stop(int status)
 {
-	u32		status;
-	u32		ctrl;
-	int		rc = IRQ_NONE;
-
-	ctrl = readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-	while ((ctrl & KW_I2C_REG_CONTROL_IFLG) &&
-		(drv_data->rc == 0)) {
-		status = readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_STATUS);
-		kirkwood_i2c_fsm(status);
-		kirkwood_i2c_do_action();
-		rc = IRQ_HANDLED;
-		ctrl = readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
-		udelay(1000);
-	}
-	return rc;
+	int control, stop_status;
+	int timeout = 1000;
+
+	/* assert STOP */
+	control = MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_STOP;
+	writel(control, &twsi->control);
+	/* wait for IDLE; IFLG won't rise so twsi_wait() is no use. */
+	do {
+		stop_status = readl(&twsi->status);
+		if (stop_status == MVTWSI_STATUS_IDLE)
+			break;
+		udelay(10); /* one clock cycle at 100 kHz */
+	} while (timeout--);
+	control = readl(&twsi->control);
+	if (stop_status != MVTWSI_STATUS_IDLE)
+		if (status == 0)
+			status = MVTWSI_ERROR(
+				MVTWSI_ERROR_TIMEOUT,
+				control, status, MVTWSI_STATUS_IDLE);
+	return status;
 }
 
-static void
-kirkwood_i2c_doio(struct i2c_msg *msg)
-{
-	int	ret;
+/*
+ * Ugly formula to convert m and n values to a frequency comes from
+ * TWSI specifications
+ */
 
-	while ((drv_data->rc == 0) && (drv_data->state != KW_I2C_STATE_IDLE)) {
-		/* poll Status register */
-		ret = kirkwood_i2c_intr();
-		if (ret == IRQ_NONE)
-			udelay(10);
-	}
+#define TWSI_FREQUENCY(m, n) \
+	((u8) (CONFIG_SYS_TCLK / (10 * (m + 1) * 2 * (1 << n))))
+
+/*
+ * These are required to be reprogrammed before enabling the controller
+ * because a reset loses them.
+ * Default values come from the spec, but a twsi_reset will change them.
+ * twsi_slave_address left uninitialized lest checkpatch.pl complains.
+ */
+
+static u8 twsi_baud_rate = 0x44;
+static u8 twsi_actual_speed = TWSI_FREQUENCY(4, 4);
+static u8 twsi_slave_address;
+
+/*
+ * Reset controller.
+ * Called@end of i2c_init unsuccessful i2c transactions.
+ * Controller reset also resets the baud rate and slave address, so
+ * re-establish them.
+ */
+static void twsi_reset(void)
+{
+	/* ensure controller will be enabled by any twsi*() function */
+	twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
+	/* reset controller */
+	writel(0, &twsi->soft_reset);
+	/* wait 2 ms -- this is what the Marvell LSP does */
+	udelay(20000);
+	/* set baud rate */
+	writel(twsi_baud_rate, &twsi->baudrate);
+	/* set slave address even though we don't use it */
+	writel(twsi_slave_address, &twsi->slave_address);
+	writel(0, &twsi->extended_slave_address);
+	/* assert STOP but don't care for the result */
+	(void) twsi_stop(0);
 }
 
-static void
-kirkwood_i2c_prepare_for_io(struct i2c_msg *msg)
+/*
+ * I2C init called by cmd_i2c when doing 'i2c reset'.
+ * Sets baud to the highest possible value not exceeding requested one.
+ */
+void i2c_init(int requested_speed, int slaveadd)
 {
-	u32	dir = 0;
-
-	drv_data->msg = msg;
-	drv_data->byte_posn = 0;
-	drv_data->bytes_left = msg->len;
-	drv_data->aborting = 0;
-	drv_data->rc = 0;
-	/* in u-boot we use no IRQs */
-	drv_data->cntl_bits = KW_I2C_REG_CONTROL_ACK | KW_I2C_REG_CONTROL_TWSIEN;
-
-	if (msg->flags & I2C_M_RD)
-		dir = 1;
-	if (msg->flags & I2C_M_TEN) {
-		drv_data->addr1 = 0xf0 | (((u32)msg->addr & 0x300) >> 7) | dir;
-		drv_data->addr2 = (u32)msg->addr & 0xff;
-	} else {
-		drv_data->addr1 = ((u32)msg->addr & 0x7f) << 1 | dir;
-		drv_data->addr2 = 0;
+	int	tmp_speed, highest_speed, n, m;
+	int	baud = 0x44; /* value at controller reset */
+
+	/* use actual speed to collect progressively higher values */
+	highest_speed = 0;
+	/* compute m, n setting for highest speed not above requested speed */
+	for (n = 0; n < 8; n++) {
+		for (m = 0; m < 16; m++) {
+			tmp_speed = TWSI_FREQUENCY(m, n);
+			if ((tmp_speed <= requested_speed)
+			 && (tmp_speed > highest_speed)) {
+				highest_speed = tmp_speed;
+				baud = (m << 3) | n;
+			}
+		}
 	}
-	/* OK, no start it (from kirkwood_i2c_execute_msg())*/
-	drv_data->action = KW_I2C_ACTION_SEND_START;
-	drv_data->state = KW_I2C_STATE_WAITING_FOR_START_COND;
-	drv_data->block = 1;
-	kirkwood_i2c_do_action();
+	/* save baud rate and slave for later calls to twsi_reset */
+	twsi_baud_rate = baud;
+	twsi_actual_speed = highest_speed;
+	twsi_slave_address = slaveadd;
+	/* reset controller */
+	twsi_reset();
 }
 
-void
-i2c_init(int speed, int slaveadd)
+/*
+ * Begin I2C transaction with expected start status, at given address.
+ * Common to i2c_probe, i2c_read and i2c_write.
+ * Expected address status will derive from direction bit (bit 0) in addr.
+ */
+static int i2c_begin(int expected_start_status, u8 addr)
 {
-	kirkwood_i2c_hw_init(speed, slaveadd);
+	int status, expected_addr_status;
+
+	/* compute expected address status from direction bit in addr */
+	if (addr & 1) /* reading */
+		expected_addr_status = MVTWSI_STATUS_ADDR_R_ACK;
+	else /* writing */
+		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
+	/* assert START */
+	status = twsi_start(expected_start_status);
+	/* send out the address if the start went well */
+	if (status == 0)
+		status = twsi_send(addr, expected_addr_status);
+	/* return ok or status of first failure to caller */
+	return status;
 }
 
-int
-i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
+/*
+ * I2C probe called by cmd_i2c when doing 'i2c probe'.
+ * Begin read, nak data byte, end.
+ */
+int i2c_probe(uchar chip)
 {
-	kirkwood_i2c_msg->buf = data;
-	kirkwood_i2c_msg->len = length;
-	kirkwood_i2c_msg->addr = dev;
-	kirkwood_i2c_msg->flags = I2C_M_RD;
-
-	kirkwood_i2c_prepare_for_io(kirkwood_i2c_msg);
-	kirkwood_i2c_doio(kirkwood_i2c_msg);
-	return drv_data->rc;
+	u8 dummy_byte;
+	int status;
+
+	/* begin i2c read */
+	status = i2c_begin(MVTWSI_STATUS_START, (chip << 1) | 1);
+	/* dummy read was accepted: receive byte but NAK it. */
+	if (status == 0)
+		status = twsi_recv(&dummy_byte);
+	/* Stop transaction */
+	twsi_stop(0);
+	/* return 0 or status of first failure */
+	return status;
 }
 
-int
-i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
+/*
+ * I2C read called by cmd_i2c when doing 'i2c read' and by cmd_eeprom.c
+ * Begin write, send address byte(s), begin read, receive data bytes, end.
+ *
+ * NOTE: some EEPROMS want a stop right before the second start, while
+ * some will choke if it is there. Deciding which we should do is eeprom
+ * stuff, not i2c, but@the moment the APIs won't let us put it in
+ * cmd_eeprom, so we have to choose here, and for the moment that'll be
+ * a repeated start without a preceding stop.
+ */
+int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
 {
-	kirkwood_i2c_msg->buf = data;
-	kirkwood_i2c_msg->len = length;
-	kirkwood_i2c_msg->addr = dev;
-	kirkwood_i2c_msg->flags = 0;
-
-	kirkwood_i2c_prepare_for_io(kirkwood_i2c_msg);
-	kirkwood_i2c_doio(kirkwood_i2c_msg);
-	return drv_data->rc;
+	int status;
+
+	/* begin i2c write to send the address bytes */
+	status = i2c_begin(MVTWSI_STATUS_START, (dev << 1));
+	/* send addr bytes */
+	while ((status == 0) && alen--)
+		status = twsi_send(addr >> (8*alen),
+			MVTWSI_STATUS_DATA_W_ACK);
+	/* begin i2c read to receive eeprom data bytes */
+	if (status == 0)
+		status = i2c_begin(
+			MVTWSI_STATUS_REPEATED_START, (dev << 1) | 1);
+	/* prepare ACK if@least one byte must be received */
+	if (length > 0)
+		twsi_control_flags |= MVTWSI_CONTROL_ACK;
+	/* now receive actual bytes */
+	while ((status == 0) && length--) {
+		/* reset NAK if we if no more to read now */
+		if (length == 0)
+			twsi_control_flags &= ~MVTWSI_CONTROL_ACK;
+		/* read current byte */
+		status = twsi_recv(data++);
+	}
+	/* Stop transaction */
+	status = twsi_stop(status);
+	/* return 0 or status of first failure */
+	return status;
 }
 
-int
-i2c_probe(uchar chip)
+/*
+ * I2C write called by cmd_i2c when doing 'i2c write' and by cmd_eeprom.c
+ * Begin write, send address byte(s), send data bytes, end.
+ */
+int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 {
-	return i2c_read(chip, 0, 0, NULL, 0);
+	int status;
+
+	/* begin i2c write to send the eeprom adress bytes then data bytes */
+	status = i2c_begin(MVTWSI_STATUS_START, (dev << 1));
+	/* send addr bytes */
+	while ((status == 0) && alen--)
+		status = twsi_send(addr >> (8*alen),
+			MVTWSI_STATUS_DATA_W_ACK);
+	/* send data bytes */
+	while ((status == 0) && (length-- > 0))
+		status = twsi_send(*(data++), MVTWSI_STATUS_DATA_W_ACK);
+	/* Stop transaction */
+	status = twsi_stop(status);
+	/* return 0 or status of first failure */
+	return status;
 }
 
+/*
+ * Bus set routine: we only support bus 0.
+ */
 int i2c_set_bus_num(unsigned int bus)
 {
-#if defined(CONFIG_I2C_MUX)
-	if (bus < CONFIG_SYS_MAX_I2C_BUS) {
-		i2c_bus_num = bus;
-	} else {
-		int	ret;
-
-		ret = i2x_mux_select_mux(bus);
-		if (ret)
-			return ret;
-		i2c_bus_num = 0;
-	}
-	i2c_bus_num_mux = bus;
-#else
 	if (bus > 0) {
 		return -1;
 	}
-
-	i2c_bus_num = bus;
-#endif
 	return 0;
 }
 
+/*
+ * Bus get routine: hard-return bus 0.
+ */
 unsigned int i2c_get_bus_num(void)
 {
-#if defined(CONFIG_I2C_MUX)
-	return i2c_bus_num_mux;
-#else
-	return i2c_bus_num;
-#endif
+	return 0;
 }
-- 
1.7.1

[U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support orion5x and kirkwood

[Prafulla Wadaskar]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> Sent: Friday, August 27, 2010 11:42 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support
> orion5x and kirkwood
>
> This rewrite of the mvtwsi driver is 25% smaller and much
> faster and simpler than the previous code.
>
> Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
> ---
>  drivers/i2c/mvtwsi.c |  742
> +++++++++++++++++++++++---------------------------
>  1 files changed, 334 insertions(+), 408 deletions(-)
>
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index a4409be..e7f08ac 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -1,10 +1,7 @@
>  /*
> - * Driver for the i2c controller on the Marvell line of host bridges
> - * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, Orion SoC family),
> - * and Kirkwood family.
> + * Driver for the TWSI (i2c) controller on the Marvell orion5x

Don't you think Kirkwood is missing here ?

>   *
> - * Based on:
> - * Author: Mark A. Greer <mgreer@mvista.com>
> + * Author: Albert Aribaud <albert.aribaud@free.fr>
>   * 2005 (c) MontaVista, Software, Inc.

I hope this line is not there in earlier version, pls confirm..

>   *
>   * See file CREDITS for list of people who contributed to this
> @@ -24,473 +21,402 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
>   * MA 02110-1301 USA
> - *
> - * ported from Linux to u-boot
> - * (C) Copyright 2009
> - * Heiko Schocher, DENX Software Engineering, hs at denx.de.
>   */
> +
>  #include <common.h>
>  #include <i2c.h>
> -#include <asm/arch/kirkwood.h>
>  #include <asm/errno.h>
>  #include <asm/io.h>
>
> -DECLARE_GLOBAL_DATA_PTR;
> +/*
> + * include a file that will provide CONFIG_I2C_MVTWSI_BASE
> + * and possibly other settings
> + */
>
> -static unsigned int i2c_bus_num __attribute__ ((section
> (".data"))) = 0;
> -#if defined(CONFIG_I2C_MUX)
> -static unsigned int i2c_bus_num_mux __attribute__ ((section
> ("data"))) = 0;
> +#if defined(CONFIG_ORION5X)
> +#include <asm/arch/orion5x.h>
> +#else
> +#error Driver mvtwsi not supported by SoC or board
>  #endif
>
> -/* Register defines */
> -#define      KW_I2C_REG_SLAVE_ADDR                   0x00
> -#define      KW_I2C_REG_DATA                         0x04
> -#define      KW_I2C_REG_CONTROL                      0x08
> -#define      KW_I2C_REG_STATUS                       0x0c
> -#define      KW_I2C_REG_BAUD                         0x0c
> -#define      KW_I2C_REG_EXT_SLAVE_ADDR               0x10
> -#define      KW_I2C_REG_SOFT_RESET                   0x1c
> -
> -#define      KW_I2C_REG_CONTROL_ACK                  0x00000004
> -#define      KW_I2C_REG_CONTROL_IFLG                 0x00000008
> -#define      KW_I2C_REG_CONTROL_STOP                 0x00000010
> -#define      KW_I2C_REG_CONTROL_START                0x00000020
> -#define      KW_I2C_REG_CONTROL_TWSIEN               0x00000040
> -#define      KW_I2C_REG_CONTROL_INTEN                0x00000080
> -
> -/* Ctlr status values */
> -#define      KW_I2C_STATUS_BUS_ERR                   0x00
> -#define      KW_I2C_STATUS_MAST_START                0x08
> -#define      KW_I2C_STATUS_MAST_REPEAT_START         0x10
> -#define      KW_I2C_STATUS_MAST_WR_ADDR_ACK          0x18
> -#define      KW_I2C_STATUS_MAST_WR_ADDR_NO_ACK       0x20
> -#define      KW_I2C_STATUS_MAST_WR_ACK               0x28
> -#define      KW_I2C_STATUS_MAST_WR_NO_ACK            0x30
> -#define      KW_I2C_STATUS_MAST_LOST_ARB             0x38
> -#define      KW_I2C_STATUS_MAST_RD_ADDR_ACK          0x40
> -#define      KW_I2C_STATUS_MAST_RD_ADDR_NO_ACK       0x48
> -#define      KW_I2C_STATUS_MAST_RD_DATA_ACK          0x50
> -#define      KW_I2C_STATUS_MAST_RD_DATA_NO_ACK       0x58
> -#define      KW_I2C_STATUS_MAST_WR_ADDR_2_ACK        0xd0
> -#define      KW_I2C_STATUS_MAST_WR_ADDR_2_NO_ACK     0xd8
> -#define      KW_I2C_STATUS_MAST_RD_ADDR_2_ACK        0xe0
> -#define      KW_I2C_STATUS_MAST_RD_ADDR_2_NO_ACK     0xe8
> -#define      KW_I2C_STATUS_NO_STATUS                 0xf8
> -
> -/* Driver states */
> -enum {
> -     KW_I2C_STATE_INVALID,
> -     KW_I2C_STATE_IDLE,
> -     KW_I2C_STATE_WAITING_FOR_START_COND,
> -     KW_I2C_STATE_WAITING_FOR_ADDR_1_ACK,
> -     KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK,
> -     KW_I2C_STATE_WAITING_FOR_SLAVE_ACK,
> -     KW_I2C_STATE_WAITING_FOR_SLAVE_DATA,
> -};
> +/*
> + * TWSI register structure
> + */
>
> -/* Driver actions */
> -enum {
> -     KW_I2C_ACTION_INVALID,
> -     KW_I2C_ACTION_CONTINUE,
> -     KW_I2C_ACTION_SEND_START,
> -     KW_I2C_ACTION_SEND_ADDR_1,
> -     KW_I2C_ACTION_SEND_ADDR_2,
> -     KW_I2C_ACTION_SEND_DATA,
> -     KW_I2C_ACTION_RCV_DATA,
> -     KW_I2C_ACTION_RCV_DATA_STOP,
> -     KW_I2C_ACTION_SEND_STOP,
> +struct  mvtwsi_registers {
> +     u32 slave_address;
> +     u32 data;
> +     u32 control;
> +     union {
> +             u32 status;     /* when reading */
> +             u32 baudrate;   /* when writing */
> +     };
> +     u32 extended_slave_address;

smaller variable names should be preffered

> +     u32 reserved[2];
> +     u32 soft_reset;
>  };
>
> -/* defines to get compatible with Linux driver */
> -#define IRQ_NONE     0x0
> -#define IRQ_HANDLED  0x01
> +/*
> + * Control register fields
> + */
>
> -#define I2C_M_TEN    0x01
> -#define I2C_M_RD     0x02
> -#define      I2C_M_REV_DIR_ADDR      0x04;
> +#define      MVTWSI_CONTROL_ACK      0x00000004
> +#define      MVTWSI_CONTROL_IFLG     0x00000008
> +#define      MVTWSI_CONTROL_STOP     0x00000010
> +#define      MVTWSI_CONTROL_START    0x00000020
> +#define      MVTWSI_CONTROL_TWSIEN   0x00000040
> +#define      MVTWSI_CONTROL_INTEN    0x00000080
>
> -struct i2c_msg {
> -     u32     addr;
> -     u32     flags;
> -     u8      *buf;
> -     u32     len;
> -};
> +/*
> + * Status register values -- only those expected in normal master
> + * operation on non-10-bit-address devices; whatever status we don't
> + * expect in nominal conditions (bus errors, arbitration losses,
> + * missing ACKs...) we just pass back to the caller as an error
> + * code.
> + */
>
> -struct kirkwood_i2c_data {
> -     int                     irq;
> -     u32                     state;
> -     u32                     action;
> -     u32                     aborting;
> -     u32                     cntl_bits;
> -     void                    *reg_base;
> -     u32                     reg_base_p;
> -     u32                     reg_size;
> -     u32                     addr1;
> -     u32                     addr2;
> -     u32                     bytes_left;
> -     u32                     byte_posn;
> -     u32                     block;
> -     int                     rc;
> -     u32                     freq_m;
> -     u32                     freq_n;
> -     struct i2c_msg          *msg;
> -};
> +#define      MVTWSI_STATUS_START             0x08
> +#define      MVTWSI_STATUS_REPEATED_START    0x10
> +#define      MVTWSI_STATUS_ADDR_W_ACK        0x18
> +#define      MVTWSI_STATUS_DATA_W_ACK        0x28
> +#define      MVTWSI_STATUS_ADDR_R_ACK        0x40
> +#define      MVTWSI_STATUS_ADDR_R_NAK        0x48
> +#define      MVTWSI_STATUS_DATA_R_ACK        0x50
> +#define      MVTWSI_STATUS_DATA_R_NAK        0x58
> +#define      MVTWSI_STATUS_IDLE              0xF8
> +
> +/*
> + * The single instance of the controller we'll be dealing with
> + */
>
> -static struct kirkwood_i2c_data __drv_data __attribute__
> ((section (".data")));
> -static struct kirkwood_i2c_data *drv_data = &__drv_data;
> -static struct i2c_msg __i2c_msg __attribute__ ((section (".data")));
> -static struct i2c_msg *kirkwood_i2c_msg = &__i2c_msg;
> +static struct  mvtwsi_registers *twsi =
> +     (struct  mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE;
>
>  /*
> -
> **************************************************************
> ***************
> - *
> - *   Finite State Machine & Interrupt Routines
> - *
> -
> **************************************************************
> ***************
> + * Returned statuses are 0 for success and nonzero otherwise.
> + * Currently, cmd_i2c and cmd_eeprom do not interpret an
> error status.
> + * Thus to ease debugging, the return status contains some
> debug info:
> + * - bits 31..24 are error class: 1 is timeout, 2 is 'status
> mismatch'.
> + * - bits 23..16 are the last value of the control register.
> + * - bits 15..8 are the last value of the status register.
> + * - bits 7..0 are the expected value of the status register.
>   */
>
> -static inline int abs(int n)
> -{
> -      if(n >= 0)
> -             return n;
> -     else
> -             return n * -1;
> -}
> +#define MVTWSI_ERROR_WRONG_STATUS    0x01
> +#define MVTWSI_ERROR_TIMEOUT         0x02
>
> -static void kirkwood_calculate_speed(int speed)
> -{
> -     int     calcspeed;
> -     int     diff;
> -     int     best_diff = CONFIG_SYS_TCLK;
> -     int     best_speed = 0;
> -     int     m, n;
> -     int     tmp[8] = {2, 4, 8, 16, 32, 64, 128, 256};
> +#define MVTWSI_ERROR(ec, lc, ls, es) (((ec << 24) & 0xFF000000) | \
> +     ((lc << 16) & 0x00FF0000) | ((ls<<8) & 0x0000FF00) |
> (es & 0xFF))
>
> -     for (n = 0; n < 8; n++) {
> -             for (m = 0; m < 16; m++) {
> -                     calcspeed = CONFIG_SYS_TCLK / (10 * (m
> + 1) * tmp[n]);
> -                     diff = abs((speed - calcspeed));
> -                     if ( diff < best_diff) {
> -                             best_diff = diff;
> -                             best_speed = calcspeed;
> -                             drv_data->freq_m = m;
> -                             drv_data->freq_n = n;
> -                     }
> +/*
> + * Wait for IFLG to raise, or return 'timeout'; then if
> status is as expected,
> + * return 0 (ok) or return 'wrong status'.
> + */
> +static int twsi_wait(int expected_status)
> +{
> +     int control, status;
> +     int timeout = 1000;
> +
> +     do {
> +             control = readl(&twsi->control);
> +             if (control & MVTWSI_CONTROL_IFLG) {
> +                     status = readl(&twsi->status);
> +                     if (status == expected_status)
> +                             return 0;
> +                     else
> +                             return MVTWSI_ERROR(
> +                                     MVTWSI_ERROR_WRONG_STATUS,
> +                                     control, status,
> expected_status);
>               }
> -     }
> +             udelay(10); /* one clock cycle at 100 kHz */
> +     } while (timeout--);
> +     status = readl(&twsi->status);
> +     return MVTWSI_ERROR(
> +             MVTWSI_ERROR_TIMEOUT, control, status, expected_status);
>  }
>
> -/* Reset hardware and initialize FSM */
> -static void
> -kirkwood_i2c_hw_init(int speed, int slaveadd)
> +/*
> + * These flags are ORed to any write to the control register
> + * They allow global setting of TWSIEN and ACK.
> + * By default none are set.
> + * twsi_start() sets TWSIEN (in case the controller was disabled)
> + * twsi_recv() sets ACK or resets it depending on expected status.
> + */
> +static u8 twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
> +
> +/*
> + * Assert the START condition, either in a single I2C transaction
> + * or inside back-to-back ones (repeated starts).
> + */
> +static int twsi_start(int expected_status)
>  {
> -     drv_data->state = KW_I2C_STATE_IDLE;
> -
> -     kirkwood_calculate_speed(speed);
> -     writel(0, CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_SOFT_RESET);
> -     writel((((drv_data->freq_m & 0xf) << 3) |
> (drv_data->freq_n & 0x7)),
> -             CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_BAUD);
> -     writel(slaveadd, CONFIG_I2C_KW_REG_BASE +
> KW_I2C_REG_SLAVE_ADDR);
> -     writel(0, CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_EXT_SLAVE_ADDR);
> -     writel(KW_I2C_REG_CONTROL_TWSIEN | KW_I2C_REG_CONTROL_STOP,
> -             CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> +     /* globally set TWSIEN in case it was not */
> +     twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
> +     /* assert START */
> +     writel(twsi_control_flags | MVTWSI_CONTROL_START,
> &twsi->control);
> +     /* wait for controller to process START */
> +     return twsi_wait(expected_status);
>  }
>
> -static void
> -kirkwood_i2c_fsm(u32 status)
> +/*
> + * Send a byte (i2c address or data).
> + */
> +static int twsi_send(u8 byte, int expected_status)
>  {
> -     /*
> -      * If state is idle, then this is likely the remnants of an old
> -      * operation that driver has given up on or the user has killed.
> -      * If so, issue the stop condition and go to idle.
> -      */
> -     if (drv_data->state == KW_I2C_STATE_IDLE) {
> -             drv_data->action = KW_I2C_ACTION_SEND_STOP;
> -             return;
> -     }
> -
> -     /* The status from the ctlr [mostly] tells us what to do next */
> -     switch (status) {
> -     /* Start condition interrupt */
> -     case KW_I2C_STATUS_MAST_START: /* 0x08 */
> -     case KW_I2C_STATUS_MAST_REPEAT_START: /* 0x10 */
> -             drv_data->action = KW_I2C_ACTION_SEND_ADDR_1;
> -             drv_data->state = KW_I2C_STATE_WAITING_FOR_ADDR_1_ACK;
> -             break;
> -
> -     /* Performing a write */
> -     case KW_I2C_STATUS_MAST_WR_ADDR_ACK: /* 0x18 */
> -             if (drv_data->msg->flags & I2C_M_TEN) {
> -                     drv_data->action = KW_I2C_ACTION_SEND_ADDR_2;
> -                     drv_data->state =
> -                             KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK;
> -                     break;
> -             }
> -             /* FALLTHRU */
> -     case KW_I2C_STATUS_MAST_WR_ADDR_2_ACK: /* 0xd0 */
> -     case KW_I2C_STATUS_MAST_WR_ACK: /* 0x28 */
> -             if ((drv_data->bytes_left == 0)
> -                             || (drv_data->aborting
> -                                     && (drv_data->byte_posn
> != 0))) {
> -                     drv_data->action = KW_I2C_ACTION_SEND_STOP;
> -                     drv_data->state = KW_I2C_STATE_IDLE;
> -             } else {
> -                     drv_data->action = KW_I2C_ACTION_SEND_DATA;
> -                     drv_data->state =
> -                             KW_I2C_STATE_WAITING_FOR_SLAVE_ACK;
> -                     drv_data->bytes_left--;
> -             }
> -             break;
> -
> -     /* Performing a read */
> -     case KW_I2C_STATUS_MAST_RD_ADDR_ACK: /* 40 */
> -             if (drv_data->msg->flags & I2C_M_TEN) {
> -                     drv_data->action = KW_I2C_ACTION_SEND_ADDR_2;
> -                     drv_data->state =
> -                             KW_I2C_STATE_WAITING_FOR_ADDR_2_ACK;
> -                     break;
> -             }
> -             /* FALLTHRU */
> -     case KW_I2C_STATUS_MAST_RD_ADDR_2_ACK: /* 0xe0 */
> -             if (drv_data->bytes_left == 0) {
> -                     drv_data->action = KW_I2C_ACTION_SEND_STOP;
> -                     drv_data->state = KW_I2C_STATE_IDLE;
> -                     break;
> -             }
> -             /* FALLTHRU */
> -     case KW_I2C_STATUS_MAST_RD_DATA_ACK: /* 0x50 */
> -             if (status != KW_I2C_STATUS_MAST_RD_DATA_ACK)
> -                     drv_data->action = KW_I2C_ACTION_CONTINUE;
> -             else {
> -                     drv_data->action = KW_I2C_ACTION_RCV_DATA;
> -                     drv_data->bytes_left--;
> -             }
> -             drv_data->state = KW_I2C_STATE_WAITING_FOR_SLAVE_DATA;
> -
> -             if ((drv_data->bytes_left == 1) || drv_data->aborting)
> -                     drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_ACK;
> -             break;
> -
> -     case KW_I2C_STATUS_MAST_RD_DATA_NO_ACK: /* 0x58 */
> -             drv_data->action = KW_I2C_ACTION_RCV_DATA_STOP;
> -             drv_data->state = KW_I2C_STATE_IDLE;
> -             break;
> -
> -     case KW_I2C_STATUS_MAST_WR_ADDR_NO_ACK: /* 0x20 */
> -     case KW_I2C_STATUS_MAST_WR_NO_ACK: /* 30 */
> -     case KW_I2C_STATUS_MAST_RD_ADDR_NO_ACK: /* 48 */
> -             /* Doesn't seem to be a device at other end */
> -             drv_data->action = KW_I2C_ACTION_SEND_STOP;
> -             drv_data->state = KW_I2C_STATE_IDLE;
> -             drv_data->rc = -ENODEV;
> -             break;
> -
> -     default:
> -             printf("kirkwood_i2c_fsm: Ctlr Error -- state: 0x%x, "
> -                     "status: 0x%x, addr: 0x%x, flags: 0x%x\n",
> -                      drv_data->state, status, drv_data->msg->addr,
> -                      drv_data->msg->flags);
> -             drv_data->action = KW_I2C_ACTION_SEND_STOP;
> -             kirkwood_i2c_hw_init(CONFIG_SYS_I2C_SPEED,
> CONFIG_SYS_I2C_SLAVE);
> -             drv_data->rc = -EIO;
> -     }
> +     /* put byte in data register for sending */
> +     writel(byte, &twsi->data);
> +     /* clear any pending interrupt -- that'll cause sending */
> +     writel(twsi_control_flags, &twsi->control);
> +     /* wait for controller to receive byte and check ACK */
> +     return twsi_wait(expected_status);
>  }
>
> -static void
> -kirkwood_i2c_do_action(void)
> +/*
> + * Receive a byte.
> + * Global mvtwsi_control_flags variable says if we should ack or nak.
> + */
> +static int twsi_recv(u8 *byte)
>  {
> -     switch(drv_data->action) {
> -     case KW_I2C_ACTION_CONTINUE:
> -             writel(drv_data->cntl_bits,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_SEND_START:
> -             writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_START,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_SEND_ADDR_1:
> -             writel(drv_data->addr1,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
> -             writel(drv_data->cntl_bits,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_SEND_ADDR_2:
> -             writel(drv_data->addr2,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
> -             writel(drv_data->cntl_bits,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_SEND_DATA:
> -             writel(drv_data->msg->buf[drv_data->byte_posn++],
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
> -             writel(drv_data->cntl_bits,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_RCV_DATA:
> -             drv_data->msg->buf[drv_data->byte_posn++] =
> -                     readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
> -             writel(drv_data->cntl_bits,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             break;
> -
> -     case KW_I2C_ACTION_RCV_DATA_STOP:
> -             drv_data->msg->buf[drv_data->byte_posn++] =
> -                     readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_DATA);
> -             drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_INTEN;
> -             writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_STOP,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             drv_data->block = 0;
> -             break;
> -
> -     case KW_I2C_ACTION_INVALID:
> -     default:
> -             printf("kirkwood_i2c_do_action: Invalid action: %d\n",
> -                     drv_data->action);
> -             drv_data->rc = -EIO;
> -             /* FALLTHRU */
> -     case KW_I2C_ACTION_SEND_STOP:
> -             drv_data->cntl_bits &= ~KW_I2C_REG_CONTROL_INTEN;
> -             writel(drv_data->cntl_bits | KW_I2C_REG_CONTROL_STOP,
> -                     CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -             drv_data->block = 0;
> -             break;
> -     }
> +     int expected_status, status;
> +
> +     /* compute expected status based on ACK bit in global
> control flags */
> +     if (twsi_control_flags & MVTWSI_CONTROL_ACK)
> +             expected_status = MVTWSI_STATUS_DATA_R_ACK;
> +     else
> +             expected_status = MVTWSI_STATUS_DATA_R_NAK;
> +     /* acknowledge *previous state* and launch receive */
> +     writel(twsi_control_flags, &twsi->control);
> +     /* wait for controller to receive byte and assert ACK or NAK */
> +     status = twsi_wait(expected_status);
> +     /* if we did receive expected byte then store it */
> +     if (status == 0)
> +             *byte = readl(&twsi->data);
> +     /* return status */
> +     return status;
>  }
>
> -static       int
> -kirkwood_i2c_intr(void)
> +/*
> + * Assert the STOP condition.
> + * This is also used to force the bus back in idle (SDA=SCL=1).
> + */
> +static int twsi_stop(int status)
>  {
> -     u32             status;
> -     u32             ctrl;
> -     int             rc = IRQ_NONE;
> -
> -     ctrl = readl(CONFIG_I2C_KW_REG_BASE + KW_I2C_REG_CONTROL);
> -     while ((ctrl & KW_I2C_REG_CONTROL_IFLG) &&
> -             (drv_data->rc == 0)) {
> -             status = readl(CONFIG_I2C_KW_REG_BASE +
> KW_I2C_REG_STATUS);
> -             kirkwood_i2c_fsm(status);
> -             kirkwood_i2c_do_action();
> -             rc = IRQ_HANDLED;
> -             ctrl = readl(CONFIG_I2C_KW_REG_BASE +
> KW_I2C_REG_CONTROL);
> -             udelay(1000);
> -     }
> -     return rc;
> +     int control, stop_status;
> +     int timeout = 1000;
> +
> +     /* assert STOP */
> +     control = MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_STOP;
> +     writel(control, &twsi->control);
> +     /* wait for IDLE; IFLG won't rise so twsi_wait() is no use. */
> +     do {
> +             stop_status = readl(&twsi->status);
> +             if (stop_status == MVTWSI_STATUS_IDLE)
> +                     break;
> +             udelay(10); /* one clock cycle at 100 kHz */
> +     } while (timeout--);
> +     control = readl(&twsi->control);
> +     if (stop_status != MVTWSI_STATUS_IDLE)
> +             if (status == 0)
> +                     status = MVTWSI_ERROR(
> +                             MVTWSI_ERROR_TIMEOUT,
> +                             control, status, MVTWSI_STATUS_IDLE);
> +     return status;
>  }
>
> -static void
> -kirkwood_i2c_doio(struct i2c_msg *msg)
> -{
> -     int     ret;
> +/*
> + * Ugly formula to convert m and n values to a frequency comes from
> + * TWSI specifications
> + */
>
> -     while ((drv_data->rc == 0) && (drv_data->state !=
> KW_I2C_STATE_IDLE)) {
> -             /* poll Status register */
> -             ret = kirkwood_i2c_intr();
> -             if (ret == IRQ_NONE)
> -                     udelay(10);
> -     }
> +#define TWSI_FREQUENCY(m, n) \
> +     ((u8) (CONFIG_SYS_TCLK / (10 * (m + 1) * 2 * (1 << n))))
> +
> +/*
> + * These are required to be reprogrammed before enabling the
> controller
> + * because a reset loses them.
> + * Default values come from the spec, but a twsi_reset will
> change them.
> + * twsi_slave_address left uninitialized lest checkpatch.pl
> complains.
> + */
> +
> +static u8 twsi_baud_rate = 0x44;

What does it indicate? pls use figurative constants/macros

> +static u8 twsi_actual_speed = TWSI_FREQUENCY(4, 4);
> +static u8 twsi_slave_address;
> +
> +/*
> + * Reset controller.
> + * Called at end of i2c_init unsuccessful i2c transactions.
> + * Controller reset also resets the baud rate and slave address, so
> + * re-establish them.
> + */
> +static void twsi_reset(void)
> +{
> +     /* ensure controller will be enabled by any twsi*() function */
> +     twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
> +     /* reset controller */
> +     writel(0, &twsi->soft_reset);
> +     /* wait 2 ms -- this is what the Marvell LSP does */
> +     udelay(20000);
> +     /* set baud rate */
> +     writel(twsi_baud_rate, &twsi->baudrate);
> +     /* set slave address even though we don't use it */
> +     writel(twsi_slave_address, &twsi->slave_address);
> +     writel(0, &twsi->extended_slave_address);
> +     /* assert STOP but don't care for the result */
> +     (void) twsi_stop(0);
>  }
>
> -static void
> -kirkwood_i2c_prepare_for_io(struct i2c_msg *msg)
> +/*
> + * I2C init called by cmd_i2c when doing 'i2c reset'.
> + * Sets baud to the highest possible value not exceeding
> requested one.
> + */
> +void i2c_init(int requested_speed, int slaveadd)
>  {
> -     u32     dir = 0;
> -
> -     drv_data->msg = msg;
> -     drv_data->byte_posn = 0;
> -     drv_data->bytes_left = msg->len;
> -     drv_data->aborting = 0;
> -     drv_data->rc = 0;
> -     /* in u-boot we use no IRQs */
> -     drv_data->cntl_bits = KW_I2C_REG_CONTROL_ACK |
> KW_I2C_REG_CONTROL_TWSIEN;
> -
> -     if (msg->flags & I2C_M_RD)
> -             dir = 1;
> -     if (msg->flags & I2C_M_TEN) {
> -             drv_data->addr1 = 0xf0 | (((u32)msg->addr &
> 0x300) >> 7) | dir;
> -             drv_data->addr2 = (u32)msg->addr & 0xff;
> -     } else {
> -             drv_data->addr1 = ((u32)msg->addr & 0x7f) << 1 | dir;
> -             drv_data->addr2 = 0;
> +     int     tmp_speed, highest_speed, n, m;
> +     int     baud = 0x44; /* value at controller reset */

Same here..

Regards..
Prafulla . .

[U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support orion5x and kirkwood

[Albert ARIBAUD]

Le 27/08/2010 14:34, Prafulla Wadaskar a ?crit :
>
>
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
>> Sent: Friday, August 27, 2010 11:42 AM
>> To: u-boot at lists.denx.de
>> Subject: [U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support
>> orion5x and kirkwood
>>
>> This rewrite of the mvtwsi driver is 25% smaller and much
>> faster and simpler than the previous code.
>>
>> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr>
>> ---
>>   drivers/i2c/mvtwsi.c |  742
>> +++++++++++++++++++++++---------------------------
>>   1 files changed, 334 insertions(+), 408 deletions(-)
>>
>> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
>> index a4409be..e7f08ac 100644
>> --- a/drivers/i2c/mvtwsi.c
>> +++ b/drivers/i2c/mvtwsi.c
>> @@ -1,10 +1,7 @@
>>   /*
>> - * Driver for the i2c controller on the Marvell line of host bridges
>> - * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, Orion SoC family),
>> - * and Kirkwood family.
>> + * Driver for the TWSI (i2c) controller on the Marvell orion5x
>
> Don't you think Kirkwood is missing here ?
>
>>    *
>> - * Based on:
>> - * Author: Mark A. Greer<mgreer@mvista.com>
>> + * Author: Albert Aribaud<albert.aribaud@free.fr>
>>    * 2005 (c) MontaVista, Software, Inc.
>
> I hope this line is not there in earlier version, pls confirm..

The Montavista (c) line is in the kirkwood_i2c file from which I 
copy-pasted the GPL notice. I'd even posted a remark myself on V1 of the 
patch, stating that this copyright line was a leftover of the old 
driver, and that I would fix it on V2. Somehow I missed it.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support orion5x and kirkwood

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Friday, August 27, 2010 6:33 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, 
> support orion5x and kirkwood
> 
> Le 27/08/2010 14:34, Prafulla Wadaskar a ?crit :
> >
> >
> >> -----Original Message-----
> >> From: u-boot-bounces at lists.denx.de
> >> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> >> Sent: Friday, August 27, 2010 11:42 AM
> >> To: u-boot at lists.denx.de
> >> Subject: [U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support
> >> orion5x and kirkwood
> >>
> >> This rewrite of the mvtwsi driver is 25% smaller and much
> >> faster and simpler than the previous code.
> >>
> >> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr>
> >> ---
> >>   drivers/i2c/mvtwsi.c |  742
> >> +++++++++++++++++++++++---------------------------
> >>   1 files changed, 334 insertions(+), 408 deletions(-)
> >>
> >> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> >> index a4409be..e7f08ac 100644
> >> --- a/drivers/i2c/mvtwsi.c
> >> +++ b/drivers/i2c/mvtwsi.c
> >> @@ -1,10 +1,7 @@
> >>   /*
> >> - * Driver for the i2c controller on the Marvell line of 
> host bridges
> >> - * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, Orion SoC family),
> >> - * and Kirkwood family.
> >> + * Driver for the TWSI (i2c) controller on the Marvell orion5x
> >
> > Don't you think Kirkwood is missing here ?
Hi Albert
Any commetns for this?

Regards..
Prafulla . .

> >
> >>    *
> >> - * Based on:
> >> - * Author: Mark A. Greer<mgreer@mvista.com>
> >> + * Author: Albert Aribaud<albert.aribaud@free.fr>
> >>    * 2005 (c) MontaVista, Software, Inc.
> >
> > I hope this line is not there in earlier version, pls confirm..
> 
> The Montavista (c) line is in the kirkwood_i2c file from which I 
> copy-pasted the GPL notice. I'd even posted a remark myself 
> on V1 of the 
> patch, stating that this copyright line was a leftover of the old 
> driver, and that I would fix it on V2. Somehow I missed it.
> 
> Amicalement,
> -- 
> Albert.
> 

[U-Boot] [PATCH V3 3/4] i2c: rewrite mvtwsi, support orion5x and kirkwood

[Albert ARIBAUD]

Le 27/08/2010 16:30, Prafulla Wadaskar a ?crit :
>>>> - * Driver for the i2c controller on the Marvell line of
>> host bridges
>>>> - * (e.g, gt642[46]0, mv643[46]0, mv644[46]0, Orion SoC family),
>>>> - * and Kirkwood family.
>>>> + * Driver for the TWSI (i2c) controller on the Marvell orion5x
>>>
>>> Don't you think Kirkwood is missing here ?
> Hi Albert
> Any commetns for this?

No worries: all your remarks are being applied in V4, especially those 
to which I did not reply; I've added kirkwood to the supported SoC list. :)

Speaking of which, I thought I might as well directly add orion5x *and* 
kirkwood includes, i.e patch V4 will have:

	#if defined(CONFIG_ORION5X)
	#include <asm/arch/orion5x.h>
	#elif defined(CONFIG_KIRKWOOD)
	#include <asm/arch/kirkwood.h>
	#else
	#error Driver mvtwsi not supported by SoC or board
	#endif

... even though I won't be adding I2C support on any kirkwood board in 
this patchset.

> Regards..
> Prafulla . .

Amicalement,
-- 
Albert.

[U-Boot] [PATCH V3 4/4] edminiv2: add I2C support using mvtwsi driver

[Albert Aribaud]

Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
---
 include/configs/edminiv2.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/configs/edminiv2.h b/include/configs/edminiv2.h
index 57dd165..36ed392 100644
--- a/include/configs/edminiv2.h
+++ b/include/configs/edminiv2.h
@@ -182,6 +182,15 @@
 #endif /* CMD_IDE */
 
 /*
+ * I2C related stuff
+ */
+#define CONFIG_I2C_DRIVER_MVTWSI
+#define CONFIG_I2C_MVTWSI_BASE		ORION5X_TWSI_BASE
+#define CONFIG_SYS_I2C_SLAVE		0x0
+#define CONFIG_SYS_I2C_SPEED		100000
+#define CONFIG_CMD_I2C
+
+/*
  *  Environment variables configurations
  */
 #define CONFIG_ENV_IS_IN_FLASH		1
-- 
1.7.1

[U-Boot] [PATCH V3 4/4] edminiv2: add I2C support using mvtwsi driver

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> Sent: Friday, August 27, 2010 11:42 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH V3 4/4] edminiv2: add I2C support 
> using mvtwsi driver
> 
> 
> Signed-off-by: Albert Aribaud <albert.aribaud@free.fr>
> ---
>  include/configs/edminiv2.h |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/edminiv2.h b/include/configs/edminiv2.h
> index 57dd165..36ed392 100644
> --- a/include/configs/edminiv2.h
> +++ b/include/configs/edminiv2.h
> @@ -182,6 +182,15 @@
>  #endif /* CMD_IDE */
>  
>  /*
> + * I2C related stuff
> + */
> +#define CONFIG_I2C_DRIVER_MVTWSI
> +#define CONFIG_I2C_MVTWSI_BASE		ORION5X_TWSI_BASE
> +#define CONFIG_SYS_I2C_SLAVE		0x0
> +#define CONFIG_SYS_I2C_SPEED		100000
> +#define CONFIG_CMD_I2C

Can you pls encapsulate this in #ifdef CONFIG_CMD_I2C ?

Regards..
Prafulla . .

> +
> +/*
>   *  Environment variables configurations
>   */
>  #define CONFIG_ENV_IS_IN_FLASH		1
> -- 
> 1.7.1
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [PATCH V3 4/4] edminiv2: add I2C support using mvtwsi driver

[Albert ARIBAUD]

Le 27/08/2010 14:20, Prafulla Wadaskar a ?crit :

> Can you pls encapsulate this in #ifdef CONFIG_CMD_I2C ?

Done, and moved #define CONFIG_CMD_I2C up right after #define 
CONFIG_CMD_IDE.

Amicalement,
-- 
Albert.