Skip to content
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

STM32L476 Update rtc_api.c #1523

Closed
wants to merge 2 commits into from
Closed

STM32L476 Update rtc_api.c #1523

wants to merge 2 commits into from

Conversation

star297
Copy link
Contributor

@star297 star297 commented Feb 2, 2016

Added RTC ISR check code for Mbed rtc_isenabled function.
Removed rtc_inited parts.

Tested on Nucleo-L476 and Disco-L476 boards with and without vBat pin powered.
Also fully tested on prototype hardware.
Targets no longer resets RTC time registers on POR or nRST including sleep modes.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 3, 2016

I thought we eliminated the static variable to keep the state of RTC. Thanks @star297 for bringing this up, and sending this patch

@bcostm @adustm Please review

@adustm
Copy link
Member

adustm commented Feb 4, 2016

In the case of a nRST
If application does
rtc_init(); -> RTC will start
If application does
if (!rtc_isenabled()) rtc_init(); ->The RTC will not start

The RTC will not start, because RTC power domain will not be switched on and LSE/LSI clock will not be linked to RTC clock.
Refer to issue #1480 that we fixed earlier.

As this rtc_isenabled function is a public function, we cannot assume any way of using it from users.

The SW variable shall be kept and PR #1523 should not be used.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 4, 2016

@star297 Please provide a code example how you tested the code. To answer the comment above

As this rtc_isenabled function is a public function, we cannot assume any way of using it from users.

Not certain what does this mean?

The RTC will not start, because RTC power domain will not be switched on and LSE/LSI clock will not be linked to RTC clock.

Is this related to if (!rtc_isenabled()) rtc_init(); ? This patch is using RTC peripheral to check if it was enabled. Does this mean, that if RTC is not, fetching RTC->ISR register might lead to a fault? You could overcome this by setting RTC clock in rtc_isenabled, to be able to read RTC registers , to get a status if RTC is configured.

@star297
Copy link
Contributor Author

star297 commented Feb 4, 2016

The int rtc_isenabled(void) function is common across all Mbed platforms to test if the RTC has been initialised and subsequently running.

This function MUST be utilised on Mbed by ALL platform vendors to enable user code compatibility across each of the platforms.

Mbed's rtc_time.c function checks #if DEVICE_RTC, if true the RTC can be initialised, first by testing the int rtc_isenabled(void) function in the platform's unique RTC api.
When a RTC function call is made, in the case of STM targets, the ISR register is checked to determine the state of the RTC. Because each platform's RTC is different the rtc_isenabled test must be done in the platform's RTC api.

Reading the ISR register at any time has never resulted in an error. If no RTC function call has been made the register always reads decimal 7, its initial power on state. Either the RTC power domain is enabled at start up or it can be read independent of this.

If the RTC has not been initialised rtc_init() is called from the Mbed rtc_time.c that will configure the RTC power domain and clocks at the same time resetting the Backup Domain and setting set_time(0);.

If the ISR register indicates the RTC is running, rtc_init() is not called as the RTC is already set up and running and will not reset Backup Domain and set_time(0); this will leave the RTC registers in tact and will remain running.

The rtc_inited parts are not required here as the rtc_isenabled function takes control. It only causes the RTC to reset on nRST or POR which is not what we want.

As this rtc_isenabled function is a public function, we cannot assume any way of using it from users.

I do not understand what you are saying, this function forms part of the Mbed system set up that is pretty much automated so users would not be concerned with it. A user can manually call rtc_init() if required. If that happens the rtc_isenabled function will still be valid.

Whilst I have been faultlessly using a form of this working example below for some time, I may still be missing some important considerations.

Simple test code here:
https://developer.mbed.org/users/star297/code/RTC-test/

Working MBED-DEV here:
https://developer.mbed.org/users/star297/code/mbed-dev/

@dbestm
Copy link
Contributor

dbestm commented Feb 9, 2016

The RTC init flag is used in the rtc_init() function before the init of the RTC but after the init of the clock which is needed to read the time and the date. So the RTC is not reset after a nRST.
If you want to remove the rtc_inited parts, you have to move the setting of the clock inside the rtc_isenabled() function.
Let us know what you prefer?
either use rtc_inited as it is done
or set the clock + use RTC init flag in rtc_enabled()
I remind you if the clocks are not set, the RTC can run but the time and date registers will not be updated.

@star297
Copy link
Contributor Author

star297 commented Feb 9, 2016

Further research results in the following quote from page 204 of the RM0351 manual:

The LSE clock is in the Backup domain, whereas the HSE and LSI clocks are not.
Consequently:
• If LSE is selected as RTC clock:
– The RTC continues to work even if the VDD supply is switched off, provided the
VBAT supply is maintained.

Therefore once rtc_init() has run the LSE clock setting remains working which explains why it does work under reset conditions, However;

• If LSI is selected as the RTC clock:
– The RTC state is not guaranteed if the VDD supply is powered off.
• If the HSE clock divided by a prescaler is used as the RTC clock:
– The RTC state is not guaranteed if the VDD supply is powered off or if the internal
voltage regulator is powered off (removing power from the VCORE domain)

This explains why my suggested method does work in LSE mode both in run and deep sleep modes.
The test code above does prove this.

I think we need to make a decision here so as not to overcomplicate this.
Both Nucleo and Disco L476 boards have the LSE 32KHz crystal fitted as standard from the factory.
We can't cater for every eventuality, I would suggest that we assume the RTC will be clocked with LSE by default and go with this. It also fits in with standard Mbed set up's across most other platforms that operate in a similar manor.

Other RTC clocking scenarios will be sub standard and detract from true RTC functionality.
This will also lead to start up delays that are particularly undesirable when waking from sleep modes.
As the second quote indicates the RTC state cannot be guaranteed under these circumstances which renders the RTC unusable from a practical point of view.
Certainly it is possible to test in the rtc_isenabled() part for these different eventualities, but is it worth it? what we do not want to do is run the clock set up code if it is already running.

Your platform, your decision.

Corrected Teensy3_1 OS_CLOCK to 96MHz.
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 15, 2016

@star297 The commit f485a47 should probably not be here.

@star297
Copy link
Contributor Author

star297 commented Feb 15, 2016

No it shouldn't, not sure why that happened.

@star297
Copy link
Contributor Author

star297 commented Feb 15, 2016

See PR #1548, I have applied those changes to this L476 target and has fixed the problem.
If the same changes are made for the L476, you can close this issue.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 18, 2016

#1551 should fix this. Please have a look

@0xc0170 0xc0170 closed this Feb 18, 2016
deepakvenugopal added a commit to deepakvenugopal/mbed-os that referenced this pull request Feb 9, 2018
…changes from c9bf20f..43c7ec2

43c7ec2 Merge branch 'release_internal' into release_external
ed76459 Merge pull request ARMmbed#1558 from ARMmbed/IOTTHD-2195
8d3bcb7 Add new function to unit tests
56f66a4 Review correction
8b7d018 Move indirect queue size public API to net_interface
d877c9e Review corrections
c25e476 Remove Eclipse project files for external release
d51f442 Merge branch 'release_internal' into release_external
135c48d Increase Thread SED buffer size for big packets
70931a7 Fix indirect queue packet ordering
10e51a4 API for changing Thread SED parent buffer size
6122d24 dereference null value issue fixed. (ARMmbed#1557)
d1378dc Clear IPv6 neighbor cache in partition change (ARMmbed#1554)
7610e91 Child neighbor entry updates (ARMmbed#1550)
c727295 cleared neighbours with child address that are not ours (ARMmbed#1549)
80b4d72 Thread partition merge mode TLV change (ARMmbed#1546)
edd7599 RLOC was updated before clearing child info (ARMmbed#1547)
a666056 router short address set to 0xfffe for non routers (ARMmbed#1543)
759ab05 delete route set and link set entries for a router ID (ARMmbed#1540)
23a1265 REED advertisement handling (Thread spec 5.16.3): (ARMmbed#1535)
0a32cb4 added active and pending timestamps to child update response (ARMmbed#1533)
d0eec80 Fix error case memory leak (ARMmbed#1537)
da9860f Pending set after link sync (ARMmbed#1526)
ffa1569 Thread router network data update after link sync (ARMmbed#1530)
3b46d8d Fix defects found by coverity (ARMmbed#1529)
3a57101 Fix compiler warnings and update traces (ARMmbed#1523)
c288227 Add extension check for partition weight drop in parent selection (ARMmbed#1521)
d8dea28 network data cleared after router forms new partition (ARMmbed#1525)
44a85e5 removed router flagging for thread_management_server file (ARMmbed#1524)
1cbced9 Merge pull request ARMmbed#1520 from ARMmbed/IOTTHD-2105_2
3d07365 Review corrections to network data clearing
22a0375 Clear network data from lost children
ffd8517 added a new thread management function (ARMmbed#1519)
80af9cb Thread BR network data clearing (ARMmbed#1518)
5a6f6b5 thread nvm valgrind uninitialized data fix (ARMmbed#1517)
d5e2198 Add API for partition weighting set (ARMmbed#1513)
8811d6f multicast forwarding scope changed and address registration updated. (ARMmbed#1516)
c277384 printf to tr_info (ARMmbed#1515)
bb21264 Thread combined nvm test (ARMmbed#1507)
76f7725 Primary BBR fixes from interop (ARMmbed#1512)
12ed5ab FHSS unit test: fixed fhss mac interface test (cherry picked from commit 805eb42e4416b00cc018dc32dceb353d0b6c8bb6)
dd21ea9 Remvoed unnecessary trace print's.
cb6e78b FHSS unit test: fixed fhss beacon tasklet test (cherry picked from commit abe6d671b058f4f069741eab24d51e4d62d550b0)
237b3d4 Fhss info print (ARMmbed#1486)
0f39a47 FHSS: Do not update synch monitor right after superframe change (cherry picked from commit 99d50ad9d7f8dad80f10c2a4303f4e75ab31a3c2)
c9a098f Fixed Timeoout force which actually never generate timeout.
957c7fb Pana server and client update:
ae230e5 FHSS: Update Beacon synch info in critical state
84bd8a4 FHSS: Synchronization must be done in critical state
fb1b163 Pan coordinator blacklist update
39fe6ba Added missing HAVE_RPL compiler flag
16a1bc5 MLE bootsrap and message timeout update
eeb2d39 enable BBR to support multicast registration in non   commercial networks (ARMmbed#1509)
4ea2bf8 uri modified. (ARMmbed#1510)
f443853 timeout corrected for neighbour entry (ARMmbed#1508)
ea93c1f Thread dev conf taken use (ARMmbed#1503)
5d5b239 bug fix in bbr start (ARMmbed#1505)
8dbd521 commented a trace. (ARMmbed#1504)
145dbdf device conf copy fix (ARMmbed#1502)
f60268f eid&random mac moved to device conf struct (ARMmbed#1497)
df18635 Let MAC choose address when mesh forwarding
42f916b fixed BBR stop to remove network data and routing information (ARMmbed#1500)
e058c2a pbbr changes (ARMmbed#1499)
1ece307 Merge pull request ARMmbed#1485 from ARMmbed/merge_release_back
dda8164 thread address handling updated. (ARMmbed#1496)
1dc21a1 thread extension fixes. (ARMmbed#1495)
543fe98 Merge branch 'release_internal'
bade70e Dua req changes (ARMmbed#1494)
1979df8 added status to MLR response and implemeted BMLR.ntf multicast. (ARMmbed#1492)
1807c01 mle class initialisation (ARMmbed#1488)
d809831 Merge pull request ARMmbed#1479 from ARMmbed/merge_release_to_master
bce812d Update license to Thread test file (ARMmbed#1483)
aaa4b1f Revert eclipse file removal

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack
git-subtree-split: 43c7ec2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants