SPI slave with DMA and hardware NSS

[hacknus]

Hello,

Thanks to all the devs for this amazing crate!
I've been using it for the last couple of months and I'm very happy. I have noticed that when setting up SPI as a slave with DMA:

    // SPI1
    let sck = gpioa.pa5;
    let miso = gpioa.pa6.into_alternate_push_pull(&mut gpioa.crl);
    let mosi = gpioa.pa7;
    let spi = Spi::spi1_slave(dp.SPI1, (sck, miso, mosi), &mut afio.mapr, MODE);

     let dma_channels = dp.DMA1.split();

    // Prepare DMA
    let spi_dma = spi.with_rx_tx_dma(dma_channels.2, dma_channels.3);

   let slave_transfer = spi_dma.read_write(&mut RX_BUF_A, &mut TX_BUF_A);


unsafe {
        NVIC::unmask(Interrupt::DMA1_CHANNEL2);
        NVIC::unmask(Interrupt::DMA1_CHANNEL3);
    }

es explained in the example, this is not enough. I need to add some unsafe stuff to actually get the DMA1_CHANNEL2 and DMA1_CHANNEL3 interrupts to fire:

 unsafe {
        // Enable DMA transfer complete interrupts
        let dma = &(*stm32f1xx_hal::pac::DMA1::ptr());

        // Configure channel 2 (RX)
        dma.ch2.cr.modify(|_, w| w.tcie().set_bit()); // Enable transfer complete interrupt

        // Configure channel 3 (TX)
        dma.ch3.cr.modify(|_, w| w.tcie().set_bit()); // Enable transfer complete interrupt

        // Start the first DMA transfer
        USING_BUFFER_A.store(true, Ordering::Relaxed);
        let slave_transfer = spi_dma.read_write(&mut RX_BUF_A, &mut TX_BUF_A);

        // Store the transfer in the global
        cortex_m::interrupt::free(|cs| {
            G_SPI1_DMA_TRANSFER.borrow(cs).replace(Some(slave_transfer));
        });
    }

and then the interrupts are:

// Correct type definition for SPI1 with DMA
// Modify your type definitions to match what the HAL expects
type Spi1RxTxDma = stm32f1xx_hal::spi::SpiRxTxDma<
    stm32f1xx_hal::pac::SPI1,
    stm32f1xx_hal::spi::Spi1NoRemap,
    (
        stm32f1xx_hal::gpio::PA5<stm32f1xx_hal::gpio::Input<stm32f1xx_hal::gpio::Floating>>,
        stm32f1xx_hal::gpio::PA6<stm32f1xx_hal::gpio::Alternate<stm32f1xx_hal::gpio::PushPull>>,
        stm32f1xx_hal::gpio::PA7<stm32f1xx_hal::gpio::Input<stm32f1xx_hal::gpio::Floating>>,
    ),
    stm32f1xx_hal::spi::Slave,
    stm32f1xx_hal::dma::dma1::C2,
    stm32f1xx_hal::dma::dma1::C3,
>;

// DMA transfer type
type Spi1DmaTransfer = stm32f1xx_hal::dma::Transfer<
    stm32f1xx_hal::dma::W,
    (&'static mut [u8; 8], &'static mut [u8; 8]), // References instead of owned buffers
    Spi1RxTxDma,
>;

static mut RX_BUF_A: [u8; 8] = [0; 8];
static mut RX_BUF_B: [u8; 8] = [0; 8];
static mut TX_BUF_A: [u8; 8] = [0xDE, 0xAD, 0xBE, 0xEF, 1, 2, 3, 4];
static mut TX_BUF_B: [u8; 8] = [0xDE, 0xAD, 0xBE, 0xEF, 1, 2, 3, 4];
 // Add these global variables at the top of your file where the other globals are defined
static USING_BUFFER_A: AtomicBool = AtomicBool::new(true);
static G_SPI1_DMA_TRANSFER: Mutex<RefCell<Option<Spi1DmaTransfer>>> =
    Mutex::new(RefCell::new(None));

#[interrupt]
fn DMA1_CHANNEL2() {
    cortex_m::interrupt::free(|_cs| {
        // Get DMA registers
        let dma = unsafe { &(*stm32f1xx_hal::pac::DMA1::ptr()) };

        // Check if transfer complete interrupt flag is set
        if dma.isr.read().tcif2().is_complete() {
            // Clear the interrupt flag
            dma.ifcr.write(|w| w.ctcif2().clear());

            usb::usb_print_bytes_ln(b"DMA1_CH2 interrupt fired");

            // With hardware NSS, we only get data when CS is asserted
            // Process the received data
            let rx_data = unsafe {
                if USING_BUFFER_A.load(Ordering::Relaxed) {
                    &RX_BUF_A
                } else {
                    &RX_BUF_B
                }
            };

            usb::usb_print_bytes_ln(b"SPI data received:");
            for &byte in rx_data.iter() {
                let mut hex_buf = [0u8; 4];
                hex_buf[0] = b'0';
                hex_buf[1] = b'x';
                let hi = byte >> 4;
                let lo = byte & 0x0F;
                hex_buf[2] = if hi < 10 { b'0' + hi } else { b'A' + (hi - 10) };
                hex_buf[3] = if lo < 10 { b'0' + lo } else { b'A' + (lo - 10) };
                usb::usb_print_bytes_ln(&hex_buf);
            }

            // Toggle buffer for next transfer
            USING_BUFFER_A.store(!USING_BUFFER_A.load(Ordering::Relaxed), Ordering::Relaxed);

            // Restart DMA
            let spi = unsafe { &(*stm32f1xx_hal::pac::SPI1::ptr()) };
            let dma1 = unsafe { &(*stm32f1xx_hal::pac::DMA1::ptr()) };

            unsafe {
                // Configure DMA channels
                // First disable channels
                dma1.ch2.cr.modify(|_, w| w.en().clear_bit());
                dma1.ch3.cr.modify(|_, w| w.en().clear_bit());

                // Wait for channels to be disabled
                while dma1.ch2.cr.read().en().bit_is_set() {}
                while dma1.ch3.cr.read().en().bit_is_set() {}

                // Clear all interrupt flags for these channels
                dma1.ifcr.write(|w| {
                    w.cgif2()
                        .set_bit()
                        .ctcif2()
                        .set_bit()
                        .chtif2()
                        .set_bit()
                        .cteif2()
                        .set_bit()
                        .cgif3()
                        .set_bit()
                        .ctcif3()
                        .set_bit()
                        .chtif3()
                        .set_bit()
                        .cteif3()
                        .set_bit()
                });

                // Set buffer addresses
                let (rx_buf, tx_buf) = if USING_BUFFER_A.load(Ordering::Relaxed) {
                    (
                        &mut RX_BUF_A as *mut _ as u32,
                        &mut TX_BUF_A as *mut _ as u32,
                    )
                } else {
                    (
                        &mut RX_BUF_B as *mut _ as u32,
                        &mut TX_BUF_B as *mut _ as u32,
                    )
                };

                // Configure RX DMA (Channel 2)
                dma1.ch2
                    .par
                    .write(|w| w.pa().bits(&spi.dr as *const _ as u32));
                dma1.ch2.mar.write(|w| w.ma().bits(rx_buf));
                dma1.ch2.ndtr.write(|w| w.ndt().bits(8));

                // Configure TX DMA (Channel 3)
                dma1.ch3
                    .par
                    .write(|w| w.pa().bits(&spi.dr as *const _ as u32));
                dma1.ch3.mar.write(|w| w.ma().bits(tx_buf));
                dma1.ch3.ndtr.write(|w| w.ndt().bits(8));

                // Configure RX DMA control register
                dma1.ch2.cr.write(|w| {
                    w.mem2mem()
                        .disabled()
                        .pl()
                        .high()
                        .msize()
                        .bits8()
                        .psize()
                        .bits8()
                        .minc()
                        .enabled()
                        .pinc()
                        .disabled()
                        .circ()
                        .disabled()
                        .dir()
                        .from_peripheral()
                        .teie()
                        .enabled()
                        .tcie()
                        .enabled()
                        .en()
                        .enabled()
                });

                // Configure TX DMA control register
                dma1.ch3.cr.write(|w| {
                    w.mem2mem()
                        .disabled()
                        .pl()
                        .high()
                        .msize()
                        .bits8()
                        .psize()
                        .bits8()
                        .minc()
                        .enabled()
                        .pinc()
                        .disabled()
                        .circ()
                        .disabled()
                        .dir()
                        .from_memory()
                        .teie()
                        .enabled()
                        .tcie()
                        .enabled()
                        .en()
                        .enabled()
                });

                // Enable SPI DMA requests
                spi.cr2
                    .modify(|_, w| w.rxdmaen().enabled().txdmaen().enabled());
            }
        }
    });
}

#[interrupt]
fn DMA1_CHANNEL3() {
    cortex_m::interrupt::free(|_cs| {
        // Get DMA registers
        let dma = unsafe { &(*stm32f1xx_hal::pac::DMA1::ptr()) };

        // Check if transfer complete interrupt flag is set
        if dma.isr.read().tcif3().is_complete() {
            // Clear the interrupt flag
            dma.ifcr.write(|w| w.ctcif3().clear());

            // Log interrupt
            usb::usb_print_bytes_ln(b"DMA1_CH3 interrupt fired");

            // No need to restart the transfer here - it's done in CHANNEL2 handler
        }
    });
}

And I also noticed that to enable hardware NSS (standard PA4 pin for SPI1), since other devices are also connected to the same SPI bus, I also required this unsafe block:

    unsafe {
        let spi_reg = &(*stm32f1xx_hal::pac::SPI1::ptr());
        spi_reg.cr1.modify(|_, w| w.ssm().clear_bit()); // Hardware NSS management
        spi_reg.cr2.modify(|_, w| w.ssoe().clear_bit()); // NSS input for slave mode
    }

The code above is definitely not pretty, I had AI help me a lot to get this running, since I have not that much experience with stuff that goes beyond using standard HAL functions.

Expected behaviour:

In my opinion, these unsafe calls should be minimized and mostly safely abstracted by the HAL.

[tmehr-enqt]

I assume you are using the 0.10 version? SPI got a bit of a rework with the changes for embedded-hal 1.0. I hope it's going to release soon.

Also your problem stems from not configuring the dma channels before pushing them into the SPI abstraction.

let dma_ch = device.DMA1.split(&mut rcc).1;
dma_ch.listen(stm32f1xx_hal::dma::Event::TransferComplete);

configures the interrupt on that channel without directly accessing the registers.

I think there should be a with_interrupt(Event) method that you can inline call but that is a design change.

device.DMA1.ifcr().reset();

resets the ifcr

device.DMA1.ch1().cr().reset();

the cr.

With 0.10 I think ifcr and cr are just members instead of functions but is should still work that way.