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

Jump to bottom

PSOC6: Modify lp_ticker to 32 bit #10267

Merged
merged 1 commit into from Apr 3, 2019
Merged

PSOC6: Modify lp_ticker to 32 bit #10267

merged 1 commit into from Apr 3, 2019

Conversation

cy-vivekp
Copy link
Contributor

Description

Increase PSoC6 lpticker to 32 bits

  • Needed for PSoC6 t o deep-sleep for more than 2 seconds
  • Max sleep with 16 bit lp_ticker (before this change) : 2sec
  • Max sleep with 32 bit lp_ticker (after this change) : 36hours

Pull request type

[ ] Fix
[ ] Refactor
[x] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@amak-cy @cydriftcloud @satya1957

Release Notes

@cy-vivekp
PSOC: Modify lp_ticker to 32 bit
7799be6 
Needed for PSoC to deep-sleep for more than 2 seconds
Max sleep with 16 bit lp_ticker (before this change) : 2sec
Max sleep with 32 bit lp_ticker (after this change)  : 36hours
@ciarmcom
Copy link
Member

@cy-vivekp, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from a team March 29, 2019 00:00
amak-cy
amak-cy reviewed Mar 29, 2019
View reviewed changes
targets/TARGET_Cypress/TARGET_PSOC6/lp_ticker.c
#define MCWDT_COUNTER0_MAX_COUNT (0xffffUL)
#define MCWDT_COUNTER1_MAX_COUNT (0xffffUL)
#define MCWDT_COUNTER2_MAX_COUNT (0xffffffffUL)
#define MAX_MCWDT_DURATION_SEC (35UL*60UL*60UL*1000UL)
Copy link

Choose a reason for hiding this comment

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

Can this value be a just a number - 126 000 000 Plus comment how it is calculated - In this way is I see that the uint32 is not overflowed

Copy link
Contributor

Choose a reason for hiding this comment

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

This question hasn't been answered. @cy-vivekp Please review

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 1, 2019

Max sleep with 16 bit lp_ticker (before this change) : 2sec

Didn't this fail the test - there should be test to deepsleep for few seconds? To catch these?

@cmonr
Copy link
Contributor

cmonr commented Apr 1, 2019

@cy-vivekp ^^^

@cmonr
Copy link
Contributor

cmonr commented Apr 2, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 2, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 1
Build artifacts

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 2, 2019

I've checked, ticker info supports 16 bit lp ticker so its fine.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 2, 2019

Waiting on the final comment to #10267 (comment)

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 3, 2019

@cy-vivekp Any update?

@amak-cy
Copy link

amak-cy commented Apr 3, 2019

@cy-vivekp Any update?

It look like @cy-vivekp wont be available until April 15.
We discussed this internally - I am ok with the current implementation. It is a matter of style. So we can resolve this question

@0xc0170 0xc0170 merged commit 0066ba9 into ARMmbed:master Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 left review comments

@cmonr cmonr cmonr approved these changes

@amak-cy amak-cy amak-cy approved these changes

Assignees
No one assigned
Labels
release-version: 5.12.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@cy-vivekp @ciarmcom @0xc0170 @cmonr @mbed-ci @amak-cy