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
commented
1 year ago
Currently our Spidev wrapper struct implements both
SpiBusandSpiDevicefrom embedded-hal. I think this is likely to lead to some confusion, especially since theSpiBusmethods 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,
SpidevBusandSpidevDeviceperhaps, where:SpidevBusimplements just theSpiBustraitspidevto not drive the associated CS pinspidevdevice on that busSpiDeviceobjects, using generic GPIO for CS control, or used when dedicated bus access is required (eg to generate smart LED waveforms)spidevSpidevDeviceimplements just theSpiDevicetraitSpiDevice, the kernel handles sharing and CS for us, even with other programsI'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.