Skip to content

Conversation

@nordic-krch
Copy link
Contributor

The order of handling RX events in the interrupt was forced to ENDRX, RXSTARTED, RXTO. It was working ok as long as buffers were not too short (compared to the baudrate) as this order assumed that RXSTARTED event is for the next buffer and that previous RXSTARTED is already handled.

When buffer is short (e.g. 1 byte) then ENDRX and RXSTARTED will cause an error as next buffer cannot be provided on time. It is better to enforce RXSTARTED, ENDRX, RXTO order. Driver is reworked. Additionally, RX interrupt sources are only enabled when RX is enabled. This approach allows to reduce number of register accesses in the interrupt handler.

Extended test to run a configuration where continuous reception happens to 1 byte buffers. Test is failing without the fix.

Complexity in the driver make it hard to apply the fix so driver is refactored. Complexity results from driver implementing 3 UART APIs and 4 receive modes for the asynchronous API:

  1. byte-by-byte counting - first mode implemented, fail to work when CPU is loaded. Interrupt on every byte
  2. ENHANCED_RX mode - reliable reception with HWFC, potential byte loss in the corner cases without HWFC
  3. Using TIMER and PPI for counting bytes for legacy SoCs (nRF52, nRF53 and nRF91) - reliable reception. Enabled in Kconfig
  4. Using TIMER and PPI for counting bytes for new SoCs (nRF54x) - reliable reception. Enabled in DT

Option 1 is removed and "ENHANCED RX" becomes the default so CONFIG_UART_NRFX_UARTE_ENHANCED_RX is deprecated.

Option 3 and 4 are aligned. Kconfig configuration for option 3 is deprecated and DT configuration is used like for option 4. To enable reliable reception without HWFC a timer node need to be added to the uart node.

&timer1 {
  status = "reserved";
};
&uart1 {
  timer = <&timer1>;
};

To pass the test with chopped TX data a dedicated TIMER for byte
counting need to be used.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Remove parts of implementation which are no longer in use.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Interrupt handler was handling events in a following order:
ENDRX, RXSTARTED, RXTO. This could lead to error if RX buffer
is short then with high baudrate RXSTARTED for the current
buffer could be handled together with ENDRX and in that case
uart_rx_buf_rsp called from RXSTARTED would return error as
reception is already finished due to handled ENDRX.

Reworking the driver to change that order to RXSTARTED, ENDRX,
RXTO.

Additionally, driver is optimize to only check HW events for
enabled interrupts. To achieve that, RX path interrupts are
all disabled then RX is disabled.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Extend test to run a scenario where receiver is continuously providing
1 byte buffers and tries to receive loselessly data to that short
buffers.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Add pull-ups to all pins when in sleep state. Otherwise, those pins are
floating and can lead to test failures.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
RX pin must have pull-up like in other boards. It is needed because
2 UART instances are connected and order of initialization is unknown
so if receiver is initialized before the transmitter then RX pin may
float and it can result in the unexpected receiver error.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
For few samples that used cellular modem on nrf9160dk a UART driver
feature was enabled to use TIMER peripheral for byte counting. It used
to be required to reliably received data even with HWFC. Since some
time default receiver has been changed and UART wit HWFC is able to
reliably receive data without need of using TIMER and PPI for byte
counting.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Some time ago a receiver mode for newer SoCs was introduced. It is
using TIMER to count RX bytes via PPI. Due to changes in the system
behavior it is not the same implementation as the one used on
legacy targets (nRF52x, nRF53x and nRF91x). New implementation is
using the Device Tree to assign a TIMER instance and old feature was
using Kconfig which is now obsolete and error prone since it is
easy to get resource usage conflict is some instances are managed
in Device Tree and some in Kconfig.

As a result to reliably receive data without HWFC user had to use
different configuration depending on the target.

Legacy implementation was using nrfx_timer and new one used HAL.

Patch attempts to align better those two similar but different
implementations by:
- deprecating Kconfig symbols for selecting TIMER instance in favor
  of the Device Tree
- extracting common part like PPI initialization and some control
 block fields
- Using TIMER HAL in both implementations.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
Aligning configurations for Nordic targets after changes in the driver.
CONFIG_UART_x_NRF_HW_ASYNC_TIMER option is deprecated. Where valid
Device Tree configuration is added.

Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>
@sonarqubecloud
Copy link

@SeppoTakalo
Copy link
Contributor

@nordic-krch is there any downsides of using timer node in DT even when HWFC is used?

I'm thinking if this is easier to document/implement on some use-cases, or even allow runtime switching of FC?

@nordic-krch
Copy link
Contributor Author

@SeppoTakalo using TIMER on legacy SoCs like nRF91 is mainly resources cost (TIMER instance + DPPI channel). In terms of performance it is similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Modem area: Networking area: Samples Samples area: Tests Issues related to a particular existing or missing test area: UART Universal Asynchronous Receiver-Transmitter platform: nRF Nordic nRFx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants