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

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

[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 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
-- 
1.7.9.5

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

[Heiko Schocher]

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

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Michal Simek]

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



-- 
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/20130919/d28b67ae/attachment.pgp>