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

split master/slave spi structures #609

Merged
merged 1 commit into from
Apr 26, 2023
Merged

split master/slave spi structures #609

merged 1 commit into from
Apr 26, 2023

Conversation

burrbull
Copy link
Contributor

No description provided.

@eZioPan
Copy link
Contributor

eZioPan commented Apr 21, 2023

Before this modification, I can use .into_push_pull_output_in_state() to let SCK/MISO/MOSI Pins pre-configure as Output Mode, and put them into .spi() / .spi_slave() to use as SPI pins. After SPI communication complete, I can .release() pins into pre-configure state, which avoid sending random data into slave pins.

#![no_std]
#![no_main]

use panic_rtt_target as _;

use stm32f4xx_hal::{gpio::PinState, hal, pac, prelude::*};

#[cortex_m_rt::entry]
fn main() -> ! {
    let dp = pac::Peripherals::take().unwrap();

    let rcc = dp.RCC.constrain();
    let clocks = rcc.cfgr.freeze();

    let gpioa = dp.GPIOA.split();

    // avoid random signal after release SPI
    let sck_pin = gpioa.pa5.into_push_pull_output_in_state(PinState::High);
    let miso_pin = gpioa.pa6.into_pull_down_input();
    let mosi_pin = gpioa.pa7.into_push_pull_output_in_state(PinState::Low);

    // assemble a SPI
    dp.SPI1.spi(
        (sck_pin, miso_pin, mosi_pin), // ERROR: trait bound not satisfied
        hal::spi::MODE_0,
        1.MHz(),
        &clocks,
    );
    loop {}
}

If I try do the same thing after this modification, I get some errors as such:

error[E0277]: the trait bound `stm32f4xx_hal::gpio::alt::spi1::Sck: From<stm32f4xx_hal::gpio::Pin<'A', 5, stm32f4xx_hal::gpio::Output>>` is not satisfied
   --> src\bin\spi_test.rs:24:9
    |
23  |     dp.SPI1.spi(
    |             --- required by a bound introduced by this call
24  |         (sck_pin, miso_pin, mosi_pin),
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<stm32f4xx_hal::gpio::Pin<'A', 5, stm32f4xx_hal::gpio::Output>>` is not implemented for `stm32f4xx_hal::gpio::alt::spi1::Sck`
    |
    = help: the following other types implement trait `From<T>`:
              <stm32f4xx_hal::gpio::alt::spi1::Sck as From<NoPin>>
              <stm32f4xx_hal::gpio::alt::spi1::Sck as From<stm32f4xx_hal::gpio::Pin<'A', 5, stm32f4xx_hal::gpio::Alternate<5>>>>
              <stm32f4xx_hal::gpio::alt::spi1::Sck as From<stm32f4xx_hal::gpio::Pin<'A', 5>>>
              <stm32f4xx_hal::gpio::alt::spi1::Sck as From<stm32f4xx_hal::gpio::Pin<'B', 3, stm32f4xx_hal::gpio::Alternate<5>>>>
              <stm32f4xx_hal::gpio::alt::spi1::Sck as From<stm32f4xx_hal::gpio::Pin<'B', 3>>>
    = note: required for `stm32f4xx_hal::gpio::Pin<'A', 5, stm32f4xx_hal::gpio::Output>` to implement `Into<stm32f4xx_hal::gpio::alt::spi1::Sck>`
note: required by a bound in `spi`
   --> D:\01_eZioPanDir\Github\stm32f4xx-hal\src\spi.rs:203:18
    |
203 |             impl Into<Self::Sck>,
    |                  ^^^^^^^^^^^^^^^ required by this bound in `_stm32f4xx_hal_spi_SpiExt::spi`

.spi() and .spi_slave() only accept Alternate<5> and original function, which is in Input mode. Thus after .release(), slave SPI pins could capture random data from wires connected.

@eZioPan
Copy link
Contributor

eZioPan commented Apr 21, 2023

After 2 changes I suggest above, I can confirm it can run SPI Slave Mode successfully on my STM32F411RET6.

@burrbull
Copy link
Contributor Author

This change was done before this PR.

Anyway output/input modes are not correct modes for SPI for F4 serie (unlike F1). Correct modes are Alternate.

Previously there was hack for setting pins in alternate mode internally and restore on release without changing type.
Now you need manually change mode after release peripheral with try_into():

let (sck, miso, mosi) = spi.release();
let sck: PA5<Output> = sck.try_into()?;
...

I could make possible to pass pin in any state although it is often misoptimize.
But in this case possibly better to use internal PullUp/PullDown to avoid trash on line like you already done with into_pull_down_input:

    let sck_pin = gpioa.pa5.internal_pull_up(true);
    let miso_pin = gpioa.pa6.internal_pull_down(true);
    let mosi_pin = gpioa.pa7.internal_pull_down(true);

    // assemble a SPI
    dp.SPI1.spi((sck_pin, miso_pin, mosi_pin)

@eZioPan
Copy link
Contributor

eZioPan commented Apr 21, 2023

thanks, I didn't realize it's a hacky way to store pin state inside the Alternate mode. And from now on, I will use the recomended .internal_pull_up().

@burrbull
Copy link
Contributor Author

thanks, I didn't realize it's a hacky way to store pin state inside the Alternate mode. And from now on, I will use the recomended .internal_pull_up().

you misunderstood me. I just said there has been a hack in code of hal to hide mode conversion from you.

P.S. This PR #610 allows to pass pins in output mode. Although I still don't think this is good way to use output here.

@eZioPan
Copy link
Contributor

eZioPan commented Apr 21, 2023

oh, sorry for my bad english, now I understand what you are talking about.
Btw, the reason I set pin to Output mode previously is that, after releasing the master SPI, I want SCK pin output a High state to avoid slave SPI pick up random signal.
But since now I understand it's better to use .internal_pull_up(), then I will follow the better way.

@burrbull
Copy link
Contributor Author

Rebased on #610

@burrbull burrbull marked this pull request as ready for review April 22, 2023 05:02
@burrbull burrbull force-pushed the spi_slave branch 2 times, most recently from 8d85ffc to 5e8662e Compare April 26, 2023 04:21
@burrbull
Copy link
Contributor Author

Merging this

@burrbull burrbull merged commit af887fe into master Apr 26, 2023
@bors bors bot deleted the spi_slave branch April 26, 2023 06:54
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

Successfully merging this pull request may close these issues.

2 participants