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

This amends my previous patch (of Sept. 17, 2013, 5:29 p.m):
Initialize both bus masters as needed in 'i2c_init'.
The behavior of this code should be unchanged from the old version if CONFIG_I2C_MULTI_BUS is not defined.
According to Heiko, this driver should be ported to the new multibus framework (which I will try to do in the near future), so I don't expect this patch to be accepted now. I am submitting this only to make sure that the current iteration of my code is self-consistent (i.e. not broken!). This version may also be helpful to anyone who still has the older framework.
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 | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c index 1c9ae30..6610869 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> @@ -72,9 +72,8 @@ 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 +#if defined(CONFIG_I2C_MULTI_BUS) || !defined(CONFIG_ZYNQ_I2C1) + /* Hard-code setup for Zynq PS I2C0 */ zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
/* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */ @@ -84,6 +83,24 @@ void i2c_init(int requested_speed, int slaveadd) /* Enable master mode, ack, and 7-bit addressing */ setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_MS | ZYNQ_I2C_CONTROL_ACKEN | ZYNQ_I2C_CONTROL_NEA); +#endif +#if defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_ZYNQ_I2C1) + /* Hard-code setup for Zynq PS I2C1 */ + zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR1; + + /* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */ + writel((16 << ZYNQ_I2C_CONTROL_DIV_B_SHIFT) | + (2 << ZYNQ_I2C_CONTROL_DIV_A_SHIFT), &zynq_i2c->control); + + /* Enable master mode, ack, and 7-bit addressing */ + setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_MS | + ZYNQ_I2C_CONTROL_ACKEN | ZYNQ_I2C_CONTROL_NEA); +#endif +#if defined(CONFIG_I2C_MULTI_BUS) + /* Initially select bus '0' */ + zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0; + current_bus = 0; +#endif }
#ifdef DEBUG

Hello Michael,
Am 18.09.2013 18:28, schrieb Michael Burr:
This amends my previous patch (of Sept. 17, 2013, 5:29 p.m):
Initialize both bus masters as needed in 'i2c_init'.
The behavior of this code should be unchanged from the old version if CONFIG_I2C_MULTI_BUS is not defined.
According to Heiko, this driver should be ported to the new multibus framework (which I will try to do in the near future), so I don't expect this patch to be accepted now. I am submitting this only to make sure that the current iteration of my code is self-consistent (i.e. not broken!). This version may also be helpful to anyone who still has the older framework.
Signed-off-by: Michael Burrmichael.burr@logicpd.com Cc: Heiko Schocherhs@denx.de Cc: Michal Simekmonstr@monstr.eu
drivers/i2c/zynq_i2c.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
Hmm... I do not really understand what I should do now with your patch? As you correct stated in your commit message, I do not accept this patch ... so I mark your patch as "Not Applicable"
BTW: such info is not good for a commit message. If you want to add more info to your patch when submitting it, but this info should not go into the commit message, as for example the info what has changed between different patchversions, you can write such text after the three "---". All text after this marker would not go into the commit message.
bye, Heiko

On 09/18/2013 06:28 PM, Michael Burr wrote:
This amends my previous patch (of Sept. 17, 2013, 5:29 p.m):
Initialize both bus masters as needed in 'i2c_init'.
The behavior of this code should be unchanged from the old version if CONFIG_I2C_MULTI_BUS is not defined.
According to Heiko, this driver should be ported to the new multibus framework (which I will try to do in the near future), so I don't expect this patch to be accepted now. I am submitting this only to make sure that the current iteration of my code is self-consistent (i.e. not broken!). This version may also be helpful to anyone who still has the older framework.
yes. Please clear this commit message.
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 | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c index 1c9ae30..6610869 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+
Probably unrelated change.
*/
#include <common.h> @@ -72,9 +72,8 @@ 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
I can't see this in Tom's master branch. Which branch/repo did you used?
+#if defined(CONFIG_I2C_MULTI_BUS) || !defined(CONFIG_ZYNQ_I2C1)
/* Hard-code setup for Zynq PS I2C0 */ zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
/* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */
@@ -84,6 +83,24 @@ void i2c_init(int requested_speed, int slaveadd) /* Enable master mode, ack, and 7-bit addressing */ setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_MS | ZYNQ_I2C_CONTROL_ACKEN | ZYNQ_I2C_CONTROL_NEA); +#endif +#if defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_ZYNQ_I2C1)
- /* Hard-code setup for Zynq PS I2C1 */
- zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR1;
- /* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */
- writel((16 << ZYNQ_I2C_CONTROL_DIV_B_SHIFT) |
(2 << ZYNQ_I2C_CONTROL_DIV_A_SHIFT), &zynq_i2c->control);
- /* Enable master mode, ack, and 7-bit addressing */
- setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_MS |
ZYNQ_I2C_CONTROL_ACKEN | ZYNQ_I2C_CONTROL_NEA);
+#endif +#if defined(CONFIG_I2C_MULTI_BUS)
- /* Initially select bus '0' */
- zynq_i2c = (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
- current_bus = 0;
+#endif
You probably have to use different logic here when you should support both i2c controllers.
Thanks, Michal
participants (3)
-
Heiko Schocher
-
Michael Burr
-
Michal Simek