DTR/RI UART

[MarkusLassila]

JIRA: SLM-105

drivers: DTR/RI virtual UART implementation
app: Take DTR/RI into use
sm_host: DTR/RI implementation for host side

Documentation changes to be done as a separate PR.

[Copilot]

Pull Request Overview

This PR implements DTR (Data Terminal Ready) and RI (Ring Indicator) virtual UART functionality for Serial Modem. It replaces the previous GPIO-based power and indication pin implementation with a standardized DTR/RI UART approach.

• Adds DTR/RI UART driver that implements the UART API with DTR flow control and RI signaling
• Migrates host library from GPIO pin management to DTR/RI automatic UART control
• Updates application and board configurations to use the new DTR/RI device tree bindings

Reviewed Changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
drivers/dtr_uart/	New DTR/RI UART driver implementation with power management and flow control
lib/sm_host/sm_host.c	Refactored to use DTR/RI automatic UART management instead of GPIO power/indicate pins
include/sm_host.h	Updated API to support DTR/RI handler registration and automatic UART configuration
app/src/	Updated application code to remove GPIO power/indicate pin handling and use DTR/RI callbacks
dts/bindings/	New device tree bindings for DTR/RI UART configuration
app/boards/	Board-specific overlays migrated from GPIO config to DTR/RI device tree nodes

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[SeppoTakalo]

I believe all .DTS files now require changes so that it does not leave UART pins floating:

This is a piece from my nrf9160_nrf52840.conf:

&pinctrl {
	uart1_sleep_pull: uart1_sleep_pull {
		group1 {
			psels = <NRF_PSEL(UART_TX, 0, 18)>,
				<NRF_PSEL(UART_RX, 0, 17)>,
				<NRF_PSEL(UART_RTS, 0, 21)>,
				<NRF_PSEL(UART_CTS, 0, 19)>;
			low-power-enable;
			bias-pull-up;
		};
	};
};

So the bias-pull-up sets up the pull-up on the pin when UART is shut down.
This is required so that we don't end up getting lots of garbage in the UART line that is sleeping.

Technically, you could separate those groups so that you only leave pull-up on lines that UART should be driving, not RX lines. I think it is safer.

[SeppoTakalo]

Get rid of unnecessary typecasts in dtr_uart.c

void * converts perfectly to other type when assigning.

diff --git a/drivers/dtr_uart/dtr_uart.c b/drivers/dtr_uart/dtr_uart.c
index 0fe97d7ede5e..75ad12becb79 100644
--- a/drivers/dtr_uart/dtr_uart.c
+++ b/drivers/dtr_uart/dtr_uart.c
@@ -69,25 +69,15 @@ struct dtr_uart_config {

static void user_callback(const struct device *dev, struct uart_event *evt);

-static inline struct dtr_uart_data *get_dev_data(const struct device *dev)
-{
-       return dev->data;
-}
-
static inline const struct dtr_uart_config *get_dev_config(const struct device *dev)
{
        return dev->config;
}

-static inline const struct device *get_dev(struct dtr_uart_data *data)
-{
-       return data->dev;
-}
-
/* --- Power Management --- */
static void power_on_uart(struct dtr_uart_data *data)
{
-       const struct dtr_uart_config *config = get_dev_config(get_dev(data));
+       const struct dtr_uart_config *config = data->dev->config;
        enum pm_device_state state = PM_DEVICE_STATE_OFF;

        int err = pm_device_state_get(config->uart, &state);
@@ -106,7 +96,7 @@ static void power_on_uart(struct dtr_uart_data *data)

static void power_off_uart(struct dtr_uart_data *data)
{
-       const struct dtr_uart_config *config = get_dev_config(get_dev(data));
+       const struct dtr_uart_config *config = data->dev->config;
        enum pm_device_state state = PM_DEVICE_STATE_OFF;

        int err = pm_device_state_get(config->uart, &state);
@@ -134,7 +124,7 @@ static void tx_complete(struct dtr_uart_data *data)

static void activate_tx(struct dtr_uart_data *data)
{
-       const struct dtr_uart_config *config = get_dev_config(get_dev(data));
+       const struct dtr_uart_config *config = data->dev->config;

        if (data->tx_buf) {
                int err;

So only proper usage of inlining the typecast is

gpio_pin_set_dt(&get_dev_config(data->dev)->ri_gpio, 0);

[SeppoTakalo]

Should these be PULL_UP | ACTIVE_LOW, so if nobody is driving the pin it will be in "inactive" state?

[MarkusLassila]

With DK's SM should run immediately when the DK is plugged in. So DTR should be asserted by default. Pushing the button down will then deassert DTR.

I am not 100% on EK board, but I would expect the same functionality to make sense.

[SeppoTakalo]

I'm OK with the code.
We should clarify the <PULL_UP | ACTIVE_HIGH/LOW> but that can be a follow up PR as well.

[SeppoTakalo]

And FYI: functionally this works against my ongoing CMUX+DTR work.

[MarkusLassila]

@SeppoTakalo: #8 (comment) this is not addressed. Follow up questions in chat.

[MarkusLassila]

Ping @SeppoTakalo to review the mdm-pwr-gpios commit.

[SeppoTakalo]

OK.

I think we could fine-tune the example overlays later, we should proceed to merging and testing this PR in CI