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

Cellular: HSI set to be source clock for WISE_1570 #7489

Merged
merged 1 commit into from Jul 13, 2018

Conversation

Projects
None yet
8 participants
@mirelachirica
Contributor

mirelachirica commented Jul 12, 2018

Description

LSE as LPUART source clock is causing WISE_1570 to have stability issues(framing errors) for AT commands communication over LPUART. Changing clock to HSI is fixing the problem.

Internal defect: IOTCELL-988

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change
@JanneKiiskila

This comment has been minimized.

Contributor

JanneKiiskila commented Jul 12, 2018

To next patch release, please. @adbridge @0xc0170

@cmonr

cmonr approved these changes Jul 12, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 12, 2018

@JanneKiiskila @mirelachirica We're having some issues with the testing CI atm. Will start once they're resolved.

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jul 12, 2018

Hi

I would suggest to follow #7469
Then, it should be possible to keep SystemClock with HSE, and configure LPUART clock with HSI

@bcostm

@bcostm

This comment has been minimized.

Contributor

bcostm commented Jul 12, 2018

Can you check PR #7498 ? Thanks.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 12, 2018

/morph build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 12, 2018

@bcostm Who are you referring to when you say "you"?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 12, 2018

@bcostm @jeromecoutant @JanneKiiskila @mirelachirica
Going to try and bring this in w/o the changes in #7498.
Bringing in #7498 before this PR creates a dependency that @JanneKiiskila and @mirelachirica would have to re-verify before moving this forward. The time constraint is that Code Freeze for the patch is about to occur in 24 hours, which would make timing everything incredibly difficult.

@bcostm @jeromecoutant Do y'all have any strong arguments as to why this needs to wait on #7498?

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jul 12, 2018

Hi
It is up to you...
I can see in this PR, that you are downgrading SystemClock from HSE to HSI, in order to get higher LPUART clock.
What @bcostm is proposing is to take #7498, then keep HSE SystemClock, and set LPUART clock to HSI as requested.

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jul 12, 2018

This can be done in 2 steps.
Merge this one, and push another one later with HSE setting

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 12, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 12, 2018

Export failure due to multiply defined symbols, but symbol does not appear related tio PR.
Retesting: /morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jul 13, 2018

@cmonr cmonr merged commit 1145d6b into ARMmbed:master Jul 13, 2018

14 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/astyle Passed, 791 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8688 cycles (-172 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
@juhoeskeli

This comment has been minimized.

Contributor

juhoeskeli commented Jul 31, 2018

Hello, I'm having an issue with this fix. The board MTB_ADV_WISE_1570 fails in SetSysClock_PLL_HSI in function HAL_RCC_OscConfig. More precisely this happens when waiting the PLL to stabilize (stm32l4xx_hal_rcc.c:813). As a workaround increasing PLL_TIMEOUT_VALUE from 2ms to 5ms solves the issue. Then it works 10/10 tries. With 4ms value it works 1/20 times.

If changing the PLL timeout value is a valid fix, maybe the timeout value could be as parameter to HAL_RCC_OscConfig?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 31, 2018

@juhoeskeli Would you mind opening an issue and referencing this PR if you haven't already?

Comments left in PRs after they're merged tend not to get the best attention.

@juhoeskeli

This comment has been minimized.

Contributor

juhoeskeli commented Aug 1, 2018

@cmonr Sounds fair.

@juhoeskeli

This comment has been minimized.

Contributor

juhoeskeli commented Aug 1, 2018

@cmonr I noticed that the issue was due to bootloader using different clock and then switching to HSI in application. By using the same clock in bootloader there is no issue. I consider this working as intended.

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7489 from mirelachirica/wise_1570_hsi_sour…
…ce_clock

Cellular: HSI set to be source clock for WISE_1570
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment