Skip to content

Fix nrf52 enabled uart count and enable uart0/1 #10753

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 12, 2019
Merged

Fix nrf52 enabled uart count and enable uart0/1 #10753

merged 1 commit into from
Jun 12, 2019

Conversation

desmond-blue
Copy link
Contributor

Description

This is a proposed fix for internally raised issue IOTDEV-2195.

The reason it hangs is we don't allocate nordic_nrf5_uart_state for UART1, so the initialing procedure for UART1 access an invalid memory space and retrieve wrong information.

Version of Nordic SDK has been changed from 14.2 to 15.0, NRFX_UART_ENABLED_COUNT doesn't represent the supported UART correctly, instead we need to use NRFX_UARTE_ENABLED_COUNT to represent count of enabled UART.

sdk_config.h is also updated to enable UART0/UART1 as default.

Pull request type

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

Reviewers

Release Notes

@ciarmcom
Copy link
Member

ciarmcom commented Jun 4, 2019

@desmond-blue, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from a team June 4, 2019 09:00
AriParkkila
AriParkkila reviewed Jun 4, 2019
View reviewed changes
targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_NRF52/serial_api.c

/**
* Array with UARTE register pointers for easy access.
*/
static NRF_UARTE_Type *nordic_nrf5_uart_register[NRFX_UART_ENABLED_COUNT] = {
static NRF_UARTE_Type *nordic_nrf5_uart_register[NRFX_UARTE_ENABLED_COUNT] = {
NRF_UARTE0,
#if UART1_ENABLED
Copy link

Choose a reason for hiding this comment

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

I guess this should be defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing this, you are correct.
UART1_ENABLED shall be defiend as 1, somehow the updating Nordic SDK to 15 disabled it.

@desmond-blue desmond-blue force-pushed the feature-fix-nrf52-uart-count branch 3 times, most recently from 7e4b8cf to d9db087 Compare June 5, 2019 03:34
@kimlep01
Copy link

kimlep01 commented Jun 5, 2019

I tested this pull request and now the board doesn't crash anymore but it looks like UART is still not working. More details in Jira ticket.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2019

I tested this pull request and now the board doesn't crash anymore but it looks like UART is still not working. More details in Jira ticket.

needs: work set , waiting for the update

@desmond-blue desmond-blue force-pushed the feature-fix-nrf52-uart-count branch from d9db087 to 5cd7f60 Compare June 5, 2019 13:54
@AriParkkila
Copy link

@0xc0170 this fix is needed in Mbed OS 5.13.

@desmond-blue
Copy link
Contributor Author

Hi @0xc0170 , I've updated the change, it is tested by the reporter, would you please merge this PR, thanks!

@adbridge
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Jun 11, 2019

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 11, 2019

See IAR logs, related failure: [Error] serial_api.c@170,51: [Pe094]: the size of an array must be greater than zero

@adbridge
Copy link
Contributor

@desmond-blue could you please take a look at the failures, they seem to be valid.

@desmond-blue desmond-blue force-pushed the feature-fix-nrf52-uart-count branch from 5cd7f60 to d5624b6 Compare June 12, 2019 07:37
@desmond-blue
Copy link
Contributor Author

sdk_config.h is updated for TARGET_MCU_NRF52832, which is used on NRF52_DK.

@adbridge
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Jun 12, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@adbridge adbridge merged commit 378e997 into ARMmbed:master Jun 12, 2019
@LDong-Arm LDong-Arm mentioned this pull request Jun 13, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@AriParkkila AriParkkila AriParkkila approved these changes

Assignees
No one assigned
Labels
release-version: 5.13.0-rc3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@desmond-blue @ciarmcom @kimlep01 @0xc0170 @AriParkkila @adbridge @mbed-ci