Skip to content

RFC: Use the SPI traits to represent a single peripheral on an SPI bus #299

Closed
@GrantM11235

Description

@GrantM11235

What?

Currently, the SPI traits are used to represent an entire SPI bus. I propose using them to represent a single peripheral on an SPI bus.

Why?

Generic device drivers want to be able to communicate with an SPI device or devices, they don't need access to the whole bus. In fact, giving the entire bus to a driver has a number of downsides:

  • The driver must manually toggle it's chip select. This is inconvenient and can lead to problems like Clarify SPI timing behaviour (currently completely broken!) #264.
  • Because the driver must hold the SPI bus struct (or at least a &mut), nothing else can use that bus for the lifetime of the driver. shared-bus exists to work around this problem, but it has limitations (edit: and it is unsound SpiProxy is still unsound, even in a single thread Rahix/shared-bus#23) due to the manual handling of chip selects.
  • A driver that communicates with two SPI devices must decide whether to take one bus (and only work if both devices are on the same bus) or two buses (and only work if the devices are on separate buses).
  • A driver has no way to communicate with two SPI devices on the same bus if they use different bus configurations (polarity, phase, speed) because there is no way to change the configuration of the bus using embedded_hal.

How?

embedded_hal

We don't need to actually change any code in embedded_hal, we just need to mandate that any implementations of the spi traits must hold their chip select pins low for the duration of the transfer.

This is similar to #180/#245, but it would be mandatory, not optional.

Generic device drivers

Generic device drivers can be updated by simply removing any manual toggling of chip select pins.

HALs

This is where it gets interesting...

HAL implementations

Instead of implementing the SPI traits directly on an spi bus, HALs will instead create device structs. Here are some simplified examples:

A device struct which shares it's bus:

pub struct Device {
	bus: Mutex<Spi1>,
	cs_pin: OutputPin,
	config: Config,
}

impl spi::Write<u8> for Device {
	fn write(&mut self, words: &[u8]) {
		self.bus.lock(|bus| {
			bus.reconfigure(self.config);
			self.cs_pin.set_low();
			bus.write(words);
			self.cs_pin.set_high();
		});
	}
}

A device struct with exclusive access to it's bus, which doesn't need a mutex and doesn't need to reconfigure the bus every time:

pub struct ExclusiveDevice<'a> {
	bus: &'a mut Spi1,
	cs_pin: OutputPin,
}

impl<'a'> spi::Write<u8> for ExclusiveDevice<'a> {
	fn write(&mut self, words: &[u8]) {
		self.cs_pin.set_low();
		self.bus.write(words);
		self.cs_pin.set_high();
	}
}

What could this look like?

let spi1 = hal::spi::Spi1::take();

let lcd_spi = spi1.new_device(lcd_cs_pin, MODE_1, MegaHertz(20));

let sensor_spi = spi1.new_device(sensor_cs_pin, MODE_2, MegaHertz(1));

let mut lcd = Lcd::new(lcd_spi, lcd_reset_pin);

let mut sensor = Sensor::new(sensor_spi);

loop {
    let data = sensor.read_data();

    display_data(&mut lcd, data);
}

Unresolved questions

  • What guarantees do we need to provide about the chip select pin? Obviously it needs to be low during a transfer, but do we need to guarantee that it goes high afterwards? What about between Transactional operations?
  • How hard is it to implement this in HALs?
  • How much overhead is introduced by reconfiguring the bus every time a trait method is used? Can it be mitigated with Transactional?
  • How would this work for the nonblocking SPI trait?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions