-
Notifications
You must be signed in to change notification settings - Fork 1k
Add support for the VBLUno51 board #290
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
Conversation
@c1728p9 @maclobdell I hope that this commit will be reviewed soon. Thank you very much |
5c080af
to
426474a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @iotvietmember, thanks for the PR. Before I can merge this you'll need to sign the contributor's agreement if you haven't already:
https://developer.mbed.org/contributor_agreement/
If you have, just let me know what mbed account you used.
UART_Config.FlowControl = UART_FLOW_CONTROL_NONE; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this a runtime setting rather than a define
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c1728p9 I used the "BOARD_VBLUNO51" macro in "records/board/vbluno51.yaml" file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flow control could be useful to other devices as well. Could you add a function to enable it like the sam3u2c has:
https://github.com/mbedmicro/DAPLink/blob/master/source/hic_hal/atmel/sam3u2c/uart.c#L397
Then you can use prerun_board_config() to enable it:
https://github.com/mbedmicro/DAPLink/blob/master/source/board/ncs36510rf.c#L21
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c1728p9 The VBLUno51, VBLUno52 boards always use
UART Hardware flow control (RTS/CTS) feature for DAPLink interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but as more and more boards use flow control this code will get bigger and bigger
#if defined(BOARD_VBLUNO51) || defined(BOARD_VBLUNO52) || ... || defined(BoardN)
If this is a runtime changeable option and is set in board specific code the all boards can use the same code without defines.
#define FUNC_1 1 | ||
#define PULL_DOWN_ENABLED (1 << 3) | ||
#define PULL_UP_ENABLED (2 << 3) | ||
#define OPENDRAIN (1 << 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you unable to use the standard pinout for your board? If not, can you document in this file which pins differ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VBLUno51 board has not used standard pinout. The following file is VBLUno51's schematic
schematic
In the VBLUn51 board, TGT_SWCLK is PIO0_9(18). While standard design used PIO0_7(16) for TGT_SWCLK
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this as a comment to this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just added comment to this file
@c1728p9 Beside that, I signed the contributor's agreement with |
426474a
to
7daeee6
Compare
@c1728p9 Could you please review this commit. Thanks |
@c1728p9 Hope that this commit would be reviewed soon. |
fc25930
to
f64d381
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @iotvietmember. This PR has conflicts with master so it needs to be rebased before I and test and merged it.
Also, I added a few more comments to the PR, none of them preventing it from getting merged though.
void prerun_board_config() | ||
{ | ||
uart_enable_flow_control(true); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick - no newline at end of file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @c1728p9 I have added new line at end of this file
source/hic_hal/nxp/lpc11u35/uart.c
Outdated
@@ -146,8 +148,8 @@ int32_t uart_set_configuration(UART_Configuration *config) | |||
parity = 0x00; | |||
break; | |||
} | |||
|
|||
if (config->FlowControl == UART_FLOW_CONTROL_RTS_CTS) { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick - extra whitesspace
@c1728p9 I have just fixed two errors which you wrote |
Thanks for the update @iotvietmember. Github won't let me merge this until it is rebased - "This branch has conflicts that must be resolved" |
@c1728p9 Thanks. Hope it will resolve soon |
Hi @iotvietmember, you'll need to use git to perform the rebase. If you want help with this let me know. |
The VBLUno51 board has BOARD_ID is 0xC006 Signed-off-by: iotvietmember <[email protected]>
60a843c
to
6074ea1
Compare
Hi @c1728p9 , I have just done the rebase. Could you please review for me. Thank you very much |
/morph test |
1 similar comment
/morph test |
@c1728p9 thank you very much |
Add support for the VBLUno51 board
Email from Sarah Marsh (mbed):
Hi,
Here is your daplink board ID: C006
The slug: VBLUNO51
When I execute mbedls, it looks like you have used the DAPLink ID for the NRF51_DK. You need your own unique ID. We are running the tests with the NRF51_DK ID mocked as your platform, but you will need to do the following to be mbed enabled:
Please submit a PR against mbed-ls (https://github.com/ARMmbed/mbed-ls), so your board can be detected with our tools.
Please also submit a PR against DAPLink (https://github.com/mbedmicro/DAPLink), so that your board is uniquely identifiable.
Thanks,
Sarah
The VBLUno51 board was added to mbed-os 5.5.2 released
ARMmbed/mbed-os#4629
ARMmbed/mbed-os#4719
Signed-off-by: iotvietmember [email protected]