All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements
@ 2016-07-21  9:57 Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 01/13] i2c: mvtwsi: Fix style violations Mario Six
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

This patch series converts the MVTWSI I2C driver to DM, fixes style violations,
improves and cleans up the code, and adds lots of documentation.

Mario Six (13):
  i2c: mvtwsi: Fix style violations
  i2c: mvtwsi: Streamline code and add documentation
  i2c: mvtwsi: Improve and fix comments
  i2c: mvtwsi: Eliminate flags parameter
  i2c: mvtwsi: Get rid of status parameter
  i2c: mvtwsi: Use 'uint' instead of 'unsigned int'
  i2c: mvtwsi: Add compatibility functions
  i2c: mvtwsi: Factor out adap parameter
  i2c: mvtwsi: Make address length variable
  i2c: mvtwsi: Add compatibility to DM
  i2c: mvtwsi: Handle zero-length offsets properly
  i2c: mvtwsi: Make delay times frequency-dependent
  i2c: mvtwsi: Add documentation

 drivers/i2c/Kconfig  |   7 +
 drivers/i2c/mvtwsi.c | 777 +++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 570 insertions(+), 214 deletions(-)

--
2.9.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 01/13] i2c: mvtwsi: Fix style violations
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 02/13] i2c: mvtwsi: Streamline code and add documentation Mario Six
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

This patch fixes seven style violations: Six superfluous spaces after
casts, and one logical continuation violation.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index bf44432..e9e7c47 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -110,27 +110,27 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
 	switch (adap->hwadapnr) {
 #ifdef CONFIG_I2C_MVTWSI_BASE0
 	case 0:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE0;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE0;
 #endif
 #ifdef CONFIG_I2C_MVTWSI_BASE1
 	case 1:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE1;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE1;
 #endif
 #ifdef CONFIG_I2C_MVTWSI_BASE2
 	case 2:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE2;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE2;
 #endif
 #ifdef CONFIG_I2C_MVTWSI_BASE3
 	case 3:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE3;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE3;
 #endif
 #ifdef CONFIG_I2C_MVTWSI_BASE4
 	case 4:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE4;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE4;
 #endif
 #ifdef CONFIG_I2C_MVTWSI_BASE5
 	case 5:
-		return (struct mvtwsi_registers *) CONFIG_I2C_MVTWSI_BASE5;
+		return (struct mvtwsi_registers *)CONFIG_I2C_MVTWSI_BASE5;
 #endif
 	default:
 		printf("Missing mvtwsi controller %d base\n", adap->hwadapnr);
@@ -312,8 +312,8 @@ static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 	for (n = 0; n < 8; n++) {
 		for (m = 0; m < 16; m++) {
 			tmp_speed = twsi_calc_freq(n, m);
-			if ((tmp_speed <= requested_speed)
-			 && (tmp_speed > highest_speed)) {
+			if ((tmp_speed <= requested_speed) &&
+			    (tmp_speed > highest_speed)) {
 				highest_speed = tmp_speed;
 				baud = (m << 3) | n;
 			}
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 02/13] i2c: mvtwsi: Streamline code and add documentation
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 01/13] i2c: mvtwsi: Fix style violations Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 03/13] i2c: mvtwsi: Improve and fix comments Mario Six
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Convert groups of logically connected preprocessor defines into proper
enums, one macro into an inline function, and add documentation
to/extend existing documentation of these items.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 113 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 38 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index e9e7c47..93bb88b 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -62,15 +62,23 @@ struct  mvtwsi_registers {
 #endif

 /*
- * Control register fields
+ * enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control
+ * register
  */
-
-#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
+enum mvtwsi_ctrl_register_fields {
+	/* Acknowledge bit */
+	MVTWSI_CONTROL_ACK	= 0x00000004,
+	/* Interrupt flag */
+	MVTWSI_CONTROL_IFLG	= 0x00000008,
+	/* Stop bit */
+	MVTWSI_CONTROL_STOP	= 0x00000010,
+	/* Start bit */
+	MVTWSI_CONTROL_START	= 0x00000020,
+	/* I2C enable */
+	MVTWSI_CONTROL_TWSIEN	= 0x00000040,
+	/* Interrupt enable */
+	MVTWSI_CONTROL_INTEN	= 0x00000080,
+};

 /*
  * On sun6i and newer IFLG is a write-clear bit which is cleared by writing 1,
@@ -84,22 +92,36 @@ struct  mvtwsi_registers {
 #endif

 /*
- * 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
+ * enum mvstwsi_status_values - Possible values of I2C controller's status
+ * register
+ *
+ * Only those statuses expected in normal master operation on
+ * non-10-bit-address devices are specified.
+ *
+ * Every status that's unexpected during normal operation (bus errors,
+ * arbitration losses, missing ACKs...) is passed back to the caller as an error
  * code.
  */
-
-#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
+enum mvstwsi_status_values {
+	/* START condition transmitted */
+	MVTWSI_STATUS_START		= 0x08,
+	/* Repeated START condition transmitted */
+	MVTWSI_STATUS_REPEATED_START	= 0x10,
+	/* Address + write bit transmitted, ACK received */
+	MVTWSI_STATUS_ADDR_W_ACK	= 0x18,
+	/* Data transmitted, ACK received */
+	MVTWSI_STATUS_DATA_W_ACK	= 0x28,
+	/* Address + read bit transmitted, ACK received */
+	MVTWSI_STATUS_ADDR_R_ACK	= 0x40,
+	/* Address + read bit transmitted, ACK not received */
+	MVTWSI_STATUS_ADDR_R_NAK	= 0x48,
+	/* Data received, ACK transmitted */
+	MVTWSI_STATUS_DATA_R_ACK	= 0x50,
+	/* Data received, ACK not transmitted */
+	MVTWSI_STATUS_DATA_R_NAK	= 0x58,
+	/* No relevant status */
+	MVTWSI_STATUS_IDLE		= 0xF8,
+};

 /*
  * MVTWSI controller base
@@ -141,20 +163,35 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
 }

 /*
- * 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.
+ * enum mvtwsi_error_class - types of I2C errors
  */
+enum mvtwsi_error_class {
+	/* The controller returned a different status than expected */
+	MVTWSI_ERROR_WRONG_STATUS       = 0x01,
+	/* The controller timed out */
+	MVTWSI_ERROR_TIMEOUT            = 0x02,
+};

-#define MVTWSI_ERROR_WRONG_STATUS	0x01
-#define MVTWSI_ERROR_TIMEOUT		0x02
-
-#define MVTWSI_ERROR(ec, lc, ls, es) (((ec << 24) & 0xFF000000) | \
-	((lc << 16) & 0x00FF0000) | ((ls<<8) & 0x0000FF00) | (es & 0xFF))
+/*
+ * mvtwsi_error() - Build I2C return code from error information
+ *
+ * For debugging purposes, this function packs some information of an occurred
+ * error into a return code. These error codes are returned from I2C API
+ * functions (i2c_{read,write}, dm_i2c_{read,write}, etc.).
+ *
+ * @ec:		The error class of the error (enum mvtwsi_error_class).
+ * @lc:		The last value of the control register.
+ * @ls:		The last value of the status register.
+ * @es:		The expected value of the status register.
+ * @return The generated error code.
+ */
+inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es)
+{
+	return ((ec << 24) & 0xFF000000)
+	       | ((lc << 16) & 0x00FF0000)
+	       | ((ls << 8) & 0x0000FF00)
+	       | (es & 0xFF);
+}

 /*
  * Wait for IFLG to raise, or return 'timeout'; then if status is as expected,
@@ -173,15 +210,15 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status)
 			if (status == expected_status)
 				return 0;
 			else
-				return MVTWSI_ERROR(
+				return mvtwsi_error(
 					MVTWSI_ERROR_WRONG_STATUS,
 					control, status, expected_status);
 		}
 		udelay(10); /* one clock cycle@100 kHz */
 	} while (timeout--);
 	status = readl(&twsi->status);
-	return MVTWSI_ERROR(
-		MVTWSI_ERROR_TIMEOUT, control, status, expected_status);
+	return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status,
+			    expected_status);
 }

 /*
@@ -265,7 +302,7 @@ static int twsi_stop(struct i2c_adapter *adap, int status)
 	control = readl(&twsi->control);
 	if (stop_status != MVTWSI_STATUS_IDLE)
 		if (status == 0)
-			status = MVTWSI_ERROR(
+			status = mvtwsi_error(
 				MVTWSI_ERROR_TIMEOUT,
 				control, status, MVTWSI_STATUS_IDLE);
 	return status;
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 03/13] i2c: mvtwsi: Improve and fix comments
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 01/13] i2c: mvtwsi: Fix style violations Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 02/13] i2c: mvtwsi: Streamline code and add documentation Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 04/13] i2c: mvtwsi: Eliminate flags parameter Mario Six
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

This patch fixes only comments/documentation: Streamline capitalization
and improve grammar/punctuation.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 129 +++++++++++++++++++++++++--------------------------
 1 file changed, 62 insertions(+), 67 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 93bb88b..7b2c611 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -14,8 +14,8 @@
 #include <asm/io.h>

 /*
- * include a file that will provide CONFIG_I2C_MVTWSI_BASE*
- * and possibly other settings
+ * Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other
+ * settings
  */

 #if defined(CONFIG_ORION5X)
@@ -51,8 +51,8 @@ struct  mvtwsi_registers {
 	u32 data;
 	u32 control;
 	union {
-		u32 status;	/* when reading */
-		u32 baudrate;	/* when writing */
+		u32 status;	/* When reading */
+		u32 baudrate;	/* When writing */
 	};
 	u32 xtnd_slave_addr;
 	u32 reserved[2];
@@ -81,8 +81,8 @@ enum mvtwsi_ctrl_register_fields {
 };

 /*
- * On sun6i and newer IFLG is a write-clear bit which is cleared by writing 1,
- * on other platforms it is a normal r/w bit which is cleared by writing 0.
+ * On sun6i and newer, IFLG is a write-clear bit, which is cleared by writing 1;
+ * on other platforms, it is a normal r/w bit, which is cleared by writing 0.
  */

 #ifdef CONFIG_SUNXI_GEN_SUN6I
@@ -194,8 +194,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es)
 }

 /*
- * Wait for IFLG to raise, or return 'timeout'; then if status is as expected,
- * return 0 (ok) or return 'wrong status'.
+ * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as
+ * expected, return 0 (ok) or 'wrong status' otherwise.
  */
 static int twsi_wait(struct i2c_adapter *adap, int expected_status)
 {
@@ -214,7 +214,7 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status)
 					MVTWSI_ERROR_WRONG_STATUS,
 					control, status, expected_status);
 		}
-		udelay(10); /* one clock cycle at 100 kHz */
+		udelay(10); /* One clock cycle at 100 kHz */
 	} while (timeout--);
 	status = readl(&twsi->status);
 	return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status,
@@ -229,12 +229,12 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

-	/* globally set TWSIEN in case it was not */
+	/* Set TWSIEN */
 	*flags |= MVTWSI_CONTROL_TWSIEN;
-	/* assert START */
+	/* Assert START */
 	writel(*flags | MVTWSI_CONTROL_START |
-				    MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
-	/* wait for controller to process START */
+	       MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
+	/* Wait for controller to process START */
 	return twsi_wait(adap, expected_status);
 }

@@ -246,42 +246,40 @@ static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status,
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

-	/* put byte in data register for sending */
+	/* Write byte to data register for sending */
 	writel(byte, &twsi->data);
-	/* clear any pending interrupt -- that'll cause sending */
+	/* Clear any pending interrupt -- that will cause sending */
 	writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
-	/* wait for controller to receive byte and check ACK */
+	/* Wait for controller to receive byte, and check ACK */
 	return twsi_wait(adap, expected_status);
 }

 /*
  * Receive a byte.
- * Global mvtwsi_control_flags variable says if we should ack or nak.
  */
 static int twsi_recv(struct i2c_adapter *adap, u8 *byte, u8 *flags)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	int expected_status, status;

-	/* compute expected status based on ACK bit in global control flags */
+	/* Compute expected status based on ACK bit in passed control flags */
 	if (*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 */
+	/* Acknowledge *previous state*, and launch receive */
 	writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
-	/* wait for controller to receive byte and assert ACK or NAK */
+	/* Wait for controller to receive byte, and assert ACK or NAK */
 	status = twsi_wait(adap, expected_status);
-	/* if we did receive expected byte then store it */
+	/* If we did receive the expected byte, store it */
 	if (status == 0)
 		*byte = readl(&twsi->data);
-	/* return status */
 	return status;
 }

 /*
  * Assert the STOP condition.
- * This is also used to force the bus back in idle (SDA=SCL=1).
+ * This is also used to force the bus back to idle (SDA = SCL = 1).
  */
 static int twsi_stop(struct i2c_adapter *adap, int status)
 {
@@ -289,15 +287,15 @@ static int twsi_stop(struct i2c_adapter *adap, int status)
 	int control, stop_status;
 	int timeout = 1000;

-	/* assert STOP */
+	/* Assert STOP */
 	control = MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_STOP;
 	writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
-	/* wait for IDLE; IFLG won't rise so twsi_wait() is no use. */
+	/* Wait for IDLE; IFLG won't rise, so we can't use twsi_wait() */
 	do {
 		stop_status = readl(&twsi->status);
 		if (stop_status == MVTWSI_STATUS_IDLE)
 			break;
-		udelay(10); /* one clock cycle at 100 kHz */
+		udelay(10); /* One clock cycle at 100 kHz */
 	} while (timeout--);
 	control = readl(&twsi->control);
 	if (stop_status != MVTWSI_STATUS_IDLE)
@@ -326,26 +324,26 @@ static void twsi_reset(struct i2c_adapter *adap)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

-	/* reset controller */
+	/* Reset controller */
 	writel(0, &twsi->soft_reset);
-	/* wait 2 ms -- this is what the Marvell LSP does */
+	/* Wait 2 ms -- this is what the Marvell LSP does */
 	udelay(20000);
 }

 /*
- * I2C init called by cmd_i2c when doing 'i2c reset'.
- * Sets baud to the highest possible value not exceeding requested one.
+ * Sets baud to the highest possible value not exceeding the requested one.
  */
 static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 					   unsigned int requested_speed)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	unsigned int tmp_speed, highest_speed, n, m;
-	unsigned int baud = 0x44; /* baudrate at controller reset */
+	unsigned int baud = 0x44; /* Baud rate after controller reset */

-	/* use actual speed to collect progressively higher values */
 	highest_speed = 0;
-	/* compute m, n setting for highest speed not above requested speed */
+	/* Successively try m, n combinations, and use the combination
+	 * resulting in the largest speed that's not above the requested
+	 * speed */
 	for (n = 0; n < 8; n++) {
 		for (m = 0; m < 16; m++) {
 			tmp_speed = twsi_calc_freq(n, m);
@@ -364,20 +362,19 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

-	/* reset controller */
+	/* Reset controller */
 	twsi_reset(adap);
-	/* set speed */
+	/* Set speed */
 	twsi_i2c_set_bus_speed(adap, speed);
-	/* set slave address even though we don't use it */
+	/* Set slave address; even though we don't use it */
 	writel(slaveadd, &twsi->slave_address);
 	writel(0, &twsi->xtnd_slave_addr);
-	/* assert STOP but don't care for the result */
+	/* Assert STOP, but don't care for the result */
 	(void) twsi_stop(adap, 0);
 }

 /*
  * 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(struct i2c_adapter *adap, int expected_start_status,
@@ -385,23 +382,23 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 {
 	int status, expected_addr_status;

-	/* compute expected address status from direction bit in addr */
-	if (addr & 1) /* reading */
+	/* Compute the expected address status from the direction bit in
+	 * the address byte */
+	if (addr & 1) /* Reading */
 		expected_addr_status = MVTWSI_STATUS_ADDR_R_ACK;
-	else /* writing */
+	else /* Writing */
 		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
-	/* assert START */
+	/* Assert START */
 	status = twsi_start(adap, expected_start_status, flags);
-	/* send out the address if the start went well */
+	/* Send out the address if the start went well */
 	if (status == 0)
 		status = twsi_send(adap, addr, expected_addr_status,
 				   flags);
-	/* return ok or status of first failure to caller */
+	/* Return 0, or the status of the first failure */
 	return status;
 }

 /*
- * I2C probe called by cmd_i2c when doing 'i2c probe'.
  * Begin read, nak data byte, end.
  */
 static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
@@ -410,26 +407,24 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 	u8 flags = 0;
 	int status;

-	/* begin i2c read */
+	/* Begin i2c read */
 	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1, &flags);
-	/* dummy read was accepted: receive byte but NAK it. */
+	/* Dummy read was accepted: receive byte, but NAK it. */
 	if (status == 0)
 		status = twsi_recv(adap, &dummy_byte, &flags);
 	/* Stop transaction */
 	twsi_stop(adap, 0);
-	/* return 0 or status of first failure */
+	/* Return 0, or the status of the first failure */
 	return status;
 }

 /*
- * 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.
+ * NOTE: Some devices want a stop right before the second start, while some
+ * will choke if it is there. Since deciding this is not yet supported in
+ * higher level APIs, we need to make a decision here, and for the moment that
+ * will be a repeated start without a preceding stop.
  */
 static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 			int alen, uchar *data, int length)
@@ -437,35 +432,34 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 	int status;
 	u8 flags = 0;

-	/* begin i2c write to send the address bytes */
+	/* Begin i2c write to send the address bytes */
 	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags);
-	/* send addr bytes */
+	/* Send address bytes */
 	while ((status == 0) && alen--)
 		status = twsi_send(adap, addr >> (8*alen),
 			MVTWSI_STATUS_DATA_W_ACK, &flags);
-	/* begin i2c read to receive eeprom data bytes */
+	/* Begin i2c read to receive data bytes */
 	if (status == 0)
 		status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START,
 				   (chip << 1) | 1, &flags);
-	/* prepare ACK if at least one byte must be received */
+	/* Prepare ACK if@least one byte must be received */
 	if (length > 0)
 		flags |= MVTWSI_CONTROL_ACK;
-	/* now receive actual bytes */
+	/* Receive actual data bytes */
 	while ((status == 0) && length--) {
-		/* reset NAK if we if no more to read now */
+		/* Set NAK if we if we have nothing more to read */
 		if (length == 0)
 			flags &= ~MVTWSI_CONTROL_ACK;
-		/* read current byte */
+		/* Read current byte */
 		status = twsi_recv(adap, data++, &flags);
 	}
 	/* Stop transaction */
 	status = twsi_stop(adap, status);
-	/* return 0 or status of first failure */
+	/* Return 0, or the status of the first failure */
 	return status;
 }

 /*
- * 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.
  */
 static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
@@ -474,19 +468,20 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 	int status;
 	u8 flags = 0;

-	/* begin i2c write to send the eeprom adress bytes then data bytes */
+	/* Begin i2c write to send first the address bytes, then the
+	 * data bytes */
 	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags);
-	/* send addr bytes */
+	/* Send address bytes */
 	while ((status == 0) && alen--)
 		status = twsi_send(adap, addr >> (8*alen),
 			MVTWSI_STATUS_DATA_W_ACK, &flags);
-	/* send data bytes */
+	/* Send data bytes */
 	while ((status == 0) && (length-- > 0))
 		status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK,
 				   &flags);
 	/* Stop transaction */
 	status = twsi_stop(adap, status);
-	/* return 0 or status of first failure */
+	/* Return 0, or the status of the first failure */
 	return status;
 }

--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 04/13] i2c: mvtwsi: Eliminate flags parameter
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (2 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 03/13] i2c: mvtwsi: Improve and fix comments Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 05/13] i2c: mvtwsi: Get rid of status parameter Mario Six
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Due to breaking boots from NOR flashes, commit d6b7757 ("i2c: mvtwsi:
Eliminate twsi_control_flags") removed the static global
twsi_control_flags variable, which kept a set of default flags that were
always or'd to the control register when writing. It was replaced with a
flags parameter, which was passed around between the functions that
needed it.

Since the twsi_control_flags variable was used just for the purposes of
a) setting the MVTWSI_CONTROL_TWSIEN on every control register write,
   and
b) setting the MVTWSI_CONTROL_ACK from twsi_i2c_read if needed,
anyway, the added overhead of another variable being passed around is no
longer justified, and we are better off implementing this flag setting
logic locally in the functions that actually write to the control
register.

Therefore, this patch sets MVTWSI_CONTROL_TWSIEN on every control
register write, replaces the twsi_i2c_read's flags parameter with a
ack_flag parameter, which tells the function whether to acknowledge the
read or not, and removes every other instance of the flags variable.
This has the added benefit that now every notion of "global default
flags" is gone, and it's much easier to see which control flags are
actually set at which point in time.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 83 ++++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 7b2c611..4a34eab 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -124,6 +124,17 @@ enum mvstwsi_status_values {
 };

 /*
+ * enum mvstwsi_ack_flags - Determine whether a read byte should be
+ * acknowledged or not.
+ */
+enum mvtwsi_ack_flags {
+	/* Send NAK after received byte */
+	MVTWSI_READ_NAK = 0,
+	/* Send ACK after received byte */
+	MVTWSI_READ_ACK = 1,
+};
+
+/*
  * MVTWSI controller base
  */

@@ -225,14 +236,12 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status)
  * Assert the START condition, either in a single I2C transaction
  * or inside back-to-back ones (repeated starts).
  */
-static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags)
+static int twsi_start(struct i2c_adapter *adap, int expected_status)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

-	/* Set TWSIEN */
-	*flags |= MVTWSI_CONTROL_TWSIEN;
 	/* Assert START */
-	writel(*flags | MVTWSI_CONTROL_START |
+	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START |
 	       MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to process START */
 	return twsi_wait(adap, expected_status);
@@ -241,15 +250,15 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status, u8 *flags)
 /*
  * Send a byte (i2c address or data).
  */
-static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status,
-		     u8 *flags)
+static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

 	/* Write byte to data register for sending */
 	writel(byte, &twsi->data);
 	/* Clear any pending interrupt -- that will cause sending */
-	writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
+	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG,
+	       &twsi->control);
 	/* Wait for controller to receive byte, and check ACK */
 	return twsi_wait(adap, expected_status);
 }
@@ -257,18 +266,18 @@ static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status,
 /*
  * Receive a byte.
  */
-static int twsi_recv(struct i2c_adapter *adap, u8 *byte, u8 *flags)
+static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	int expected_status, status;
+	int expected_status, status, control;

-	/* Compute expected status based on ACK bit in passed control flags */
-	if (*flags & MVTWSI_CONTROL_ACK)
-		expected_status = MVTWSI_STATUS_DATA_R_ACK;
-	else
-		expected_status = MVTWSI_STATUS_DATA_R_NAK;
+	/* Compute expected status based on passed ACK flag */
+	expected_status = ack_flag ? MVTWSI_STATUS_DATA_R_ACK :
+			  MVTWSI_STATUS_DATA_R_NAK;
 	/* Acknowledge *previous state*, and launch receive */
-	writel(*flags | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
+	control = MVTWSI_CONTROL_TWSIEN;
+	control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0;
+	writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to receive byte, and assert ACK or NAK */
 	status = twsi_wait(adap, expected_status);
 	/* If we did receive the expected byte, store it */
@@ -378,7 +387,7 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
  * Expected address status will derive from direction bit (bit 0) in addr.
  */
 static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
-		     u8 addr, u8 *flags)
+		     u8 addr)
 {
 	int status, expected_addr_status;

@@ -389,11 +398,10 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 	else /* Writing */
 		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
 	/* Assert START */
-	status = twsi_start(adap, expected_start_status, flags);
+	status = twsi_start(adap, expected_start_status);
 	/* Send out the address if the start went well */
 	if (status == 0)
-		status = twsi_send(adap, addr, expected_addr_status,
-				   flags);
+		status = twsi_send(adap, addr, expected_addr_status);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -404,14 +412,13 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 {
 	u8 dummy_byte;
-	u8 flags = 0;
 	int status;

 	/* Begin i2c read */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1, &flags);
+	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1);
 	/* Dummy read was accepted: receive byte, but NAK it. */
 	if (status == 0)
-		status = twsi_recv(adap, &dummy_byte, &flags);
+		status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK);
 	/* Stop transaction */
 	twsi_stop(adap, 0);
 	/* Return 0, or the status of the first failure */
@@ -430,29 +437,23 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 			int alen, uchar *data, int length)
 {
 	int status;
-	u8 flags = 0;

 	/* Begin i2c write to send the address bytes */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags);
+	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
 	while ((status == 0) && alen--)
 		status = twsi_send(adap, addr >> (8*alen),
-			MVTWSI_STATUS_DATA_W_ACK, &flags);
+			MVTWSI_STATUS_DATA_W_ACK);
 	/* Begin i2c read to receive data bytes */
 	if (status == 0)
 		status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START,
-				   (chip << 1) | 1, &flags);
-	/* Prepare ACK if@least one byte must be received */
-	if (length > 0)
-		flags |= MVTWSI_CONTROL_ACK;
-	/* Receive actual data bytes */
-	while ((status == 0) && length--) {
-		/* Set NAK if we if we have nothing more to read */
-		if (length == 0)
-			flags &= ~MVTWSI_CONTROL_ACK;
-		/* Read current byte */
-		status = twsi_recv(adap, data++, &flags);
-	}
+				   (chip << 1) | 1);
+	/* Receive actual data bytes; set NAK if we if we have nothing more to
+	 * read */
+	while ((status == 0) && length--)
+		status = twsi_recv(adap, data++,
+				   length > 0 ?
+				   MVTWSI_READ_ACK : MVTWSI_READ_NAK);
 	/* Stop transaction */
 	status = twsi_stop(adap, status);
 	/* Return 0, or the status of the first failure */
@@ -466,19 +467,17 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 			int alen, uchar *data, int length)
 {
 	int status;
-	u8 flags = 0;

 	/* Begin i2c write to send first the address bytes, then the
 	 * data bytes */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1), &flags);
+	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
 	while ((status == 0) && alen--)
 		status = twsi_send(adap, addr >> (8*alen),
-			MVTWSI_STATUS_DATA_W_ACK, &flags);
+			MVTWSI_STATUS_DATA_W_ACK);
 	/* Send data bytes */
 	while ((status == 0) && (length-- > 0))
-		status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK,
-				   &flags);
+		status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK);
 	/* Stop transaction */
 	status = twsi_stop(adap, status);
 	/* Return 0, or the status of the first failure */
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 05/13] i2c: mvtwsi: Get rid of status parameter
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (3 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 04/13] i2c: mvtwsi: Eliminate flags parameter Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 06/13] i2c: mvtwsi: Use 'uint' instead of 'unsigned int' Mario Six
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

The twsi_stop function contains a parameter "status," which is used to
pass in the current exit status of the function calling twsi_stop, and
either return this status unchanged if it indicates an error, or return
twsi_stop's exit status if it does not indicate an error.

While not massively complicated, this adds another purpose to the
twsi_stop function, which should have the sole purpose of asserting a
STOP condition on the bus (and not manage the exit status of its
caller).

Therefore, we move the exit status management into the caller functions
by introducing a "stop_status" variable and returning either the status
before the twsi_stop call (kept in the "status" variable), or the status
from the twsi_stop call, depending on which indicates an error.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 4a34eab..d0e3c3f 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -290,10 +290,11 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag)
  * Assert the STOP condition.
  * This is also used to force the bus back to idle (SDA = SCL = 1).
  */
-static int twsi_stop(struct i2c_adapter *adap, int status)
+static int twsi_stop(struct i2c_adapter *adap)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	int control, stop_status;
+	int status = 0;
 	int timeout = 1000;

 	/* Assert STOP */
@@ -308,10 +309,8 @@ static int twsi_stop(struct i2c_adapter *adap, int status)
 	} 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);
+		status = mvtwsi_error(MVTWSI_ERROR_TIMEOUT,
+				      control, status, MVTWSI_STATUS_IDLE);
 	return status;
 }

@@ -379,7 +378,7 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
 	writel(slaveadd, &twsi->slave_address);
 	writel(0, &twsi->xtnd_slave_addr);
 	/* Assert STOP, but don't care for the result */
-	(void) twsi_stop(adap, 0);
+	(void) twsi_stop(adap);
 }

 /*
@@ -420,7 +419,7 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 	if (status == 0)
 		status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK);
 	/* Stop transaction */
-	twsi_stop(adap, 0);
+	twsi_stop(adap);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -436,7 +435,8 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 			int alen, uchar *data, int length)
 {
-	int status;
+	int status = 0;
+	int stop_status;

 	/* Begin i2c write to send the address bytes */
 	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1));
@@ -455,9 +455,9 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 				   length > 0 ?
 				   MVTWSI_READ_ACK : MVTWSI_READ_NAK);
 	/* Stop transaction */
-	status = twsi_stop(adap, status);
+	stop_status = twsi_stop(adap);
 	/* Return 0, or the status of the first failure */
-	return status;
+	return status != 0 ? status : stop_status;
 }

 /*
@@ -466,7 +466,7 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 			int alen, uchar *data, int length)
 {
-	int status;
+	int status, stop_status;

 	/* Begin i2c write to send first the address bytes, then the
 	 * data bytes */
@@ -479,9 +479,9 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 	while ((status == 0) && (length-- > 0))
 		status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK);
 	/* Stop transaction */
-	status = twsi_stop(adap, status);
+	stop_status = twsi_stop(adap);
 	/* Return 0, or the status of the first failure */
-	return status;
+	return status != 0 ? status : stop_status;
 }

 #ifdef CONFIG_I2C_MVTWSI_BASE0
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 06/13] i2c: mvtwsi: Use 'uint' instead of 'unsigned int'
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (4 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 05/13] i2c: mvtwsi: Get rid of status parameter Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 07/13] i2c: mvtwsi: Add compatibility functions Mario Six
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Since some additional parameters will be added in the course of this
patch series (especially with the addition of DM support), we replace
the longer "unsigned int" declarations with "uint" declarations to keep
the parameter lists more readable.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index d0e3c3f..9ddd3ee 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -314,7 +314,7 @@ static int twsi_stop(struct i2c_adapter *adap)
 	return status;
 }

-static unsigned int twsi_calc_freq(const int n, const int m)
+static uint twsi_calc_freq(const int n, const int m)
 {
 #ifdef CONFIG_SUNXI
 	return CONFIG_SYS_TCLK / (10 * (m + 1) * (1 << n));
@@ -341,12 +341,12 @@ static void twsi_reset(struct i2c_adapter *adap)
 /*
  * Sets baud to the highest possible value not exceeding the requested one.
  */
-static unsigned int twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
-					   unsigned int requested_speed)
+static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
+				   uint requested_speed)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	unsigned int tmp_speed, highest_speed, n, m;
-	unsigned int baud = 0x44; /* Baud rate after controller reset */
+	uint tmp_speed, highest_speed, n, m;
+	uint baud = 0x44; /* Baud rate after controller reset */

 	highest_speed = 0;
 	/* Successively try m, n combinations, and use the combination
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 07/13] i2c: mvtwsi: Add compatibility functions
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (5 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 06/13] i2c: mvtwsi: Use 'uint' instead of 'unsigned int' Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 08/13] i2c: mvtwsi: Factor out adap parameter Mario Six
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

To prepare for the DM conversion, we add a layer of compatibility
functions to be used by both the legacy and the DM functions.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 47 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 9ddd3ee..f1bfd5d 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -341,8 +341,8 @@ static void twsi_reset(struct i2c_adapter *adap)
 /*
  * Sets baud to the highest possible value not exceeding the requested one.
  */
-static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
-				   uint requested_speed)
+static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
+				     uint requested_speed)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	uint tmp_speed, highest_speed, n, m;
@@ -366,14 +366,14 @@ static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 	return 0;
 }

-static void twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
+static void __twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);

 	/* Reset controller */
 	twsi_reset(adap);
 	/* Set speed */
-	twsi_i2c_set_bus_speed(adap, speed);
+	__twsi_i2c_set_bus_speed(adap, speed);
 	/* Set slave address; even though we don't use it */
 	writel(slaveadd, &twsi->slave_address);
 	writel(0, &twsi->xtnd_slave_addr);
@@ -408,7 +408,7 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 /*
  * Begin read, nak data byte, end.
  */
-static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
+static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip)
 {
 	u8 dummy_byte;
 	int status;
@@ -432,8 +432,8 @@ static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
  * higher level APIs, we need to make a decision here, and for the moment that
  * will be a repeated start without a preceding stop.
  */
-static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
-			int alen, uchar *data, int length)
+static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
+			   int alen, uchar *data, int length)
 {
 	int status = 0;
 	int stop_status;
@@ -463,8 +463,8 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 /*
  * Begin write, send address byte(s), send data bytes, end.
  */
-static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
-			int alen, uchar *data, int length)
+static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
+			    int alen, uchar *data, int length)
 {
 	int status, stop_status;

@@ -484,6 +484,35 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 	return status != 0 ? status : stop_status;
 }

+static void twsi_i2c_init(struct i2c_adapter *adap, int speed,
+			  int slaveadd)
+{
+	__twsi_i2c_init(adap, speed, slaveadd);
+}
+
+static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
+				   uint requested_speed)
+{
+	return __twsi_i2c_set_bus_speed(adap, requested_speed);
+}
+
+static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
+{
+	return __twsi_i2c_probe_chip(adap, chip);
+}
+
+static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
+			 int alen, uchar *data, int length)
+{
+	return __twsi_i2c_read(adap, chip, addr, alen, data, length);
+}
+
+static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
+			  int alen, uchar *data, int length)
+{
+	return __twsi_i2c_write(adap, chip, addr, alen, data, length);
+}
+
 #ifdef CONFIG_I2C_MVTWSI_BASE0
 U_BOOT_I2C_ADAP_COMPLETE(twsi0, twsi_i2c_init, twsi_i2c_probe,
 			 twsi_i2c_read, twsi_i2c_write,
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 08/13] i2c: mvtwsi: Factor out adap parameter
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (6 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 07/13] i2c: mvtwsi: Add compatibility functions Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 09/13] i2c: mvtwsi: Make address length variable Mario Six
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

To be able to use the compatibility layer from the DM functions, we
factor the adap parameter out of all functions, and pass the actual
register base instead.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 97 +++++++++++++++++++++++++---------------------------
 1 file changed, 46 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index f1bfd5d..688efc2 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -208,9 +208,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es)
  * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as
  * expected, return 0 (ok) or 'wrong status' otherwise.
  */
-static int twsi_wait(struct i2c_adapter *adap, int expected_status)
+static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	int control, status;
 	int timeout = 1000;

@@ -236,39 +235,35 @@ static int twsi_wait(struct i2c_adapter *adap, int expected_status)
  * Assert the START condition, either in a single I2C transaction
  * or inside back-to-back ones (repeated starts).
  */
-static int twsi_start(struct i2c_adapter *adap, int expected_status)
+static int twsi_start(struct mvtwsi_registers *twsi, int expected_status)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-
 	/* Assert START */
 	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START |
 	       MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to process START */
-	return twsi_wait(adap, expected_status);
+	return twsi_wait(twsi, expected_status);
 }

 /*
  * Send a byte (i2c address or data).
  */
-static int twsi_send(struct i2c_adapter *adap, u8 byte, int expected_status)
+static int twsi_send(struct mvtwsi_registers *twsi, u8 byte,
+		     int expected_status)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-
 	/* Write byte to data register for sending */
 	writel(byte, &twsi->data);
 	/* Clear any pending interrupt -- that will cause sending */
 	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG,
 	       &twsi->control);
 	/* Wait for controller to receive byte, and check ACK */
-	return twsi_wait(adap, expected_status);
+	return twsi_wait(twsi, expected_status);
 }

 /*
  * Receive a byte.
  */
-static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag)
+static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	int expected_status, status, control;

 	/* Compute expected status based on passed ACK flag */
@@ -279,7 +274,7 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag)
 	control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0;
 	writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to receive byte, and assert ACK or NAK */
-	status = twsi_wait(adap, expected_status);
+	status = twsi_wait(twsi, expected_status);
 	/* If we did receive the expected byte, store it */
 	if (status == 0)
 		*byte = readl(&twsi->data);
@@ -290,9 +285,8 @@ static int twsi_recv(struct i2c_adapter *adap, u8 *byte, int ack_flag)
  * Assert the STOP condition.
  * This is also used to force the bus back to idle (SDA = SCL = 1).
  */
-static int twsi_stop(struct i2c_adapter *adap)
+static int twsi_stop(struct mvtwsi_registers *twsi)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	int control, stop_status;
 	int status = 0;
 	int timeout = 1000;
@@ -328,10 +322,8 @@ static uint twsi_calc_freq(const int n, const int m)
  * Controller reset also resets the baud rate and slave address, so
  * they must be re-established afterwards.
  */
-static void twsi_reset(struct i2c_adapter *adap)
+static void twsi_reset(struct mvtwsi_registers *twsi)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-
 	/* Reset controller */
 	writel(0, &twsi->soft_reset);
 	/* Wait 2 ms -- this is what the Marvell LSP does */
@@ -341,10 +333,9 @@ static void twsi_reset(struct i2c_adapter *adap)
 /*
  * Sets baud to the highest possible value not exceeding the requested one.
  */
-static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
+static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
 				     uint requested_speed)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
 	uint tmp_speed, highest_speed, n, m;
 	uint baud = 0x44; /* Baud rate after controller reset */

@@ -366,26 +357,25 @@ static uint __twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 	return 0;
 }

-static void __twsi_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
+static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
+			  int slaveadd)
 {
-	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-
 	/* Reset controller */
-	twsi_reset(adap);
+	twsi_reset(twsi);
 	/* Set speed */
-	__twsi_i2c_set_bus_speed(adap, speed);
+	__twsi_i2c_set_bus_speed(twsi, speed);
 	/* Set slave address; even though we don't use it */
 	writel(slaveadd, &twsi->slave_address);
 	writel(0, &twsi->xtnd_slave_addr);
 	/* Assert STOP, but don't care for the result */
-	(void) twsi_stop(adap);
+	(void) twsi_stop(twsi);
 }

 /*
  * Begin I2C transaction with expected start status, at given address.
  * Expected address status will derive from direction bit (bit 0) in addr.
  */
-static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
+static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
 		     u8 addr)
 {
 	int status, expected_addr_status;
@@ -397,10 +387,10 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 	else /* Writing */
 		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
 	/* Assert START */
-	status = twsi_start(adap, expected_start_status);
+	status = twsi_start(twsi, expected_start_status);
 	/* Send out the address if the start went well */
 	if (status == 0)
-		status = twsi_send(adap, addr, expected_addr_status);
+		status = twsi_send(twsi, addr, expected_addr_status);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -408,18 +398,18 @@ static int i2c_begin(struct i2c_adapter *adap, int expected_start_status,
 /*
  * Begin read, nak data byte, end.
  */
-static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip)
+static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip)
 {
 	u8 dummy_byte;
 	int status;

 	/* Begin i2c read */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1) | 1);
+	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1);
 	/* Dummy read was accepted: receive byte, but NAK it. */
 	if (status == 0)
-		status = twsi_recv(adap, &dummy_byte, MVTWSI_READ_NAK);
+		status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK);
 	/* Stop transaction */
-	twsi_stop(adap);
+	twsi_stop(twsi);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -432,30 +422,30 @@ static int __twsi_i2c_probe_chip(struct i2c_adapter *adap, uchar chip)
  * higher level APIs, we need to make a decision here, and for the moment that
  * will be a repeated start without a preceding stop.
  */
-static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
-			   int alen, uchar *data, int length)
+static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
+			   uint addr, int alen, uchar *data, int length)
 {
 	int status = 0;
 	int stop_status;

 	/* Begin i2c write to send the address bytes */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1));
+	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
 	while ((status == 0) && alen--)
-		status = twsi_send(adap, addr >> (8*alen),
+		status = twsi_send(twsi, addr >> (8*alen),
 			MVTWSI_STATUS_DATA_W_ACK);
 	/* Begin i2c read to receive data bytes */
 	if (status == 0)
-		status = i2c_begin(adap, MVTWSI_STATUS_REPEATED_START,
+		status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START,
 				   (chip << 1) | 1);
 	/* Receive actual data bytes; set NAK if we if we have nothing more to
 	 * read */
 	while ((status == 0) && length--)
-		status = twsi_recv(adap, data++,
+		status = twsi_recv(twsi, data++,
 				   length > 0 ?
 				   MVTWSI_READ_ACK : MVTWSI_READ_NAK);
 	/* Stop transaction */
-	stop_status = twsi_stop(adap);
+	stop_status = twsi_stop(twsi);
 	/* Return 0, or the status of the first failure */
 	return status != 0 ? status : stop_status;
 }
@@ -463,23 +453,23 @@ static int __twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 /*
  * Begin write, send address byte(s), send data bytes, end.
  */
-static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
-			    int alen, uchar *data, int length)
+static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
+			    uint addr, int alen, uchar *data, int length)
 {
 	int status, stop_status;

 	/* Begin i2c write to send first the address bytes, then the
 	 * data bytes */
-	status = i2c_begin(adap, MVTWSI_STATUS_START, (chip << 1));
+	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
 	while ((status == 0) && alen--)
-		status = twsi_send(adap, addr >> (8*alen),
+		status = twsi_send(twsi, addr >> (8*alen),
 			MVTWSI_STATUS_DATA_W_ACK);
 	/* Send data bytes */
 	while ((status == 0) && (length-- > 0))
-		status = twsi_send(adap, *(data++), MVTWSI_STATUS_DATA_W_ACK);
+		status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK);
 	/* Stop transaction */
-	stop_status = twsi_stop(adap);
+	stop_status = twsi_stop(twsi);
 	/* Return 0, or the status of the first failure */
 	return status != 0 ? status : stop_status;
 }
@@ -487,30 +477,35 @@ static int __twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 static void twsi_i2c_init(struct i2c_adapter *adap, int speed,
 			  int slaveadd)
 {
-	__twsi_i2c_init(adap, speed, slaveadd);
+	struct mvtwsi_registers *twsi = twsi_get_base(adap);
+	__twsi_i2c_init(twsi, speed, slaveadd);
 }

 static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 				   uint requested_speed)
 {
-	return __twsi_i2c_set_bus_speed(adap, requested_speed);
+	struct mvtwsi_registers *twsi = twsi_get_base(adap);
+	return __twsi_i2c_set_bus_speed(twsi, requested_speed);
 }

 static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 {
-	return __twsi_i2c_probe_chip(adap, chip);
+	struct mvtwsi_registers *twsi = twsi_get_base(adap);
+	return __twsi_i2c_probe_chip(twsi, chip);
 }

 static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 			 int alen, uchar *data, int length)
 {
-	return __twsi_i2c_read(adap, chip, addr, alen, data, length);
+	struct mvtwsi_registers *twsi = twsi_get_base(adap);
+	return __twsi_i2c_read(twsi, chip, addr, alen, data, length);
 }

 static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 			  int alen, uchar *data, int length)
 {
-	return __twsi_i2c_write(adap, chip, addr, alen, data, length);
+	struct mvtwsi_registers *twsi = twsi_get_base(adap);
+	return __twsi_i2c_write(twsi, chip, addr, alen, data, length);
 }

 #ifdef CONFIG_I2C_MVTWSI_BASE0
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 09/13] i2c: mvtwsi: Make address length variable
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (7 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 08/13] i2c: mvtwsi: Factor out adap parameter Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 10/13] i2c: mvtwsi: Add compatibility to DM Mario Six
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

The length of the address parameter of the __twsi_i2c_read and
__twsi_i2c_write functions is fixed to four bytes.

As a final step in the preparation of the DM conversion, we make the
length of this parameter variable by turning it into an array of bytes,
and convert the 32 bit value that's passed to the legacy functions into
a four-byte-array on the fly.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 688efc2..3325c4b 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -423,7 +423,7 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip)
  * will be a repeated start without a preceding stop.
  */
 static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
-			   uint addr, int alen, uchar *data, int length)
+			   u8 *addr, int alen, uchar *data, int length)
 {
 	int status = 0;
 	int stop_status;
@@ -432,8 +432,7 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
 	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
 	while ((status == 0) && alen--)
-		status = twsi_send(twsi, addr >> (8*alen),
-			MVTWSI_STATUS_DATA_W_ACK);
+		status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK);
 	/* Begin i2c read to receive data bytes */
 	if (status == 0)
 		status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START,
@@ -454,7 +453,7 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
  * Begin write, send address byte(s), send data bytes, end.
  */
 static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
-			    uint addr, int alen, uchar *data, int length)
+			    u8 *addr, int alen, uchar *data, int length)
 {
 	int status, stop_status;

@@ -462,9 +461,8 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
 	 * data bytes */
 	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
 	/* Send address bytes */
-	while ((status == 0) && alen--)
-		status = twsi_send(twsi, addr >> (8*alen),
-			MVTWSI_STATUS_DATA_W_ACK);
+	while ((status == 0) && (alen-- > 0))
+		status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK);
 	/* Send data bytes */
 	while ((status == 0) && (length-- > 0))
 		status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK);
@@ -498,14 +496,28 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 			 int alen, uchar *data, int length)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	return __twsi_i2c_read(twsi, chip, addr, alen, data, length);
+	u8 addr_bytes[4];
+
+	addr_bytes[0] = (addr >> 0) & 0xFF;
+	addr_bytes[1] = (addr >> 8) & 0xFF;
+	addr_bytes[2] = (addr >> 16) & 0xFF;
+	addr_bytes[3] = (addr >> 24) & 0xFF;
+
+	return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length);
 }

 static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 			  int alen, uchar *data, int length)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	return __twsi_i2c_write(twsi, chip, addr, alen, data, length);
+	u8 addr_bytes[4];
+
+	addr_bytes[0] = (addr >> 0) & 0xFF;
+	addr_bytes[1] = (addr >> 8) & 0xFF;
+	addr_bytes[2] = (addr >> 16) & 0xFF;
+	addr_bytes[3] = (addr >> 24) & 0xFF;
+
+	return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length);
 }

 #ifdef CONFIG_I2C_MVTWSI_BASE0
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 10/13] i2c: mvtwsi: Add compatibility to DM
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (8 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 09/13] i2c: mvtwsi: Make address length variable Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 11/13] i2c: mvtwsi: Handle zero-length offsets properly Mario Six
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

This patch adds the necessary functions and Kconfig entry to make the
MVTWSI I2C driver compatible with the driver model.

A possible device tree entry might look like this:

i2c at 11100 {
	compatible = "marvell,mv64xxx-i2c";
	reg = <0x11000 0x20>;
	clock-frequency = <100000>;
	u-boot,i2c-slave-addr = <0x0>;
};

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
- Correct register base assignment to using dev_get_addr_ptr, as pointed out by
  Stefan Roese.

---
 drivers/i2c/Kconfig  |   7 ++++
 drivers/i2c/mvtwsi.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 6e22bba..b3e8405 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -154,6 +154,13 @@ config SYS_I2C_UNIPHIER_F
 	  Support for UniPhier FIFO-builtin I2C controller driver.
 	  This I2C controller is used on PH1-Pro4 or newer UniPhier SoCs.

+config SYS_I2C_MVTWSI
+	bool "Marvell I2C driver"
+	depends on DM_I2C
+	help
+	  Support for Marvell I2C controllers as used on the orion5x and
+	  kirkwood SoC families.
+
 source "drivers/i2c/muxes/Kconfig"

 endmenu
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 3325c4b..f694527 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -12,12 +12,18 @@
 #include <i2c.h>
 #include <asm/errno.h>
 #include <asm/io.h>
+#ifdef CONFIG_DM_I2C
+#include <dm.h>
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;

 /*
  * Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other
  * settings
  */

+#ifndef CONFIG_DM_I2C
 #if defined(CONFIG_ORION5X)
 #include <asm/arch/orion5x.h>
 #elif (defined(CONFIG_KIRKWOOD) || defined(CONFIG_ARCH_MVEBU))
@@ -27,6 +33,7 @@
 #else
 #error Driver mvtwsi not supported by SoC or board
 #endif
+#endif /* CONFIG_DM_I2C */

 /*
  * TWSI register structure
@@ -61,6 +68,19 @@ struct  mvtwsi_registers {

 #endif

+#ifdef CONFIG_DM_I2C
+struct mvtwsi_i2c_dev {
+	/* TWSI Register base for the device */
+	struct mvtwsi_registers *base;
+	/* Number of the device (determined from cell-index property) */
+	int index;
+	/* The I2C slave address for the device */
+	u8 slaveadd;
+	/* The configured I2C speed in Hz */
+	uint speed;
+};
+#endif /* CONFIG_DM_I2C */
+
 /*
  * enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control
  * register
@@ -134,6 +154,7 @@ enum mvtwsi_ack_flags {
 	MVTWSI_READ_ACK = 1,
 };

+#ifndef CONFIG_DM_I2C
 /*
  * MVTWSI controller base
  */
@@ -172,6 +193,7 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)

 	return NULL;
 }
+#endif

 /*
  * enum mvtwsi_error_class - types of I2C errors
@@ -358,7 +380,7 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
 }

 static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
-			  int slaveadd)
+			    int slaveadd)
 {
 	/* Reset controller */
 	twsi_reset(twsi);
@@ -472,6 +494,7 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
 	return status != 0 ? status : stop_status;
 }

+#ifndef CONFIG_DM_I2C
 static void twsi_i2c_init(struct i2c_adapter *adap, int speed,
 			  int slaveadd)
 {
@@ -561,3 +584,89 @@ U_BOOT_I2C_ADAP_COMPLETE(twsi5, twsi_i2c_init, twsi_i2c_probe,
 			 CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE, 5)

 #endif
+#else /* CONFIG_DM_I2C */
+
+static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
+				 u32 chip_flags)
+{
+	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
+	return __twsi_i2c_probe_chip(dev->base, chip_addr);
+}
+
+static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed)
+{
+	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
+	return __twsi_i2c_set_bus_speed(dev->base, speed);
+}
+
+static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
+{
+	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
+
+	dev->base = dev_get_addr_ptr(bus);
+
+	if (!dev->base)
+		return -ENOMEM;
+
+	dev->index = fdtdec_get_int(gd->fdt_blob, bus->of_offset,
+				    "cell-index", -1);
+	dev->slaveadd = fdtdec_get_int(gd->fdt_blob, bus->of_offset,
+				       "u-boot,i2c-slave-addr", 0x0);
+	dev->speed = fdtdec_get_int(gd->fdt_blob, bus->of_offset,
+				    "clock-frequency", 100000);
+	return 0;
+}
+
+static int mvtwsi_i2c_probe(struct udevice *bus)
+{
+	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
+	__twsi_i2c_init(dev->base, dev->speed, dev->slaveadd);
+	return 0;
+}
+
+static int mvtwsi_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
+{
+	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
+	struct i2c_msg *dmsg, *omsg, dummy;
+
+	memset(&dummy, 0, sizeof(struct i2c_msg));
+
+	/* We expect either two messages (one with an offset and one with the
+	 * actual data) or one message (just data or offset/data combined) */
+	if (nmsgs > 2 || nmsgs == 0) {
+		debug("%s: Only one or two messages are supported.", __func__);
+		return -1;
+	}
+
+	omsg = nmsgs == 1 ? &dummy : msg;
+	dmsg = nmsgs == 1 ? msg : msg + 1;
+
+	if (dmsg->flags & I2C_M_RD)
+		return __twsi_i2c_read(dev->base, dmsg->addr, omsg->buf,
+				       omsg->len, dmsg->buf, dmsg->len);
+	else
+		return __twsi_i2c_write(dev->base, dmsg->addr, omsg->buf,
+					omsg->len, dmsg->buf, dmsg->len);
+}
+
+static const struct dm_i2c_ops mvtwsi_i2c_ops = {
+	.xfer		= mvtwsi_i2c_xfer,
+	.probe_chip	= mvtwsi_i2c_probe_chip,
+	.set_bus_speed	= mvtwsi_i2c_set_bus_speed,
+};
+
+static const struct udevice_id mvtwsi_i2c_ids[] = {
+	{ .compatible = "marvell,mv64xxx-i2c", },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(i2c_mvtwsi) = {
+	.name = "i2c_mvtwsi",
+	.id = UCLASS_I2C,
+	.of_match = mvtwsi_i2c_ids,
+	.probe = mvtwsi_i2c_probe,
+	.ofdata_to_platdata = mvtwsi_i2c_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct mvtwsi_i2c_dev),
+	.ops = &mvtwsi_i2c_ops,
+};
+#endif /* CONFIG_DM_I2C */
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 11/13] i2c: mvtwsi: Handle zero-length offsets properly
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (9 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 10/13] i2c: mvtwsi: Add compatibility to DM Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 12/13] i2c: mvtwsi: Make delay times frequency-dependent Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 13/13] i2c: mvtwsi: Add documentation Mario Six
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Zero-length offsets are not properly handled by the driver. When a read
operation with a zero-length offset is started, a START condition is
asserted, and since no offset bytes are transferred, a repeated START is
issued immediately after, which confuses the controller.

To fix this, we send the first START only if any address bytes need to
be sent, and keep track of the expected start status accordingly.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index f694527..8552fba 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -449,16 +449,21 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
 {
 	int status = 0;
 	int stop_status;
-
-	/* Begin i2c write to send the address bytes */
-	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
-	/* Send address bytes */
-	while ((status == 0) && alen--)
-		status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK);
+	int expected_start = MVTWSI_STATUS_START;
+
+	if (alen > 0) {
+		/* Begin i2c write to send the address bytes */
+		status = i2c_begin(twsi, expected_start, (chip << 1));
+		/* Send address bytes */
+		while ((status == 0) && alen--)
+			status = twsi_send(twsi, *(addr++),
+					   MVTWSI_STATUS_DATA_W_ACK);
+		/* Send repeated STARTs after the initial START */
+		expected_start = MVTWSI_STATUS_REPEATED_START;
+	}
 	/* Begin i2c read to receive data bytes */
 	if (status == 0)
-		status = i2c_begin(twsi, MVTWSI_STATUS_REPEATED_START,
-				   (chip << 1) | 1);
+		status = i2c_begin(twsi, expected_start, (chip << 1) | 1);
 	/* Receive actual data bytes; set NAK if we if we have nothing more to
 	 * read */
 	while ((status == 0) && length--)
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 12/13] i2c: mvtwsi: Make delay times frequency-dependent
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (10 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 11/13] i2c: mvtwsi: Handle zero-length offsets properly Mario Six
@ 2016-07-21  9:57 ` Mario Six
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 13/13] i2c: mvtwsi: Add documentation Mario Six
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Some devices using the MVTWSI driver have the option to run at speeds
faster than Standard Mode (100kHZ). On the Armada 38x controllers, this
is actually necessary, since due to erratum FE-8471889, a timing
violation concerning repeated starts prevents the controller from
working correctly in Standard Mode. One of the workarounds recommended
in the erratum is to set the bus to Fast Mode (400kHZ) operation and
ensure all connected devices are set to Fast Mode.

In the current version of the driver, however, the delay times are
hard-coded to 10ms, corresponding to Standard Mode operation. To take
full advantage of the faster modes, we would need to either keep the
currently configured I2C speed in a globally accessible variable, or
pass it to the necessary functions as a parameter. For DM, the first
option is not a problem, and we can simply keep the speed in the private
data of the driver. For the legacy interface, however, we would need to
introduce a static variable, which would cause problems with boots from
NOR flashes; see commit d6b7757 "i2c: mvtwsi: Eliminate
twsi_control_flags."

As to not clutter the interface with yet another parameter, we therefore
keep the default 10ms delays for the legacy functions.

In DM mode, we make the delay time dependant on the frequency to allow
taking full advantage of faster modes of operation (tested with up to
1MHZ frequency on Armada MV88F6820).

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
- Add forgotten #include.

---
 drivers/i2c/mvtwsi.c | 127 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 85 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 8552fba..f060cd1 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -12,6 +12,7 @@
 #include <i2c.h>
 #include <asm/errno.h>
 #include <asm/io.h>
+#include <linux/compat.h>
 #ifdef CONFIG_DM_I2C
 #include <dm.h>
 #endif
@@ -78,6 +79,8 @@ struct mvtwsi_i2c_dev {
 	u8 slaveadd;
 	/* The configured I2C speed in Hz */
 	uint speed;
+	/* The current length of a clock period (depending on speed) */
+	uint tick;
 };
 #endif /* CONFIG_DM_I2C */

@@ -154,7 +157,15 @@ enum mvtwsi_ack_flags {
 	MVTWSI_READ_ACK = 1,
 };

+inline uint calc_tick(uint speed)
+{
+	/* One tick = the duration of a period at the specified speed in ns (we
+	 * add 100 ns to be on the safe side) */
+	return (1000000000u / speed) + 100;
+}
+
 #ifndef CONFIG_DM_I2C
+
 /*
  * MVTWSI controller base
  */
@@ -230,7 +241,8 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es)
  * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as
  * expected, return 0 (ok) or 'wrong status' otherwise.
  */
-static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status)
+static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status,
+		     uint tick)
 {
 	int control, status;
 	int timeout = 1000;
@@ -246,7 +258,7 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status)
 					MVTWSI_ERROR_WRONG_STATUS,
 					control, status, expected_status);
 		}
-		udelay(10); /* One clock cycle at 100 kHz */
+		ndelay(tick); /* One clock cycle */
 	} while (timeout--);
 	status = readl(&twsi->status);
 	return mvtwsi_error(MVTWSI_ERROR_TIMEOUT, control, status,
@@ -257,20 +269,21 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status)
  * Assert the START condition, either in a single I2C transaction
  * or inside back-to-back ones (repeated starts).
  */
-static int twsi_start(struct mvtwsi_registers *twsi, int expected_status)
+static int twsi_start(struct mvtwsi_registers *twsi, int expected_status,
+		      uint tick)
 {
 	/* Assert START */
 	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_START |
 	       MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to process START */
-	return twsi_wait(twsi, expected_status);
+	return twsi_wait(twsi, expected_status, tick);
 }

 /*
  * Send a byte (i2c address or data).
  */
 static int twsi_send(struct mvtwsi_registers *twsi, u8 byte,
-		     int expected_status)
+		     int expected_status, uint tick)
 {
 	/* Write byte to data register for sending */
 	writel(byte, &twsi->data);
@@ -278,13 +291,14 @@ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte,
 	writel(MVTWSI_CONTROL_TWSIEN | MVTWSI_CONTROL_CLEAR_IFLG,
 	       &twsi->control);
 	/* Wait for controller to receive byte, and check ACK */
-	return twsi_wait(twsi, expected_status);
+	return twsi_wait(twsi, expected_status, tick);
 }

 /*
  * Receive a byte.
  */
-static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag)
+static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag,
+		     uint tick)
 {
 	int expected_status, status, control;

@@ -296,7 +310,7 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag)
 	control |= ack_flag == MVTWSI_READ_ACK ? MVTWSI_CONTROL_ACK : 0;
 	writel(control | MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* Wait for controller to receive byte, and assert ACK or NAK */
-	status = twsi_wait(twsi, expected_status);
+	status = twsi_wait(twsi, expected_status, tick);
 	/* If we did receive the expected byte, store it */
 	if (status == 0)
 		*byte = readl(&twsi->data);
@@ -307,7 +321,7 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag)
  * Assert the STOP condition.
  * This is also used to force the bus back to idle (SDA = SCL = 1).
  */
-static int twsi_stop(struct mvtwsi_registers *twsi)
+static int twsi_stop(struct mvtwsi_registers *twsi, uint tick)
 {
 	int control, stop_status;
 	int status = 0;
@@ -321,7 +335,7 @@ static int twsi_stop(struct mvtwsi_registers *twsi)
 		stop_status = readl(&twsi->status);
 		if (stop_status == MVTWSI_STATUS_IDLE)
 			break;
-		udelay(10); /* One clock cycle at 100 kHz */
+		ndelay(tick); /* One clock cycle */
 	} while (timeout--);
 	control = readl(&twsi->control);
 	if (stop_status != MVTWSI_STATUS_IDLE)
@@ -376,21 +390,32 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
 		}
 	}
 	writel(baud, &twsi->baudrate);
-	return 0;
+
+	/* Wait for controller for one tick */
+#ifdef CONFIG_DM_I2C
+	ndelay(calc_tick(highest_speed));
+#else
+	ndelay(10000);
+#endif
+	return highest_speed;
 }

 static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
-			    int slaveadd)
+			    int slaveadd, uint *actual_speed)
 {
 	/* Reset controller */
 	twsi_reset(twsi);
 	/* Set speed */
-	__twsi_i2c_set_bus_speed(twsi, speed);
+	*actual_speed = __twsi_i2c_set_bus_speed(twsi, speed);
 	/* Set slave address; even though we don't use it */
 	writel(slaveadd, &twsi->slave_address);
 	writel(0, &twsi->xtnd_slave_addr);
 	/* Assert STOP, but don't care for the result */
-	(void) twsi_stop(twsi);
+#ifdef CONFIG_DM_I2C
+	(void) twsi_stop(twsi, calc_tick(*actual_speed));
+#else
+	(void) twsi_stop(twsi, 10000);
+#endif
 }

 /*
@@ -398,7 +423,7 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
  * Expected address status will derive from direction bit (bit 0) in addr.
  */
 static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
-		     u8 addr)
+		     u8 addr, uint tick)
 {
 	int status, expected_addr_status;

@@ -409,10 +434,10 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
 	else /* Writing */
 		expected_addr_status = MVTWSI_STATUS_ADDR_W_ACK;
 	/* Assert START */
-	status = twsi_start(twsi, expected_start_status);
+	status = twsi_start(twsi, expected_start_status, tick);
 	/* Send out the address if the start went well */
 	if (status == 0)
-		status = twsi_send(twsi, addr, expected_addr_status);
+		status = twsi_send(twsi, addr, expected_addr_status, tick);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -420,18 +445,19 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
 /*
  * Begin read, nak data byte, end.
  */
-static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip)
+static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip,
+				 uint tick)
 {
 	u8 dummy_byte;
 	int status;

 	/* Begin i2c read */
-	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1);
+	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1) | 1, tick);
 	/* Dummy read was accepted: receive byte, but NAK it. */
 	if (status == 0)
-		status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK);
+		status = twsi_recv(twsi, &dummy_byte, MVTWSI_READ_NAK, tick);
 	/* Stop transaction */
-	twsi_stop(twsi);
+	twsi_stop(twsi, tick);
 	/* Return 0, or the status of the first failure */
 	return status;
 }
@@ -445,7 +471,8 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip)
  * will be a repeated start without a preceding stop.
  */
 static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
-			   u8 *addr, int alen, uchar *data, int length)
+			   u8 *addr, int alen, uchar *data, int length,
+			   uint tick)
 {
 	int status = 0;
 	int stop_status;
@@ -453,25 +480,25 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,

 	if (alen > 0) {
 		/* Begin i2c write to send the address bytes */
-		status = i2c_begin(twsi, expected_start, (chip << 1));
+		status = i2c_begin(twsi, expected_start, (chip << 1), tick);
 		/* Send address bytes */
 		while ((status == 0) && alen--)
 			status = twsi_send(twsi, *(addr++),
-					   MVTWSI_STATUS_DATA_W_ACK);
+					   MVTWSI_STATUS_DATA_W_ACK, tick);
 		/* Send repeated STARTs after the initial START */
 		expected_start = MVTWSI_STATUS_REPEATED_START;
 	}
 	/* Begin i2c read to receive data bytes */
 	if (status == 0)
-		status = i2c_begin(twsi, expected_start, (chip << 1) | 1);
+		status = i2c_begin(twsi, expected_start, (chip << 1) | 1, tick);
 	/* Receive actual data bytes; set NAK if we if we have nothing more to
 	 * read */
 	while ((status == 0) && length--)
 		status = twsi_recv(twsi, data++,
 				   length > 0 ?
-				   MVTWSI_READ_ACK : MVTWSI_READ_NAK);
+				   MVTWSI_READ_ACK : MVTWSI_READ_NAK, tick);
 	/* Stop transaction */
-	stop_status = twsi_stop(twsi);
+	stop_status = twsi_stop(twsi, tick);
 	/* Return 0, or the status of the first failure */
 	return status != 0 ? status : stop_status;
 }
@@ -480,21 +507,24 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
  * Begin write, send address byte(s), send data bytes, end.
  */
 static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
-			    u8 *addr, int alen, uchar *data, int length)
+			    u8 *addr, int alen, uchar *data, int length,
+			    uint tick)
 {
 	int status, stop_status;

 	/* Begin i2c write to send first the address bytes, then the
 	 * data bytes */
-	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1));
+	status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1), tick);
 	/* Send address bytes */
 	while ((status == 0) && (alen-- > 0))
-		status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK);
+		status = twsi_send(twsi, *(addr++), MVTWSI_STATUS_DATA_W_ACK,
+				   tick);
 	/* Send data bytes */
 	while ((status == 0) && (length-- > 0))
-		status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK);
+		status = twsi_send(twsi, *(data++), MVTWSI_STATUS_DATA_W_ACK,
+				   tick);
 	/* Stop transaction */
-	stop_status = twsi_stop(twsi);
+	stop_status = twsi_stop(twsi, tick);
 	/* Return 0, or the status of the first failure */
 	return status != 0 ? status : stop_status;
 }
@@ -504,20 +534,21 @@ static void twsi_i2c_init(struct i2c_adapter *adap, int speed,
 			  int slaveadd)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	__twsi_i2c_init(twsi, speed, slaveadd);
+	__twsi_i2c_init(twsi, speed, slaveadd, NULL);
 }

 static uint twsi_i2c_set_bus_speed(struct i2c_adapter *adap,
 				   uint requested_speed)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	return __twsi_i2c_set_bus_speed(twsi, requested_speed);
+	__twsi_i2c_set_bus_speed(twsi, requested_speed);
+	return 0;
 }

 static int twsi_i2c_probe(struct i2c_adapter *adap, uchar chip)
 {
 	struct mvtwsi_registers *twsi = twsi_get_base(adap);
-	return __twsi_i2c_probe_chip(twsi, chip);
+	return __twsi_i2c_probe_chip(twsi, chip, 10000);
 }

 static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
@@ -531,7 +562,8 @@ static int twsi_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 	addr_bytes[2] = (addr >> 16) & 0xFF;
 	addr_bytes[3] = (addr >> 24) & 0xFF;

-	return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length);
+	return __twsi_i2c_read(twsi, chip, addr_bytes, alen, data, length,
+			       10000);
 }

 static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
@@ -545,7 +577,8 @@ static int twsi_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 	addr_bytes[2] = (addr >> 16) & 0xFF;
 	addr_bytes[3] = (addr >> 24) & 0xFF;

-	return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length);
+	return __twsi_i2c_write(twsi, chip, addr_bytes, alen, data, length,
+				10000);
 }

 #ifdef CONFIG_I2C_MVTWSI_BASE0
@@ -595,13 +628,17 @@ static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
 				 u32 chip_flags)
 {
 	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
-	return __twsi_i2c_probe_chip(dev->base, chip_addr);
+	return __twsi_i2c_probe_chip(dev->base, chip_addr, dev->tick);
 }

 static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed)
 {
 	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
-	return __twsi_i2c_set_bus_speed(dev->base, speed);
+
+	dev->speed = __twsi_i2c_set_bus_speed(dev->base, speed);
+	dev->tick = calc_tick(dev->speed);
+
+	return 0;
 }

 static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
@@ -625,7 +662,11 @@ static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
 static int mvtwsi_i2c_probe(struct udevice *bus)
 {
 	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
-	__twsi_i2c_init(dev->base, dev->speed, dev->slaveadd);
+	uint actual_speed;
+
+	__twsi_i2c_init(dev->base, dev->speed, dev->slaveadd, &actual_speed);
+	dev->speed = actual_speed;
+	dev->tick = calc_tick(dev->speed);
 	return 0;
 }

@@ -648,10 +689,12 @@ static int mvtwsi_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)

 	if (dmsg->flags & I2C_M_RD)
 		return __twsi_i2c_read(dev->base, dmsg->addr, omsg->buf,
-				       omsg->len, dmsg->buf, dmsg->len);
+				       omsg->len, dmsg->buf, dmsg->len,
+				       dev->tick);
 	else
 		return __twsi_i2c_write(dev->base, dmsg->addr, omsg->buf,
-					omsg->len, dmsg->buf, dmsg->len);
+					omsg->len, dmsg->buf, dmsg->len,
+					dev->tick);
 }

 static const struct dm_i2c_ops mvtwsi_i2c_ops = {
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2 13/13] i2c: mvtwsi: Add documentation
  2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
                   ` (11 preceding siblings ...)
  2016-07-21  9:57 ` [U-Boot] [PATCH v2 12/13] i2c: mvtwsi: Make delay times frequency-dependent Mario Six
@ 2016-07-21  9:57 ` Mario Six
  12 siblings, 0 replies; 14+ messages in thread
From: Mario Six @ 2016-07-21  9:57 UTC (permalink / raw)
  To: u-boot

Add full documentation to all driver functions.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Stefan Roese <sr@denx.de>
---

Changes for v2:
None

---
 drivers/i2c/mvtwsi.c | 163 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 144 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index f060cd1..ab7481a 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -157,6 +157,12 @@ enum mvtwsi_ack_flags {
 	MVTWSI_READ_ACK = 1,
 };

+/*
+ * calc_tick() - Calculate the duration of a clock cycle from the I2C speed
+ *
+ * @speed:	The speed in Hz to calculate the clock cycle duration for.
+ * @return The duration of a clock cycle in ns.
+ */
 inline uint calc_tick(uint speed)
 {
 	/* One tick = the duration of a period at the specified speed in ns (we
@@ -167,9 +173,11 @@ inline uint calc_tick(uint speed)
 #ifndef CONFIG_DM_I2C

 /*
- * MVTWSI controller base
+ * twsi_get_base() - Get controller register base for specified adapter
+ *
+ * @adap:	Adapter to get the register base for.
+ * @return Register base for the specified adapter.
  */
-
 static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
 {
 	switch (adap->hwadapnr) {
@@ -238,8 +246,10 @@ inline uint mvtwsi_error(uint ec, uint lc, uint ls, uint es)
 }

 /*
- * Wait for IFLG to raise, or return 'timeout.' Then, if the status is as
- * expected, return 0 (ok) or 'wrong status' otherwise.
+ * twsi_wait() - Wait for I2C bus interrupt flag and check status, or time out.
+ *
+ * @return Zero if status is as expected, or a non-zero code if either a time
+ *	   out occurred, or the status was not the expected one.
  */
 static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status,
 		     uint tick)
@@ -266,8 +276,17 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status,
 }

 /*
- * Assert the START condition, either in a single I2C transaction
- * or inside back-to-back ones (repeated starts).
+ * twsi_start() - Assert a START condition on the bus.
+ *
+ * This function is used in both single I2C transactions and inside
+ * back-to-back transactions (repeated starts).
+ *
+ * @twsi:		The MVTWSI register structure to use.
+ * @expected_status:	The I2C bus status expected to be asserted after the
+ *			operation completion.
+ * @tick:		The duration of a clock cycle at the current I2C speed.
+ * @return Zero if status is as expected, or a non-zero code if either a time
+ *	   out occurred or the status was not the expected one.
  */
 static int twsi_start(struct mvtwsi_registers *twsi, int expected_status,
 		      uint tick)
@@ -280,7 +299,17 @@ static int twsi_start(struct mvtwsi_registers *twsi, int expected_status,
 }

 /*
- * Send a byte (i2c address or data).
+ * twsi_send() - Send a byte on the I2C bus.
+ *
+ * The byte may be part of an address byte or data.
+ *
+ * @twsi:		The MVTWSI register structure to use.
+ * @byte:		The byte to send.
+ * @expected_status:	The I2C bus status expected to be asserted after the
+ *			operation completion.
+ * @tick:		The duration of a clock cycle at the current I2C speed.
+ * @return Zero if status is as expected, or a non-zero code if either a time
+ *	   out occurred or the status was not the expected one.
  */
 static int twsi_send(struct mvtwsi_registers *twsi, u8 byte,
 		     int expected_status, uint tick)
@@ -295,7 +324,17 @@ static int twsi_send(struct mvtwsi_registers *twsi, u8 byte,
 }

 /*
- * Receive a byte.
+ * twsi_recv() - Receive a byte on the I2C bus.
+ *
+ * The static variable mvtwsi_control_flags controls whether we ack or nak.
+ *
+ * @twsi:		The MVTWSI register structure to use.
+ * @byte:		The byte to send.
+ * @ack_flag:		Flag that determines whether the received byte should
+ *			be acknowledged by the controller or not (sent ACK/NAK).
+ * @tick:		The duration of a clock cycle at the current I2C speed.
+ * @return Zero if status is as expected, or a non-zero code if either a time
+ *	   out occurred or the status was not the expected one.
  */
 static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag,
 		     uint tick)
@@ -318,8 +357,15 @@ static int twsi_recv(struct mvtwsi_registers *twsi, u8 *byte, int ack_flag,
 }

 /*
- * Assert the STOP condition.
- * This is also used to force the bus back to idle (SDA = SCL = 1).
+ * twsi_stop() - Assert a STOP condition on the bus.
+ *
+ * This function is also used to force the bus back to idle state (SDA =
+ * SCL = 1).
+ *
+ * @twsi:	The MVTWSI register structure to use.
+ * @tick:	The duration of a clock cycle at the current I2C speed.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out
+ *	   occurred.
  */
 static int twsi_stop(struct mvtwsi_registers *twsi, uint tick)
 {
@@ -344,6 +390,13 @@ static int twsi_stop(struct mvtwsi_registers *twsi, uint tick)
 	return status;
 }

+/*
+ * twsi_calc_freq() - Compute I2C frequency depending on m and n parameters.
+ *
+ * @n:		Parameter 'n' for the frequency calculation algorithm.
+ * @m:		Parameter 'm' for the frequency calculation algorithm.
+ * @return The I2C frequency corresponding to the passed m and n parameters.
+ */
 static uint twsi_calc_freq(const int n, const int m)
 {
 #ifdef CONFIG_SUNXI
@@ -354,9 +407,12 @@ static uint twsi_calc_freq(const int n, const int m)
 }

 /*
- * Reset controller.
- * Controller reset also resets the baud rate and slave address, so
- * they must be re-established afterwards.
+ * twsi_reset() - Reset the I2C controller.
+ *
+ * Resetting the controller also resets the baud rate and slave address, hence
+ * they must be re-established after the reset.
+ *
+ * @twsi:	The MVTWSI register structure to use.
  */
 static void twsi_reset(struct mvtwsi_registers *twsi)
 {
@@ -367,7 +423,15 @@ static void twsi_reset(struct mvtwsi_registers *twsi)
 }

 /*
- * Sets baud to the highest possible value not exceeding the requested one.
+ * __twsi_i2c_set_bus_speed() - Set the speed of the I2C controller.
+ *
+ * This function sets baud rate to the highest possible value that does not
+ * exceed the requested rate.
+ *
+ * @twsi:		The MVTWSI register structure to use.
+ * @requested_speed:	The desired frequency the controller should run at
+ *			in Hz.
+ * @return The actual frequency the controller was configured to.
  */
 static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
 				     uint requested_speed)
@@ -400,6 +464,18 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
 	return highest_speed;
 }

+/*
+ * __twsi_i2c_init() - Initialize the I2C controller.
+ *
+ * @twsi:		The MVTWSI register structure to use.
+ * @speed:		The initial frequency the controller should run at
+ *			in Hz.
+ * @slaveadd:		The I2C address to be set for the I2C master.
+ * @actual_speed:	A output parameter that receives the actual frequency
+ *			in Hz the controller was set to by the function.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out
+ *	   occurred.
+ */
 static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
 			    int slaveadd, uint *actual_speed)
 {
@@ -419,8 +495,21 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
 }

 /*
- * Begin I2C transaction with expected start status, at given address.
- * Expected address status will derive from direction bit (bit 0) in addr.
+ * i2c_begin() - Start a I2C transaction.
+ *
+ * Begin a I2C transaction with a given expected start status and chip address.
+ * A START is asserted, and the address byte is sent to the I2C controller. The
+ * expected address status will be derived from the direction bit (bit 0) of
+ * the address byte.
+ *
+ * @twsi:			The MVTWSI register structure to use.
+ * @expected_start_status:	The I2C status the controller is expected to
+ *				assert after the address byte was sent.
+ * @addr:			The address byte to be sent.
+ * @tick:			The duration of a clock cycle at the current
+ *				I2C speed.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out or
+ *	   unexpected I2C status occurred.
  */
 static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
 		     u8 addr, uint tick)
@@ -443,7 +532,16 @@ static int i2c_begin(struct mvtwsi_registers *twsi, int expected_start_status,
 }

 /*
- * Begin read, nak data byte, end.
+ * __twsi_i2c_probe_chip() - Probe the given I2C chip address.
+ *
+ * This function begins a I2C read transaction, does a dummy read and NAKs; if
+ * the procedure succeeds, the chip is considered to be present.
+ *
+ * @twsi:	The MVTWSI register structure to use.
+ * @chip:	The chip address to probe.
+ * @tick:	The duration of a clock cycle at the current I2C speed.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out or
+ *	   unexpected I2C status occurred.
  */
 static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip,
 				 uint tick)
@@ -463,12 +561,26 @@ static int __twsi_i2c_probe_chip(struct mvtwsi_registers *twsi, uchar chip,
 }

 /*
- * Begin write, send address byte(s), begin read, receive data bytes, end.
+ * __twsi_i2c_read() - Read data from a I2C chip.
+ *
+ * This function begins a I2C write transaction, and transmits the address
+ * bytes; then begins a I2C read transaction, and receives the data bytes.
  *
  * NOTE: Some devices want a stop right before the second start, while some
  * will choke if it is there. Since deciding this is not yet supported in
  * higher level APIs, we need to make a decision here, and for the moment that
  * will be a repeated start without a preceding stop.
+ *
+ * @twsi:	The MVTWSI register structure to use.
+ * @chip:	The chip address to read from.
+ * @addr:	The address bytes to send.
+ * @alen:	The length of the address bytes in bytes.
+ * @data:	The buffer to receive the data read from the chip (has to have
+ *		a size of at least 'length' bytes).
+ * @length:	The amount of data to be read from the chip in bytes.
+ * @tick:	The duration of a clock cycle at the current I2C speed.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out or
+ *	   unexpected I2C status occurred.
  */
 static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
 			   u8 *addr, int alen, uchar *data, int length,
@@ -504,7 +616,20 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi, uchar chip,
 }

 /*
- * Begin write, send address byte(s), send data bytes, end.
+ * __twsi_i2c_write() - Send data to a I2C chip.
+ *
+ * This function begins a I2C write transaction, and transmits the address
+ * bytes; then begins a new I2C write transaction, and sends the data bytes.
+ *
+ * @twsi:	The MVTWSI register structure to use.
+ * @chip:	The chip address to read from.
+ * @addr:	The address bytes to send.
+ * @alen:	The length of the address bytes in bytes.
+ * @data:	The buffer containing the data to be sent to the chip.
+ * @length:	The length of data to be sent to the chip in bytes.
+ * @tick:	The duration of a clock cycle at the current I2C speed.
+ * @return Zero if the operation succeeded, or a non-zero code if a time out or
+ *	   unexpected I2C status occurred.
  */
 static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
 			    u8 *addr, int alen, uchar *data, int length,
--
2.9.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-07-21  9:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-21  9:57 [U-Boot] [PATCH v2 00/13] i2c: mvtwsi: DM conversion and improvements Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 01/13] i2c: mvtwsi: Fix style violations Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 02/13] i2c: mvtwsi: Streamline code and add documentation Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 03/13] i2c: mvtwsi: Improve and fix comments Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 04/13] i2c: mvtwsi: Eliminate flags parameter Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 05/13] i2c: mvtwsi: Get rid of status parameter Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 06/13] i2c: mvtwsi: Use 'uint' instead of 'unsigned int' Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 07/13] i2c: mvtwsi: Add compatibility functions Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 08/13] i2c: mvtwsi: Factor out adap parameter Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 09/13] i2c: mvtwsi: Make address length variable Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 10/13] i2c: mvtwsi: Add compatibility to DM Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 11/13] i2c: mvtwsi: Handle zero-length offsets properly Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 12/13] i2c: mvtwsi: Make delay times frequency-dependent Mario Six
2016-07-21  9:57 ` [U-Boot] [PATCH v2 13/13] i2c: mvtwsi: Add documentation Mario Six

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.