-
Notifications
You must be signed in to change notification settings - Fork 221
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
Examples: Added proper sleep mode control register config to uno-timer.rs #356
base: main
Are you sure you want to change the base?
Conversation
…r.rs example Previously in the example the sleep mode control register was never configured for sleep mode. To do this, the sleep enable (SE) bit has to be set active. Due to the examples layout, it was not apparent that the controller didn't go to sleep, but it is easily proven by putting any kind of visible functionality inside the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uff, thanks for catching this!
In the datasheet, I read
To avoid the MCU entering the sleep mode unless it is the programmer’s purpose, it is recommended to write the Sleep Enable (SE) bit to one just before the execution of the SLEEP instruction and to clear it immediately after waking up.
so maybe the register write should rather be next to the actual sleep instruction? It doesn't make a difference here, but we must expect that people just copy-paste the examples, so doing the "right thing" here is a good idea.
Going even one step further: Maybe avr-hal
should provide a small helper which does the right sequence. So something like
pub fn enter_sleep(smcr: &pac::cpu::SMCR) {
smcr.modify(|_, w| w.se().set_bit())
avr_device::asm::sleep();
smcr.modify(|_, w| w.se().clear_bit())
}
What do you think?
Sleep functionality being quite an important part of AVR programming, I really like the idea of having a helper function for it. Having a bit of friendly abstraction for the different sleep modes wouldn't hurt either in the future :) |
Do you want to take a stab at adding the helper function or would you prefer me doing it? Implementation wise, I think it needs to go into the |
I'd be happy to try and implement the changes myself. I'll start by adding the "go to sleep" -helper to the crates mentioned by you. |
I'm also interested in this. I have a working example with asm macro for using power-down with sleep and waking up via wdt interrupt. I tried to rewrite my asm code with your example but it didn't work. You could find the whole example here: https://github.com/mayms/stuff/blob/main/nano-power-down/src/main.rs Working asm code:
Not working adaption:
Any idea what I missed? |
@mayms, the equivalent of your avr_device::interrupt::free(|_| {
avr_device::asm::wdr();
dp.WDT.wdtcsr.write(|w| w.wdce().set_bit().wde().set_bit());
dp.WDT.wdtcsr.write(|w| {
w.wdce()
.clear_bit()
.wdie()
.set_bit()
.wde()
.clear_bit()
.wdph()
.set_bit()
});
dp.CPU.smcr.write(|w| w.sm().bits(0b010).se().set_bit());
});
let wdtcsr = dp.WDT.wdtcsr.read().bits();
let smcr = dp.CPU.smcr.read().bits(); The generated assembly is slightly different, it looks like this: in r17, 0x3f ; 63
cli
wdr
ldi r24, 0x18 ; 24
sts 0x0060, r24 ; 0x800060 <__TEXT_REGION_LENGTH__+0x7f8060>
ldi r24, 0x60 ; 96
sts 0x0060, r24 ; 0x800060 <__TEXT_REGION_LENGTH__+0x7f8060>
ldi r24, 0x02 ; 2
call 0x12e ; 0x12e <<T as core::convert::From<T>>::from::heefc31865f28f97a>
add r24, r24
andi r24, 0x0E ; 14
ori r24, 0x01 ; 1
out 0x33, r24 ; 51
out 0x3f, r17 ; 63 I didn't check, but hopefully this works better? A few notes about what is different:
|
@Rahix, thank you for taking time to solve my mistake and for giving me such a detailed response. Your solution works fine. Minimal code footprint is not a requirement for now. Next thing on my list is to deactivate BOD to bring consumption further down. What do you think about lib support. Maybe I can help there. |
Previously in the example the sleep mode control register was never configured for sleep mode.
To do this, the sleep enable (SE) bit has to be set active. Due to the examples layout, it was not apparent that the controller didn't go to sleep, but it is easily proven by putting any kind of visible functionality inside the loop.