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

login
register
mail settings
Submitter Michael Burr
Date Sept. 24, 2013, 4:32 p.m.
Message ID <EFBFAB0BC299D345A30F813A3C85DA8701253860@EDPR-EX01.logicpd.com>
Download mbox | patch
Permalink /patch/277540/
State Superseded
Delegated to: Heiko Schocher
Headers show

Comments

Michael Burr - Sept. 24, 2013, 4:32 p.m.
> 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(-)
Michal Simek - Sept. 25, 2013, 6:09 a.m.
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.
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

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) {