Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C HAL driver problem with HAL_I2C_Mem_Read_IT #7

Closed
pavel-a opened this issue Dec 7, 2019 · 14 comments
Closed

I2C HAL driver problem with HAL_I2C_Mem_Read_IT #7

pavel-a opened this issue Dec 7, 2019 · 14 comments
Labels
bug Something isn't working good first issue Good for newcomers hal HAL-LL driver-related issue or pull-request. internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system st community Also reported by users on the community.st.com
Milestone

Comments

@pavel-a
Copy link

pavel-a commented Dec 7, 2019

Possible bug in HAL_I2C_Mem_Read_IT, reported in the forum.

In stm32f4xx_hal_i2c.c near line 4597

@ALABSTM
Copy link
Contributor

ALABSTM commented Dec 12, 2019

Hi Pavel,

Thank you for this one other issue you reported. I will forward it to our development team for analysis. I will let you updated as soon as I get an answer.

With regards,

@ALABSTM ALABSTM added the st community Also reported by users on the community.st.com label Dec 12, 2019
@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 2, 2020

Hi Pavel,

I hope you are doing well. Would you mind centralizing here the details that could be needed by our development teams to analyze this issue (board reference, IDE, compiler, and their versions)?

Thank you,

@ALABSTM ALABSTM moved this from To do to Assigned in stm32cube-mcu-fw-dashboard Jan 2, 2020
@JRASTM
Copy link

JRASTM commented Jan 2, 2020

Hi,

In our development phase, we already manage, mean validate a latency treatment of different flags.

By the way, as a no regression we have run our test on a discovery STM32F412G-DISCO board with an external EEPROM @100khz on I2C1/PB6/PB7 connection, and no issue detected by our tests.

Can you help us to reproduce the issue, by giving us more details like :

  • which sensor did you use ?
  • which speed did you use ?
  • shared a schematic of your HW implementation ?

Thanks and regards

@pavel-a
Copy link
Author

pavel-a commented Jan 3, 2020

Hi Ali,
Pinged the reporter on the forum. (link above). There may be a certain holiday in their part of the world, you know... -- p

@ALABSTM ALABSTM moved this from Assigned to In progress in stm32cube-mcu-fw-dashboard Jan 3, 2020
@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 3, 2020

Hi Pavel,

No problem. We will wait till the reporter on the forum answers you. Thank you very much for your implication and your several contributions so far. We do appreciate.

With regards,

@zacwbond
Copy link

zacwbond commented Jan 27, 2020

I'm not able to share a schematic, but we have an STM32F412RGT acting as an I2C master using I2C1 on PB8 (SCL) and PB9(SDA). The clock speed is 400 kHz. There are three slaves on the bus, and the one we are talking to when the problem occurs is an LIS2HH12TR accelerometer.

As described in the post, I think the problem is that HAL_I2C_EV_IRQHandler() can't manage the case where both TXE and BTF are set. If you can get TXE and BTF set at the same time, you should see the same lockup we're observing.

Probably you could use some higher priority ISR to inhibit handling the TXE interrupt long enough that BTF is set, too, and then see what happens when HAL_I2C_EV_IRQHandler() finally executes.

@JRASTM
Copy link

JRASTM commented Jan 28, 2020

Hi,

Understood the hypothesis, but by double check the handler, in both case RXNE or TXE with a BTF, the most important flag checked is BTF thanks to the last part of if condition of RXNE or TXE.

Here is an extract, you can find that TXE is checked only if BTF is reset, if BTF set it is BTF flag condition that is execute :
if ((I2C_CHECK_FLAG(sr1itflags, I2C_FLAG_TXE) != RESET) && (I2C_CHECK_IT_SOURCE(itsources, I2C_IT_BUF) != RESET) && (I2C_CHECK_FLAG(sr1itflags, I2C_FLAG_BTF) == RESET))

Now by checking the code of hal i2c versus current hal i2c in our development branch, there are some issues corrected around Mem HAL I2C interface.

Like this one in particularity : ST Internal Reference 50144 integrated in version FW package F4 V1.24.2
This bug have been found using hal mem interface in case an error occurs during exchange between slave.

May be an alignement with HAL I2C will solved the issue.

Regards,
Jerome.

@zacwbond
Copy link

Hi Jerome,

I agree with what you're saying: if TXE and BTF are both set, then the BTF is what is handled by calling I2C_MasterTransmit_BTF(). My point is that I don't think that's what should happen in the memory read case.

Suppose you do a memory read by calling HAL_I2C_Mem_Read_IT(). That sets the state to HAL_I2C_STATE_BUSY_RX. The SB and ADDR interrupts get handled properly, so that the next expected interrupt is TXE after the read address is sent. But if handling of that TXE gets held off, then both BTF and TXE will be set.

When HAL_I2C_EV_IRQHandler() is finally called, the conditional that you describe results in the execution of I2C_MasterTransmit_BTF(). But because the state is HAL_I2C_STATE_BUSY_RX, that function becomes a no-op, the interrupts are not cleared, and they continue recurring forever.

Please explain if I am missing something in this execution sequence.

Thanks,
-Zac

@JRASTM
Copy link

JRASTM commented Jan 29, 2020

Hi and ok,

I will try to reproduce this situation and come back to you

Regards,
Jerome.

@JRASTM
Copy link

JRASTM commented Feb 4, 2020

Hi,

Come back with news, i have take time to try to reproduce the issue by adding a delay treatment in IRQ handler.

And i confirm your statement, there is well an issue between TXE and BTF introduce by a latency treatment of flag on time.

I will treat this issue and come back with a fix candidate.

Is it possible to raise an internal ticket to follow this issue ?

Regards,
Jerome.

@ALABSTM
Copy link
Contributor

ALABSTM commented Feb 4, 2020

Hi Jerome,

Thank you for your analysis. An internal bug tracker will be created.

With regards,

@ALABSTM
Copy link
Contributor

ALABSTM commented Feb 4, 2020

ST Internal Reference: 80462

@ALABSTM ALABSTM added bug Something isn't working internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system labels Feb 4, 2020
@JRASTM
Copy link

JRASTM commented Feb 19, 2020

Hi all,

For your information, fix available and integrated in ST internal database.

Regards
Jerome

@ALABSTM
Copy link
Contributor

ALABSTM commented Feb 28, 2020

Hi everybody,

@JRLSTM, thank you for your answers and for the fix.

@pavel-a and @zacwbond, thank you for having reported this issue. The fix will not be available in the STM32CubeF4 v1.25.0 package, but in a later one. In the meanwhile, please find attached this patch.

We hope this solves the problem from your side. I think this issue could be closed now.

With regards,

@ALABSTM ALABSTM closed this as completed Feb 28, 2020
stm32cube-mcu-fw-dashboard automation moved this from In progress to Done Feb 28, 2020
@ALABSTM ALABSTM added the good first issue Good for newcomers label Apr 3, 2020
@ALABSTM ALABSTM added this to the v1.25.1 milestone Sep 21, 2020
@ALABSTM ALABSTM added the hal HAL-LL driver-related issue or pull-request. label Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers hal HAL-LL driver-related issue or pull-request. internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system st community Also reported by users on the community.st.com
Projects
Development

No branches or pull requests

4 participants