stm32-rs / stm32f1xx-hal

A Rust embedded-hal HAL impl for the STM32F1 family based on japarics stm32f103xx-hal
Apache License 2.0
570 stars 179 forks source link

GPIO pin PB5 and PB4 does not work correctly #77

Closed MauroMombelli closed 5 years ago

MauroMombelli commented 5 years ago

Hi,

after managing to run the demo blink on led PC13, I wanted to also blink led on PB3, PB4 and PB5.

PC13 and PB5 works fine, PB3 and PB4 don't. I am aware they may be used for some debug stuff but I set debug to false to Cargo.toml. Also if pin would be in use by something else, i would expect compilation error

full code:

// std and main are not available for bare metal software
#![deny(unsafe_code)]  //  Don't allow unsafe code in this file.
//#![deny(warnings)] //  If the Rust compiler generates a warning, stop the compilation with an error.
#![no_std]

#![no_main] 

use nb::block;

use stm32f1xx_hal::{

    prelude::*,

    pac,

    timer::Timer,

};

#[cortex_m_rt::entry]

fn main() -> ! {

    // Get access to the core peripherals from the cortex-m crate

    let cp = cortex_m::Peripherals::take().unwrap();

    // Get access to the device specific peripherals from the peripheral access crate

    let dp = pac::Peripherals::take().unwrap();

    // Take ownership over the raw flash and rcc devices and convert them into the corresponding

    // HAL structs

    let mut flash = dp.FLASH.constrain();

    let mut rcc = dp.RCC.constrain();

    // Freeze the configuration of all the clocks in the system and store

    // the frozen frequencies in `clocks`

    let clocks = rcc.cfgr.freeze(&mut flash.acr);

    // Acquire the GPIOC peripheral

    let mut gpiob = dp.GPIOB.split(&mut rcc.apb2);
    let mut gpioc = dp.GPIOC.split(&mut rcc.apb2);

    // Configure gpio C pin 13 as a push-pull output. The `crh` register is passed to the function

    // in order to configure the port. For pins 0-7, crl should be passed instead.

    let mut led = gpioc.pc13.into_push_pull_output(&mut gpioc.crh);
    let mut led1 = gpiob.pb3.into_push_pull_output(&mut gpiob.crl);
    let mut led2 = gpiob.pb4.into_push_pull_output(&mut gpiob.crl);
    let mut led3 = gpiob.pb5.into_push_pull_output(&mut gpiob.crl);
    led1.set_high();
    led2.set_high();
    led3.set_high();
    // Configure the syst timer to trigger an update every second

    let mut timer = Timer::syst(cp.SYST, 1.hz(), clocks);

    // Wait for the timer to trigger an update and change the state of the LED

    loop {

        block!(timer.wait()).unwrap();

        led.set_high();
        led1.set_high();
        led2.set_high();
        led3.set_high();

        block!(timer.wait()).unwrap();

        led.set_low();
        led1.set_low();
        led2.set_low();
        led3.set_low();
    }

}

#[panic_handler]
fn my_panic(_info: &core::panic::PanicInfo) -> ! {
    loop {}
}

cargo.toml

[package]
name = "afro32"
version = "0.1.0"
authors = ["lesto <mombelli.mauro@gmail.com>"]
edition = "2018"

[profile.release]
# optimize for size ('z' would optimize even more)
opt-level = 's'
# link with link time optimization (lto).
lto = true
# enable debugging in release mode.
debug = false

[dependencies]
cortex-m = "0.6.0"
nb = "0.1.2"
cortex-m-rt = "0.6.8"
stm32f1 = "0.7.1"

[dependencies.stm32f1xx-hal]
version = "0.3.0"
features = ["stm32f100", "rt"]
geomatsi commented 5 years ago

Hi @MauroMombelli , Well, by default PB3 and PB4 pins are mapped to TDO/TRACESWO and NJTRST functions respectively. Tweaking debug option in Cargo.toml does not remap them into GPIO. So you need to do the following before switching those pins to push/pull GPIO output:

    // Configure TDO/TRACESWO as PB3 and NJTRST as PB4
    afio.mapr
        .mapr()
        .modify(|_, w| unsafe { w.swj_cfg().bits(0b010) });

You may refer to section 9.3.5 in Reference manual for stm32f10x MCU for more details.

By the way, alternatively, if you don't need JTAG/SWD at all in runtime, you may use the following call:

afio.mapr.disable_jtag();

Regards, Sergey

MauroMombelli commented 5 years ago

The reason why I disabled the debug in the toml. Problem is PB4 start high, and this is not good on my board; I need to disable this debug at startup.

Also this debug stuff should be optional, normally disabled, and SWD can work fine without.

So my suggestion is, if you really want to keep this enabled by default if not a big warning, at least put a mention on the docs :/

On Mon, Jul 8, 2019, 10:27 Sergey Matyukevich notifications@github.com wrote:

Hi @MauroMombelli https://github.com/MauroMombelli , Well, by default PB3 and PB4 pins are mapped to TDO/TRACESWO and NJTRST functions respectively. Tweaking debug option in Cargo.toml does not remap them into GPIO. So you need to do the following before switching those pins to push/pull GPIO output:

// Configure TDO/TRACESWO as PB3 and NJTRST as PB4
afio.mapr
    .mapr()
    .modify(|_, w| unsafe { w.swj_cfg().bits(0b010) });

You may refer to section 9.3.5 in Reference manual for stm32f10x MCU https://www.st.com/content/ccc/resource/technical/document/reference_manual/59/b9/ba/7f/11/af/43/d5/CD00171190.pdf/files/CD00171190.pdf/jcr:content/translations/en.CD00171190.pdf for more details.

By the way, alternatively, if you don't need JTAG/SWD at all in runtime, you may use the following call:

afio.mapr.disable_jtag();

Regards, Sergey

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stm32-rs/stm32f1xx-hal/issues/77?email_source=notifications&email_token=ABQTZUOPCH72VMFEMZ3NAG3P6L27VA5CNFSM4H6WZKG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZMLMLQ#issuecomment-509130286, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQTZUJELXGKNQRTULND27TP6L27VANCNFSM4H6WZKGQ .

geomatsi commented 5 years ago

Hmm... I don't think I understand you. This is not about Rust or Cargo, it is about hardware properties. By default those pins are mapped to JTAG/SWD functions on MCU startup. You are free to remap them in your code and keep using PB3 and PB4 as normal GPIOs. And the simplest way to do it is to use disable_jtag() method mentioned above. As for debug option in Cargo.toml, it does not control hardware (e.g. pin mapping), it controls availability of debug info in resulting binary.

MauroMombelli commented 5 years ago

By default those pins are mapped to JTAG/SWD functions on MCU startup

oh, you got me curious and indeed they are debug on reset, so somehow i got lucky to not have problem with bare CMSIS (will have to take a look into it, maybe is a compilation setting), as I am porting known working C code I assumed by default only SWD was enabled (so PB3, PB4 and PA15 released, and only PA13 and PA14 used by SWD)

afio.mapr.disable_jtag();

this is confusing, will disable both jtag and swd or jtag only? (see table below for different possible setup, RM0008 Rev 20 page 177/1134)

image

TeXitoi commented 5 years ago

https://github.com/stm32-rs/stm32f1xx-hal/blob/0189db9f0929870eae01bb4faabc53d419f9c075/src/afio.rs#L60

010 in your table, so only jtag

MauroMombelli commented 5 years ago

thanks, I just took a more direct approach and tested directly, and can confirm it works, closing.

Just as quick info, this mean I could use some pin as usart and while in use, also use them as gpio or other, right? the library is not preventing this kind of error? or is just a weird edge case that need a bugfix?

therealprof commented 5 years ago

Huh? No, it should not let you do that.

MauroMombelli commented 5 years ago

then the code in the first post should have return compilation error as I use those pin as GPIO but they should have been "consumed" by the jtag, no?

TeXitoi commented 5 years ago

This library doesn't handle correctly the starting state of the debugger pins. That's a tricky fix.

Rust ownership system will not allow you to use a pin as UART and GPIO at the same time, it will just not compile, except if you cheet with unsafe.

therealprof commented 5 years ago

Well, it might be possible to work around that by utilising a special state which the PINs are in by default and to get them into one of the regular states you'd have to call a special function. If someone wants to do a PR to implement that it would be welcome.

MauroMombelli commented 5 years ago

thanks guys, this is very interesting and while I would participate a lot in the discussion, right now I don't even know what this code is actually doing, so maybe in a couple of years :)