NCS36510 deep sleep function fix #5396
Conversation
Removed the NOP commands and put in a FLASH ready routine to wait until the FLASH tells the system it's ready to execute code (i.e. read).
Updates to deep sleep function
|
/morph build |
|
@danclement Thanks for sending the fix. Regarding merge commit: Is merge commit necessary for this changeset? |
Build : SUCCESSBuild number : 363 Triggering tests/morph test |
Test : FAILUREBuild number : 175 |
|
I am not power user of GitHub and Fork, branch, pull request flows, so sorry for the merge commit. I made a mistake somewhere in my branch of mbed OS and I was trying to fix it before doing the pull request. Sorry if that messes things up for whatever reason. @0xc0170 |
You can still squash those 2 locally and push with force to overwrite it , or we can in this case during integration. Can you please check the test, it fails for IAR for lp ticker, seems like deepsleep test case for lp ticker time outs. |
There was a problem hiding this comment.
From the test results it looks like the problem still occurs with this fix. @danclement I don't think you'll be able to fix this problem with trial an error. This seems like it may be a more deeply rooted problem. I would recommend working with the ncs36510 design team to root cause this and ensure this problem is fixed.
|
|
||
| // After waking from interrupt, wait for flash banks to be powered up and ready | ||
| // TODO: Later need to make sure this still works well for a dual stack or dual FLASH application | ||
| while ((FLASHREG->STATUS.BITS.FLASH_A_BUSY == 1)); |
There was a problem hiding this comment.
Does flash actually become busy after deep sleep WFI, or is this only when erasing or programming flash? A quick test you could try is adding a count to the loop:
while ((FLASHREG->STATUS.BITS.FLASH_A_BUSY == 1)) {
busy_count++;
}
There was a problem hiding this comment.
Russ, Martin,
I will have to look into it deeper apparently. I will add it to our backlog but it may take some time to get back into it.
Should I close this pull request and make a new one when the issue is solved or is it better to keep this one?
There was a problem hiding this comment.
@c1728p9 Russ,
I looked at this again yesterday and i think adding this while loop is needed and I think it needs to be in front of the oscillator code.
The ready signal is absolutely a part of the power up sequence, it's clearly documented in our FLASH design spec.
I moved the code to right after the WFI instruction and the test passes GCC and IAR. I tried to test using ARM but for some reason the path is not able to resolve itself.
Russ, you mentioned you did not think having the wait for ready code right after WFI is appropriate. I don't understand that. WFI is wait for interrupt so I assume the chip is in deep sleep at that point. As soon as the interrupt comes, the internal 32MHz oscillator will power up and the system will try to run code. If the FLASH isn't actually ready then I think this may be an issue and could cause a crash.
The code where it's waiting for the oscillator is only to switch the system over to the accurate crystal based 32MHz clock.
So I would like to move the wait for FLASH ready to right after the WFI and resubmit this pull request. If you don't agree let me know and we can discuss.
There was a problem hiding this comment.
I moved the code to right after the WFI instruction and the test passes GCC and IAR. I tried to test using ARM but for some reason the path is not able to resolve itself.
So I would like to move the wait for FLASH ready to right after the WFI and resubmit this pull request. If you don't agree let me know and we can discuss.
I would assume this is the proper place - immediately after WFI call completes.
ARM then still failing tests with this patch . You can rebase this PR to accomodate your proposed changes, however what is with ARMCC?
The ready signal is absolutely a part of the power up sequence, it's clearly documented in our FLASH design spec.
It is always good to provide references that a reader can verify (even in the commit msg, so we can look it up via a reference there or add a brief description from the reference manual there).
There was a problem hiding this comment.
The ready signal is absolutely a part of the power up sequence, it's clearly documented in our FLASH design spec.
@danclement so the ncs36510 returns from WFI before the flash isn't ready? Why does the program not crash immediately if it is executing from flash before flash is ready? Does reading from flash before it is ready return a dummy value without triggering a hardfault? If so is that dummy value random or a fixed value such as 0xff or 0x00? Also, how long does it take for flash to become ready?
|
Russ,
Maybe I don’t understand what the WFI instruction does. From reading the ARM documentation it is an instruction that tells the system to wait for an interrupt. So I had assumed that this was the last thing that happened after going into the deep sleep condition.
I thought that’s why putting the loop to wait for flash ready made sense right after that instruction.
I’m not an ARM MCU expert so maybe I’m oversimplifying things.
I think that maybe we should get on a call with Mac and discuss this to make sure I don’t miss something important here. I appreciate you pushing back as there must be something going on I don’t fully understand yet.
Thanks,
Best Regards,
Dan Clement
|
Build : SUCCESSBuild number : 483 Triggering tests/morph test |
Test : FAILUREBuild number : 303 |
Exporter Build : ABORTEDBuild number : 101 |
|
Closing due to inactivity. Feel free to reopen if more progress is made. |
|
@danclement - any update on this one? |
Description
This pull request is to fix an issues flagged in #5065.
Status
READY
Fully tested by mbed CLI tests.
@maclobdell
@0xc0170
@0x6d61726b