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

Prepare LPSPI driver for embedded-hal 1.0 rework #149

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Finomnis
Copy link
Contributor

@Finomnis Finomnis commented Dec 22, 2023

Some fixes/modifications to the existing LPSPI driver.

This is in preparation for the embedded-hal 1.0 rework, which will introduce a new high level driver that wraps this low level one.

Changes:

  • Remove CS_PIN from Spi device
    • Rationale: eh1 primarily uses software CS pins. There is an extension for hardware CS pin support, but I think we should add it later as a special case instead of by default. This will simplify API design a lot.
  • Split Spi into Spi + SpiCsPin in the board subcrate
  • Add initial state to gpio::Output constructor. This is important for cases where the state needs to be set before enabling the pin.
  • Fix/rework check for valid frame sizes (existing implementation did not cover all corner cases)
  • Rework SPI clock initialization
    • Make sure we never get a baud rate faster than specified (slower is pretty much always ok for peripherals)
    • Configure PCSSCK and SCKPCS properly for contiguous transfers
  • Some improvements in reset logic
  • Add soft_reset that cancels the current transfer without requiring a full re-initialization
  • Move more values into enqueue_transaction so that multiple transactions with different settings can be enqueued in series
  • Add flush function that waits for the current transaction to be finished whilst repeatedly checking for errors
  • Fix disabled() not waiting for the current transfer to be finished
  • Take set_watermark out of disabled, because it doesn't require it.

@Finomnis
Copy link
Contributor Author

Finomnis commented Feb 24, 2024

@mciantyre Sorry that I was gone for a while. Don't know how much time I will be able to spend on the SPI device, but I'll try to do a little :)

I finally finished this PR. I didn't update examples yet, but I think there's no point really in updating them until we have a properly reworked driver.

Any feedback?

@Finomnis
Copy link
Contributor Author

Finomnis commented Mar 2, 2024

@mciantyre Ping :)

@teburd
Copy link
Member

teburd commented Mar 17, 2024

I’d say squash this and I’ll merge, changes look good. Ian is presumably busy with other endeavors at the moment.

@Finomnis
Copy link
Contributor Author

@teburd I don't think I agree time to continue this any time soon, so let's keep that one open because it's a breaking change.

@mciantyre
Copy link
Member

There's useful non-breaking changes in this branch, changes that could help today's users. I extracted them into #156.

@Finomnis
Copy link
Contributor Author

@mciantyre Welcome back! Glad to see your life has cooled down enough to dabble in some Rust again :)

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