rp-rs / rp-hal

A Rust Embedded-HAL for the rp series microcontrollers
https://crates.io/crates/rp2040-hal
1.32k stars 222 forks source link

CS handling in SPI #480

Open cimbul opened 1 year ago

cimbul commented 1 year ago

I'm new to the embedded-hal ecosystem, so apologies if this is a dumb question. It may also have more to do with the PL022 hardware than this crate.

I'm trying to configure a TMC2130 stepper driver with SPI (see chapter 4 in the datasheet). It uses a 40-bit datagram, and says that the CS pin should be asserted low for the whole datagram, but it needs to come up after each one. This seems to be because of daisy chaining: if you send more than 40 bits while CS is low, it echoes the leading bits out to the next device.

As suggested by the TMC datasheet, I'm using Mode 3. The RP2040/PL022 datasheet describes the behavior of the CS pin with SPH = 1 as follows:

After all bits have been transferred, in the case of a single word transmission, the SSPFSSOUT line is returned to its idle HIGH state one SSPCLKOUT period after the last bit has been captured.

For continuous back-to-back transmissions, the SSPFSSOUT pin remains in its active-LOW state, until the final bit of the last word has been captured, and then returns to its idle state as the previous section describes.

However, I can't tell what the PL022 considers the "last word" in the transmission to set CS back to idle. When I send multiple datagrams with successive spi.transfer(&mut datagram) calls, the TMC just echoes the commands back to the RP2040 as though it's daisy chained and expecting CS to rise. I tried adding a short delay—several SCK cycles' worth—between the spi.transfer() calls and got the same result. Either way, I would expect the FIFOs to be empty between calls thanks to the way spi.transfer() is implemented.

The only thing that has worked so far is configuring the CS pin as a GPIO and manually pulsing it between datagrams:

type Datagram = [u8; 5];

fn driver_send<T: Transfer<u8>>(
    out: &mut impl Write,
    spi: &mut T,
    cs: &mut impl OutputPin,
    datagrams: &[Datagram]
) -> Result<(), T::Error> {
    let mut buf: Datagram = Default::default();
    for datagram in datagrams {
        // Send a command (40-bit datagram) to the TMC driver
        buf.copy_from_slice(datagram);
        let _ = write!(out, "→ {:02x?}\r\n", buf);
        cs.set_low().ok().unwrap();
        spi.transfer(&mut buf)?;
        cs.set_high().ok().unwrap();

        // TODO: Ensure CS stays high for at least 20ns??

        // Receive the response datagram, sending zeros as a dummy command        
        buf.fill(0);
        cs.set_low().ok().unwrap();
        spi.transfer(&mut buf)?;
        cs.set_high().ok().unwrap();
        let _ = write!(out, "← {:02x?}\r\n", buf);
    }

    Ok(())
}

#[entry]
fn main() -> ! {
    // ...

    pins.gpio19.into_mode::<hal::gpio::FunctionSpi>(); // TX
    pins.gpio18.into_mode::<hal::gpio::FunctionSpi>(); // SCK
    pins.gpio16.into_mode::<hal::gpio::FunctionSpi>(); // RX

    // This is what I was trying before:
    // pins.gpio17.into_mode::<hal::gpio::FunctionSpi>(); // CSn

    // Now I'm handling CS manually:
    let mut cs = pins.gpio17.into_push_pull_output();
    cs.set_high().unwrap();

    let mut spi = hal::spi::Spi::<_, _, 8>::new(pac.SPI0).init(
        &mut pac.RESETS,
        125_u32.MHz(),
        2_u32.MHz(),
        &embedded_hal::spi::MODE_3,
    );

    driver_send(&mut uart, &mut spi, &mut cs, INIT_COMMANDS)
        .expect("could not initialize driver");
    driver_send(&mut uart, &mut spi, &mut cs, STATUS_COMMANDS)
        .expect("could not get initial driver status");

    // ...
}

Is that typically what we need to do to control the CS signal? It seems like asking for timing problems, but so far it seems to work. Of course, I could probably handle this with PIO, but I'm just curious if there's something I'm missing about the native SPI support.

jannic commented 1 year ago

I don't have a solution at hand for your issue. But I think this is an interesting question for the embedded-hal API in general, as it would be good if it was possible to write a generic TMC2130 driver on top of the embedded-hal abstraction, without directly programming the RP2040 SPI peripheral. Perhaps you could ask in https://matrix.to/#/#rust-embedded:matrix.org as well?

cimbul commented 1 year ago

Recapping the conversation on the Matrix channel:

The SPI module in latest embedded-hal v1 alpha distinguishes between the SpiBus trait, with no CS handling, and SpiDevice, which is associated with a particular CS line and has a transaction() method with explicit requirements around CS.

That isn't available yet in rp2040-hal, and it's not clear if there's a way to implement it with the PL022 rather than a software-controlled GPIO, but it does address the long-term driver design question.

I'm good with this, but I'll leave it up to the maintainers to decide whether the issue can be closed.

jannic commented 1 year ago

Let's keep it open, this is at least a valid feature request.

ptpaterson commented 1 year ago

Using the hardware controlled CS pin, in master configuration, for multi-byte frames, is not possible in all SPI modes.

I just submitted PR #488 to implement the latest SPI traits, and was going to include SpiDevice because of the hardware control. But turns out hardware control has limits/ quirks that mean providing a generic HAL implementation for SpiDevice is not going to be possible. And I am convinced that is reasonable at this point.

I came across this discussion which helped me grok the datasheet on the hardware control of the CS pin. https://forums.raspberrypi.com/viewtopic.php?t=309859

I moved my SpiDevice implementation to my application code base, but it's still quite simple. To be fair, I don't have to worry about shared SPI bus in my application.

use core::convert::Infallible;
use eh1_0_alpha::{
    digital::OutputPin,
    spi::{self as eh1_spi, ErrorType, SpiBus},
};

pub struct SpiDevice<SPI, CS> {
    bus: SPI,
    cs: CS,
}

impl<SPI, CS> SpiDevice<SPI, CS>
where
    SPI: SpiBus<Error = Infallible>,
    CS: OutputPin<Error = Infallible>,
{
    pub fn new(bus: SPI, cs: CS) -> Self {
        let mut cs = cs;
        cs.set_high().unwrap();
        Self { bus, cs }
    }
}

impl<SPI, CS> ErrorType for SpiDevice<SPI, CS>
where
    SPI: SpiBus<Error = Infallible>,
    CS: OutputPin<Error = Infallible>,
{
    type Error = Infallible;
}

impl<SPI, CS> eh1_spi::SpiDevice for SpiDevice<SPI, CS>
where
    SPI: SpiBus<Error = Infallible>,
    CS: OutputPin<Error = Infallible>,
{
    type Bus = SPI;

    fn transaction<R>(
        &mut self,
        f: impl FnOnce(&mut SPI) -> Result<R, <Self::Bus as ErrorType>::Error>,
    ) -> Result<R, Self::Error> {
        self.cs.set_low()?;
        let result = f(&mut self.bus);
        self.bus.flush()?;
        self.cs.set_high()?;
        result
    }
}
ptpaterson commented 1 year ago

@cimbul pointed out that the latest embedded-hal traits distinguish between the SpiBus, representing ownership of the bus, and SpiDevice, which could share the bus with other devices. It's that potential sharing that means the implementation aught to be user-specific.

embedded-hal notes that

HALs must not add infrastructure for sharing at the [SpiBus] level. User code owning a [SpiBus] must have the guarantee of exclusive access.

eldruin commented 1 year ago

It should be noted that SpiDevice implementations can come from other crates like embedded-hal-bus. What @ptpaterson has implemented is basically spi::ExclusiveDevice. Currently it only contains that but there is more to come.