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

SPI struct implements both SpiBus and SpiDevice #99

Open
adamgreig opened this issue Sep 27, 2023 · 1 comment
Open

SPI struct implements both SpiBus and SpiDevice #99

adamgreig opened this issue Sep 27, 2023 · 1 comment

Comments

@adamgreig
Copy link
Member

Currently our Spidev wrapper struct implements both SpiBus and SpiDevice from embedded-hal. I think this is likely to lead to some confusion, especially since the SpiBus methods will still actually be using a shared SPI bus, toggling CS, and multiplexing with any other spidev devices on the same bus.

We discussed this a bit in #87 and while the main problem there is resolved by the new embedded-hal design, I think the split suggested is still worth considering. The proposal is to have two wrapper structs, SpidevBus and SpidevDevice perhaps, where:

  • SpidevBus implements just the SpiBus trait
    • we configure spidev to not drive the associated CS pin
    • we explain in the documentation that when this struct is used, it must be the only active spidev device on that bus
    • we encourage users to give it a CS pin that is not connected to anything, since whatever pin is assigned to it in the device tree can't be re-used as GPIO (and therefore can't be used as a CS)
    • this struct can then be used with any of the bus-multiplexing crates (like embedded-hal-bus) to generate lots of SpiDevice objects, using generic GPIO for CS control, or used when dedicated bus access is required (eg to generate smart LED waveforms)
    • we are basically lying to the kernel about it being a single device, because there's no way to request exclusive bus access with spidev
  • SpidevDevice implements just the SpiDevice trait
    • it's already a perfect fit for SpiDevice, the kernel handles sharing and CS for us, even with other programs
    • it will work just as users expect
    • but their device tree has to be set up to create the device and use the right CS

I'm happy to implement this as a PR if it sounds good to people, but it'd be great to get some feedback first or any suggestions on how it might be better done, especially if there's any clever spidev-related tricks that might be useful.

@eldruin
Copy link
Member

eldruin commented Sep 28, 2023

Sounds great to me!

adamgreig added a commit to adamgreig/linux-embedded-hal that referenced this issue Oct 8, 2023
adamgreig added a commit to adamgreig/linux-embedded-hal that referenced this issue Oct 8, 2023
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

No branches or pull requests

2 participants