diff mbox

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

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

Commit Message

Michael Burr Sept. 17, 2013, 5:29 p.m. UTC 
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(-)

Comments

Heiko Schocher Sept. 18, 2013, 7:13 a.m. UTC | #1 
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
Michael Burr Sept. 18, 2013, 3:01 p.m. UTC | #2 
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@denx.de] 
Sent: Wednesday, September 18, 2013 2:14 AM
To: Michael Burr
Cc: u-boot@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
diff mbox

Patch

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
 }