drivers: serial: nrf: Use flow control from device tree #25999
Conversation
nordic-krch
added
area: UART
nordic-krch
requested review from
carlescufi,
erwango,
ioannisg,
jhedberg,
joerchan,
lemrey and
Vudentz
as code owners
zephyrbot
added
area: Devicetree
area: Boards
area: Bluetooth
area: Samples
|
All checks are passing now. Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
nordic-krch
force-pushed
the
nrf_uart_flow_control
branch
from
4c3edae
to
ef638b7
Compare
| @@ -1,5 +1,4 @@ | |||
| CONFIG_GPIO=y | |||
| CONFIG_UART_0_NRF_FLOW_CONTROL=y | |||
There was a problem hiding this comment.
Name of this file is wrong. It should be nrf5340pdk_nrf5340_cpuapp.conf. Could you correct it on the occasion?
drivers/serial/uart_nrfx_uarte.c
Outdated
| @@ -1452,8 +1464,8 @@ static int uarte_nrfx_pm_control(struct device *dev, u32_t ctrl_command, | |||
| (UARTE_PROP(idx, pin_prop)), \ | |||
| (NRF_UARTE_PSEL_DISCONNECTED)) | |||
|
|
|||
| #define UARTE_RTS_CTS_PINS_SET(idx) \ | |||
| (UARTE_HAS_PROP(idx, rts_pin) && UARTE_HAS_PROP(idx, cts_pin)) | |||
| #define UARTE_RTS_OR_CTS_PINS_SET(idx) \ | |||
There was a problem hiding this comment.
What about naming it HW_FLOW_CONTROL_AVAILABLE?
drivers/serial/uart_nrfx_uarte.c
Outdated
| #define HWFC_CONFIG_CHECK(idx) \ | ||
| BUILD_ASSERT( \ | ||
| (UARTE_PROP(idx, hw_flow_control) && \ | ||
| (UARTE_HAS_PROP(idx, rts_pin) || UARTE_HAS_PROP(idx, cts_pin)))\ |
There was a problem hiding this comment.
You could use UARTE_RTS_OR_CTS_PINS_SET(idx) here, for better readability. This line should be also indented more.
| @@ -140,7 +140,7 @@ | |||
| }; | |||
|
|
|||
| &uart0 { | |||
| compatible = "nordic,nrf-uart"; | |||
| compatible = "nordic,nrf-uarte"; | |||
There was a problem hiding this comment.
Was that intentional? If so, it should be mentioned in the commit message, or perhaps better done in a separate commit.
There was a problem hiding this comment.
removed, it was accidentally added.
what do you mean? DTS for bbc-micro has HWFC off for uart0 and sample does not modify it so it will have hwfc off. |
nordic-krch
force-pushed
the
nrf_uart_flow_control
branch
from
ef638b7
to
7d842d2
Compare
|
@anangl can you re-review? |
Right. HWFC is required for hci-uart though. I'm thinking we should actually remove the BBC microbit board configuration then. |
|
@nordic-krch pls, rebase for conflict resolution |
Cleaned up flow control configuration. Added support for using only cts or only rts. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Cleaned up flow control configuration. Added support for using only cts or only rts. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Removed flow control configuration from Kconfig and updated samples to use device tree for that. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Conf file was not renamed after board renaming. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
nordic-krch
force-pushed
the
nrf_uart_flow_control
branch
from
7d842d2
to
33fa59f
Compare
Removed flow control configuration from Kconfig and started to use device tree. Added option to have single direction flow control with only rts or cts pin.