stm32-rs / stm32f4xx-hal

A Rust embedded-hal HAL for all MCUs in the STM32 F4 family
BSD Zero Clause License
573 stars 213 forks source link

Bug for SPI read: wrong last bit in every byte #564

Closed qwerty19106 closed 1 year ago

qwerty19106 commented 1 year ago

I use stm32f4xx-hal=0.14.0, cortex-m-rtic = 1.1, tested on STM32F411CE and STM32F446RE.

Simple SPI initialization spawn this bug at the some SYSCLK and PCLK frequencies. For example, STM32F411CE spawn bug at SYSCLK=100.MHz(), but at SYSCLK=80.MHz() no errors.

        let spi1 = dp.SPI1.spi(
            (gpioa.pa5, gpioa.pa6, gpioa.pa7),
            hal::spi::MODE_3,
            2.MHz(),
            &clocks,
        );

Presumably it occurs due to slow GPIO speed. This code fix bit error in my case:

        let mut sck1 = gpioa.pa5.into_push_pull_output();
        let mut miso1 = gpioa.pa6.into_push_pull_output();
        let mut mosi1 = gpioa.pa7.into_push_pull_output();
        sck1.set_speed(Speed::VeryHigh);
        miso1.set_speed(Speed::VeryHigh);
        mosi1.set_speed(Speed::VeryHigh);

        let spi1 = dp.SPI1.spi(
            (sck1.into_input(), miso1.into_input(), mosi1.into_input()),
            hal::spi::MODE_3,
            2.MHz(),
            &clocks,
        );

See issue with the same error in other HAL for details.

I want create PR, but I need help. The set_speed is method of Pin, but Spi::new get Pins<SPI>. I propose add pub trait SetSpeed to add it into Pins<SPI> implementation. Are there other ways to fix this?

burrbull commented 1 year ago

Correct pin mode for SPI and other peripherals is Alternate<N>. There is 3 mode converts in your workaround code: Input -> Output -> Input -> Alternate. Last inside SPI initialization code. Should be 1. I've added possibility to pass pins to peripheral in any mode, but it was not very good idea. Possibly I'll delete it in future.

set/set_speed is implemented for alternate pins. So your code should looks like:

        let sck1 = gpioa.pa5.into_alternate().speed(Speed::VeryHigh);
        let miso1 = gpioa.pa6.into_alternate().speed(Speed::VeryHigh);
        let mosi1 = gpioa.pa7.into_alternate().speed(Speed::VeryHigh);

        let spi1 = dp.SPI1.spi(
            (sck1, miso1, mosi1),
            hal::spi::MODE_3,
            2.MHz(),
            &clocks,
        );

Anyway it would be good to add description of this potential issue in spi docs/examples.