Skip to content

Conversation

@manio
Copy link
Contributor

@manio manio commented Sep 20, 2023

Currently the get_rx_fifo_count() is using fifo_cnt, which is unreliable for ESP32, according to the errata:
https://www.espressif.com/sites/default/files/documentation/esp32_errata_en.pdf section 3.17

This commit is fixing the code for the ESP32 according to the workaround example.

Main discussion and my debugging results:
#765

@manio manio force-pushed the fifo_counter_fix branch 2 times, most recently from 6d40f34 to d8af234 Compare September 20, 2023 20:30
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix!

Edit: oops, spoke too soon 😁 Other than the build error, looks good 😉

Currently the `get_rx_fifo_count()` is using `fifo_cnt`, which is unreliable
for ESP32, according to the errata:
https://www.espressif.com/sites/default/files/documentation/esp32_errata_en.pdf
section 3.17

This commit is fixing the code for the ESP32 according to the
workaround example.

Main discussion and my debugging results:
esp-rs#765
@manio
Copy link
Contributor Author

manio commented Sep 21, 2023

Should be good now, I also rebased again to get rid of CHANGELOG conflict.

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for your contribution.

@manio
Copy link
Contributor Author

manio commented Sep 21, 2023

Thanks! 😁

SergioGasquez pushed a commit to SergioGasquez/esp-hal that referenced this pull request Sep 22, 2023
Currently the `get_rx_fifo_count()` is using `fifo_cnt`, which is unreliable
for ESP32, according to the errata:
https://www.espressif.com/sites/default/files/documentation/esp32_errata_en.pdf
section 3.17

This commit is fixing the code for the ESP32 according to the
workaround example.

Main discussion and my debugging results:
esp-rs#765
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants