RM Error: I2C Master "Restart Read" Flowchart wrong

[flyer31]

Thank you for new I2C interface, this really is supergreat compared to the cumbersome STM32F4 solution. My new STM32H7 I2C code is only about 20-30% of the STM32F4 version - I use a state machine and needed > 10 partly extremely cumbersome states in STM32F4, but now in STM32H7 only 4 states left, which look very nice and clear.

Also thank you for the flowcharts in the RM chapter 46 "I2C" (I use the version 2018 July). They are very helpful for programming.

I hope I do not nerve you with the hint, that the flowchart in Figure 553 (Chapter 46.4.8 "I2C master mode") in fact has a quite severe error. And I think this is the relevant flow chart for "99% of CPU I2C code" - this flowchart concerns such basic things as e. g. reading AD values from some simple I2C AD converter, which I think covers 99% of users interested in I2C... (Write Reg-Number to "Conversion result reg", then Restart with Read and Read conversion register, this is how very many such tiny AD converters work). So this flowchart really is quite crucial I think.

If you look at the question "I2C_ISR.TC=1?" in this flow chart, you will easily see that if programmed like this, it will be useless, because directly after Write to I2C_TXDR the answer here NEVER can be Yes... . (I2C will need about 100usec for this I2C_TXDR byte to be transmitted .. and this is a myriad of time for STM32H7).

I did not invest the time for a graphical flowchart myself, please excuse, but in "lazy C writing" I pogrammed like this:

if( !All Bytes transmitted ){
  if( !I2C_ISR_TXIS)
    return; // continue next time
  I2C_TXDR= Next Byte;
  if( All Bytes transmitted and AutoEnd set)
    return; // and all done... (Autoend set, Stop will be done automatically)
  else
    return; // continue next time
}
if( !I2C_ISR_TC)
  return; // continue next time
Restart I2C with new Slave address and I2C_CR2_RD_WRN=1
return; // and continue next time with reading

Your code in this "lazy writing style" was (only the right wing of this figure):

if( !I2C_ISR_TXIS)
  return; // continue next time
I2C_TXDR= Next Byte;
if( !All Bytes transmitted )
  return; // continue next time
if( !I2C_ISR_TC)   //<<< this is stupid / wrong here
  return; // and all done... (Autoend set, Stop will be done automatically)
Restart I2C with new Slave address and I2C_CR2_RD_WRN=1
return; // and continue next time with reading

One further nice thing I recognized only by "happy accident" (see my other post for I2C):

Nicely now it seems to be possible to enforce the I2C to write 2 CLK pulses on SCL line with I2C timing. This possibility is definitely necessary to "free an I2C bus hanging by some "stupid I2C slave who missed an SCL, and holding SDA line low"". This now works by I2C reset (clear I2C_CR1_PE and then set again to 1). This will produce 2 CLK pulses on SCL with I2C_TIMINGR configured baudrate. This is VERY nice and helpful and necessary. I am quite sure that 99% of I2C users need this urgently in their software somewhere for error recovery.

But this is NOWHERE described in the manual as I see it. Please spend a note in 46.7.1 (description of I2C_CR1 register), and possibly also in 46.4.5 "Sofware reset", e. g. the following: "Note: Software reset will create 2 SCL pulses with I2C_TIMINGR baudrate, even if SDA line is kept low. This is necessary to release a "hanging I2C bus" in master mode, if some slave without timout functionality missed an SCL pulse and keeps SDA in low state."