M487: Add some bugfixes #5366

ccli8 · 2017-10-24T02:59:35Z

Description

This PR includes the following bugfixes with NUMAKER_PFM_M487 target:

Enable uVisor core debug message output via semihosting
Fix premature lp_ticker interrupt

0xc0170 · 2017-10-27T17:00:20Z

@Patater Is there anyone else who should review uvisor changes? please add as reviewer

Patater

uVisor-related changes look OK to me.

Patater · 2017-10-30T15:17:01Z

targets/TARGET_NUVOTON/TARGET_M480/lp_ticker.c

    wakeup_tick = timestamp;
-
+    


Please don't add trailing whitespace.

@Patater I remove trailing whitespace in 44131d8.

alzix · 2017-10-30T17:10:33Z

targets/TARGET_NUVOTON/TARGET_M480/device/startup_M480.c

+     */
+#ifdef TARGET_DEBUG
+#if defined(MBED_CONF_TARGET_UVISOR_DEBUG_SEMIHOSTING_ENABLE) && MBED_CONF_TARGET_UVISOR_DEBUG_SEMIHOSTING_ENABLE
+    uvisor_api.debug_semihosting_enable();


Hi @ccli8 ,
I think you are missing the point of having uvisor_api.debug_semihosting_enable().

The idea was to have a single binary that can be executed both with debugger attached and debugger detached. There is no way to reliably programmatically determine whether debugger is attached (on all ther platforms). So we added this function to prevent MCU from halting on BKPT instruction.
We need a debugger to be attached to get the semihosting prints. In such a case - you can add a simply call to uvisor_api.debug_semihosting_enable() to the debugger init script.

Calling uvisor_api.debug_semihosting_enable() function statically guarded by ifdef is misaligned with our goal.

Please refer to the https://github.com/ARMmbed/uvisor/blob/master/docs/lib/DEBUGGING.md

@alzix The NUMAKER_PFM_M487 target doesn't support GDB debugger. Currently I can debug by following the Build with GCC and Debug with uVIsion guide. Do you know how to call uvisor_api.debug_semihosting_enable() in uVision debugger?

There is field under Debugger settings tab in uVision5 that allows specifying Initialization file.
uvisor_api.debug_semihosting_enable() can be called from it.
You will probably need to clear the Run to main() checkbox, otherwise initialization file commands will only be executed once you reach main.

@alzix The approach is failed on my uVision 5.24. I try with another simple example and find C function call by debugger doesn't work whether in Initialization File, Command Window, or Watch Window.

@alzix I remove the commit with uvisor_api.debug_semihosting_enable(). If there are other means to enable uvisor semi-hosting in Keil debugger, please let me know. Thanks.

Old lp_ticker handles past event, but it has a bug with premature go-off. The bug can re-produce on mbed-os-tests-mbed_drivers-lp_timeout/mbed-os-tests-mbed_hal-lp_us_tickers (mbed-os commit: 9c1fd48). Because upper layer (mbed-os/hal/mbed_ticker_api.c) has handled past event, this code can be removed from lp_ticker. The similar fix also applies to us_ticker.

theotherjimmy · 2017-11-13T16:25:36Z

@Patater @alzix Could you take another look?

alzix · 2017-11-14T06:51:54Z

LGTM

0xc0170 · 2017-11-14T08:54:09Z

@ccli8 There's travis again not reporting. Are you logged into travis? I was told that would help to eliminate abuse detected statuses. I can not locate this PR in travis. I'll close and open

Please update the pull request thread with the latest status (in this case this was reworked to remove uvisor changes) and only tickers fix is present.

ccli8 · 2017-11-14T09:01:26Z

@0xc0170 I don't log in travis. I will update status after you re-open this PR.

ccli8 · 2017-11-14T09:16:24Z

Current status for this PR:

Fix premature lp_ticker interrupt

Enable uVisor core debug message output via semihosting (removed)

0xc0170 · 2017-11-14T09:20:48Z

Abuse detected in the logs.

@ccli8 I was asking about travis logging, because I received a response from travis help team yesterday:

For the time being, a work around would be to have someone in the XXX organization create an account with Travis. They can do so right here: https://www.travis-ci.org This should prevent these pull requests from being flagged. If you still encounter issues after that, please don't hesitate to reach out and we'll investigate again.

They mention the organization however seems like this is valid also for a user :( Therefore I was asking if you can at least once log into travis, this would not flag this PR as abuse. This is the second time I am seeing this abuse detected status in travis this month.

As my reopening did not help, the only way I know is to open a new PR or try to logging to travis CI and we can reopen this again.

ccli8 · 2017-11-14T09:36:46Z

@0xc0170 I've logged in travis CI. Please re-try the travis test.

ccli8 · 2017-11-20T01:27:47Z

@0xc0170 Could the abuse issue be fixed? Or I need to raise another PR?

0xc0170 · 2017-11-20T14:43:45Z

I contacted your colleague @cyliangtw the previous week to resolve this. I resent the email just now including you also there to agree on the next steps with this PR.

Lets send new pull request.

ccli8 · 2017-11-22T07:06:15Z

@0xc0170 I resend the PR in #5552.

0xc0170 requested a review from Patater October 25, 2017 17:57

0xc0170 added the needs: review label Oct 25, 2017

0xc0170 approved these changes Oct 25, 2017

View reviewed changes

Patater requested a review from alzix October 30, 2017 15:15

Patater suggested changes Oct 30, 2017

View reviewed changes

alzix suggested changes Oct 30, 2017

View reviewed changes

adbridge added needs: work and removed needs: review labels Oct 31, 2017

ccli8 added 2 commits November 8, 2017 14:13

[M487] Remove trailing space in lp_ticker/us_ticker

44131d8

ccli8 force-pushed the nuvoton branch from 03bee14 to 44131d8 Compare November 8, 2017 06:41

theotherjimmy added needs: review and removed needs: work labels Nov 13, 2017

0xc0170 closed this Nov 14, 2017

0xc0170 removed the needs: review label Nov 14, 2017

0xc0170 reopened this Nov 14, 2017

0xc0170 closed this Nov 14, 2017

0xc0170 reopened this Nov 20, 2017

0xc0170 closed this Nov 20, 2017

ccli8 mentioned this pull request Nov 22, 2017

M487: Fix premature lp_ticker interrupt #5552

Closed

0xc0170 mentioned this pull request Nov 22, 2017

Nuvoton: fix lpticker interrupt #5553

Merged

M487: Add some bugfixes #5366

M487: Add some bugfixes #5366

Uh oh!

Conversation

ccli8 commented Oct 24, 2017

Description

Uh oh!

0xc0170 commented Oct 27, 2017

Uh oh!

Patater left a comment

Choose a reason for hiding this comment

Uh oh!

Patater Oct 30, 2017

Choose a reason for hiding this comment

Uh oh!

ccli8 Nov 8, 2017

Choose a reason for hiding this comment

Uh oh!

alzix Oct 30, 2017

Choose a reason for hiding this comment

Uh oh!

ccli8 Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

alzix Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

ccli8 Oct 31, 2017

Choose a reason for hiding this comment

Uh oh!

ccli8 Nov 8, 2017

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Nov 13, 2017

Uh oh!

alzix commented Nov 14, 2017

Uh oh!

0xc0170 commented Nov 14, 2017

Uh oh!

ccli8 commented Nov 14, 2017

Uh oh!

ccli8 commented Nov 14, 2017

Uh oh!

0xc0170 commented Nov 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccli8 commented Nov 14, 2017

Uh oh!

ccli8 commented Nov 20, 2017

Uh oh!

0xc0170 commented Nov 20, 2017

Uh oh!

ccli8 commented Nov 22, 2017

Uh oh!

Uh oh!

0xc0170 commented Nov 14, 2017 •

edited

Loading