diff --git a/src/spi/for_embedded_hal_1.rs b/src/spi/for_embedded_hal_1.rs new file mode 100644 index 00000000..9a3f747e --- /dev/null +++ b/src/spi/for_embedded_hal_1.rs @@ -0,0 +1,296 @@ +//! A rewrite of the SPI wrappers that uses the [`embedded-hal` 1.0 model](embedded_hal::spi) of +//! SPI. +//! +//! This is a dedicated module because the abstractions of that version differ significantly from +//! what was in 0.2 that inspired the original implementation. With the next breaking release of +//! `riot-wrappers`, this module will replace the [`super`] module (possibly with a compatibility +//! alias). +//! +//! # HAL author notes +//! +//! The `embedded-hal` documentations has some [notes for HAL +//! authors](https://docs.rs/embedded-hal/latest/embedded_hal/spi/index.html#for-hal-authors); not +//! all those are followed here: +//! +//! * "HALS **must not** add infrastructure for sharing at the `SpiBus` level": RIOT already has +//! that infrastructure; users don't have a guarantee on exclusive, but that's not even SPI +//! specific: For all it's worth, the SPI hardware can be multiplexed into (say) I2C hardware, +//! or anything else (hey maybe it's bit banging and the bitbanging core is just busy). At any +//! rate, this crate is not going out of its way to do that, it's a consequence of being in the +//! operating system, and technically it's not worse than being preempted by some interrupt. + +use crate::error::{NegativeErrorExt, NumericError}; +use core::convert::Infallible; +use core::num::NonZero; +use embedded_hal::spi::{ErrorType, Mode, Operation}; + +/// A RIOT SPI device combined with complete with mode and clock configuration, but no particular +/// CS pin. +/// +/// Note that while this implements [`embedded-hal::SpiBus`], it is not exclusive (because no +/// peripheral in RIOT is); when accessed while another "owner" uses it, operations only start when +/// the other party is done. +pub struct SpiBus { + bus: riot_sys::spi_t, + mode: riot_sys::spi_mode_t, + clk: riot_sys::spi_clk_t, +} + +/// A RIOT SPI device combined with its CS pin, complete with mode and clock configuration. +pub struct SpiDevice { + bus: SpiBus, + cs: riot_sys::spi_cs_t, +} + +impl SpiBus { + /// Creates a new SPI device, given its RIOT bus number (equivalent to running + /// `SPI_DEV(number)`). + /// + /// By default, the clock speed is set to the lowest speed supported by the hardware, and the + /// mode is set to SPI mode number 0 (the most common one). + pub fn from_number(number: u32) -> Self { + // SAFETY: This is designed to be called with any number. (Whether the device is then valid + // will show later). + let bus = unsafe { riot_sys::macro_SPI_DEV(number) }; + Self { + bus, + mode: riot_sys::spi_mode_t_SPI_MODE_0, + clk: riot_sys::spi_clk_t_SPI_CLK_100KHZ, + } + } + + /// Uses the bus with a particular CS pin, creating an [`embedded_hal::SpiDevice`]. + /// + /// Convenience alias for [`SpiDevice::new()`] for builder style construction. + #[cfg(riot_module_periph_gpio)] + pub fn into_device(self, cs: crate::gpio::GPIO) -> Result { + SpiDevice::new(self, cs) + } + + // This family of speed setters is deliberately by-function, because this can easily be kept + // available no matter how RIOT decides to support arbitrary speeds. + + /// Sets the speed to 100KHz. + pub fn with_speed_100khz(self) -> Self { + Self { + clk: riot_sys::spi_clk_t_SPI_CLK_100KHZ, + ..self + } + } + + /// Sets the speed to 400KHz. + pub fn with_speed_400khz(self) -> Self { + Self { + clk: riot_sys::spi_clk_t_SPI_CLK_400KHZ, + ..self + } + } + + /// Sets the speed to 1MHz. + pub fn with_speed_1mhz(self) -> Self { + Self { + clk: riot_sys::spi_clk_t_SPI_CLK_1MHZ, + ..self + } + } + + /// Sets the speed to 5MHz. + pub fn with_speed_5mhz(self) -> Self { + Self { + clk: riot_sys::spi_clk_t_SPI_CLK_5MHZ, + ..self + } + } + + /// Sets the speed to 10MHz. + pub fn with_speed_10mhz(self) -> Self { + Self { + clk: riot_sys::spi_clk_t_SPI_CLK_10MHZ, + ..self + } + } + + /// Sets the device's mode. + pub fn with_mode(self, mode: Mode) -> Self { + Self { + mode: match mode { + embedded_hal::spi::MODE_0 => riot_sys::spi_mode_t_SPI_MODE_0, + embedded_hal::spi::MODE_1 => riot_sys::spi_mode_t_SPI_MODE_1, + embedded_hal::spi::MODE_2 => riot_sys::spi_mode_t_SPI_MODE_2, + embedded_hal::spi::MODE_3 => riot_sys::spi_mode_t_SPI_MODE_3, + }, + ..self + } + } +} + +impl ErrorType for SpiBus { + type Error = Infallible; +} + +// In RIOT, there is no type-level distinction between a bus and a device -- if it's acquired with +// GPIO_UNDEF as the CS pin, it matches the SpiBus pattern, and if it has a pin in it, it matches +// the SpiDevice pattern. (The distinction of whether something is owned exclusively or not can not +// be made in RIOT as it has no concept of exclusive device ownership.) +// +// To avoid re-implementing everything and especially the split transfer logic, this is hooking +// into the transaction function, which is largely modeled after the HAL SpiDevice's API (taking an +// Operations list like SpiDevice::transaction()). +// +// This may or may not be efficient depending on what the compiler inlines, but really, if you want +// efficient, go with SpiDevice anyway for hardware CS. Another downside of this approach is that +// it means that ZTimer is a dependency even when not using SpiDevice; sticking with it for overall +// simplicity. +impl embedded_hal::spi::SpiBus for SpiBus { + fn read(&mut self, words: &mut [u8]) -> Result<(), Infallible> { + transaction( + &self, + riot_sys::inline::GPIO_UNDEF.try_into().unwrap(), + &mut [Operation::Read(words)], + ); + Ok(()) + } + fn write(&mut self, words: &[u8]) -> Result<(), ::Error> { + transaction( + &self, + riot_sys::inline::GPIO_UNDEF.try_into().unwrap(), + &mut [Operation::Write(words)], + ); + Ok(()) + } + fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Infallible> { + transaction( + &self, + riot_sys::inline::GPIO_UNDEF.try_into().unwrap(), + &mut [Operation::Transfer(read, write)], + ); + Ok(()) + } + fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Infallible> { + transaction( + &self, + riot_sys::inline::GPIO_UNDEF.try_into().unwrap(), + &mut [Operation::TransferInPlace(words)], + ); + Ok(()) + } + fn flush(&mut self) -> Result<(), Infallible> { + // See also comment on `SpiDevice for SpiDevice` + Ok(()) + } +} + +impl SpiDevice { + /// and its CS GPIO pin + #[cfg(riot_module_periph_gpio)] + pub fn new(bus: SpiBus, cs: crate::gpio::GPIO) -> Result { + let cs = cs.to_c(); + (unsafe { riot_sys::spi_init_cs(bus.bus, cs) }).negative_to_error()?; + Ok(Self { bus, cs }) + } +} + +impl ErrorType for SpiDevice { + type Error = Infallible; +} + +impl embedded_hal::spi::SpiDevice for SpiDevice { + // No need to implement flush(): It's not very explicit, but as spi_release() docs say that + // after release, the SPI bus should be powered down, that only works if release blocks until + // that is done. + + fn transaction(&mut self, ops: &mut [Operation<'_, u8>]) -> Result<(), Infallible> { + transaction(&self.bus, self.cs, ops); + Ok(()) + } +} + +fn transaction(bus: &SpiBus, cs: riot_sys::spi_cs_t, ops: &mut [Operation<'_, u8>]) { + unsafe { riot_sys::spi_acquire(bus.bus, cs, bus.mode, bus.clk) }; + let len = ops.len(); + for (index, op) in ops.iter_mut().enumerate() { + let cont = index != len - 1; + match op { + Operation::Read(bytes) => unsafe { + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont, + core::ptr::null(), + bytes.as_mut_ptr() as _, + bytes.len().try_into().expect("usize and size_t match"), + ); + }, + Operation::Write(bytes) => unsafe { + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont, + bytes.as_ptr() as _, + core::ptr::null_mut(), + bytes.len().try_into().expect("usize and size_t match"), + ); + }, + Operation::Transfer(read, write) => unsafe { + let common_length = core::cmp::min(read.len(), write.len()); + let read_longer = read + .len() + .checked_sub(write.len()) + .and_then(|n| NonZero::try_from(n).ok()); + let write_longer = write + .len() + .checked_sub(read.len()) + .and_then(|n| NonZero::try_from(n).ok()); + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont || read_longer.is_some() || write_longer.is_some(), + write.as_ptr() as _, + read.as_mut_ptr() as _, + common_length.try_into().expect("usize and size_t match"), + ); + if let Some(read_longer) = read_longer { + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont, + core::ptr::null(), + read[common_length..].as_mut_ptr() as _, + read_longer + .get() + .try_into() + .expect("usize and size_t match"), + ); + } + if let Some(write_longer) = write_longer { + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont, + write[common_length..].as_ptr() as _, + core::ptr::null_mut(), + write_longer + .get() + .try_into() + .expect("usize and size_t match"), + ); + } + }, + Operation::TransferInPlace(bytes) => unsafe { + riot_sys::spi_transfer_bytes( + bus.bus, + cs, + cont, + bytes.as_ptr() as _, + bytes.as_mut_ptr() as _, + bytes.len().try_into().expect("usize and size_t match"), + ); + }, + Operation::DelayNs(time) => { + crate::ztimer::Clock::usec().sleep(crate::ztimer::Ticks(time.div_ceil(1000))); + } + } + } + // SAFETY: as per C API. + unsafe { riot_sys::spi_release(bus.bus) }; +} diff --git a/src/spi/mod.rs b/src/spi/mod.rs index b3740b84..e304471a 100644 --- a/src/spi/mod.rs +++ b/src/spi/mod.rs @@ -1,3 +1,6 @@ +#[cfg(riot_module_ztimer_usec)] +pub mod for_embedded_hal_1; + use core::convert::Infallible; use embedded_hal_0_2::blocking; use riot_sys::{ diff --git a/tests/spi/Cargo.toml b/tests/spi/Cargo.toml new file mode 100644 index 00000000..61b990a2 --- /dev/null +++ b/tests/spi/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "riot-wrappers-test-spi" +version = "0.1.0" +edition = "2021" +publish = false + +[lib] +crate-type = ["staticlib"] + +[profile.release] +panic = "abort" + +[dependencies] +# critical-section: We don't *use* this, but the embedded-hal-bus crate also contains variants that rely on critical sections, so we have to enable it. +riot-wrappers = { path = "../..", features = [ "set_panic_handler", "panic_handler_format", "provide_critical_section_1_0" ] } +embedded-hal = "1" +embedded-hal-bus = "0.2.0" + +# Actually this is only required on samr21 and other non-atomic platforms to +# make the static_cell work, but for sake of setup simplicity we'll keep this +# enabled everywhere. (A better solution would be to have the RIOT build system +# managed crates provide this dependency on platforms where it is needed). +portable-atomic = { version = "1.6", default-features = false, features = [ "critical-section" ] } diff --git a/tests/spi/Makefile b/tests/spi/Makefile new file mode 100644 index 00000000..b21be9e9 --- /dev/null +++ b/tests/spi/Makefile @@ -0,0 +1,9 @@ +APPLICATION = riot-wrappers-test-spi +APPLICATION_RUST_MODULE = riot_wrappers_test_spi +BASELIBS += $(APPLICATION_RUST_MODULE).module +FEATURES_REQUIRED += rust_target +FEATURES_REQUIRED += periph_gpio +FEATURES_REQUIRED += periph_spi +USEMODULE += ztimer_usec + +include $(RIOTBASE)/Makefile.include diff --git a/tests/spi/src/lib.rs b/tests/spi/src/lib.rs new file mode 100644 index 00000000..654614dc --- /dev/null +++ b/tests/spi/src/lib.rs @@ -0,0 +1,104 @@ +//! This is a primitive SPI test. +//! +//! It performs different reads and writes in different modes. The precise results depend on +//! the voltage on the MISO pin, which is usually indeterminate. +#![no_std] + +use embedded_hal::spi::SpiDevice; + +use riot_wrappers::println; +use riot_wrappers::riot_main; + +riot_main!(main); + +fn main() { + // SPI device and CS pin that the test may drive. + // + // Boards should only be added if the attached peripherals are safe to use, no matter what gets + // written there. + let (spi_num, cs_num) = match riot_wrappers::BOARD { + "particle-xenon" => (0, (0, 31)), + _ => panic!("No "), + }; + + let cs = riot_wrappers::gpio::GPIO::from_port_and_pin(cs_num.0, cs_num.1).unwrap(); + let spi = riot_wrappers::spi::for_embedded_hal_1::SpiBus::from_number(spi_num) + // arbitrary parameters + .with_speed_1mhz() + .with_mode(embedded_hal::spi::MODE_2); + + println!("Testing with software CS"); + // Writing a test for the SpiBus would be annoyingly repetitive compared to the one for + // SpiDevice; using the SpiBus through the embedded-hal-bus mechanism instead. + // + // Also, there's not really anything about this test where the use of the CS pin makes any + // difference in the outcome, it's more to cover the full API surface. + let Ok(mut spi_with_soft_cs) = embedded_hal_bus::spi::ExclusiveDevice::new( + spi, + cs.configure_as_output(riot_wrappers::gpio::OutputMode::Out).unwrap(), + riot_wrappers::ztimer::Clock::usec(), + ); + test_on_device(&mut spi_with_soft_cs); + + // ExclusiveDevice has no destructuring finalizer, so we just rebuild the pieces, which RIOT + // lets us do -- and it won't cause any sort of practical trouble because the exclusive device + // is not used any more, probably even dropped already. + let cs = riot_wrappers::gpio::GPIO::from_port_and_pin(cs_num.0, cs_num.1).unwrap(); + let spi = riot_wrappers::spi::for_embedded_hal_1::SpiBus::from_number(spi_num) + // arbitrary parameters + .with_speed_1mhz() + .with_mode(embedded_hal::spi::MODE_2); + + println!("Testing with hardware CS"); + // It is not guaranteed that this is really hardware CS; could just as well be performed by + // RIOT internally. + let mut spi_with_hard_cs = spi.into_device(cs).unwrap(); + test_on_device(&mut spi_with_hard_cs); + + println!("Both tests done."); +} + +// This is a bit .unwrap()py even though all our devices have infallible SPI (and CS) operations at +// runtime; can't use `let Ok(()) = …;` because the ExclusiveDevice has an Either type, and I haven't found an easy way to require that the associated error of D is +// uninhabited. +fn test_on_device(spi: &mut D) { + println!("Plain transfer in place:"); + let mut buf = [0, 0, 0x12, 0x34]; + println!("Writing {:?}, …", buf); + spi.transfer_in_place(&mut buf).unwrap(); + println!("read {:?}.", buf); + + println!("Write from flash:"); + // Writing from flash makes a difference eg. on nrf52: That peripheral is DMA only and can not + // read flash. + let buf = [0, 0, 0x12, 0x34]; + println!("Writing {:?}.", buf); + spi.write(&buf).unwrap(); + + println!("Performing complex sequence:"); + let writebuf = [0; 300]; + let mut readbuf1 = [0x12, 0x34, 0x56, 0x78]; + let mut readbuf2 = [0x12, 0x34, 0x56, 0x78]; + use embedded_hal::spi::Operation; + let mut operations = [ + Operation::Write(&writebuf), + Operation::DelayNs(123), + Operation::Read(&mut readbuf1), + Operation::Read(&mut readbuf2), + ]; + spi.transaction(&mut operations).unwrap(); + println!( + "Wrote [0; 300], read into {:?} and {:?}", + readbuf1, readbuf2 + ); + + println!("Plain transfer in place:"); + let writebuf = [0, 0]; + let mut readbuf = [0xff; 10]; + spi.transfer(&mut readbuf, &writebuf).unwrap(); + println!( + "In mixed transfer, wrote [0; 2], and continued reading into {:?}.", + readbuf + ); +}