[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

Zynq PS has two I2C bus masters (called 'I2C0' and 'I2C1'):
> Implement 'i2c_set_bus_num' routine to select from these.
Support I2C devices which do not use internal addresses:
> Handle case of 'alen == 0' in 'i2c_read', 'i2c_write'.
This supports PCA9548 bus multiplexer on Xilinx ZC702 board.
Tested cases of 'alen == 0' and 'alen == 1' on this board.
Further minor corrections:
> Write 'address' register before 'data' register.
> Write 'transfer_size' register before 'address' register.

Signed-off-by: Michael Burr <michael.burr@logicpd.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Heiko Schocher <hs@denx.de>
Cc: Michal Simek <monstr@monstr.eu>
---
 drivers/i2c/zynq_i2c.c |   87 ++++++++++++++++++++++++++++++------------------
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c
index ce2d23f..1c9ae30 100644
--- a/drivers/i2c/zynq_i2c.c
+++ b/drivers/i2c/zynq_i2c.c
@@ -7,7 +7,7 @@
  *
  * Copyright (c) 2012-2013 Xilinx, Michal Simek
  *
- * SPDX-License-Identifier:	GPL-2.0+
+ * SPDX-License-Identifier:    GPL-2.0+
  */
 
 #include <common.h>
@@ -64,18 +64,19 @@ struct zynq_i2c_registers {
 #define ZYNQ_I2C_FIFO_DEPTH		16
 #define ZYNQ_I2C_TRANSFERT_SIZE_MAX	255 /* Controller transfer limit */
 
-#if defined(CONFIG_ZYNQ_I2C0)
-# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR0
-#else
-# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR1
+#ifdef CONFIG_I2C_MULTI_BUS
+static unsigned int current_bus;
 #endif
-
-static struct zynq_i2c_registers *zynq_i2c =
-	(struct zynq_i2c_registers *)ZYNQ_I2C_BASE;
+static struct zynq_i2c_registers *zynq_i2c;
 
 /* I2C init called by cmd_i2c when doing 'i2c reset'. */
 void i2c_init(int requested_speed, int slaveadd)
 {
+#ifdef CONFIG_I2C_MULTI_BUS
+	current_bus = 0;
+#endif
+	zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
+
 	/* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */
 	writel((16 << ZYNQ_I2C_CONTROL_DIV_B_SHIFT) |
 		(2 << ZYNQ_I2C_CONTROL_DIV_A_SHIFT), &zynq_i2c->control);
@@ -187,26 +188,29 @@ 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);
 	/* Start reading data */
-	writel(dev, &zynq_i2c->address);
 	writel(length, &zynq_i2c->transfer_size);
+	writel(dev, &zynq_i2c->address);
 
 	/* Wait for data */
 	do {
@@ -244,17 +248,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) {
@@ -277,14 +282,32 @@ int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 
 int i2c_set_bus_num(unsigned int bus)
 {
-	/* Only support bus 0 */
-	if (bus > 0)
+	if (bus >= CONFIG_SYS_MAX_I2C_BUS)
 		return -1;
+
+	switch (bus) {
+	case 0:
+		zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
+		break;
+#ifdef CONFIG_I2C_MULTI_BUS
+	case 1:
+		zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR1;
+		break;
+#endif
+	default:
+		return -1;
+	}
+#ifdef CONFIG_I2C_MULTI_BUS
+	current_bus = bus;
+#endif
 	return 0;
 }
 
 unsigned int i2c_get_bus_num(void)
 {
-	/* Only support bus 0 */
+#ifdef CONFIG_I2C_MULTI_BUS
+	return current_bus;
+#else
 	return 0;
+#endif
 }
-- 
1.7.9.5

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Heiko Schocher]

Hello Michael,

a "How to send patches", Jagan Teki responded to you, so I only comment
your patch ...

Am 17.09.2013 19:29, schrieb Michael Burr:
> Zynq PS has two I2C bus masters (called 'I2C0' and 'I2C1'):
>> Implement 'i2c_set_bus_num' routine to select from these.
> Support I2C devices which do not use internal addresses:
>> Handle case of 'alen == 0' in 'i2c_read', 'i2c_write'.
> This supports PCA9548 bus multiplexer on Xilinx ZC702 board.
> Tested cases of 'alen == 0' and 'alen == 1' on this board.
> Further minor corrections:
>> Write 'address' register before 'data' register.
>> Write 'transfer_size' register before 'address' register.
>
> Signed-off-by: Michael Burr<michael.burr@logicpd.com>
> Cc: Albert Aribaud<albert.u.boot@aribaud.net>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Michal Simek<monstr@monstr.eu>
> ---
>   drivers/i2c/zynq_i2c.c |   87 ++++++++++++++++++++++++++++++------------------
>   1 file changed, 55 insertions(+), 32 deletions(-)

Thanks for your work!

But please use the new i2c multibus/multiadapter framework for adding
multibus support for this driver. Currently 4 i2c drivers are ported
to this new framework, see:

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=880540decfb855e96bc14ac84ac7784669e4b382
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1f2ba722ac06393d6abe6d4734824d3b98ea9108
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=00f792e0df9ae942427e44595a0f4379582accee
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=ea818dbbcd59300b56014ac2d67798a54994eb9b

You can use them as a base for your work

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

Thanks for the feedback, Heiko!

I will take a look at the new framework...

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: Heiko Schocher [mailto:hs at denx.de] 
Sent: Wednesday, September 18, 2013 2:14 AM
To: Michael Burr
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

Hello Michael,

a "How to send patches", Jagan Teki responded to you, so I only comment
your patch ...

Am 17.09.2013 19:29, schrieb Michael Burr:
> Zynq PS has two I2C bus masters (called 'I2C0' and 'I2C1'):
>> Implement 'i2c_set_bus_num' routine to select from these.
> Support I2C devices which do not use internal addresses:
>> Handle case of 'alen == 0' in 'i2c_read', 'i2c_write'.
> This supports PCA9548 bus multiplexer on Xilinx ZC702 board.
> Tested cases of 'alen == 0' and 'alen == 1' on this board.
> Further minor corrections:
>> Write 'address' register before 'data' register.
>> Write 'transfer_size' register before 'address' register.
>
> Signed-off-by: Michael Burr<michael.burr@logicpd.com>
> Cc: Albert Aribaud<albert.u.boot@aribaud.net>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Michal Simek<monstr@monstr.eu>
> ---
>   drivers/i2c/zynq_i2c.c |   87 ++++++++++++++++++++++++++++++------------------
>   1 file changed, 55 insertions(+), 32 deletions(-)

Thanks for your work!

But please use the new i2c multibus/multiadapter framework for adding
multibus support for this driver. Currently 4 i2c drivers are ported
to this new framework, see:

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=880540decfb855e96bc14ac84ac7784669e4b382
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1f2ba722ac06393d6abe6d4734824d3b98ea9108
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=00f792e0df9ae942427e44595a0f4379582accee
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=ea818dbbcd59300b56014ac2d67798a54994eb9b

You can use them as a base for your work

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

Changes:
'zynq_i2c.c':
Adapted driver for compliance with "new" I2C driver model
(CONFIG_SYS_I2C).
Support for 0-length register address in 'i2c_write', 'i2c_read'.
'i2c.h', 'i2c_core.c':
Support for Texas Instruments PCA9548 bus multiplexer.

Tested:
Xilinx ZC702 eval board (single bus master 'I2C0' with multiplexer
PCA9548).
Write and read registers with addresses of length 0 and 1.

Note:
Mainline code does not work "out of the box" on ZC702 at this time;
ad hoc adaptations based on 'u-boot-xlnx' repository were needed
in order to test on this board. (Those adapatations are not included
with this patch.)

Signed-off-by: Michael Burr <michael.burr@logicpd.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Michal Simek <monstr@monstr.eu>
---
 drivers/i2c/i2c_core.c |    5 ++
 drivers/i2c/zynq_i2c.c |  148 +++++++++++++++++++++++++++++-------------------
 include/i2c.h          |    3 +
 3 files changed, 97 insertions(+), 59 deletions(-)

diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
index d1072e8..b263562 100644
--- a/drivers/i2c/i2c_core.c
+++ b/drivers/i2c/i2c_core.c
@@ -138,6 +138,11 @@ static int i2c_mux_set(struct i2c_adapter *adap, int mux_id, int chip,
 			return -1;
 		buf = (uint8_t)((channel & 0x07) | (1 << 3));
 		break;
+	case I2C_MUX_PCA9548_ID:
+		if (channel > 7)
+			return -1;
+		buf = (uint8_t)(0x01 << channel);
+		break;
 	default:
 		printf("%s: wrong mux id: %d\n", __func__, mux_id);
 		return -1;
diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c
index ce2d23f..be27966 100644
--- a/drivers/i2c/zynq_i2c.c
+++ b/drivers/i2c/zynq_i2c.c
@@ -7,6 +7,8 @@
  *
  * Copyright (c) 2012-2013 Xilinx, Michal Simek
  *
+ * Copyright (c) 2013 Logic PD, Michael Burr
+ *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
@@ -64,18 +66,28 @@ struct zynq_i2c_registers {
 #define ZYNQ_I2C_FIFO_DEPTH		16
 #define ZYNQ_I2C_TRANSFERT_SIZE_MAX	255 /* Controller transfer limit */
 
-#if defined(CONFIG_ZYNQ_I2C0)
-# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR0
-#else
-# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR1
-#endif
-
-static struct zynq_i2c_registers *zynq_i2c =
-	(struct zynq_i2c_registers *)ZYNQ_I2C_BASE;
+static struct zynq_i2c_registers *i2c_select(struct i2c_adapter *adap)
+{
+	return adap->hwadapnr ?
+	       /* Zynq PS I2C1 */
+	       (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR1 :
+	       /* Zynq PS I2C0 */
+	       (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
+}
 
 /* I2C init called by cmd_i2c when doing 'i2c reset'. */
-void i2c_init(int requested_speed, int slaveadd)
+static void zynq_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
 {
+	struct zynq_i2c_registers *zynq_i2c = i2c_select(adap);
+
+	if (speed != 100000)
+		debug("Warning: requested speed not supported.\n");
+	if (slaveaddr)
+		debug("Warning: slave mode not supported.\n");
+
+	/* The following _assumes_ clock rate cpu_1x = 111 MHz */
+	/* This could use improvement! Also see 'i2c_set_bus_speed' below */
+
 	/* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */
 	writel((16 << ZYNQ_I2C_CONTROL_DIV_B_SHIFT) |
 		(2 << ZYNQ_I2C_CONTROL_DIV_A_SHIFT), &zynq_i2c->control);
@@ -86,7 +98,7 @@ void i2c_init(int requested_speed, int slaveadd)
 }
 
 #ifdef DEBUG
-static void zynq_i2c_debug_status(void)
+static void zynq_i2c_debug_status(struct zynq_i2c_registers *zynq_i2c)
 {
 	int int_status;
 	int status;
@@ -128,7 +140,7 @@ static void zynq_i2c_debug_status(void)
 #endif
 
 /* Wait for an interrupt */
-static u32 zynq_i2c_wait(u32 mask)
+static u32 zynq_i2c_wait(struct zynq_i2c_registers *zynq_i2c, u32 mask)
 {
 	int timeout, int_status;
 
@@ -139,7 +151,7 @@ static u32 zynq_i2c_wait(u32 mask)
 			break;
 	}
 #ifdef DEBUG
-	zynq_i2c_debug_status();
+	zynq_i2c_debug_status(zynq_i2c);
 #endif
 	/* Clear interrupt status flags */
 	writel(int_status & mask, &zynq_i2c->interrupt_status);
@@ -151,17 +163,19 @@ static u32 zynq_i2c_wait(u32 mask)
  * I2C probe called by cmd_i2c when doing 'i2c probe'.
  * Begin read, nak data byte, end.
  */
-int i2c_probe(u8 dev)
+static int zynq_i2c_probe(struct i2c_adapter *adap, uint8_t chip)
 {
+	struct zynq_i2c_registers *zynq_i2c = i2c_select(adap);
+
 	/* Attempt to read a byte */
 	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
 		ZYNQ_I2C_CONTROL_RW);
 	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
 	writel(0xFF, &zynq_i2c->interrupt_status);
-	writel(dev, &zynq_i2c->address);
+	writel(chip, &zynq_i2c->address);
 	writel(1, &zynq_i2c->transfer_size);
 
-	return (zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP |
+	return (zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP |
 		ZYNQ_I2C_INTERRUPT_NACK) &
 		ZYNQ_I2C_INTERRUPT_COMP) ? 0 : -ETIMEDOUT;
 }
@@ -170,14 +184,18 @@ int i2c_probe(u8 dev)
  * 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.
  */
-int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
+static int zynq_i2c_read(struct i2c_adapter *adap, uint8_t chip, uint addr,
+			 int alen, uint8_t *buffer, int len)
 {
+	struct zynq_i2c_registers *zynq_i2c;
 	u32 status;
 	u32 i = 0;
-	u8 *cur_data = data;
+	u8 *cur_data = buffer;
+
+	zynq_i2c = i2c_select(adap);
 
 	/* Check the hardware can handle the requested bytes */
-	if ((length < 0) || (length > ZYNQ_I2C_TRANSFERT_SIZE_MAX))
+	if ((len < 0) || (len > ZYNQ_I2C_TRANSFERT_SIZE_MAX))
 		return -EINVAL;
 
 	/* Write the register address */
@@ -187,30 +205,33 @@ 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);
-
-	/* 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;
+	if (alen) {
+		clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW);
+		writel(chip, &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, 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);
 	/* Start reading data */
-	writel(dev, &zynq_i2c->address);
-	writel(length, &zynq_i2c->transfer_size);
+	writel(len, &zynq_i2c->transfer_size);
+	writel(chip, &zynq_i2c->address);
 
 	/* Wait for data */
 	do {
-		status = zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP |
+		status = zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP |
 			ZYNQ_I2C_INTERRUPT_DATA);
 		if (!status) {
 			/* Release the bus */
@@ -218,15 +239,15 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
 			return -ETIMEDOUT;
 		}
 		debug("Read %d bytes\n",
-		      length - readl(&zynq_i2c->transfer_size));
-		for (; i < length - readl(&zynq_i2c->transfer_size); i++)
+		      len - readl(&zynq_i2c->transfer_size));
+		for (; i < len - readl(&zynq_i2c->transfer_size); i++)
 			*(cur_data++) = readl(&zynq_i2c->data);
 	} while (readl(&zynq_i2c->transfer_size) != 0);
 	/* All done... release the bus */
 	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
 
 #ifdef DEBUG
-	zynq_i2c_debug_status();
+	zynq_i2c_debug_status(zynq_i2c);
 #endif
 	return 0;
 }
@@ -235,30 +256,35 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
  * I2C write called by cmd_i2c when doing 'i2c write' and by cmd_eeprom.c
  * Begin write, send address byte(s), send data bytes, end.
  */
-int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
+static int zynq_i2c_write(struct i2c_adapter *adap, uint8_t chip, uint addr,
+			  int alen, uint8_t *buffer, int len)
 {
-	u8 *cur_data = data;
+	struct zynq_i2c_registers *zynq_i2c;
+	u8 *cur_data = buffer;
+
+	zynq_i2c = i2c_select(adap);
 
 	/* Write the register address */
 	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
 		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;
+	writel(chip, &zynq_i2c->address);
+	if (alen) {
+		while (alen--)
+			writel(addr >> (8*alen), &zynq_i2c->data);
+		/* Start the tranfer */
+		if (!zynq_i2c_wait(zynq_i2c, 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--) {
+	while (len--) {
 		writel(*(cur_data++), &zynq_i2c->data);
 		if (readl(&zynq_i2c->transfer_size) == ZYNQ_I2C_FIFO_DEPTH) {
-			if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
+			if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP)) {
 				/* Release the bus */
 				clrbits_le32(&zynq_i2c->control,
 					     ZYNQ_I2C_CONTROL_HOLD);
@@ -270,21 +296,25 @@ int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 	/* All done... release the bus */
 	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
 	/* Wait for the address and data to be sent */
-	if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP))
+	if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP))
 		return -ETIMEDOUT;
 	return 0;
 }
 
-int i2c_set_bus_num(unsigned int bus)
+static uint zynq_i2c_set_bus_speed(struct i2c_adapter *adap, uint speed)
 {
-	/* Only support bus 0 */
-	if (bus > 0)
+	/* struct zynq_i2c_registers *zynq_i2c = i2c_select(adap); */
+
+	/* Bus-speed selection is not implemented */
+	if (speed != 100000)
 		return -1;
-	return 0;
-}
 
-unsigned int i2c_get_bus_num(void)
-{
-	/* Only support bus 0 */
 	return 0;
 }
+
+U_BOOT_I2C_ADAP_COMPLETE(ZYNQ_I2C0, zynq_i2c_init, zynq_i2c_probe,
+			 zynq_i2c_read, zynq_i2c_write,
+			 zynq_i2c_set_bus_speed, 100000, 0, 0)
+U_BOOT_I2C_ADAP_COMPLETE(ZYNQ_I2C1, zynq_i2c_init, zynq_i2c_probe,
+			 zynq_i2c_read, zynq_i2c_write,
+			 zynq_i2c_set_bus_speed, 100000, 0, 1)
diff --git a/include/i2c.h b/include/i2c.h
index 8fd17d1..683affe 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -40,6 +40,7 @@
 #define CONFIG_SYS_I2C_MAX_HOPS		0
 #define CONFIG_SYS_NUM_I2C_BUSES	ll_entry_count(struct i2c_adapter, i2c)
 #else
+
 /* we use i2c muxes */
 #undef CONFIG_SYS_I2C_DIRECT_BUS
 #endif
@@ -135,6 +136,8 @@ extern struct i2c_bus_hose	i2c_bus[];
 #define I2C_MUX_PCA9544		{I2C_MUX_PCA9544_ID, "PCA9544A"}
 #define I2C_MUX_PCA9547_ID	4
 #define I2C_MUX_PCA9547		{I2C_MUX_PCA9547_ID, "PCA9547A"}
+#define I2C_MUX_PCA9548_ID	5
+#define I2C_MUX_PCA9548		{I2C_MUX_PCA9548_ID, "PCA9548"}
 #endif
 
 #ifndef I2C_SOFT_DECLARATIONS
-- 
1.7.9.5

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Heiko Schocher]

Hello Michael,

Am 20.09.2013 22:40, schrieb Michael Burr:
> Changes:
> 'zynq_i2c.c':
> Adapted driver for compliance with "new" I2C driver model
> (CONFIG_SYS_I2C).
> Support for 0-length register address in 'i2c_write', 'i2c_read'.
> 'i2c.h', 'i2c_core.c':
> Support for Texas Instruments PCA9548 bus multiplexer.

Your commit message implements, that your patch contains 3
independent changes ...  it would be better to seperate your
patch in 3 pieces ... so if your patch introduces a problem,
it is better to identify, which part it introduced ...

Can you do this easy? Except of this, I have only one minor
Codingstyle comment

> Tested:
> Xilinx ZC702 eval board (single bus master 'I2C0' with multiplexer
> PCA9548).
> Write and read registers with addresses of length 0 and 1.
>
> Note:
> Mainline code does not work "out of the box" on ZC702 at this time;
> ad hoc adaptations based on 'u-boot-xlnx' repository were needed
> in order to test on this board. (Those adapatations are not included
> with this patch.)

? Where can I find this repository?

> Signed-off-by: Michael Burr<michael.burr@logicpd.com>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Michal Simek<monstr@monstr.eu>
> ---
>   drivers/i2c/i2c_core.c |    5 ++
>   drivers/i2c/zynq_i2c.c |  148 +++++++++++++++++++++++++++++-------------------
>   include/i2c.h          |    3 +
>   3 files changed, 97 insertions(+), 59 deletions(-)
[...]
> diff --git a/include/i2c.h b/include/i2c.h
> index 8fd17d1..683affe 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -40,6 +40,7 @@
>   #define CONFIG_SYS_I2C_MAX_HOPS		0
>   #define CONFIG_SYS_NUM_I2C_BUSES	ll_entry_count(struct i2c_adapter, i2c)
>   #else
> +

Change not needed!

>   /* we use i2c muxes */
>   #undef CONFIG_SYS_I2C_DIRECT_BUS
>   #endif
> @@ -135,6 +136,8 @@ extern struct i2c_bus_hose	i2c_bus[];
>   #define I2C_MUX_PCA9544		{I2C_MUX_PCA9544_ID, "PCA9544A"}
>   #define I2C_MUX_PCA9547_ID	4
>   #define I2C_MUX_PCA9547		{I2C_MUX_PCA9547_ID, "PCA9547A"}
> +#define I2C_MUX_PCA9548_ID	5
> +#define I2C_MUX_PCA9548		{I2C_MUX_PCA9548_ID, "PCA9548"}
>   #endif
>
>   #ifndef I2C_SOFT_DECLARATIONS

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

See below... (My replies are tagged with "[MRB:]".)

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: Heiko Schocher [mailto:hs at denx.de] 
Sent: Sunday, September 22, 2013 2:33 AM
To: Michael Burr
Cc: u-boot at lists.denx.de; Michal Simek
Subject: Re: [PATCH] i2c:zynq: I2C multi-bus support on Zynq

Hello Michael,

Am 20.09.2013 22:40, schrieb Michael Burr:
> Changes:
> 'zynq_i2c.c':
> Adapted driver for compliance with "new" I2C driver model 
> (CONFIG_SYS_I2C).
> Support for 0-length register address in 'i2c_write', 'i2c_read'.
> 'i2c.h', 'i2c_core.c':
> Support for Texas Instruments PCA9548 bus multiplexer.

Your commit message implements, that your patch contains 3 independent changes ...  it would be better to seperate your patch in 3 pieces ... so if your patch introduces a problem, it is better to identify, which part it introduced ...

Can you do this easy?

[MRB:] I will give it a try...

Except of this, I have only one minor Codingstyle comment

> Tested:
> Xilinx ZC702 eval board (single bus master 'I2C0' with multiplexer 
> PCA9548).
> Write and read registers with addresses of length 0 and 1.
>
> Note:
> Mainline code does not work "out of the box" on ZC702 at this time; ad 
> hoc adaptations based on 'u-boot-xlnx' repository were needed in order 
> to test on this board. (Those adapatations are not included with this 
> patch.)

? Where can I find this repository?

[MRB:] At the following address. (I believe Michal Simek is the person who manages this.)

http://github.com/Xilinx/u-boot-xlnx/

> Signed-off-by: Michael Burr<michael.burr@logicpd.com>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Michal Simek<monstr@monstr.eu>
> ---
>   drivers/i2c/i2c_core.c |    5 ++
>   drivers/i2c/zynq_i2c.c |  148 +++++++++++++++++++++++++++++-------------------
>   include/i2c.h          |    3 +
>   3 files changed, 97 insertions(+), 59 deletions(-)
[...]
> diff --git a/include/i2c.h b/include/i2c.h index 8fd17d1..683affe 
> 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -40,6 +40,7 @@
>   #define CONFIG_SYS_I2C_MAX_HOPS		0
>   #define CONFIG_SYS_NUM_I2C_BUSES	ll_entry_count(struct i2c_adapter, i2c)
>   #else
> +

Change not needed!

[MRB:] Oops. I'll get rid of that...

>   /* we use i2c muxes */
>   #undef CONFIG_SYS_I2C_DIRECT_BUS
>   #endif
> @@ -135,6 +136,8 @@ extern struct i2c_bus_hose	i2c_bus[];
>   #define I2C_MUX_PCA9544		{I2C_MUX_PCA9544_ID, "PCA9544A"}
>   #define I2C_MUX_PCA9547_ID	4
>   #define I2C_MUX_PCA9547		{I2C_MUX_PCA9547_ID, "PCA9547A"}
> +#define I2C_MUX_PCA9548_ID	5
> +#define I2C_MUX_PCA9548		{I2C_MUX_PCA9548_ID, "PCA9548"}
>   #endif
>
>   #ifndef I2C_SOFT_DECLARATIONS

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michal Simek]

>> Tested:
>> Xilinx ZC702 eval board (single bus master 'I2C0' with multiplexer 
>> PCA9548).
>> Write and read registers with addresses of length 0 and 1.
>>
>> Note:
>> Mainline code does not work "out of the box" on ZC702 at this time; ad 
>> hoc adaptations based on 'u-boot-xlnx' repository were needed in order 
>> to test on this board. (Those adapatations are not included with this 
>> patch.)
> 
> ? Where can I find this repository?
> 
> [MRB:] At the following address. (I believe Michal Simek is the person who manages this.)
> 
> http://github.com/Xilinx/u-boot-xlnx/

That's xilinx repo. But you should remove this from commit message.
If there is something wrong in the mainline u-boot initialization
in connection to this you should send patch for it.
As I wrote you in other email we are going to invest our time
to sync configuration from this repo with mainline in the next merge
window because also these changes when they are reviewed
will go to the next u-boot version.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130924/cec6145e/attachment.pgp>

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

Sorry, Michal

I only wrote that part to explain how I was able to test on the 
ZC702 board (in case somebody else should want to reproduce
the results).

I understand that you will sync the 'u-boot-xlnx' repo with mainline
at some time in the future, on your own schedule. I was not trying
to suggest that anything else be done right now.

Thanks again for your feedback,

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 at monstr.eu] 
Sent: Tuesday, September 24, 2013 3:31 AM
To: Michael Burr
Cc: hs at denx.de; u-boot at lists.denx.de
Subject: Re: [PATCH] i2c:zynq: I2C multi-bus support on Zynq

>> Tested:
>> Xilinx ZC702 eval board (single bus master 'I2C0' with multiplexer 
>> PCA9548).
>> Write and read registers with addresses of length 0 and 1.
>>
>> Note:
>> Mainline code does not work "out of the box" on ZC702 at this time; 
>> ad hoc adaptations based on 'u-boot-xlnx' repository were needed in 
>> order to test on this board. (Those adapatations are not included 
>> with this
>> patch.)
> 
> ? Where can I find this repository?
> 
> [MRB:] At the following address. (I believe Michal Simek is the person 
> who manages this.)
> 
> http://github.com/Xilinx/u-boot-xlnx/

That's xilinx repo. But you should remove this from commit message.
If there is something wrong in the mainline u-boot initialization in connection to this you should send patch for it.
As I wrote you in other email we are going to invest our time to sync configuration from this repo with mainline in the next merge window because also these changes when they are reviewed will go to the next u-boot version.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michal Simek]

On 09/24/2013 04:58 PM, Michael Burr wrote:
> Sorry, Michal
> 
> I only wrote that part to explain how I was able to test on the 
> ZC702 board (in case somebody else should want to reproduce
> the results).
> 
> I understand that you will sync the 'u-boot-xlnx' repo with mainline
> at some time in the future, on your own schedule. I was not trying
> to suggest that anything else be done right now.
> 
> Thanks again for your feedback,

I got this but keep in your mind that Heiko or others will apply this
patch by git-am and your commit message will be the part of u-boot
source code.
That's why there should be focus on problems which you are trying to solve.

If you need to point/explain something then write it below your SoB line
and below "---" which ensure that git-am will remove these messages.

I would suggest to you to run git format-patch and then running
git am < patch to see what will be there.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130924/f6b1c332/attachment.pgp>

[U-Boot] [PATCH] i2c:zynq: I2C multi-bus support on Zynq

[Michael Burr]

Oh, OK. I see what you mean. I'm still very new to this process,
so it takes some practice to "learn the customs". Thanks for
clarifying.

So, in future I will keep those explanations below the "---" line.

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 at monstr.eu] 
Sent: Tuesday, September 24, 2013 10:19 AM
To: Michael Burr
Cc: hs at denx.de; u-boot at lists.denx.de
Subject: Re: [PATCH] i2c:zynq: I2C multi-bus support on Zynq

On 09/24/2013 04:58 PM, Michael Burr wrote:
> Sorry, Michal
> 
> I only wrote that part to explain how I was able to test on the
> ZC702 board (in case somebody else should want to reproduce the 
> results).
> 
> I understand that you will sync the 'u-boot-xlnx' repo with mainline 
> at some time in the future, on your own schedule. I was not trying to 
> suggest that anything else be done right now.
> 
> Thanks again for your feedback,

I got this but keep in your mind that Heiko or others will apply this patch by git-am and your commit message will be the part of u-boot source code.
That's why there should be focus on problems which you are trying to solve.

If you need to point/explain something then write it below your SoB line and below "---" which ensure that git-am will remove these messages.

I would suggest to you to run git format-patch and then running git am < patch to see what will be there.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform