2009-06-10 04:01:39     TWI - BF537 - write problem ?

Document created by Aaronwu Employee on Sep 18, 2013
Version 1Show Document
  • View in full screen mode

2009-06-10 04:01:39     TWI - BF537 - write problem ?

Andreas Neubacher (AUSTRIA)

Message: 75423   

 

i2c write in u-boot does not transfer data on bf537 rev 0.2

in u-boot-2009.03 from svn we want to use the function i2c_transfer() in bfin-twi_i2c.c in the following way:

 

unsigned char data = 0x01;

i2c_transfer(0x5f, 0x01, 1, &data, 1, 0)

 

 

 

which should set up an i2c write transfer to send one byte address and one byte data; i2c is initialized as per call to i2c_init()

 

we are observing via scope that only 0x5f and the address is transmitted, the data byte is not transmitted!

when looking at the code in i2c_transfer(), only the address len (1 byte) is programmed in MASTER_CTL, but not the data transfer length !?

 

 

 

     /* prime the pump */

        if (msg.alen) {

                len = msg.alen;

                debugi("first byte=0x%02x", *msg.abuf);

                bfin_write_TWI_XMT_DATA8(*(msg.abuf++));

                --msg.alen;

        } else if (!(msg.flags & I2C_M_READ) && msg.len) {

                debugi("first byte=0x%02x", *msg.buf);

                bfin_write_TWI_XMT_DATA8(*(msg.buf++));

                --msg.len;

        }

 

        // ...

 

        /* Master enable */

        bfin_write_TWI_MASTER_CTL(

                        (bfin_read_TWI_MASTER_CTL() & FAST) |

                        (min(len, 0xff) << 6) | MEN |

                        ((msg.flags & I2C_M_READ) ? MDIR : 0)

 

 

note that we have no problem reading data from the i2c chip; only write fails !!

 

is i2c write known to work in u-boot? it appears it can't upstream u-boot repos. seem to have the same code as is in u-boot-2009.03

we can easily reproduce on the u-boot command line using imd / imw commands

the trivial fix to program transfer length 2 when setting up MASTER_CTL does not remedy the problem

 

is anybody using the TWI in the uboot? - any problems?

 

thx, andy

QuoteReplyEditDelete

 

 

2009-06-10 04:19:59     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75439   

 

the i2c_transfer() function is marked static for a reason -- you arent supposed to use it.  use the interfaces that are clearly defined and exported: i2c_read/i2c_write and other functions defined in include/i2c.h.

QuoteReplyEditDelete

 

 

2009-06-10 04:38:28     Re: TWI - BF537 - write problem ?

Andreas Neubacher (AUSTRIA)

Message: 75440   

 

we definitley use the read/write functions, but the inside of the "i2c_read" and "i2c_write" the i2c_transfer is called (bfin-twi_i2c.c)

QuoteReplyEditDelete

 

 

2009-06-10 04:50:33     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75442   

 

you said you wanted to use i2c_transfer().  the answer is dont use it.  if you try to, that isnt supported, so you're on your own.

QuoteReplyEditDelete

 

 

2009-06-10 08:06:27     Re: TWI - BF537 - write problem ?

Andreas Neubacher (AUSTRIA)

Message: 75456   

 

the i2c_write looks like:

 

int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)

{

   return i2c_transfer(chip, addr, alen, buffer, len, I2C_M_WRITE);

}

 

- we are using the i2c_write, i2c_read functions !!

- the i2c_transfer() was just to point out where we guess the problem

- as you can see above there should be no difference anyway

QuoteReplyEditDelete

 

 

2009-06-10 14:16:06     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75472   

 

you didnt say you were using the i2c read/write functions originally

 

show the exact imd/imw commands that you're using and the exact output, as well as the exact C code that you are executing

QuoteReplyEditDelete

 

 

2009-06-10 15:06:29     Re: TWI - BF537 - write problem ?

Peter Meerwald (AUSTRIA)

Message: 75477   

 

Mike,

 

the u-boot commands are

 

imd 0x5f 0x1 1 (which correctly reads/displays the expected byte value)

 

imw 0x5f 0x1 0x1 1 (which transmits 0x5f 0x01 as we have verified with the scope, but not the final data byte, 0x01; no error message is returned)

 

the C code is the unmodified u-boot-2009.03/drivers/i2c/bfin-twi_i2c.c (svn r1719)

 

we've mentioned the i2c_transfer() code above as the observed behaviour (last byte not transmitted) seems to be explainable by the setup of the TWI_MASTER_CTL register

 

it would be tremendously helpful if you could confirm the issue and/or review the logic in i2c_transfer(), wait_for_completion()

 

please let us know if you need further details (I don't have access to the board ATM but can gather any additional info early next week); we were hoping that you could quickly check and reproduce the behaviour

 

thank you for your attention!

QuoteReplyEditDelete

 

 

2009-06-10 15:14:43     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75478   

 

i'm not talking about the i2c code you're calling, but the i2c code you've written.  your original post made it sound like you were writing C code to call the i2c layers.

 

i dont have any i2c devices laying around atm so i cant verify any operations -- the i2c bus requires slaves to ack master communication, so i cant simply wiggle the wires and look at it with a scope.

QuoteReplyEditDelete

 

 

2009-06-11 13:22:02     Re: TWI - BF537 - write problem ?

Peter Meerwald (AUSTRIA)

Message: 75550   

 

the transfer length to be set up in TWI_MASTER_CTL is computed incorrectly in case of i2c_write(); the total transfer length consists of address AND data transfer length, only the address transfer length is set up

 

issue is fixed by the attached patch, please apply

 

in case of a combined read/write operation (due to i2c_read()) the code is correct as the transfer first only handles the address data and is then restarted with the data transfer length in wait_for_completion()

 

the code in wait_for_completion() is rather timing sensitive and heavy debug output leads to FIFO buffer underruns (therefore I could not validate my earlier speculation)

 

code has been tested on bf537 rev 0.2 and one byte read/write and multibyte read/write

 

let me know if you have any questions

 

twi-i2c-write-fix.patch

QuoteReplyEditDelete

 

 

2009-06-11 23:26:54     Re: TWI - BF537 - write problem ?

Sonic Zhang (CHINA)

Message: 75567   

 

All SYNCs in your patch are not necessary.

QuoteReplyEditDelete

 

 

2009-06-16 11:06:34     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75830   

 

like Sonic said, did you actually need all of those SSYNC()'s or did you only need the last change ?

 

-        len = msg.alen;

+        len = (msg.flags & I2C_M_COMBO) ? msg.alen : msg.alen+len;

 

QuoteReplyEditDelete

 

 

2009-06-16 11:20:34     Re: TWI - BF537 - write problem ?

Peter Meerwald (AUSTRIA)

Message: 75833   

 

the SSYNC()s are taken from the hardware reference manual, I guess they are not actually needed for the hardware I'm testing on

 

the crucial line is

 

-        len = msg.alen;

+        len = (msg.flags & I2C_M_COMBO) ? msg.alen : msg.alen+len;

 

 

p.

 

QuoteReplyEditDelete

 

 

2009-06-16 11:28:34     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75834   

 

the people writing the HRM dont invest significant time in verifying actual SSYNC requirements in the example.  unless there is an explicit statement saying "an SSYNC is required" ...

 

plus if you look at the whole function, SSYNC's are issued only at required junctions -- clearing interrupt status and reprogramming of the control register.

 

at any rate, i dont think this change is correct for all cases.  if you read the end of wait_for_completion(), you'll see that the driver will process the msg length.

 

you also didnt say what device exactly you're attempting to work with ...

QuoteReplyEditDelete

 

 

2009-06-16 11:43:54     Re: TWI - BF537 - write problem ?

Peter Meerwald (AUSTRIA)

Message: 75837   

 

the code at the end of wait_for_completion() you are referring to is

 

        if (int_stat & MCOMP) {

            debugi("processing MCOMP");

            bfin_write_TWI_INT_STAT(MCOMP);

            SSYNC();

            if (msg->flags & I2C_M_COMBO && msg->len) {

                bfin_write_TWI_MASTER_CTL((bfin_read_TWI_MASTER_CTL() & ~RSTART) |

                    (min(msg->len, 0xff) << 6) | MEN | MDIR);

                SSYNC();

            } else

                break;

        }

 

 

note that the transfer is restarted only in case (msg->flags & I2C_M_COMBO) is true; I2C_M_COMBO is used to denote write-followed-by-read i2c transfers (such as where you write the register address to the slave device and read back the register value from the slave device)

 

I2C_M_COMBO is not set for write-only transfers

 

I agree that the wait_for_completion() code is complex and handles many cases; the code is definitely broken for write-only transfers; if you think the patch is correct (i.e. adds more trouble than it solves) please be specific

 

there may be corner cases where the code behaves incorrect, e.g. for write transfers > 0xff bytes where a STOP code has to be set explicitly, but that's a different story and not related to the patch

QuoteReplyEditDelete

 

 

2009-06-16 12:05:29     Re: TWI - BF537 - write problem ?

Mike Frysinger (UNITED STATES)

Message: 75840   

 

sorry, you're right of course.  i inverted the logic of your proposed change in my head.  and the device i was working with when i wrote this code was something that needed a combo transfer which is why i know that works ;).

 

so if that change works for you, i'll go ahead and merge it.

QuoteReplyEditDelete

 

 

2009-06-16 12:49:03     Re: TWI - BF537 - write problem ?

Peter Meerwald (AUSTRIA)

Message: 75844   

 

I've tested with a ks8993 ethernet switch which is configured via i2c; both read and write work OK for me with above patch

 

please go ahead and merge

Attachments

Outcomes