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

STM32 RTC : bypass shadow registers #7365

Merged
merged 1 commit into from Jul 12, 2018

Conversation

Projects
None yet
7 participants
@jeromecoutant
Contributor

jeromecoutant commented Jun 28, 2018

Description

RTC implementation has been reworked a little in order to gain reactivity and avoid some latency.

Major impact is LPTICKER feature.

Tests are OK on my side for each STM32 family and each tool chain.

@marcemmers @bulislaw @c1728p9 @ashok-rao

Pull request type

[ ] Fix
[x] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@cmonr cmonr requested review from bulislaw and ashok-rao Jun 29, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 29, 2018

@jeromecoutant Needs rebase now. While rebasing, can you add extend the commit msg (which registers are shadow? Because some chips as I noticed do not have them as I read in the comment code - from changes not clear to me how is this actually addressing the issue
to gain reactivity and avoid some latency - how did you achieve these? ).

@0xc0170 0xc0170 added needs: work and removed needs: review labels Jun 29, 2018

@jeromecoutant jeromecoutant dismissed stale reviews from bulislaw and c1728p9 via efabe7b Jun 29, 2018

@jeromecoutant jeromecoutant force-pushed the jeromecoutant:PR_RTC_SHADOW branch from 254772e to efabe7b Jun 29, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 30, 2018

@marcemmers @bulislaw @c1728p9 @ashok-rao PR has been updated. Would y'all mind re-reviewing?

// Change the RTC current date/time
if (HAL_RTC_SetDate(&RtcHandle, &dateStruct, RTC_FORMAT_BIN) != HAL_OK) {
error("HAL_RTC_SetDate error\n");
}
if (HAL_RTC_SetTime(&RtcHandle, &timeStruct, RTC_FORMAT_BIN) != HAL_OK) {
error("HAL_RTC_SetTime error\n");
}
#if DEVICE_LPTICKER && !MBED_CONF_TARGET_LPTICKER_LPTIM
while (Read_SubSeconds != (RTC->SSR)) {

This comment has been minimized.

@c1728p9

c1728p9 Jul 1, 2018

Contributor

Is it possible that SSR increments and you get stuck in this loop?

This comment has been minimized.

@jeromecoutant

jeromecoutant Jul 3, 2018

Contributor

SSR sub seconds register is reset to PREDIV_S value every second.
This prescaler value is set only during init, so I would say we can't get stuck...

@0xc0170 0xc0170 added needs: review and removed needs: work labels Jul 2, 2018

@c1728p9

c1728p9 approved these changes Jul 9, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 11, 2018

Looks like this'll need a rebase before it can continue :/

@cmonr cmonr added needs: work and removed needs: review labels Jul 11, 2018

STM32 RTC : bypass shadow registers
- RTC_SSR for the subseconds
- RTC_TR for the time
- RTC_DR for the date

These registers were accessed through shadow registers which are synchronized with PCLK1 (APB1 clock).
They are now accessed directly in order to avoid waiting for the synchronization duration.

@jeromecoutant jeromecoutant force-pushed the jeromecoutant:PR_RTC_SHADOW branch from efabe7b to 1052993 Jul 11, 2018

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jul 11, 2018

@cmonr Rebase is done

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 11, 2018

/morph build

@cmonr cmonr added needs: CI and removed needs: work labels Jul 11, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 11, 2018

Build : SUCCESS

Build number : 2571
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7365/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels Jul 11, 2018

@cmonr cmonr merged commit e1df16e into ARMmbed:master Jul 12, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 791 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9626 cycles (-559 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@jeromecoutant jeromecoutant deleted the jeromecoutant:PR_RTC_SHADOW branch Jul 12, 2018

@mattbrown015

This comment has been minimized.

Contributor

mattbrown015 commented on targets/TARGET_STM/rtc_api.c in 1052993 Jul 31, 2018

@jeromecoutant This generates an unused variable warning when building with "target.lpticker_lptim": 0.

It appears you're calling rtc_read_lp() because of its side effect of updating LP_continuous_time.

I don't understand what's going on here, and don't need to, but the warning worried me and made my build output look untidy.

Is it definitely necessary to call rtc_read_lp()? And, if so, can we remove the unused variable or add MBED_UNUSED? My preference would be the former, why have source code that doesn't do anything??

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment