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

Renesas: Add LPTICKER #7552

Merged
merged 2 commits into from Aug 1, 2018

Conversation

Projects
None yet
5 participants
@TomoYamanaka
Contributor

TomoYamanaka commented Jul 19, 2018

Description

I implemented LPTICKER feature for Renesas mbed boards.
Test result is here:
Test_PEACH_GCC.txt
Test_LYCHEE_GCC.txt

Pull request type

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

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-hal Jul 19, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 23, 2018

Looking at the implementation, what are results from sleep test (mainly lp ticker test case there) ?

@TomoYamanaka

This comment has been minimized.

Contributor

TomoYamanaka commented Jul 23, 2018

@TomoYamanaka

This comment has been minimized.

Contributor

TomoYamanaka commented Jul 30, 2018

@ARMmbed/mbed-os-hal Hi anyone of members, could you review this?

@mprse

mprse approved these changes Jul 30, 2018

Looks like implementation of ticker HAL API is consistent with the requirements.

Could you check if your implementation passes tests-mbed_hal-lp_ticker and tests-mbed_hal-common_tickers from PR #7508 - this adds requirements and test cases for ticker free function. It is still under development, but I think it would be good to check.

GIC_ClearPendingIRQ(LP_TICKER_TIMER_IRQn);

MTU2TIER &= ~MTU2_TIER_n_TGIEA;
mtu2_free();

This comment has been minimized.

@mprse

mprse Jul 30, 2018

Member

I guess that lp_ticker_inited should be set to 0, so re-initialisation will work.

@TomoYamanaka

This comment has been minimized.

Contributor

TomoYamanaka commented Jul 31, 2018

@mprse
Along your advice, I added lp_ticker_inited = 0 in lp_ticker_free() of lp_ticker.c and checked the developing ticker test(PR #7508), unfortunately "lp ticker overflow test" only was failed.

Log is here:

[1533002556.38][CONN][RXD] >>> Running case #18: 'lp ticker overflow test'...
[1533002556.42][CONN][INF] found KV pair in stream: {{__testcase_start;lp ticker overflow test}}, queued...
[1533002558.27][CONN][RXD] :350::FAIL: Expected 1 Was 2
[1533002558.31][CONN][INF] found KV pair in stream: {{__testcase_finish;lp ticker overflow test;0;1}}, queued...
[1533002558.41][CONN][RXD] >>> 'lp ticker overflow test': 0 passed, 1 failed with reason 'Assertion Failed'

Since PR #7508 is under development, I would like to suggest that go ahead this PR and I will submit new PR against the above problem If necessary after PR #7508 was merged.

Could you agree this? I will appreciate if you can tell me your opinion.

@mprse

This comment has been minimized.

Member

mprse commented Jul 31, 2018

@TomoYamanaka PR #7508 adds additional test cases for ticker free function, so I think that lp ticker overflow test will also fail on original version of tests-mbed_hal-common_tickers.

This test case performs the following actions:

  • waits for the counter overflow edge (if possible - when it should not take more than 30 sec),
  • detects rollover and checks if the counter still counts from 0 and ticker interrupt has not been fired after rollover - this is ok,
  • then sets interrupt and checks if interrupt scheduling still works after rollover- looks like we have problem here. After rollover intFlag which is incremented in ticker interrupt handler is 0 - which is ok (ticker interrupt has not been triggered after rollover). But then we set interrupt 100 ticks in the future and wait over 100 ticks. In this case the result is wrong: 350::FAIL: Expected 1 Was 2. It looks like ticker interrupt handler has been called twice - this should be investigated.

This is a long shot, but maybe interrupt is already pending and fires after the ticker interrupt is enabled while setting the interrupt (GIC_EnableIRQ(LP_TICKER_TIMER_IRQn);). Maybe before enabling ticker interrupt we should clear ticker interrupt flag by means of GIC_ClearPendingIRQ(LP_TICKER_TIMER_IRQn);.

Answering to your question I agree that you should go ahead with this PR. But it looks like morph does not cover Renesas boards, so it would be good to provide here test run results with all tests passed for each compiler before this one is merged.

cc @0xc0170 @cmonr

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 31, 2018

/morph build

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 31, 2018

Answering to your question I agree that you should go ahead with this PR. But it looks like morph does not cover Renesas boards, so it would be good to provide here test run results with all tests passed for each compiler before this one is merged.

Tests are provide in the first post, should be also (at least informed no changes in tests after adding another changes here).

@TomoYamanaka

This comment has been minimized.

Contributor

TomoYamanaka commented Jul 31, 2018

The Ticker test result for commit db266a0 is here:

Test_PEACH_GCC_Ticker.txt
Test_LYCHEE_GCC_Ticker.txt

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 31, 2018

TomoYamanaka added some commits Jul 19, 2018

Implementation of LPTICKER feature for Renesas mbed boards
Although other venders implement this feature by using RTC, in my H/W(RZ_A1), I cannot use RTC because it does not satisfy the spec of LP Ticker (ms order and low frequency between 8 KHz and 64 KHz).
Therefore I implemented this feature by creating 1024 division by MTU2(Multi function Timer pulse Unit 2) in order to satisfy this spec.
As a result of investigating, the most unaffected channel among MTU2 placed on GR-PEACH and GR-LYCHEE was channel 3, so I use channel 3 for this feature.

- mbed_drv_cfg.h
  I added a macro of MTU2 channel to this file for commonalizing code for GR-PEACH and GR-LYCHEE, and referenced it's macro at us_ticker.c.
- targets.json
  I added a macro for enabling LP Ticker.
- mtu2.c mtu2.h
  I defined fuction of MTU2's clock supply and stop.
  Because MTU2 is utilized by pwm driver too, those function were referenced at lp_ticker driver and pwm driver.

- lp_ticker.c lp_ticker_init()
  In order to satisfy the LP Ticker spec, I implemented by creating 1024 division by MTU2.
  When an interrupt is required, it will be set with ticker_set_interrupt().

- lp_ticker.c lp_ticker_free()
  This function stops the counting and powerdown the lp_ticker.

- lp_ticker.c lp_read()
  This function returns the timer counter of MTU2.

- lp_ticker.c lp_ticker_set_interrupt()
  In order to satisfy specifications, I implemented lp_ticker_set_interrupt() function.

- lp_ticker.c lp_ticker_fire_interrupt()
  In order to satisfy spec, I implemented lp_ticker_fire_interrupt() function.
  Also I added GIC_EnableIRQ for allowing the interrupt at end of function.

- lp_ticker.c lp_ticker_get_info()
  To satisfy the spec, I implemented lp_ticker_get_info() function. The value of freq includes rounding off.
Add the clear process of "inited" flag in lp_ticker_free()
I added "lp_ticker_inited = 0" in lp_ticker_free() of lp_ticker.c, so
re-initialization will work.

@TomoYamanaka TomoYamanaka force-pushed the TomoYamanaka:feature-lp-ticker branch from db266a0 to 87496df Jul 31, 2018

@TomoYamanaka

This comment has been minimized.

Contributor

TomoYamanaka commented Jul 31, 2018

Sorry, I rebased my commit to modify the build failure. Could you trigger again?
The Ticker test result for commit 87496df is here:

Test_LYCHEE_GCC_Ticker.txt
Test_PEACH_GCC_Ticker.txt

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 31, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 31, 2018

Build : SUCCESS

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

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.

@0xc0170 0xc0170 changed the title from Implementation of LPTICKER feature for Renesas mbed boards to Renesas: Add LPTICKER Aug 1, 2018

@0xc0170 0xc0170 merged commit b74a1dd into ARMmbed:master Aug 1, 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, 793 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10343 cycles (+673 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

@0xc0170 0xc0170 removed the ready for merge label Aug 1, 2018

@TomoYamanaka TomoYamanaka deleted the TomoYamanaka:feature-lp-ticker branch Aug 2, 2018

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