Skip to content

Conversation

@MarkusLassila
Copy link
Contributor

Add Asynchronous UART implementation, which does not drop data when automatic hardware-flow-control is set in the device tree.

With automatic hardware flow control, the CTS pin will be automatically deactivated when there are no more asynchronous UART RX buffers available. After buffer space becomes available, and UART RX is restarted, the CTS pin will be activated.

@MarkusLassila
Copy link
Contributor Author

Ping @SeppoTakalo

@MarkusLassila MarkusLassila force-pushed the zephyr-ppp-fix-upstream branch from 1ccdaed to 91d81e5 Compare March 25, 2025 15:19
Copy link
Contributor

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

Could you try making the receive buffers smaller in the uart backend test suite

static uint8_t backend_receive_buffer[4096];
and add a sleep here
ret = modem_pipe_receive(pipe, receive_buffer, sizeof(receive_buffer));
to try to invoke the need for flowcontrol?

I have a suspicion bytes may be lost by disabling RX outright rather than using flow control (as disable may flush the rx buffer) :)

@SeppoTakalo
Copy link
Contributor

I tried this a little bit.

Using nRF9151, I'm now able to reliably use 1 Mbit/s baud rate on the serial port without losing data on serial buffers.
So this kind of implementation significantly improves the upload performance.

@MarkusLassila
Copy link
Contributor Author

@bjarki-andreasen, sorry about bit of a delay.

Currently the test does not actually test flow control and I don't think that I can change it as I do not believe that there are devices in the CI infrastructure that would have the suitable wiring. I tested this manually in MarkusLassila#1.

With hw-flow-control enabled, no data is dropped. The RX buffers are not flushed when UART is disabled.

Copy link
Contributor

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

Please cherry pick updated test from #89250 and add a test case which enables the new CONFIG_TEST_HW_FLOW_CONTROL option.

@MarkusLassila MarkusLassila force-pushed the zephyr-ppp-fix-upstream branch 3 times, most recently from 7cae5c3 to a0f7c88 Compare April 30, 2025 11:30
Update the board overlays to support hardware flow control for
testing.

Signed-off-by: Bjarki Arge Andreasen <[email protected]>
Extend test modem uart backend test suite to support testing hw
flow control, which is performed by using a small receive
buffer for the modem uart backend, and slowing down the read
of received data, ensuring the buffer will be overrun if hw flow
control is not working.

Signed-off-by: Bjarki Arge Andreasen <[email protected]>
@MarkusLassila
Copy link
Contributor Author

@bjarki-andreasen: For whatever the reason, it does not seem that the CI ran the modem.backends tests.

@MarkusLassila MarkusLassila force-pushed the zephyr-ppp-fix-upstream branch from 68ff3e4 to 994aa6c Compare May 2, 2025 08:51
@bjarki-andreasen
Copy link
Contributor

ping @SeppoTakalo @tomi-font

Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

Reviewed all but modem_backend_uart_async_hwfc.c. Looks good overall, just some minor remarks.

@MarkusLassila MarkusLassila force-pushed the zephyr-ppp-fix-upstream branch 2 times, most recently from ce08cc4 to 307bdc1 Compare May 5, 2025 05:43
@MarkusLassila MarkusLassila requested a review from tomi-font May 5, 2025 05:45
tomi-font
tomi-font previously approved these changes May 5, 2025
@tomi-font tomi-font requested a review from bjarki-andreasen May 5, 2025 07:31
SeppoTakalo
SeppoTakalo previously approved these changes May 5, 2025
Add Asynchronous UART implementation, which does not drop data
when automatic hardware-flow-control is set in the device tree.

With automatic hardware flow control, the CTS pin will be
automatically deactivated when there are no more asynchronous
UART RX buffers available. After buffer space becomes available,
and UART RX is restarted, the CTS pin will be activated.

Signed-off-by: Markus Lassila <[email protected]>
@MarkusLassila MarkusLassila dismissed stale reviews from SeppoTakalo and tomi-font via e7dfa4c May 5, 2025 09:59
@MarkusLassila MarkusLassila force-pushed the zephyr-ppp-fix-upstream branch from 307bdc1 to e7dfa4c Compare May 5, 2025 09:59
@MarkusLassila
Copy link
Contributor Author

@tomi-font, @SeppoTakalo: Seems that a minor change dismissed your reviews. Apologies.

@kartben kartben merged commit 10bd2de into zephyrproject-rtos:main May 5, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants