-
Notifications
You must be signed in to change notification settings - Fork 3k
Implementation of SLEEP feature for Renesas mbed boards #6501
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
Implementation of SLEEP feature for Renesas mbed boards #6501
Conversation
a682b61
to
1c92778
Compare
volatile uint8_t dummy_8; | ||
|
||
CPG.STBCR1 = 0x00; | ||
dummy_8 = CPG.STBCR1; |
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.
Why is this dummy read needed?
volatile uint8_t wk_CPGSTBCR12 = CPGSTBCR12; | ||
volatile uint8_t wk_CPGSTBCR13 = CPGSTBCR13; | ||
|
||
CPGSTBCR3 = 0xFF; |
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.
What are all of these registers doing?
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.
There is many constants/values that are not clear (@cmonr pointed few). Can you provide some description for those? Naming, code comments, git commit message helps.
can you share the tests results as part of this?
Sorry for my late reply. |
1c92778
to
d19aa20
Compare
I modified this commit code with writing out and reflected the above comments, added the description. |
volatile uint8_t dummy_8; | ||
|
||
core_util_critical_section_enter(); | ||
wk_CPGSTBCR3 = CPGSTBCR3; |
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.
what are these lines doing? Can you add a commentaas for some you did (line 130 for instance). Could be for entire block. By looking at the lines, I have no understanding of why lines 116 - 127 are doing.
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.
This is a process for temporarily saving register values for powerdown the peripheral module.
I added a comment in the code.
module_standby_out(); | ||
core_util_critical_section_exit(); | ||
|
||
(void)dummy_8; |
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.
specific meaning to do this dummy8 after waking up?
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.
It is intended to implicitly avoid warnings to unused variables and has no particular meaning.
The updated commit message 👍 I still do not understand some lines in the code that should be documented? Please review my latest comments if we could add some clarification to the implementation (what is the intention with those device specific values) |
I implemented the SLEEP feature for Rnesas mbed boards. The mainly changing is here. - hal_sleep() To satisfy the mbed specificationm, I implemented this function newly by using "sleep" that is one of low power mode that is incorporated in our hardware(RZ_A1). In the "sleep", peripheral and memory state are maintained, and the peripherals continue to work and can generate interrupts. - hal_deepsleep() To satisfy the mbed specificationm, I implemented this function newly by combined using "sleep" and "module standby" that is the low power mode that is incorporated in our hardware(RZ_A1). The "module standby" is peripheral module's powerdown. Also in case of our "module standby", it need to read register as dummy when access to each register.
d19aa20
to
d247407
Compare
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.
👍
BTW, the jenkins CI failure - there are few PR affected - github is investigating these ref failures. /morph build |
Build : FAILUREBuild number : 1677 |
/morph build |
Build : FAILUREBuild number : 1683 |
the failure are not related to this , but its on the branch. investigating |
@mprse The commit 7faa2ac217ff2d964637 breaks the build (I reverted it, and can compile). Can you please review? |
Looks like solution has been provided here PR #6492. It has been merged yesterday, so I think now should be all ok. |
/morph build |
Build : SUCCESSBuild number : 1697 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 1331 |
Test : SUCCESSBuild number : 1493 |
Description
I implemented SLEEP feature in GR-PEACH.
Pull request type
[X] Fix
[ ] Refactor
[ ] New target
[] Feature
[ ] Breaking change