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

SPI BIDI mode not working #520

Open
daft-panda opened this issue Aug 26, 2022 · 2 comments
Open

SPI BIDI mode not working #520

daft-panda opened this issue Aug 26, 2022 · 2 comments

Comments

@daft-panda
Copy link

daft-panda commented Aug 26, 2022

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] 👎

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] 👍

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.

@burrbull
Copy link
Contributor

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 stm32-rs/stm32f1xx-hal#181 and than retry.

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

@daft-panda
Copy link
Author

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(())
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants