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

Extending LPSPI pin trait to support non-zero chip select #33

Closed
ameliamariecollins opened this issue Mar 13, 2023 · 5 comments · Fixed by #34
Closed

Extending LPSPI pin trait to support non-zero chip select #33

ameliamariecollins opened this issue Mar 13, 2023 · 5 comments · Fixed by #34

Comments

@ameliamariecollins
Copy link
Contributor

ameliamariecollins commented Mar 13, 2023

imxrt-iomuxc does not have a way to express non-zero LPSPI chip selects. We only identify PCS0 pins, along with the data and clock pins. Without a way to identify non-zero LPSPI chip selects in the type system, we cannot use imxrt-iomuxc APIs to configure PCS1, PCS2, and PCS3 pads.

This issue tracks support for an API to express these pins. This issue does not track MCU pin implementations; we'll use subtasks of #14 to track that effort.

(The original issue continues below.)


        iomuxc::lpspi::prepare(&mut pins.p37); // lpspi4 PCS0-1
        iomuxc::lpspi::prepare(&mut pins.p36); // lpspi4 PCS0-2

results in:

92 |         iomuxc::lpspi::prepare(&mut pins.p36); // lpspi4 PCS0
   |         ---------------------- ^^^^^^^^^^^^^ the trait `teensy4_bsp::imxrt_hal::iomuxc::lpspi::Pin` is not implemented for `Pad<1075806596, 1075807092>`

But those are truly are PSC0-1 and PCS0-2 for LPSPI4 (Teensy 4.1).

Is there a better way to use the HAL for a SPI bus that has multiple peripherals and therefore needs multiple PCS lines where each can be enabled when needed?

Many thanks!

@mciantyre mciantyre changed the title PSC0-1 and PCS0-2 cannot be prepared for LPSPI4 Extending LPSPI pin trait to support non-zero chip select Mar 13, 2023
@mciantyre
Copy link
Member

There's an overarching issue: we have no way to express non-zero LPSPI chip selects in the type system. This is why contributions like #12 skip those pin implementations. I've re-written this issue's title and description accordingly.

Once we have both of

  1. a way to express non-zero chip selects in the type system (this issue),
  2. 1060 pin implementations for those non-zero chip selects (TBD issue; using Complete pin impls #14 for now),

then those two iomuxc::lpspi::prepare calls will compile as expected. I'd be happy to mentor you or other contributors to add these features.


If you're looking to make progress without these larger contributions, there may be limited workarounds:

  • Try using the lower-level imxrt-iomuxc APIs to set the alternate ("alt") value. The alt values are documented in the reference manual. You may also need to set daisy registers yourself; however, I don't think today's imxrt-iomuxc makes this easy for external users.
  • Go even lower: use imxrt-ral directly. This is less user friendly than the first workaround, but it's all powerful.

@ameliamariecollins
Copy link
Contributor Author

I would be very grateful to be mentored. This will familiarize me with the code base, and allow me to contribute to it, both goals of mine. Having a smooth, wieldy API is always satisfying, and so I'd love to do it right and also Rustically.

@mciantyre
Copy link
Member

mciantyre commented Mar 14, 2023

Sounds great. I expect this to be a straightforward feature. Here's the gist of an approach...

To start, we'll need three more pub enums in the lpspi.rs module: We can call them Pcs1, Pcs2, and Pcs3. These three new enums will also need an impl Signal and impl Sealed, just like all the other pub enums.

For your learning:

  • There's a "why empty enums?" discussion in the API documentation.
  • The sealed traits pattern is documented here.

After that, we're doing similar work as #12, but we're adding implementations for the 1060's PCS1, PCS2, and PCS3 pins. It's a route, manual "translate a table into Rust" process. Here is a copy of the reference manual (RM). Chapter 10 has the pin muxing. Scan the left-most column for the LPSPI instances. There will be rows for the PCS[1,2,3] pins. All we need to do is add those to the imxrt1060/lpspi.rs module.

It doesn't look like there are any daisy registers associated with non-zero chip selects. I'm hastily scanning the RM to make this claim (and there's no auto-generated daisy registers already in the module). That means we can set the daisy fields to None. Supporting optional LPSPI daisy registers is a new, breaking change. Since this new work depends on that feature, we'll only be able to release it in the next release series (0.3).

@ameliamariecollins
Copy link
Contributor Author

ameliamariecollins commented Mar 14, 2023

Superb! Thank you for the help and info. I will start on this ASAP.

Many thanks for your help and encouragement.

@ameliamariecollins
Copy link
Contributor Author

ameliamariecollins commented Mar 14, 2023

Okay, looks like your workflow is branch-for-maintenance, not branch-for-development. I see that you've bumped the package version to 0.3, so I will branch from master.

@mciantyre mciantyre linked a pull request Mar 15, 2023 that will close this issue
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 a pull request may close this issue.

2 participants