Skip to content

NCS36510 deep sleep function fix #5396

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

NCS36510 deep sleep function fix #5396

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5a038e
Updates to deep sleep function
danclement Oct 27, 2017
7f7c63f
Merge pull request #1 from danclement/danclement-deepsleepfix
danclement Oct 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions targets/TARGET_ONSEMI/TARGET_NCS36510/sleep.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
* $Rev: 0.1 $
* $Date: 01-21-2016 $
******************************************************************************
* Copyright 2016 Semiconductor Components Industries LLC (d/b/a �ON Semiconductor�).
* Copyright 2016 Semiconductor Components Industries LLC (d/b/a “ON Semiconductor”).
* All rights reserved. This software and/or documentation is licensed by ON Semiconductor
* under limited terms and conditions. The terms and conditions pertaining to the software
* and/or documentation are available at http://www.onsemi.com/site/pdf/ONSEMI_T&C.pdf
* (�ON Semiconductor Standard Terms and Conditions of Sale, Section 8 Software�) and
* (“ON Semiconductor Standard Terms and Conditions of Sale, Section 8 Software”) and
* if applicable the software license agreement. Do not use this software and/or
* documentation unless you have carefully read and you agree to the limited terms and
* conditions. By using this software and/or documentation, you agree to the limited
Expand Down Expand Up @@ -57,21 +57,23 @@ void fncs36510_sleep(void)

void fncs36510_deepsleep(void)
{
/** Set SLEEPDEEP (SCR) and unset COMA to select deep sleep mode */
/** Set SLEEPDEEP (SCR) and unset COMA to select deep sleep mode */
SCB->SCR |= SCB_SCR_SLEEPDEEP_Msk;
PMUREG->CONTROL.BITS.ENCOMA = DISABLE;

/** Enter into deep sleep mode */
__ISB();
__WFI();
__NOP();
__NOP();

/** Wait for the external 32MHz to be power-ed up & running
* Re-power down the 32MHz internal osc
*/
while (!CLOCKREG->CSR.BITS.XTAL32M);
PMUREG->CONTROL.BITS.INT32M = 1;

// 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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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++;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@danclement danclement Oct 31, 2017 •
edited by adbridge
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

@0xc0170 0xc0170 Nov 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

}

void fncs36510_coma(void)
Expand Down