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

LPSPI sampling configuration API #135

Closed
wants to merge 5 commits into from

Conversation

dstric-aqueduct
Copy link
Contributor

@dstric-aqueduct dstric-aqueduct commented Apr 19, 2023

Expose a public method to allow users to change the sampling point of an LPSPI peripheral.

I've experienced RX FIFO overruns when the SAMPLE: SAMPLE_1 (delayed edge) is used with non-hardware controlled chip select pins (ie, manually driving a CS pin with GPIO embedded_hal traits). Changing this configuration setting to SAMPLE_0 fixed the issue.

Tested on a Teensy 4.0 and 4.1 with a Trinamic TMC5130 stepper driver IC.

When using CS0 (pin 10 on Teensy 4.X), the SAMPLE_1 setting works fine. When using pin 4, the SAMPLE_0 must be used to avoid overruns. I'm curious if anyone else has experienced this issue.

Expose a public method to allow users to change sampling point of an LPSPI peripheral.

I've experienced RX FIFO overruns when the SAMPLE: SAMPLE_1 (delayed edge) is used with non-hardware controlled chip select pins (ie, manually driving a CS pin with GPIO embedded_hal traits). Changing this configuration setting to SAMPLE_0 fixed the issue.

Tested on a Teensy 4.0 and 4.1.
remove text
fix formatting
Copy link
Member

@mciantyre mciantyre left a comment

Choose a reason for hiding this comment

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

New API and docs LGTM. However, the method set_sample_point should be attached to Disabled. From the 1060 reference manual (rev 3 section 48.5.1.9.2),

CFGR1 should only be written when LPSPI is disabled.


We've included the "delayed edge by default" behavior since this driver was originally integrated. Now that the user can change the setting themselves, we could remove that implicit behavior and keep the hardware default. That's a future change for a breaking release.

When using CS0 (pin 10 on Teensy 4.X), the SAMPLE_1 setting works fine. When using pin 4, the SAMPLE_0 must be used to avoid overruns. I'm curious if anyone else has experienced this issue.

Sorry, not sure why this sampling point setting would be based on chip select. But if this is the difference between hardware- and software-controlled chip selects, I'm curious if there's a timing issue between CS changes and the LPSPI transactions.

src/common/lpspi.rs Outdated Show resolved Hide resolved
@dstric-aqueduct
Copy link
Contributor Author

Moved method to Disabled in 38b7398.

@dstric-aqueduct
Copy link
Contributor Author

Whoops - accidentally closed when responding - Ian, can you reopen?

@mciantyre
Copy link
Member

I squashed these commits, added a changelog entry, and merged. If there's any follow-on work between the close and now, we can discuss in a new PR.

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.

None yet

2 participants