Skip to content

Conversation

@liebman
Copy link
Contributor

@liebman liebman commented Sep 21, 2024

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

When starting a TX transfer using the PARL_IO peripheral the TX clock was disabled and re-enabled at the start of start_write_bytes_dma. The TRM says that the clock should be re-enabled after tx_start.

Testing

  • Logic analyzer
  • running a hub75 display via 16bit PARL_IO TX

@liebman liebman changed the title per TRM the TX clock should only be re-enabled after tx_start PARL_IO: fix for garbage output at the start of some TX operations Sep 21, 2024
@liebman liebman marked this pull request as ready for review September 21, 2024 21:25
@bugadani
Copy link
Contributor

Can we test this somehow, like counting expected number of edges with PCNT if the problem is reproducible with narrower outputs?

@liebman
Copy link
Contributor Author

liebman commented Sep 22, 2024

Can we test this somehow, like counting expected number of edges with PCNT if the problem is reproducible with narrower outputs?

I think thats possible, if we use the valid output and count clocks when valid is high. (thats how I finally isolated it). I'll give that a try.

@liebman
Copy link
Contributor Author

liebman commented Sep 22, 2024

Added tests. Interestingly the error only happens in async, I added tests for both sync and async for both 8bit and 16bit

@liebman
Copy link
Contributor Author

liebman commented Sep 22, 2024

Oh - the H2 has only 8 bit PARL_IO!

@liebman
Copy link
Contributor Author

liebman commented Sep 22, 2024

So the esp32h2 PCNT can't keep up with 20MHz, I reduced the tests to 10MHz and the H2 should be ok. Can't go much lower or the initial bug does not trigger.

@liebman
Copy link
Contributor Author

liebman commented Sep 22, 2024

No idea why the esp32 test_i2s_loopback test failed - nothing changed there.

@bugadani
Copy link
Contributor

No idea why the esp32 test_i2s_loopback test failed - nothing changed there.

Don't worry about it

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.

Thanks!

@bugadani bugadani enabled auto-merge September 23, 2024 10:05
@bugadani bugadani added this pull request to the merge queue Sep 23, 2024
Merged via the queue into esp-rs:main with commit 987f00b Sep 23, 2024
@liebman liebman deleted the debug-parl_io branch January 26, 2025 18:58
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