BUG - Not working in release mode #180
Conversation
|
Removing any one of the three statements
and there is no crash |
|
Same thing when based on main Also, same thing when based on main and using rustc 1.84 |
|
Some optimization bug maybe? I don't know. Try Or some issue with What happen if |
|
I just ran it resulting in the following output: I'm on Rust (I have the exact same nucleo)
Edit, same behavior as you in release mode :( |
|
Thanks. Same problem with - pin2.is_high().unwrap(); // <--- Booom
+ pin2.is_high().ok(); // <--- Booomalso same thing with |
Have you tried in release mode? For me this works fine in non-release mode |
|
Yeah my bad, I'm going to try to compare the ASM but that might be a futile endeavor. |
No worries. That would be awesome! I'll try to replace some of the hal calls with raw pointer and see if I can see any changes. This feels a bit scary |
|
Just found that this works but not this let value = afrh_ptr.read_volatile();
- defmt::println!("{}", value);
afrh_ptr.write_volatile(value); |
|
And then again this works - let value = afrh_ptr.read_volatile();
- afrh_ptr.write_volatile(value);
+ let _value = afrh_ptr.read_volatile(); // <- dont use read value(which sould be 0)
+ afrh_ptr.write_volatile(0); // Write 0 which according to RM0440 is the reset value(so no change in theory) |
|
Add peripheral reset. Line 260 in 116e6d6 |
Hm, this does not work for me. Same issue. |
|
Further trimmed down, still the problem #[entry]
fn main() -> ! {
unsafe {
(*stm32::RCC::ptr())
.ahb2enr()
.modify(|_, w| w.gpioaen().set_bit());
let afrh_ptr = 0x4800_0024 as *mut u32;
let value = afrh_ptr.read_volatile();
afrh_ptr.write_volatile(value);
defmt::println!("hej");
(0x4800_0010 as *const u32).read_volatile(); // IDR input data register
}
loop {}
}This works for me #[entry]
fn main() -> ! {
unsafe {
(*stm32::RCC::ptr())
.ahb2enr()
.modify(|_, w| w.gpioaen().set_bit());
let afrh_ptr = 0x4800_0024 as *mut u32;
let value = afrh_ptr.read_volatile();
afrh_ptr.write_volatile(0); // <------------------------------------- This is changed to 0
defmt::println!("hej");
(0x4800_0010 as *const u32).read_volatile();
}
loop {}
} |
|
using cargo asm: then running with |
|
stm32g4xx-hal/src/rcc/enable.rs Lines 35 to 42 in 116e6d6 Set bit and then reset. |
|
I can confirm the fault occurs upon writing to afrh: |
|
Wait... SWDIO and SWDCLK are AF0. Are you somehow accidentally changing that when you write a non-zero value to afrh? |
|
Ah, yes, you are. In release mode, you need to wait to read the GPIO register to allow the AHB bus to propagate the RCC change enabling GPIOA. Notice the value read is actually the Then by the time you write it to |
|
You can avoid this with a |
|
Proof: #[entry]
fn main() -> ! {
let dp = stm32::Peripherals::take().expect("cannot take peripherals");
let mut rcc = dp.RCC.constrain();
let gpioa = dp.GPIOA.split(&mut rcc);
cortex_m::asm::delay(1);
let _pin: stm32g4xx_hal::gpio::gpioa::PA8<stm32g4xx_hal::gpio::Alternate<AF6>> =
gpioa.pa8.into_alternate();
let mut pin2 = gpioa.pa9.into_pull_up_input();
defmt::println!("hej");
pin2.is_high().unwrap(); // <--- Booom
loop {}
} |
Like this #[entry]
fn main() -> ! {
unsafe {
(*stm32::RCC::ptr())
.ahb2enr()
.modify(|_, w| w.gpioaen().set_bit());// Enable
(*stm32::RCC::ptr()).ahb2rstr().modify(|_, w| w.gpioarst().set_bit());
(*stm32::RCC::ptr()).ahb2rstr().modify(|_, w| w.gpioarst().clear_bit());
let afrh_ptr = 0x4800_0024 as *mut u32;
let value = afrh_ptr.read_volatile();
afrh_ptr.write_volatile(value);
defmt::println!("hej");
(0x4800_0010 as *const u32).read_volatile();
}
loop {}
}this appears to work! |
|
Do those register writes lock the core until the AHB bus flushes or do they happen to take at least one clock cycle? |
|
With ed2504b the original example seems to work again! |
|
From the RM
|
|
Huh, I always thought it was one cycle, I guess I've been getting lucky with 1 cycle delay. |
|
I can not find any information on the need for the rcc reset after the enable. Is the reset needed for the reset itself or for the delay it happens to provide? |
|
I'm not sure if you saw what I did here:
This is what I do in proto-hal and it works fine, I suspect the reset is simply a way to do the delay. But maybe I don't know something. |
|
@AdinAck Sorry did not see #180 (comment) interesting. Thanks a lot for the help both of you @AdinAck and @burrbull ! |
I've opened G4 RM and now I'm not sure. Looks like it is different from how F4 works. |
|
Just some more evidence. I promised an ASM analysis, so here you go: It was pure coincidence that commenting out some of the Erroneous ASM (from original code in this PR): movw r0, #4172 ; r0 is becoming the address of rcc::ahb2enr
movs r4, #4 ; r4 is becoming the address of gpioa::afrh
movt r0, #16386 ; (16386 << 16) | 4172 = 0x4002_104c (rcc base address is 0x4002_1000 + ahb2enr offset of 0x4c, gpioaen is bit 0)
movt r4, #18432 ; (18432 << 16) | 4 = 0x4800_0004 (gpioa base address is 0x4800_0000 i have no idea why it added 4)
ldr r1, [r0] ; read rcc::ahb2enr
movs r2, #2
orr r1, r1, #1 ; set gpioaen bit
str r1, [r0] ; write rcc::ahb2enr
ldr r0, [r4, #32] ; read gpioa::afrh (one instruction later!) (also this offset of 32 ends up at the correct address for gpioa::afrh)Functional ASM (original code with println commented out): movw r0, #4172
movs r2, #6
movt r0, #16386
movs r3, #2
ldr r1, [r0]
orr r1, r1, #1
str r1, [r0] ; write rcc::ahb2enr
movs r0, #4
movt r0, #18432
ldr r1, [r0, #32] ; read gpioa::afrhNow there are two instructions in between the peripheral enable and peripheral read, sufficient time. The compiler just deferred the construction of the |
|
We know This is almost certainly not a deterministic compiler feature, and just so happens to be how it behaves currently. But is still interesting nonetheless. It would be cool if the compiler was able to reason about the number of cycles between operations, but I imagine pipelining and caching and such would make this infeasible. |
Rcc enable *and* reset gpio in split method. Calling `reset` as well ensures that enough time has passed between `enable` and the first register access to the peripheral. Failing to do this might lead to very hard to debug issues, see #180.
This short example does not work in release mode:
Tested on a Nucleo-G474RE