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

Nuvoton: Fix mbed_hal-sleep test failed #8049

Merged
merged 7 commits into from
Oct 1, 2018

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Sep 10, 2018

Description

This PR tries to fix mbed_hal-sleep test failure on Nuvoton targets. It includes:

  1. Replace wait_us with nu_busy_wait_us in lp ticker since wait_us is not available with us ticker layer shutting down in the test.
  2. Fix serial corruption due to entering deep sleep
  3. Update M2351 secure library/executable

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

nu_busy_wait_us is a replacement for wait_us when intermediary us ticker layer is disabled.
nu_delay_cycle_x4 is a replacement for wait_us when us ticker is not available.
Replace wait_us with nu_busy_wait_us in lp_ticker since wait_us is not allowed in sleep test
which would suspend us ticker layer on which wait_us relies. nu_busy_wait_us is implemented
by calling us ticker HAL API directly rather than relying on us ticker layer.
Prevent deep sleep when there is still any character being transmitted on the UART.
This allows tickless to be safely enabled.
Add secure functions CLK_PowerDown_S/CLK_Idle_S
1. Remove hal_sleep/hal_deepsleep from secure library
2. Add CLK_Idle_S/CLK_PowerDown_S
@adbridge
Copy link
Contributor

@ARMmbed/mbed-os-hal please review

@adbridge adbridge requested a review from a team September 10, 2018 11:13
@jamesbeyond
Copy link
Contributor

Hi @fkjagodzinski , do you mind have a look?

@@ -1136,4 +1139,23 @@ static int serial_is_irq_en(serial_t *obj, SerialIrq irq)
}

#endif // #if DEVICE_SERIAL_ASYNCH

bool serial_can_deep_sleep(void)
Copy link
Member

Choose a reason for hiding this comment

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

I think the code of this fun is the same for every target in this PR. It might be a good idea to move it to a common file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkjagodzinski serial_can_deep_sleep is recommended in #7698 by @c1728p9 , but it is not defined in HAL spec. I can only fix on Nuvoton targets.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I wasn't aware of the origin of this function. This could be a useful HAL feature. 👍

Anyway, my original comment was only about your new code. It might be convenient not to keep a few copies of one function. In this case it would stay inside targets/TARGET_NUVOTON/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkjagodzinski Currently, on Nuvoton targets, HAL implementations (e.g. serial_api.c) for each target (NUMAKER_PFM_NUC472, NUMAKER_PFM_M453, etc.) are branched. Except these HAL implementations are resolved to merge into one, serial_can_deep_sleep would be kept branched for consistency.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, no problem for me.

@@ -1,797 +1,873 @@
:020000040000FA
:1000000000080020050F0000D90B0000D90B0000EC
:1000000000080020AB100000190D0000190D0000C1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this file and also another object file in the same folder contain a license file? (https://github.com/ARMmbed/mbed-os/tree/a65e3d42e53e5c46c8a5bd74f17c5d0e7cef0f1b/targets/TARGET_NUVOTON/TARGET_M2351/TARGET_NUMAKER_PFM_M2351/TARGET_M23_NS)

Would be good to have it as a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 The Permissive Binary License meets the scenario. Could we reuse this license just like it is attached in https://github.com/ARMmbed/mbed-os/tree/master/tools/bootloaders/MTB_MTS_DRAGONFLY?
@cyliangtw

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

Regarding this hex file - what does it do as it has suffix -example ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 Thanks.

Regarding this hex file - what does it do as it has suffix -example ?

TrustZone target requires two codes, secure and non-secure. Secure code would run first to set up security environment and then bring up non-secure code. NuMaker-mbed-TZ-secure-example.hex is the default secure code (and cmse_lib.o is the secure gateway library exported by it). It is built from:
https://github.com/OpenNuvoton/NuMaker-mbed-TZ-secure-example
We choose .hex rather than .bin as output format because secure/non-secure code may not start at address 0x0. To flash secure/non-secure code on M2351, we would drag-n-drop NuMaker-mbed-TZ-secure-example.hex first and then user program e.g. mbed-os-example-blinky.hex.

@cmonr
Copy link
Contributor

cmonr commented Sep 29, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 29, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Sep 30, 2018

@mbed-ci
Copy link

mbed-ci commented Sep 30, 2018

@0xc0170 0xc0170 merged commit 59ce41f into ARMmbed:master Oct 1, 2018
@ccli8 ccli8 deleted the nuvoton_fix_hal_sleep branch October 2, 2018 03:16
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.

7 participants