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

Support switching SPI pins for difference devices with same SPI peripheral #2876

Open
ProfFan opened this issue Jan 2, 2025 · 1 comment
Labels
peripheral:spi SPI peripheral

Comments

@ProfFan
Copy link
Contributor

ProfFan commented Jan 2, 2025

Motivations

The ESP32 series has very limited number of SPI peripherals (only SPI2/3). It is possible to use CS pins to hook multiple SPI devices under the same SPI master, but this raises signal integrity issues (with multiple slaves on a long branching SPI bus).

It is much better if the SPI branching is done inside the chip.

Solution

The ESP32's GPIO interconnect can hook multiple outputs to the same peripheral output. Thus one could duplicate the MOSI/SCLK pins. The MISO cannot be duplicated but can be switched at runtime.

Alternatives

None

Additional context

I have a PoC implementation (very dirty):

/// SPI device on a shared bus with multiple MISO/MOSI pins
///
/// Only support ESP32 devices.
pub struct MultipinSpiDevice<'a, M: RawMutex, BUS, CS, P: 'static> {
    bus: &'a Mutex<M, BUS>,
    cs: CS,
    signal: esp_hal::gpio::InputSignal,
    miso_pin: PeripheralRef<'static, P>,
    phantom: PhantomData<P>,
}

impl<M, BUS, CS, P, Word> spi::SpiDevice<Word> for MultipinSpiDevice<'_, M, BUS, CS, P>
where
    M: RawMutex,
    BUS: spi::SpiBus<Word>,
    CS: OutputPin,
    P: PeripheralInput + esp_hal::peripheral::Peripheral<P = P>,
    <P as esp_hal::peripheral::Peripheral>::P: esp_hal::gpio::InputPin,
    Word: Copy + 'static,
{
    async fn transaction(
        &mut self,
        operations: &mut [spi::Operation<'_, Word>],
    ) -> Result<(), Self::Error> {
        let mut bus = self.bus.lock().await;
        self.cs.set_low().map_err(SpiDeviceError::Cs)?;
        self.signal.connect_to(self.miso_pin.reborrow());

        let op_res = 'ops: {
            for op in operations {
                let res = match op {
                    Operation::Read(buf) => bus.read(buf).await,
                    Operation::Write(buf) => bus.write(buf).await,
                    Operation::Transfer(read, write) => bus.transfer(read, write).await,
                    Operation::TransferInPlace(buf) => bus.transfer_in_place(buf).await,
                    Operation::DelayNs(ns) => match bus.flush().await {
                        Err(e) => Err(e),
                        Ok(()) => {
                            embassy_time::Timer::after_nanos(*ns as _).await;
                            Ok(())
                        }
                    },
                };
                if let Err(e) = res {
                    break 'ops Err(e);
                }
            }
            Ok(())
        };

        // On failure, it's important to still flush and deassert CS.
        let flush_res = bus.flush().await;
        let cs_res = self.cs.set_high();

        let op_res = op_res.map_err(SpiDeviceError::Spi)?;
        flush_res.map_err(SpiDeviceError::Spi)?;
        cs_res.map_err(SpiDeviceError::Cs)?;

        Ok(op_res)
    }
}


// Usage
let mut spidev = MultipinSpiDevice::new(
        spi_mutex,
        imu_cs,
        esp_hal::gpio::InputSignal::FSPIQ,
        imu_miso,
    );

which works pretty well with my application, but I think my implementation looks very bad. Is there a better way (that follows esp-hal's style) to do this?

@ProfFan ProfFan added the status:needs-attention This should be prioritized label Jan 2, 2025
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Jan 2, 2025
@bugadani
Copy link
Contributor

bugadani commented Jan 6, 2025

The problem is, that if such a device is used on an SpiBus, all other devices need to set pins as well, or this one has to reset them back to the original pin assignment. It's probably reasonable enough to keep this implementation in your app for now, but we'll probably end up needing something similar eventually.

One tip I have for you: You can use your SPI bus type and the Instance trait it implements, to obtain an Info reference and look up the signal from that. It's not something we are trying to stabilise right now, so these bits are most likely hidden in the documentation and we don't have examples, sorry for that.

@bjoernQ bjoernQ added peripheral:spi SPI peripheral and removed status:needs-attention This should be prioritized labels Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peripheral:spi SPI peripheral
Projects
Status: Todo
Development

No branches or pull requests

3 participants