stm32f4xx-hal icon indicating copy to clipboard operation
stm32f4xx-hal copied to clipboard

SPI BIDI mode not working

Open daft-panda opened this issue 3 years ago • 2 comments

The SPI BIDI mode does not seem to function, it never reads anything other than 255 or 0.

I'm using a STEVAL-FCU001 board with a STM32F401 and trying to interface with the LPS22HD sensor which is present on SPI2 together with a LSM6DSL and LIS2MDL sensor. The SPI is 3-wire.

Using the same setup for both examples below, the one using the HAL SPI methods never works. All GPIO pins are configured correctly (PP, AF5, Very High Speed) and equally for both examples. The resulting config in the SPI2 CR1 reg is equal. Both examples attempt to read the WHO_AM_I register on the LPS22HD at 0x0F. tx is USART1.

HAL SPI example, does not work:

        let mode = Mode {
            polarity: Polarity::IdleHigh,
            phase: Phase::CaptureOnSecondTransition,
        };
        let mut spi2 = Spi::new_bidi(dp.SPI2, (spi2_clk, NoPin, spi2_sda), mode, 3.MHz(), &clocks);
        // set 3-wire SPI mode
        lps22hb_cs.set_low();
        spi2.write(&[0x10, 0x1]).unwrap();
        lps22hb_cs.set_high();

        let mut txb: [u8; 1] = [0x0F | 0x80];
        lps22hb_cs.set_low();
        let r = spi2.transfer(&mut txb).unwrap();
        lps22hb_cs.set_high();
        write!(tx, "177 == {:?}", r).unwrap();

Writes 177 == [0] :-1:

Working example, based on the C firmware sample from STM. I have included every SPI2 reg access with the exception of a NOP SPI2 SPE OFF-ON sequence.

unsafe {
            dp.SPI2.cr1.modify(|_, w| {
                w.cpha()
                    .second_edge()
                    .bidimode()
                    .bidirectional()
                    .br()
                    .div32()
                    .cpol()
                    .idle_high()
                    .mstr()
                    .master()
                    .ssm()
                    .enabled()
                    .ssi()
                    .slave_not_selected()
                    .bidioe()
                    .output_enabled()
                    .spe()
                    .enabled()
            });

            lps22hb_cs.set_low();
            let wv: [u8; 2] = [0x10, 0x1];
            // set 3-wire SPI mode
            for i in wv {
                while dp.SPI2.sr.read().txe().bit_is_clear() {}

                dp.SPI2.dr.modify(|_, w| w.dr().bits(i as u16));

                while dp.SPI2.sr.read().txe().bit_is_clear() {}
                while dp.SPI2.sr.read().bsy().bit_is_set() {}
            }
            lps22hb_cs.set_high();

            let wv: [u8; 1] = [0x0F | 0x80];
            let mut rv: [u16; 1] = [0];

            for (i, a) in wv.iter().enumerate() {
                lps22hb_cs.set_low();

                while dp.SPI2.sr.read().txe().bit_is_clear() {}

                dp.SPI2.dr.write(|w| w.bits(*a as u32));

                while dp.SPI2.sr.read().txe().bit_is_clear() {}
                while dp.SPI2.sr.read().bsy().bit_is_set() {}

                dp.SPI2.cr1.modify(|_, v| {
                    v.bidioe().clear_bit()
                });

                for _l in 0..70 {
                    dsb();
                }
                dp.SPI2.cr1.modify(|_, w| w.spe().clear_bit());

                while dp.SPI2.sr.read().rxne().bit_is_clear() {}
                rv[i] = dp.SPI2.dr.read().dr().bits();
                while dp.SPI2.sr.read().bsy().bit_is_set() {}

                lps22hb_cs.set_high();
                dp.SPI2.cr1.modify(|_, v| {
                    v.bidioe().set_bit();
                    v.spe().set_bit()
                });
            }

            write!(tx, "177 == {:?}", rv).unwrap();
        }

Writes 177 == [177] :+1:

There seem to be 2 key differences with the latter example compared to the HAL SPI example: the SPI2 interface is disabled after write before read and a comically large amount of DSB instructions are issued following the clear of the BIDIOE bit in CR1.

daft-panda avatar Aug 26 '22 18:08 daft-panda

cc @no111u3

I think we should implement optimized versions of transmission procedures like it described in RM::SPI::3.5 section and partially done for f1 in https://github.com/stm32-rs/stm32f1xx-hal/pull/181 and than retry.

About example. dsb loop looks like some kind of pause. Reading when spi is disabled looks strange for me.

burrbull avatar Aug 27 '22 04:08 burrbull

In case someone runs into this issue as well, this is a working BIDI SPI implementation conforming to embedded_hal.

pub struct BidiSPI<SPI> {
    pub spi: SPI,
}

impl<SPI> BidiSPI<SPI>
where
    SPI: Instance,
{
    pub fn new(spi: SPI) -> BidiSPI<SPI> {
        spi.cr1.modify(|_, w| {
            w.cpha()
                .second_edge()
                .bidimode()
                .bidirectional()
                .br()
                .div32()
                .cpol()
                .idle_high()
                .mstr()
                .master()
                .ssm()
                .enabled()
                .ssi()
                .slave_not_selected()
                .bidioe()
                .output_enabled()
                .spe()
                .enabled()
        });

        BidiSPI { spi }
    }

    fn read_word(&self) -> u8 {
        self.spi.cr1.modify(|_, w| w.spe().clear_bit());
        self.spi.cr1.modify(|_, v| v.bidioe().clear_bit());

        self.spi.cr1.modify(|_, w| w.spe().set_bit());
        for _l in 0..70 {
            dsb();
        }
        self.spi.cr1.modify(|_, v| v.spe().clear_bit());

        while self.spi.sr.read().rxne().bit_is_clear() {}
        let word = self.spi.dr.read().dr().bits() as u8;
        while self.spi.sr.read().bsy().bit_is_set() {}

        self.spi.cr1.modify(|_, v| v.bidioe().set_bit());
        self.spi.cr1.modify(|_, v| v.spe().set_bit());

        word
    }

    fn write_word(&self, word: u8) {
        while self.spi.sr.read().txe().bit_is_clear() {}

        self.spi.dr.modify(|_, w| w.dr().bits(word as u16));

        while self.spi.sr.read().txe().bit_is_clear() {}
        while self.spi.sr.read().bsy().bit_is_set() {}
    }
}

impl<SPI> embedded_hal::blocking::spi::Transfer<u8> for BidiSPI<SPI>
where
    SPI: Instance,
{
    type Error = stm32f4xx_hal::spi::Error;

    /// transfer n bytes from the address at words[0]. follows the STM sample code
    fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Self::Error> {
        // to support multiple byte transfer you have to toggle the CS pin between words
        // this trait does not allow to hold mutable pin references with a diferent lifetime
        assert!(
            words.len() == 1,
            "only one word transfers are supported without CS pin control"
        );
        if let Some(word) = words.iter_mut().next() {
            self.write_word(*word | 0x80);
            *word = self.read_word();
        }

        Ok(words)
    }
}

impl<SPI> embedded_hal::blocking::spi::Write<u8> for BidiSPI<SPI>
where
    SPI: Instance,
{
    type Error = stm32f4xx_hal::spi::Error;

    fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> {
        for w in words {
            self.write_word(*w);
        }
        Ok(())
    }
}

daft-panda avatar Sep 08 '22 18:09 daft-panda