diff mbox

[U-Boot] I2C: Zynq: Support for 0-length register address

Message ID EFBFAB0BC299D345A30F813A3C85DA8701253860@EDPR-EX01.logicpd.com
State Superseded
Delegated to: Heiko Schocher
Headers show

Commit Message

Michael Burr Sept. 24, 2013, 4:32 p.m. UTC 
> Fixed bug with alen == 0 in 'i2c_write', 'i2c_read'
Further minor corrections:
> Write 'address' register before 'data' register.
> Write 'transfer_size' register before 'address' register.

Tested:
Xilinx ZC702 eval board.
Write and read registers with addresses of length 0 and 1.

Signed-off-by: Michael Burr <michael.burr@logicpd.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Michal Simek <monstr@monstr.eu>
---
 drivers/i2c/zynq_i2c.c |   43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

Comments

Michal Simek Sept. 25, 2013, 6:09 a.m. UTC | #1 
On 09/24/2013 06:32 PM, Michael Burr wrote:
>> Fixed bug with alen == 0 in 'i2c_write', 'i2c_read'
> Further minor corrections:
>> Write 'address' register before 'data' register.
>> Write 'transfer_size' register before 'address' register.

This is still not acceptable format (I mean reply in commit message).

From minor correction create separate small patches.
It can be just one line patch but it worth to have it
in connection to bisecting.

> 
> Tested:
> Xilinx ZC702 eval board.
> Write and read registers with addresses of length 0 and 1.

This could go below "---"

Thanks,
Michal
Michael Burr Sept. 26, 2013, 3:43 p.m. UTC | #2 
Michael,

Again, sorry. I will continue to work on improving my style
for commit messages in the future.

Michael Burr  //  Software Engineer II

Logic PD
T // 612.436.7273
NOTICE: Important disclaimers and limitations apply to this email.
Please see this web page for our disclaimers and limitations:
http://logicpd.com/email-disclaimer/

-----Original Message-----
From: Michal Simek [mailto:monstr@monstr.eu] 
Sent: Wednesday, September 25, 2013 1:10 AM
To: Michael Burr
Cc: u-boot@lists.denx.de; Heiko Schocher
Subject: Re: [PATCH] I2C: Zynq: Support for 0-length register address

On 09/24/2013 06:32 PM, Michael Burr wrote:
>> Fixed bug with alen == 0 in 'i2c_write', 'i2c_read'
> Further minor corrections:
>> Write 'address' register before 'data' register.
>> Write 'transfer_size' register before 'address' register.

This is still not acceptable format (I mean reply in commit message).

From minor correction create separate small patches.
It can be just one line patch but it worth to have it
in connection to bisecting.

> 
> Tested:
> Xilinx ZC702 eval board.
> Write and read registers with addresses of length 0 and 1.

This could go below "---"

Thanks,
Michal
diff mbox

Patch

diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c
index ce2d23f..9cbd3e4 100644
--- a/drivers/i2c/zynq_i2c.c
+++ b/drivers/i2c/zynq_i2c.c
@@ -187,20 +187,22 @@  int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
 	 * Temporarily disable restart (by clearing hold)
 	 * It doesn't seem to work.
 	 */
-	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW |
-		ZYNQ_I2C_CONTROL_HOLD);
+	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
 	writel(0xFF, &zynq_i2c->interrupt_status);
-	while (alen--)
-		writel(addr >> (8*alen), &zynq_i2c->data);
-	writel(dev, &zynq_i2c->address);
+	if (alen) {
+		clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW);
+		writel(dev, &zynq_i2c->address);
+		while (alen--)
+			writel(addr >> (8*alen), &zynq_i2c->data);
 
-	/* Wait for the address to be sent */
-	if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
-		/* Release the bus */
-		clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
-		return -ETIMEDOUT;
+		/* Wait for the address to be sent */
+		if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
+			/* Release the bus */
+			clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
+			return -ETIMEDOUT;
+		}
+		debug("Device acked address\n");
 	}
-	debug("Device acked address\n");
 
 	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
 		ZYNQ_I2C_CONTROL_RW);
@@ -244,17 +246,18 @@  int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 		ZYNQ_I2C_CONTROL_HOLD);
 	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW);
 	writel(0xFF, &zynq_i2c->interrupt_status);
-	while (alen--)
-		writel(addr >> (8*alen), &zynq_i2c->data);
-	/* Start the tranfer */
 	writel(dev, &zynq_i2c->address);
-	if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
-		/* Release the bus */
-		clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
-		return -ETIMEDOUT;
+	if (alen) {
+		while (alen--)
+			writel(addr >> (8*alen), &zynq_i2c->data);
+		/* Start the tranfer */
+		if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
+			/* Release the bus */
+			clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
+			return -ETIMEDOUT;
+		}
+		debug("Device acked address\n");
 	}
-
-	debug("Device acked address\n");
 	while (length--) {
 		writel(*(cur_data++), &zynq_i2c->data);
 		if (readl(&zynq_i2c->transfer_size) == ZYNQ_I2C_FIFO_DEPTH) {