cpu/samd21: implement low power modes#2309
Conversation
cpu/samd21/lpm_arch.c
Outdated
There was a problem hiding this comment.
My mistake sorry! Should be
current_mode = LPM_OFF;
|
Just tried with the diff below and couldn't see any difference on our energy consumption test equipment. diff --git a/core/kernel_init.c b/core/kernel_init.c
index a57f119..c18b6e6 100644
--- a/core/kernel_init.c
+++ b/core/kernel_init.c
@@ -64,8 +64,8 @@ static void *idle_thread(void *arg)
lpm_set(LPM_IDLE);
}
else {
- lpm_set(LPM_IDLE);
- /* lpm_set(LPM_SLEEP); */
+ //lpm_set(LPM_IDLE);
+ lpm_set(LPM_SLEEP);
/* lpm_set(LPM_POWERDOWN); */
} |
|
I'll have access to my lab again next week. I'll give it a go then. Thanks |
|
@saurabh984 I addressed your comments. |
cpu/samd21/lpm_arch.c
Outdated
There was a problem hiding this comment.
@LudwigOrtmann [email protected] Haven't had a chance to try it yet :(
|
@LudwigOrtmann I tried to test this PR with: I couldn't awake the board after |
|
@PeterKietzmann Try commenting out the cpu_init() (shown below) code from |
|
@saurabh984, I commented this line out. With this change I could also awake from |
|
@saurabh984 did you experience the same? |
9f184dd to
45554bf
Compare
|
@PeterKietzmann, from p1048 to 1051/1138 (samr21 datasheet), you can find the power consumption of the samr21. There is also a schematic about how to use the measurement pin. |
|
Here are my results with @PeterKietzmann source code (above):
Here I've got 0,39mA but I can't wake up, Interruption doesn't work anymore. If I keep By the way, @PeterKietzmann, you should use a |
|
@bapclenet what do you mean by the SCB is not mentioned for power management? edit: I am speaking from experience with CM3/4 CPUs, I did not look up CM0 docs on this. |
|
Sorry, my bad, I didn't know that before, I've just seen it. |
|
@bapclenet your results seem somehow reasonable even if they appear to be kind of small, comparing to the datasheet (worse are mine :-) ). But honestly I didn't compare the clock setup etc. . Now the question is: is there anybody out who has a deeper knowledge with this MCU and with (low) power modes? Otherwise I need to spend some time in going into detail. |
|
@bapclenet can you explain in detail how you build your setup? Looking at the datasheet on page 1054 it feels like I can't directly access the needed pins or have to use an external power supply not from USB. Sorry for all these questions. I don't want to spend too much time in this and you already did these steps. |
|
@PeterKietzmann, I use a Multimeter from Agilent.
There are the same pins as in the datasheet. What I'm doing is removing the header (Datasheet p4 - CURRENT MEASUREMENT HEADER) and plug them to my multimeter. I think (not sure about that), whatever the source is, there is a regulator which transforms 5V to 3.3V, and after the transformation, there are those two pins where you can measure the current before going to the MCU. Does it make sens? |
|
Yes and no. Measuring the current through the "current measurement header"-pins is reasonable and the way I already did (some time ago). What I don't understand is the hint you gave me with the schematic in the datasheet (p. 1054). Anyway, if I see it right and understand correctly you just power your board by the pins and not by an USB connector and you measure the current through the measurement extension header. |
|
I think this PR will need a lot of attention so I don't think it will make it for the release. What's the status BTW? |
|
@MichelRottleuthner did you already give it a try? |
|
@PeterKietzmann Nope. It's on my todo list but I think it will take a few weeks before I have time for that. |
|
Maybe @aabadie wants to take a look ;) |
|
Hey all. Can I help get this merged in for the upcoming release? I have used it, along with a few other changes, and have a SAMR21 running at 5.2uA idle and 15uA average sampling temperature at 1Hz |
|
@immesys, you're welcome to help us merging this PR :-) |
|
So, the good news is yes, it is waking up fine from lpm off, the bad news is that the code is a little messy (as things get when you are just trying to make it work). I am busy splitting things up into independent PRs, so things like adding radio sleep to netdev, switching the xtimer source to ULP OSC32. I could really use a shepherd, as some of these changes are in core code and change default behavior (I use FLL instead of PLL saving a bit of energy, and I source it from the ULP instead of the 8mhz RC, which costs accuracy but saves energy). I am busy running experiments to quantify the benefit of each change individually so that the riot core devs can decide if each one is worth it. |
|
@kaspar030, @gebart, would you be willing to act as a shepherd? I would also volunteer but have only time in about two weeks from now. |
|
@immesys it's great to see your efforts improving the sam* support in RIOT! Would it help you to merge this PR "as is" (or with slight adoptions which you would need to propose ;-) )? |
|
I use this file as-is, so it would definitely help to merge it. The slight changes elsewhere are too invasive to just merge, I will need to discuss them with the core maintainers to work out the best way to do them. |
|
@LudwigKnuepfer would you rebase this PR once more? If not @immesys can you take it over (closing this one and opening an updated PR) |
|
I would not mind rebasing, but it didn't actually have any changes, I merge it as-is with no conflicts. In fact at the moment the idle thread (in kernel_init.c) doesn't actually put anything to sleep so merging this file shouldn't even change the behavior of existing code (which is either a feature or a problem depending on how you look at things) |
|
@immesys I do agree with you but regardless of that I'd like to see Murdock pass, so the PR should be updated. (It's still based on Travis, so kind of old :-) ) |
|
I will rebase. |
4cf9261 to
aa599bf
Compare
|
Should I also squash and mark as ready for CI? |
|
Yes please! But probably I'm just back on Monday. |
|
@LudwigKnuepfer, squash? |
aa599bf to
cabb200
Compare
|
@OlegHahm I did. |
|
Congrats for this PR! |

TODO: needs testing