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

Add new target MTB_ADV_WISE_1530 #6601

Merged
merged 1 commit into from Apr 16, 2018

Conversation

Projects
None yet
8 participants
@KariHaapalehto
Contributor

KariHaapalehto commented Apr 11, 2018

Description

Add new target MTB_ADV_WISE_1530.
MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 includes same usi chip,
so MTB_USI_WM_BN_BM_22 has been changed to common USI_WM_BN_BM_22 target, which
MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 will inherit

Pull request type

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

Add new target MTB_ADV_WISE_1530.
MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 includes same usi chip,
so common USI_WM_BN_BM_22 target has been created.
MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 are inheting the common usi target
@SeppoTakalo

I like this approach.
Now they both can share the same code&same drivers.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Apr 12, 2018

This is same as ODIN approach, but I'm not sure if things will break due to clock sources.. the changes are fine by me, but we need to ensure greentea passes (with the exception of RTC, LP_Timer as being discussed in the other issue)..

@ashok-rao

have you checked clock sources are same on both modules?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Apr 12, 2018

@kjbracey-arm please review

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 12, 2018

This is same as ODIN approach, but I'm not sure if things will break due to clock sources.. the changes are fine by me, but we need to ensure greentea passes (with the exception of RTC, LP_Timer as being discussed in the other issue)..

@KariHaapalehto Please for new targets, share testing results (all 3 compilers)

@KariHaapalehto

This comment has been minimized.

Contributor

KariHaapalehto commented Apr 12, 2018

wise1530_iar.txt
usi22_iar.txt
usi22_gccarm.txt
usi22_arm.txt
wise1530_arm.txt
wise1530_gccarm.txt

Test results added for 3 compilers for both MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 tagets. Wise-1530 passed all tests and usi failed only with "mbed-os-tests-mbed_drivers-rtc"-test.
Ticker tests when through without problems this time.

@ashok-rao

ashok-rao approved these changes Apr 12, 2018 edited

LGTM. Not sure why lp_ticker tests passed without issues this time.. RTC is still crashing though..

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Apr 13, 2018

Maybe it is fixed in master now?
Or is it unstable testcase?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

Test results added for 3 compilers for both MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22 tagets. Wise-1530 passed all tests and usi failed only with "mbed-os-tests-mbed_drivers-rtc"-test.

RTC should be disabled if its failing tests until it's fixed.
@ashok-rao I recall there is lp ticker issue created and known issue until 5.9. What about RTC?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 13, 2018

Build : SUCCESS

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

Triggering tests

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

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Apr 13, 2018

@0xc0170 , RTC is failing with TIMEOUT. I'm thinking it might be because of different clock sources. The module has only LSI clock.. no external sources. And maybe that's why it is failing with timeout. @KariHaapalehto , can you remove RTC for the USI MTB and re-run the test? But removing RTC would remove a very fundamental feature.. :-(

@KariHaapalehto

This comment has been minimized.

Contributor

KariHaapalehto commented Apr 13, 2018

About the lp_ticker issue, I just notice that test were not passed, they were skipped. There isn't "LOWPOWERTIMER" -flag defined at the target.json

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

@KariHaapalehto , can you remove RTC for the USI MTB and re-run the test? But removing RTC would remove a very fundamental feature.. :-(

Shall be fixed. Can anyone look at the failure and investiagate?

@KariHaapalehto

This comment has been minimized.

Contributor

KariHaapalehto commented Apr 13, 2018

RTC can not be removed just from MTB_USI_WM_BN_BM_22 and MTB_ADV_WISE_1530.
RTC is defined at FAMILY_STM32, which USI_WM_BN_BM_22 inherits. So if we remove it, it will be removed from all targets that inherits FAMILY_STM32

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Apr 13, 2018

Hi
You can add:
"device_has_remove": ["RTC"],

But as started in #6591 what is your clock configuration?
Do you have an external HSE, LSE ?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Apr 13, 2018

device_has_remove is undocumented.
Reference: https://os.mbed.com/docs/v5.8/tools/adding-and-configuring-targets.html#macros-macros-add-and-macros-remove

@0xc0170 do you know who would be able to document it? Or can we even rely on it?

Also, who would be correct person to verify our clock configurations. I don't think we understand enough this area. For us, socket&Wifi tests are passing, so we just assume that those are correct.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

device_has_remove is undocumented.
Reference: https://os.mbed.com/docs/v5.8/tools/adding-and-configuring-targets.html#macros-macros-add-and-macros-remove

I recall there was something somewhere as I pointed this configuration in another PR but from the link, it's not there and some other _remove are explained. It should be added if not in there
@theotherjimmy

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Apr 13, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 13, 2018

@KariHaapalehto This target then shall do device_has_remove for RTC as it fails

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Apr 13, 2018

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: work labels Apr 16, 2018

@cmonr cmonr merged commit fe44dc0 into ARMmbed:master Apr 16, 2018

12 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/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9571 cycles (-192 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 16, 2018

@juhoeskeli

This comment has been minimized.

Contributor

juhoeskeli commented Apr 30, 2018

The device has issue with RTC clock initialization. In targets/TARGET_STM/rtc_api.c rtc_init the board gets stuck on line "if (HAL_RCC_OscConfig(&RCC_OscInitStruct) != HAL_OK)." under MBED_CONF_TARGET_LSE_AVAILABLE.

By setting "overrides": {"lse_available": 0} to USI_WM_BN_BM_22 in targets.json the initialization passes.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 3, 2018

@juhoeskeli Can you send a fix for this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment