Skip to content

Conversation

@LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented May 13, 2019

Description

(Depends on #10597)

LPTIM is currently disabled on L073RZ due to a hardware bug (see #10537). As an alternative, RTC is used for lp ticker. This brings a new problem that RTC-based lp ticker appears to have a high latency (of a few hundred microseconds. See #9962 for more details.

After discussing with @c1728p9, we disable lp ticker feature on L073RZ for now. This also implies tests in TESTS/mbed_drivers/lp_* will not apply to this target until we re-enable LPTIM-based lp ticker once it's fixed.

Pull request type

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

@ciarmcom ciarmcom requested review from a team and c1728p9 May 13, 2019 17:00
@ciarmcom
Copy link
Member

@LDong-Arm, thank you for your changes.
@c1728p9 @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

#define MBED_SYS_TIMER_H

#if DEVICE_LPTICKER || defined(DOXYGEN_ONLY)
#if MBED_TICKLESS || defined(DOXYGEN_ONLY)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This impacts all the targets!
This should be checked very carefully, and should be out of this "L073RZ" PR...

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, PR introduction talks about fixing one target but there are fixes included impacting larger set of targets. Can we split these (l073 will be quick one to get in)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently SysTimer cannot be built when lp ticker is disabled, leading to build errors of L073RZ. I'll move this SysTimer change to a separate pull request, and make the L073RZ change depend on it.

LDong-Arm added 3 commits May 14, 2019 16:35
SysTimer depends on tickless which runs on us ticker instead
of lp ticker on some targets.
If a ticker is unavailable its macro is undefined.
Currently LPTIM is disabled on L073RZ by

    4bead92 Use the RTC for the low power ticker on L073RZ

to work around a hardware bug. But the latency of RTC is too
high (a few hundred microseconds), leading to lp_timeout
drift test failure.

For now, disable low power ticker on L073RZ altogether until
LPTIM is fixed and re-enabled.
"FLASH",
"MPU"
],
"device_has_remove": ["LPTICKER"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep this update out of the PR.
Thx

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

You should change the PR title,
and explain more what was the issue, and what is corrected now ?

@LDong-Arm LDong-Arm changed the title Disable LPTICKER on L073RZ and fix build Disable LPTICKER on L073RZ May 16, 2019
@LDong-Arm
Copy link
Contributor Author

LDong-Arm commented May 16, 2019

You should change the PR title,
and explain more what was the issue, and what is corrected now ?

Thanks, I've amended the title and description. Now this PR is limited to only disabling lpticker and is blocked on #10597.

Could we change the label to "needs: preceding PR"?

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