I2C library

[Folknology]

I was getting strange results using Xmos lib_i2c recently with inconsistent reg16 (addr8) read and writes (not getting back what I expected)
It appears that the write interface extension is the culprit

Code: Select all

inline i2c_regop_res_t write_reg16_addr8(client interface i2c_master_if i,
                                           uint8_t device_addr, uint8_t reg,
                                           uint16_t data) {
    uint8_t a_data[3] = {reg, data, data >> 8};
    size_t n;
    i.write(device_addr, a_data, 3, n, 1);
    if (n == 0)
      return I2C_REGOP_DEVICE_NACK;
    if (n < 3)
      return I2C_REGOP_INCOMPLETE;
    return I2C_REGOP_SUCCESS;
  }

I believe the the data[3] line should be:

Code: Select all

    uint8_t a_data[3] = {reg, data >> 8, data & 0xFF};

i.e. MSB then LSB, would be the same for all reg16 writes (addr8 & 16)

Or am I misunderstanding something fundamental about lib_i2c?

regards
Al

[henk]

Hi,

I don't know whether there is a standard ordering for MSB/LSB in 16-bit I2C, but the one chip that I do know does MSB first; so that supports swapping them.

Masking with 0xFF should not make a difference; it is stored in a uint8. If it does make a difference then that would be a compiler error?

Which file did you find that piece of code in?

Cheers,
Henk

[Folknology]

Hi Henk

Hi,

I don't know whether there is a standard ordering for MSB/LSB in 16-bit I2C, but the one chip that I do know does MSB first; so that supports swapping them.

Masking with 0xFF should not make a difference; it is stored in a uint8. If it does make a difference then that would be a compiler error?

Which file did you find that piece of code in?

Cheers,
Henk

All of the chips I have used with the library are MSB first, hence why I asked, moreover if you look at the equivalent read code 'Read_reg16_addr8' it implements MSB first, suggesting 'write_reg16_addr8' should also do so to be consistent. In fact the thing that alerted me to the issue was our embedded I2C tests which write to registers and then read them back to check that the I2C peripheral chain is sound and operational. As the Xmos I2C library is is currently coded all of our tests fail!

You will find the code in the lib_i2c/api/i2c.h (line 335)

I seem to remember requiring the 0xFF masking to get my tests to pass, if I get time this week I can double check this.

P.S. I also checked the older Xmos channel based code SC_I2C on github and this does do MSB first.

P.P.S. Based on a quick perusal of common I2C chips we use here, all of those with 16bit registers (as well as those with 12/14 bit) are MSB first.

regards
AL

[Folknology]

OK I got a chance to do some more checking, I removed the mask and did a clean build changing from ->

Code: Select all

 uint8_t a_data[3] = {reg, data >> 8, data & 0xFF};

To ->

Code: Select all

uint8_t a_data[3] = {reg, data >> 8, data};

for line 335

Things behaved correctly, all of our I2C tests passed, all is well with the world...

So will you guys be changing lib_i2c, do you need to test these changes internally?
Given that it is not on a public repo like the Xmos xcore code on github, I presume you will have to do all relevant changes on your internal VCS and then issue a new release?

What about 'write_reg16', will you also change that?

regards
Al

[henk]

Thanks for testing that Al,

It will be fixed.

Just to let you know; there is a public access repo: <http://github.com/xmos/lib_i2c>, that you can fork and perform pull requests for less trivial bugs; I raised an issue on it <https://github.com/xmos/lib_i2c/issues/3>

[Folknology]

Thanks for testing that Al,

It will be fixed.

Just to let you know; there is a public access repo: <http://github.com/xmos/lib_i2c>, that you can fork and perform pull requests for less trivial bugs; I raised an issue on it <https://github.com/xmos/lib_i2c/issues/3>

I didn't know about the Xmos public repo (apart from xcore), has this been around for a while? It will be really useful being able to access it on github like this.

regards
Al

[Ross]

I didn't know about the Xmos public repo (apart from xcore), has this been around for a while? It will be really useful being able to access it on github like this.

Various repo's have been opened up fairly recently