public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Anatolij Gustschin <agust@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/4 V5] I2C: mxc_i2c rework
Date: Fri, 14 Oct 2011 18:28:55 +0200	[thread overview]
Message-ID: <20111014182855.0d670e0e@wker> (raw)
In-Reply-To: <1316719332-31185-1-git-send-email-marek.vasut@gmail.com>

Hi Marek,

On Thu, 22 Sep 2011 21:22:12 +0200
Marek Vasut <marek.vasut@gmail.com> wrote:

> Rewrite the mxc_i2c driver.
>  * This version is much closer to Linux implementation.
>  * Fixes IPG_PERCLK being incorrectly used as clock source
>  * Fixes behaviour of the driver on iMX51
>  * Clean up coding style a bit ;-)
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jason Hui <jason.hui@linaro.org>
> ---
>  drivers/i2c/mxc_i2c.c |  422 +++++++++++++++++++++++++++++++++----------------
>  1 files changed, 289 insertions(+), 133 deletions(-)

Unfortunately this patch breaks accessing the I2C EEPROM on
imx31_phycore board. On this board the U-Boot environment
is stored in the I2C EEPROM. With this patch applied reading
the environment doesn't work correctly, I always get "bad CRC"
warning and fall back to default environment. Some EEPROM data
dumps using i2c commands reveal that the EEPROM data addressing
is broken, this is due to the wrong swapping of address bytes
in the code below:

> +/*
> + * Write register address
> + */
> +int i2c_imx_set_reg_addr(uint addr, int alen)
>  {
> -	int i, retry = 0;
> -	for (retry = 0; retry < 3; retry++) {
> -		if (wait_idle())
> +	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < (8 * alen); i += 8) {
> +		writeb((addr >> i) & 0xff, &i2c_regs->i2dr);
> +
> +		ret = i2c_imx_trx_complete();
> +		if (ret)
>  			break;

Applying the patch below fixes the EEPROM addressing issue,
i2c commands seem to work correctly:
dumping the EEPROM data to memory by

uboot> i2c read 0x52 0.2 1000 80000000
uboot> md 80000000 1
80000000: cf6bcdbd    ..k.

and calculating the checksum by

uboot> crc 80000004 ffc
CRC32 for 80000004 ... 80000fff ==> cf6bcdbd

shows that it works, but when the board boots, the
reading of the environment still doesn't work. Bad CRC
is always reported, even with the below patch applied.
Reverting this driver rework commit fixes the issue.

Any idea where the problem could be?

Thanks,
Anatolij


diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index a805bf6..9984c2a 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -291,11 +291,10 @@ int i2c_imx_set_chip_addr(uchar chip, int read)
 int i2c_imx_set_reg_addr(uint addr, int alen)
 {
 	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-	int i;
+	int ret = 0;
 
-	for (i = 0; i < (8 * alen); i += 8) {
-		writeb((addr >> i) & 0xff, &i2c_regs->i2dr);
+	while (alen--) {
+		writeb((addr >> (alen * 8)) & 0xff, &i2c_regs->i2dr);
 
 		ret = i2c_imx_trx_complete();
 		if (ret)

  parent reply	other threads:[~2011-10-14 16:28 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-15  0:09 [U-Boot] [PATCH 0/4] Clock fix and MXC I2C rework series Marek Vasut
2011-09-15  0:09 ` [U-Boot] [PATCH 1/4] MX5: Modify the PLL decoding algorithm Marek Vasut
2011-09-22  3:20   ` Jason Hui
2011-09-23  9:15     ` Stefano Babic
2011-09-23  9:43   ` [U-Boot] [PATCH 1/4 V2] " Marek Vasut
2011-09-15  0:09 ` [U-Boot] [PATCH 2/4] MX5: Add AHB clock reporting and fix IPG clock reporting Marek Vasut
2011-09-22  3:05   ` Jason Hui
2011-09-22  3:41     ` Marek Vasut
2011-09-22  4:46       ` Jason Hui
2011-09-22 19:20   ` [U-Boot] [PATCH 2/4 V2] " Marek Vasut
2011-09-23  2:09     ` Jason Hui
2011-09-15  0:09 ` [U-Boot] [PATCH 3/4] MX5: Clean up the output of "clocks" command Marek Vasut
2011-09-19 10:03   ` Stefano Babic
2011-09-22  1:58   ` Jason Hui
2011-09-15  0:09 ` [U-Boot] [PATCH 4/4] I2C: mxc_i2c rework Marek Vasut
2011-09-15  4:16   ` [U-Boot] [PATCH 4/4 V2] " Marek Vasut
2011-09-19  6:13     ` Heiko Schocher
2011-09-20  2:30     ` [U-Boot] [PATCH 4/4 V3] " Marek Vasut
2011-09-20  2:35       ` [U-Boot] [PATCH 4/4 V4] " Marek Vasut
2011-09-22  2:45         ` Jason Hui
2011-09-22  3:43           ` Marek Vasut
2011-09-22  4:54             ` Jason Hui
2011-09-22  5:47               ` Marek Vasut
2011-09-22  6:49                 ` Jason Hui
2011-09-22 19:22         ` [U-Boot] [PATCH 4/4 V5] " Marek Vasut
2011-09-23  3:32           ` Jason Hui
2011-09-23  4:31             ` Fabio Estevam
2011-09-23  6:21             ` Marek Vasut
2011-09-23  7:46             ` Stefano Babic
2011-09-26  6:30               ` Heiko Schocher
2011-10-14 16:28           ` Anatolij Gustschin [this message]
2011-09-19 10:11   ` [U-Boot] [PATCH 4/4] " Stefano Babic
2011-09-19 10:24     ` Marek Vasut
2011-09-20 10:07     ` Jason Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111014182855.0d670e0e@wker \
    --to=agust@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox