Discussion:
I2C error on STM32F103 [2 Attachments]
(too old to reply)
m***@public.gmane.org
2014-04-15 21:27:05 UTC
Permalink
Hi Greg,

We have encountered following issue with the I2C driver of the STM32 family (NuttX 6.33). We are using a STMF103 chip and the goal is to read one byte from some device on the I2C1 bus.

The following minimal example shows how we use the driver:

// initialize i2c device struct i2c_dev_s *dev;
struct i2c_msg_s msg[2];

dev = up_i2cinitialize(1);
I2C_SETFREQUENCY(dev, 400000);



// allocate buffers
const uint8_t read_length = 1;
uint8_t read_register = 0x00;
uint8_t data[read_length] = { 0 };


// setup message
msg[0].addr = 0x01;
msg[0].flags = 0;
msg[0].buffer = &read_register;
msg[0].length = 1;


msg[1].addr = 0x01;
msg[1].flags = I2C_M_READ;
msg[1].buffer = data;
msg[1].length = read_length;


// transmit
int ret = I2C_TRANSFER(dev, msg, 2);

When we tried this we observed that the logic NAKs after receiving the byte, but instead of immediately sending a STOP it NAKS three more bytes (see attachement: motor_controller_error.png). This specific device was tolerant to this strange behavior, however we have an IMU (MPU6050), which blocks the bus when it doesn't receive the STOP after the first NAK.

We examined this issue and found that there is no simple solution to this problem , but the logic in the I2C ISR seems to have some flaws. This issue may be related to the STM32F103 only, because I tested this with a PX4 (STM32F4) and there the ISR sets the STOP correctly (no additional NAKs after the first one).

My colleague has rewritten the ISR and we now get the desired behavior (see attachment: motor_controller_noerror.png). I also did a quick test on the PX4 and it seems to work fine. We would be happy to contribute our solution, but I guess it needs some proper testing from a third party.

How do you suggest we should proceed?

Thanks,
Max
s***@public.gmane.org
2014-04-16 18:42:16 UTC
Permalink
Hi, Max,

This is a difficult decision. I am not personally using STM32 or I2C at the moment so I cannot test or verify the changes.

I know that other people (including PX4) have complained about some unreliable behavior int he I2C driver.

I think that based on that, it would be best to incorporate the changes. If anyone has any different thoughts please state them.

So, yes, please do contribute the patch and I will check it in. As I mentioned, I cannot do any testing now so there is some risk. But I don't see anyway to avoid that risk.

Greg
max.kriegleder-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-09 13:24:04 UTC
Permalink
Hi Greg,

We believe we identified the source of the error. The read of a single byte needs to be treated differently than a 2 byte read, and both need to be treated differently than a 3 or more byte read (this information is provided in the reference manual). This is not the case for the current driver.

I think this problem was discussed somewhat in a different thread - https://groups.yahoo.com/neo/groups/nuttx/conversations/topics/5438 https://groups.yahoo.com/neo/groups/nuttx/conversations/topics/5438
At least looking at the proposed fix I see that the special case of a 1 byte read was added. Unfortunately there are other issues with this fix (POLLED version doesn't work).

My colleague Patrizio Simona has written a fix for all described cases (interrupt driven and polled). Mostly the changes apply to the transfer logic. We have tested this fix on a STM32F103 and we don't see any issues. The fix is attached for further testing and possible integration with the nuttx source.

Max
max.kriegleder-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-13 12:12:26 UTC
Permalink
Hi Greg,

We did more research on this topic and we believe we found the source of the bug. A 1 byte read from the I2C bus needs to be treated differently than a 2 byte read, and both need to be treated differently than a 3 or more byte read. The correct procedure is described in the STM32 reference manual (e.g. here http://www.st.com/web/en/resource/technical/document/reference_manual/CD00171190.pdf http://www.st.com/web/en/resource/technical/document/reference_manual/CD00171190.pdf).

There is another thread (https://groups.yahoo.com/neo/groups/nuttx/conversations/topics/5438) where I believe somebody ran into the same issue (at least I read this from the proposed patch). We tested this patch, but there seem to be other problems especially with the POLLED version.

My colleague Patrizio Simona re-wrote the transfer logic according to the reference manual. We tested this on a STM32F103 and everything works fine in the POLLED and interrupted version for transfer of 1,2, >=3 byte transfers.

The code is attached for further testing and inclusion to the nuttx source.

Max
spudarnia-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-13 12:59:00 UTC
Permalink
Post by max.kriegleder-/***@public.gmane.org [nuttx]
The code is attached for further testing and inclusion to the nuttx source.
Thanks, Max. I am a little busy this morning. But I will try to make time to review and incorporate the changes before the end of my day.

Greg
Michael Smith drziplok-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-13 16:00:03 UTC
Permalink
Max,

Did you have a chance to test this on F2/4 series parts? There are some subtle differences between the F1 and later I2C controllers, and what I’ve seen suggests they need slightly different handling in some cases.
Post by m***@public.gmane.org
Hi Greg,
We did more research on this topic and we believe we found the source of the bug. A 1 byte read from the I2C bus needs to be treated differently than a 2 byte read, and both need to be treated differently than a 3 or more byte read. The correct procedure is described in the STM32 reference manual (e.g. here http://www.st.com/web/en/resource/technical/document/reference_manual/CD00171190.pdf).
There is another thread (https://groups.yahoo.com/neo/groups/nuttx/conversations/topics/5438) where I believe somebody ran into the same issue (at least I read this from the proposed patch). We tested this patch, but there seem to be other problems especially with the POLLED version.
My colleague Patrizio Simona re-wrote the transfer logic according to the reference manual. We tested this on a STM32F103 and everything works fine in the POLLED and interrupted version for transfer of 1,2, >=3 byte transfers.
The code is attached for further testing and inclusion to the nuttx source.
Max
spudarnia-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-13 19:27:02 UTC
Permalink
Max, Mike,
Did you have a chance to test this on F2/4 series parts? There are some subtle differences between the F1 and later I2C controllers, and what I’ve seen suggests they need slightly different handling in some cases.
I know that some of the changes in this I2C file do apply only to to the F103. I am not sure if that covers the whole F1 family or not. It will take some investigation and feedback to determine what platforms the changes really apply to.

My plan is to make this an alternate I2C driver that is selectable form 'make menuconfig'. Perhaps it should be the default for F1s? Anyway, because there are uncertainties, I want to make the code changes available but also not disrupt anyone who is actively using I2C without (I think that includes most F4 users at least).

This incorporation is going very slowly. The replacement I2C file has been reformatted and does not following the NuttX coding style so it is very difficult to pick out just what the changes are. I hope to get this checked in tomorrow.

Greg
spudarnia-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-14 13:56:59 UTC
Permalink
Hi, Max,

I have finally checked in Patrizio Simona I2C changes. I carefully reviewed that code, mostly for confromance to NuttX coding style and also for recent changes to "standard" I2C driver.

The coding style was very different and there are probably other changes. So many (hopefully cosmetic) changes were made. As a consequence, the driver will need to be re-verified. You can see the files that I changed here: NuttX / GIT / Commit [5f9e14] http://sourceforge.net/p/nuttx/git/ci/5f9e14d7e3426a42783d6e034f86a559d39a2e9c/

NuttX / GIT / Commit [5f9e14] http://sourceforge.net/p/nuttx/git/ci/5f9e14d7e3426a42783d6e034f86a559d39a2e9c/ Add an alternate STM32 I2C driver that works around errata in the F103 chip (and maybe others). From Patrizio Simona



View on sourceforge.net http://sourceforge.net/p/nuttx/git/ci/5f9e14d7e3426a42783d6e034f86a559d39a2e9c/
Preview by Yahoo





Also, you can see that there are now two STM32 I2C drivers. I retained the old I2C driver which I suspect is correct for the STM32 F4 and created a new alternate I2C driver that is enabled by default for the STM32 F103 "Performance Line". There drivers should be combined some time in the future but I think we need to do a little testing first and also to clarify which F1 parts needs the workaround.

Thanks to you and Patrizio Simona for the carefully thought out work.

Greg
max.kriegleder-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-14 16:57:29 UTC
Permalink
Hi Greg,

Great! I am sorry for the trouble and we will try to write code that complies with the nuttx standards in the future. We will verify the new version, but unfortunately we won't be able to do so before next Monday. I will also get a STM32 F4 board for further testing.

One word about the old driver. I think it is also not correct for the F4 as I don't see anywhere in the code a proper handling of the 1 byte read, which should be done according to the reference manual. I assume that most devices are tolerant to this non-textbook 1 byte read, but we happened to run into a device (MP6050) which blocked the bus because of this.

I will get back to this with hopefully more information after Monday.

Max
max.kriegleder-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-14 17:13:58 UTC
Permalink
Hi Greg,

Great! I am sorry for the trouble and we will try to write code that complies with the nuttx standards in the future. We will verify the new version, but unfortunately we won't be able to do so before next Monday. I will also try this on a STM32 F4 board.

Max
Kosma Moczek kosma-OqCRPfiDy+Q@public.gmane.org [nuttx]
2014-05-14 17:15:45 UTC
Permalink
Post by Michael Smith drziplok-***@public.gmane.org [nuttx]
Did you have a chance to test this on F2/4 series parts? There are some
subtle differences between the F1 and later I2C controllers, and what I've
seen suggests they need slightly different handling in some cases.
Just a little tip: STM32 I2C is so bad that ST wrote a library that's
supposed to provide a unified interface across the F1/F2/F4/L1 families,
see here:

http://www.st.com/web/catalog/tools/FM147/CL1794/SC961/SS1743/PF258336

I can't vouch for its correctness - I wouldn't bet much on that I2C
implementation - but it might a be a good source of information on
differences between chip families.
--
Kosma Moczek
http://www.kosma.pl/
Mike Smith drziplok-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-14 17:20:11 UTC
Permalink
Post by Kosma Moczek kosma-OqCRPfiDy+***@public.gmane.org [nuttx]
http://www.st.com/web/catalog/tools/FM147/CL1794/SC961/SS1743/PF258336
I can't vouch for its correctness - I wouldn't bet much on that I2C implementation - but it might a be a good source of information on differences between chip families.
Many of us have referred to UM1029 on various occasions trying to make the STM32 i2c work less badly, with mixed success.

Worth noting is that the F2/F4 support in that package is deprecated (read: unsupported).
spudarnia-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-14 17:36:32 UTC
Permalink
With regard to coding style ... You are, of course, free to use any coding style that you like and I claim no superiority of any one syle over another. However, adopting a common coding style can make it easier for use to exchange files and patches and also reduces the risk of introducing changes.

I think that we can all agree that we don't have all of the correct answers for an I2C solution across the whole family. And, from this discussion, it seems like no one really has that solution. For now, I chose to carry this as two alternative versions of the same driver only to minimize risk. I am not working with either STM32 or I2C now so I cannot offer much additional. My hope is that those of you who are users of STM32 I2C can please advise me and send patches as necessary so that we can resolve this.

Greg
max.kriegleder-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-22 21:45:03 UTC
Permalink
Hi Greg,

We tested the changes you made and everything works as expected (on the STM32F103). We made some minor changes (reduced footprint, removed a bug) and I have attached our final version.

I had also time to test this driver on the STM32F4Discovery and there is a problem as you guys anticipated. The POLLED version seems to work fine, but the interrupted version only works for writing. In read mode the driver hangs after the write address and register address were sent (the logic doesn't trigger the second start). Thus for the moment we need to keep both drivers as you suggested. Given, however, that we now have a driver working for both chips it should be hopefully straightforward to merge them into a single driver.

I will try to look into this sometime next week.

Max
spudarnia-/E1597aS9LQAvxtiuMwx3w@public.gmane.org [nuttx]
2014-05-23 14:45:29 UTC
Permalink
Hi, Max,
Post by max.kriegleder-/***@public.gmane.org [nuttx]
We tested the changes you made and everything works as expected (on the STM32F103). We made some minor changes (reduced footprint, removed a bug) and I have attached our final version.
You changes are in the repository now. Thanks for the update.

Greg

Loading...