From 01e1dcb2e88607d4aa04f169a081397644196066 Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Fri, 12 Sep 2025 12:50:41 +0300 Subject: [PATCH] SLM: Refactor DTR+RI UART handler Work in progress: Not ready. * Refactor to use just Zephyr GPIO API * Use Devicetree to define pins * Refactor DTR UP/DOWN events to ASSERT/DEASSERT * Logic level 1 is asserted, use DT to change 0/1 level of that. * Remove support of DTE device, that is separate driver TODO: * sw_dtr should not keep RX buffers. Instead when RX is enabled, feed buffer to UART from application call. * When buffer is needed, make a callback to app to request a buffer. * When UART is powered down because of DTR, block RX_STOP and RX_DISABLE events going to application, so app thinks the UART keeps on going. Signed-off-by: Seppo Takalo --- ...erlay-zephyr-modem-nrf9160dk-nrf52840.conf | 28 +- ...ay-zephyr-modem-nrf9160dk-nrf52840.overlay | 9 +- drivers/serial/uart_nrf_sw_dtr_uart.c | 838 ++++++------------ .../serial/nordic,nrf-sw-dtr-uart.yaml | 21 +- 4 files changed, 293 insertions(+), 603 deletions(-) diff --git a/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.conf b/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.conf index 9666340ec8ad..0732f5049ca0 100644 --- a/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.conf +++ b/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.conf @@ -5,4 +5,30 @@ # # nRF52 <=> nRF91 interface pin 4 (see https://docs.nordicsemi.com/bundle/ug_nrf91_dk/page/UG/nrf91_DK/board_controller.html) -CONFIG_SLM_POWER_PIN=22 +CONFIG_SLM_POWER_PIN=-1 + +CONFIG_USE_SEGGER_RTT=n +# Where console messages (printk) are output. +# By itself, SLM does not output any. +CONFIG_RTT_CONSOLE=n +CONFIG_UART_CONSOLE=y +# Where SLM logs are output. +CONFIG_LOG_BACKEND_RTT=n +CONFIG_LOG_BACKEND_UART=y + +CONFIG_SHELL=y +CONFIG_GPIO=y +CONFIG_GPIO_SHELL=y +CONFIG_PM_DEVICE_SHELL=y + +CONFIG_UART_1_INTERRUPT_DRIVEN=n +CONFIG_UART_1_ASYNC=y +CONFIG_UART_USE_RUNTIME_CONFIGURE=y +CONFIG_UART_ASYNC_API=y + +CONFIG_SLM_SMS=n +CONFIG_SLM_GNSS=n +CONFIG_SLM_NRF_CLOUD=n +CONFIG_SLM_GPIO=n + +CONFIG_SIZE_OPTIMIZATIONS_AGGRESSIVE=y diff --git a/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.overlay b/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.overlay index 45d7faa85b13..b32d5581ccd8 100644 --- a/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.overlay +++ b/applications/serial_lte_modem/overlay-zephyr-modem-nrf9160dk-nrf52840.overlay @@ -8,11 +8,18 @@ / { chosen { - ncs,slm-uart = &uart1; + ncs,slm-uart = &dtr_uart; }; }; &uart1 { current-speed = <115200>; hw-flow-control; + status = "okay"; + dtr_uart: nrf-sw-dtr-uart { + compatible = "nordic,nrf-sw-dtr-uart"; + dtr-gpios = <&interface_to_nrf52840 4 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>; + ri-gpios = <&interface_to_nrf52840 5 (GPIO_ACTIVE_LOW)>; + status = "okay"; + }; }; diff --git a/drivers/serial/uart_nrf_sw_dtr_uart.c b/drivers/serial/uart_nrf_sw_dtr_uart.c index 365fa89897bd..1aaea35a621b 100644 --- a/drivers/serial/uart_nrf_sw_dtr_uart.c +++ b/drivers/serial/uart_nrf_sw_dtr_uart.c @@ -8,81 +8,41 @@ #include #include #include -#include #include #include #include +/* + * DTR (Data Terminal Ready) Logic: + * + * This driver implements DTR flow control where DTR input levels directly + * correspond to DTR assertion/deassertion events: + * + * DTR Input Level 0 → DTR_DEASSERTED → UART inactive (powered down) + * DTR Input Level 1 → DTR_ASSERTED → UART active (powered up and ready) + * + * Internal state representation (matches input level): + * - dtr_state = 0: DTR deasserted, UART inactive + * - dtr_state = 1: DTR asserted, UART active + */ + LOG_MODULE_REGISTER(dtr_uart, CONFIG_NRF_SW_DTR_UART_LOG_LEVEL); #define DT_DRV_COMPAT nordic_nrf_sw_dtr_uart -/* States. */ -enum dtr_state { - - /* DTR is up and we are disabled. */ - DTR_UP, - - /* DTR is pulled down to activate UART. */ - DTR_UP_TO_DOWN, - - /* DTR is down and we are active. */ - DTR_DOWN, - - /* DTR is pulled up to disable UART. */ - DTR_DOWN_TO_UP, -}; - -static const char *dtr_state_str(enum dtr_state state) -{ - switch (state) { - case DTR_UP: - return "UP"; - case DTR_UP_TO_DOWN: - return "UP to DOWN"; - case DTR_DOWN: - return "DOWN"; - case DTR_DOWN_TO_UP: - return "DOWN to UP"; - } - - return ""; -} - enum dtr_uart_event { - - /* DTR is up DTE/DCE will be be disabled. */ - DTR_UART_EVENT_UP = 0, - - /* DTR is down, DTE/DCE will be activated. */ - DTR_UART_EVENT_DOWN, - - /* RX buffer has been received. */ - DTR_UART_EVENT_RX_BUF_RECEIVED, - /* RX disabled event. */ DTR_UART_EVENT_RX_DISABLED, - /* DTE event: DCE is ready or sufficient time has passed to assume it is. */ - DTR_UART_EVENT_DCE_READY, - - /* DTE event: No traffic between DTE and DCE for a configured timeout -> DTR UP. */ + /* DTE event: No traffic between DTE and DCE for a configured timeout -> DTR deasserted. */ DTR_UART_EVENT_IDLE, }; static const char *dtr_event_str(enum dtr_uart_event event) { switch (event) { - case DTR_UART_EVENT_UP: - return "DTR UP"; - case DTR_UART_EVENT_DOWN: - return "DTR DOWN"; - case DTR_UART_EVENT_RX_BUF_RECEIVED: - return "RX BUF RECEIVED"; case DTR_UART_EVENT_RX_DISABLED: return "RX DISABLED"; - case DTR_UART_EVENT_DCE_READY: - return "DCE READY"; case DTR_UART_EVENT_IDLE: return "DTR IDLE"; } @@ -127,30 +87,6 @@ struct dtr_uart_data { /* Device structure */ // MARKUS TODO: This is us, should be a nicer way to get. const struct device *dev; - /* Physical UART device */ - const struct device *uart; - - /* Has the application called rx_enable?*/ - bool app_rx_enable; - - /* Data Terminal Equipment role. */ - bool dte_role; - - /* Data Terminal Ready pin. */ - nrfx_gpiote_pin_t dtr_pin; - - /* DTR pin for DCE uses toggle on high-to-low, instead of level trigger */ - bool dtr_pin_toggle; - - /* Debounce time for DTR pin. */ - uint32_t dtr_pin_debounce; - - /* Input pin trigger scheduled due to debounce. */ - nrfx_gpiote_trigger_t input_pin_trigger; - - /* Ring Indicator pin. */ - nrfx_gpiote_pin_t ri_pin; - /* Timer used for TX timeouting. */ struct k_timer tx_timer; @@ -158,29 +94,25 @@ struct dtr_uart_data { const uint8_t *tx_buf; size_t tx_len; - /* Current RX buffer. */ - uint8_t *rx_buf; - size_t rx_len; - bool rx_buf_requested; + bool app_rx_enable; + bool rx_active; uart_callback_t user_callback; void *user_data; - /* DTR state */ - enum dtr_state dtr_state; + /* DTR state: 0 = deasserted (UART inactive), 1 = asserted (UART active) */ + bool dtr_state; + struct gpio_callback dtr_cb; /* Worker and message queue for DTR events */ struct k_work event_queue_work; struct k_msgq event_queue; enum dtr_uart_event event_queue_buf[8]; - struct k_work dtr_event_work; + struct k_work_delayable dtr_work; /* One timeout event can run at a time */ enum dtr_timeout_event dtr_timeout_event; struct k_work_delayable dtr_timeout_work; - - /* Delayed activation for input trigger. */ - struct k_work_delayable input_trigger_work; }; /* Forward declarations. */ @@ -188,11 +120,10 @@ static void user_callback(const struct device *dev, struct uart_event *evt); /* Configuration structured. */ struct dtr_uart_config { - bool dte_role; - nrfx_gpiote_pin_t dtr_pin; - nrfx_gpiote_pin_t ri_pin; - bool dtr_pin_toggle; - uint32_t dtr_pin_debounce; + /* Physical UART device */ + const struct device *uart; + struct gpio_dt_spec dtr_gpio; + struct gpio_dt_spec ri_gpio; }; static inline struct dtr_uart_data *get_dev_data(const struct device *dev) @@ -210,39 +141,14 @@ static inline const struct device *get_dev(struct dtr_uart_data *data) return data->dev; } -#define GPIOTE_NODE(gpio_node) DT_PHANDLE(gpio_node, gpiote_instance) -#define GPIOTE_INST_AND_COMMA(gpio_node) \ - IF_ENABLED(DT_NODE_HAS_PROP(gpio_node, gpiote_instance), ( \ - [DT_PROP(gpio_node, port)] = \ - NRFX_GPIOTE_INSTANCE(DT_PROP(GPIOTE_NODE(gpio_node), instance)),)) - -static const nrfx_gpiote_t *get_gpiote(nrfx_gpiote_pin_t pin) -{ - static const nrfx_gpiote_t gpiote[GPIO_COUNT] = { - DT_FOREACH_STATUS_OKAY(nordic_nrf_gpio, GPIOTE_INST_AND_COMMA) - }; - - return &gpiote[pin >> 5]; -} - -static void dtr_down(struct dtr_uart_data *data) -{ - LOG_INF("dtr_down"); - nrf_gpio_pin_clear(data->dtr_pin); -} - -static void dtr_up(struct dtr_uart_data *data) -{ - LOG_INF("dtr_up"); - nrf_gpio_pin_set(data->dtr_pin); -} - static void activate_tx(struct dtr_uart_data *data) { + const struct dtr_uart_config *config = get_dev_config(get_dev(data)); + if (data->tx_buf && data->tx_len > 0) { int err; - err = uart_tx(data->uart, data->tx_buf, data->tx_len, SYS_FOREVER_US); + err = uart_tx(config->uart, data->tx_buf, data->tx_len, SYS_FOREVER_US); if (err < 0) { LOG_ERR("TX: Not started (error: %d)", err); // tx_complete(data); @@ -252,12 +158,14 @@ static void activate_tx(struct dtr_uart_data *data) static void deactivate_tx(struct dtr_uart_data *data) { + const struct dtr_uart_config *config = get_dev_config(get_dev(data)); + // MARKUS TODO: This needs to take care of scenario where we have not yet sent at all. if (data->tx_buf) { int err; // MARKUS TODO: We must wait for the abort to take place before we disable UART. - err = uart_tx_abort(data->uart); + err = uart_tx_abort(config->uart); if (err < 0) { LOG_ERR("TX: Abort (error: %d)", err); // tx_complete(data); @@ -273,56 +181,37 @@ static void tx_complete(struct dtr_uart_data *data) static void deactivate_rx(struct dtr_uart_data *data) { + const struct dtr_uart_config *config = get_dev_config(get_dev(data)); int err; + if (!data->rx_active) { + LOG_WRN("RX: Not active"); + return; + } + data->rx_active = false; /* abort rx */ - err = uart_rx_disable(data->uart); + err = uart_rx_disable(config->uart); if (err) { LOG_ERR("RX: Failed to disable (err: %d)", err); } } -static void request_rx_buffer(struct dtr_uart_data *data) -{ - if (data->rx_buf == NULL) { - LOG_INF("RX: No buffer to enable RX -> Request buffer"); - - struct uart_event evt = { - .type = UART_RX_BUF_REQUEST, - }; - - /* Buffer should be immediately received. */ - data->rx_buf_requested = true; - user_callback(get_dev(data), &evt); - data->rx_buf_requested = false; - } - - if (data->rx_buf) { - LOG_INF("RX: Buffer is ready"); - } else { - // MARKUS TODO: Try again later? - } -} - static void activate_rx(struct dtr_uart_data *data) { - int err; - - __ASSERT(data->rx_buf != NULL && data->rx_len > 0, "RX: No buffer to enable RX"); + if (data->rx_active) { + LOG_WRN("RX: Already active"); + return; + } - // MARKUS TODO: This should use events for retrial. - err = uart_rx_enable(data->uart, data->rx_buf, data->rx_len, 2000); - if (err < 0) { - LOG_ERR("RX: Enabling failed (err: %d)", err); - if (err == -EBUSY) { - k_sleep(K_MSEC(10)); - err = uart_rx_enable(data->uart, data->rx_buf, data->rx_len, 2000); - } - __ASSERT(err == 0 || err == -EALREADY, "RX: Enabling failed (err:%d)", err); + if (!data->app_rx_enable) { + LOG_WRN("RX: Not enabled by app"); + return; } - data->rx_buf = NULL; - data->rx_len = 0; + struct uart_event evt = { + .type = UART_RX_BUF_REQUEST, + }; + user_callback(get_dev(data), &evt); } static void event_queue_delegate(struct dtr_uart_data *data, enum dtr_uart_event evt) @@ -358,27 +247,19 @@ static void cancel_dtr_timeout_event(struct dtr_uart_data *data) k_work_cancel_delayable(&data->dtr_timeout_work); } -static void notify_idle(struct dtr_uart_data *data) -{ - if (IS_ENABLED(CONFIG_NRF_SW_DTR_UART_DTE_IDLE) && data->dte_role && - data->dtr_state == DTR_DOWN) { - set_dtr_timeout_event(data, DTR_TIMEOUT_DTR_IDLE, - K_MSEC(CONFIG_NRF_SW_DTR_UART_DTE_IDLE_TIMEOUT_MS)); - } -} - static int power_on_uart(struct dtr_uart_data *data) { + const struct dtr_uart_config *config = get_dev_config(get_dev(data)); enum pm_device_state state = PM_DEVICE_STATE_OFF; - int err = pm_device_state_get(data->uart, &state); + int err = pm_device_state_get(config->uart, &state); if (err) { LOG_ERR("Failed to get PM device state: %d", err); return err; } if (state != PM_DEVICE_STATE_ACTIVE) { /* Power on UART module */ - err = pm_device_action_run(data->uart, PM_DEVICE_ACTION_RESUME); + err = pm_device_action_run(config->uart, PM_DEVICE_ACTION_RESUME); if (err) { LOG_ERR("Failed to resume UART device: %d", err); } @@ -389,16 +270,17 @@ static int power_on_uart(struct dtr_uart_data *data) static int power_off_uart(struct dtr_uart_data *data) { + const struct dtr_uart_config *config = get_dev_config(get_dev(data)); enum pm_device_state state = PM_DEVICE_STATE_ACTIVE; - int err = pm_device_state_get(data->uart, &state); + int err = pm_device_state_get(config->uart, &state); if (err) { LOG_ERR("Failed to get PM device state: %d", err); return err; } if (state != PM_DEVICE_STATE_SUSPENDED) { /* Power off UART module */ - err = pm_device_action_run(data->uart, PM_DEVICE_ACTION_SUSPEND); + err = pm_device_action_run(config->uart, PM_DEVICE_ACTION_SUSPEND); if (err) { LOG_ERR("Failed to suspend UART device: %d", err); } @@ -406,70 +288,11 @@ static int power_off_uart(struct dtr_uart_data *data) return err; } -static void event_down_work(struct dtr_uart_data *data) -{ - if (data->dtr_state == DTR_DOWN || data->dtr_state == DTR_UP_TO_DOWN) { - LOG_WRN("DTR is already %s, ignoring event", "down"); - return; - } - - data->dtr_state = DTR_UP_TO_DOWN; - - if (!data->dte_role) { - /* DCE - stop RI */ - cancel_dtr_timeout_event(data); - nrf_gpio_pin_clear(data->ri_pin); - } - - /* Power on UART module */ - const int err = power_on_uart(data); - if (err) { - LOG_ERR("Failed to resume UART device: %d", err); - } - - request_rx_buffer(data); - activate_rx(data); - - if (data->dte_role) { - /* DTE - Wait for DCE to be ready. */ - dtr_down(data); - // MARKUS TODO: Configurable DTR activation timeout. - set_dtr_timeout_event(data, DTR_TIMEOUT_DTR_ACTIVATION, K_MSEC(100)); - - /* Continues in event_dce_ready_work. */ - - } else { - /* DCE - is ready at this point. */ - data->dtr_state = DTR_DOWN; - activate_tx(data); - } -} - -static void event_up_work(struct dtr_uart_data *data) -{ - if (data->dtr_state == DTR_UP || data->dtr_state == DTR_DOWN_TO_UP) { - LOG_WRN("DTR is already %s, ignoring event", "up"); - return; - } - - data->dtr_state = DTR_DOWN_TO_UP; - - if (data->dte_role) { - /* DTE */ - dtr_up(data); - } - - deactivate_tx(data); - deactivate_rx(data); - - /* Wait for UART to be disabled, before powering it down. */ -} - static void event_rx_disabled_work(struct dtr_uart_data *data) { - if (data->dtr_state != DTR_DOWN_TO_UP) { - LOG_INF("RX disabled event in state %s, ignoring event", - dtr_state_str(data->dtr_state)); + data->rx_active = false; + if (data->dtr_state == 1) { + LOG_INF("RX disabled event in state %d, not suspending", data->dtr_state); return; } @@ -478,19 +301,6 @@ static void event_rx_disabled_work(struct dtr_uart_data *data) if (err) { LOG_ERR("Failed to suspend UART device: %d", err); } - - data->dtr_state = DTR_UP; -} - -static void event_dce_ready_work(struct dtr_uart_data *data) -{ - /* DCE has notified DTE that it is ready, or sufficient time has passed. */ - data->dtr_state = DTR_DOWN; - - /* Send data that is ready to be transmitted. */ - activate_tx(data); - - notify_idle(data); } /**************************** WORKERS **********************************/ @@ -499,36 +309,21 @@ static void event_queue_work_fn(struct k_work *work) struct dtr_uart_data *data = CONTAINER_OF(work, struct dtr_uart_data, event_queue_work); enum dtr_uart_event evt; - // if (data->user_callback == NULL || !data->app_rx_enable) { - // LOG_INF("DTR events are not ready to be processed."); - // return; - // } - while (k_msgq_get(&data->event_queue, &evt, K_NO_WAIT) == 0) { - LOG_INF("DTR event \"%s\" in state %s", dtr_event_str(evt), - dtr_state_str(data->dtr_state)); + LOG_INF("DTR event \"%s\" in state %d", dtr_event_str(evt), data->dtr_state); switch (evt) { - case DTR_UART_EVENT_UP: case DTR_UART_EVENT_IDLE: - event_up_work(data); break; case DTR_UART_EVENT_RX_DISABLED: event_rx_disabled_work(data); break; - case DTR_UART_EVENT_DOWN: - event_down_work(data); - break; - case DTR_UART_EVENT_DCE_READY: - event_dce_ready_work(data); - break; default: LOG_WRN("Unknown event: %d", evt); break; } - LOG_INF("DTR event \"%s\" done, new state %s", dtr_event_str(evt), - dtr_state_str(data->dtr_state)); + LOG_INF("DTR event \"%s\" done, new state %d", dtr_event_str(evt), data->dtr_state); } } @@ -538,16 +333,15 @@ static void dtr_timeout_work_fn(struct k_work *work) struct dtr_uart_data *data = CONTAINER_OF(delayed_work, struct dtr_uart_data, dtr_timeout_work); - LOG_INF("DTR timeout event \"%s\" in state %s", dtr_timeout_str(data->dtr_timeout_event), - dtr_state_str(data->dtr_state)); + LOG_INF("DTR timeout event \"%s\" in state %d", dtr_timeout_str(data->dtr_timeout_event), + data->dtr_state); switch (data->dtr_timeout_event) { case DTR_TIMEOUT_RI: - nrf_gpio_pin_clear(data->ri_pin); + gpio_pin_set_dt(&get_dev_config(get_dev(data))->ri_gpio, 0); break; case DTR_TIMEOUT_DTR_ACTIVATION: - event_queue_delegate(data, DTR_UART_EVENT_DCE_READY); - break; + /* TODO: handle error */ case DTR_TIMEOUT_DTR_IDLE: event_queue_delegate(data, DTR_UART_EVENT_IDLE); break; @@ -561,175 +355,53 @@ static void dtr_timeout_work_fn(struct k_work *work) static void ri_start(struct dtr_uart_data *data) { - nrf_gpio_pin_set(data->ri_pin); + gpio_pin_set_dt(&get_dev_config(get_dev(data))->ri_gpio, 1); set_dtr_timeout_event(data, DTR_TIMEOUT_RI, K_MSEC(1000)); - } /******************************* GPIO ********************************/ -/* Handler is called when transition in state is detected which indicates - * that for DCE, DTR is toggled or for DTE, RI is toggled. - */ -static void input_pin_handler(nrfx_gpiote_pin_t pin, - nrfx_gpiote_trigger_t trigger, - void *context) +static void uart_dtr_input_gpio_callback(const struct device *port, struct gpio_callback *cb, + uint32_t pins) { - nrfx_gpiote_trigger_disable(get_gpiote(pin), pin); - - LOG_INF("Input pin handler called, pin: %d, trigger: %d", pin, trigger); - - struct dtr_uart_data *data = context; - - if (data->dte_role) { - /* DTE - RI pin change from low to high triggers event. */ - if (data->dtr_state != DTR_DOWN && trigger == NRFX_GPIOTE_TRIGGER_HIGH) { - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - - /* MARKUS TODO: If we do not want to immediately bring the DTR DOWN with RI, - * we would need to request a buffer, which the application - * would take as a note that RI has risen. Buffer providing - * or rx_enable would then be a sign to bring the DTR DOWN. */ - } - } else { - /* DCE - DTR triggered. */ - if (!data->dtr_pin_toggle) { - /* DTR pin level change triggers a corresponding event. */ - if (trigger == NRFX_GPIOTE_TRIGGER_LOW) { - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - } else { - event_queue_delegate(data, DTR_UART_EVENT_UP); - } - } else if (trigger == NRFX_GPIOTE_TRIGGER_LOW) { - /* DTR pin change from high to low triggers toggle. */ - if (data->dtr_state == DTR_UP || data->dtr_state == DTR_DOWN_TO_UP) { - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - } else { - event_queue_delegate(data, DTR_UART_EVENT_UP); - } - } - } - data->input_pin_trigger = trigger == NRFX_GPIOTE_TRIGGER_LOW ? NRFX_GPIOTE_TRIGGER_HIGH - : NRFX_GPIOTE_TRIGGER_LOW; - k_work_schedule(&data->input_trigger_work, K_MSEC(data->dtr_pin_debounce)); -} - -static void input_trigger_work_fn(struct k_work *work) -{ - struct k_work_delayable *delayed_work = CONTAINER_OF(work, struct k_work_delayable, work); - struct dtr_uart_data *data = - CONTAINER_OF(delayed_work, struct dtr_uart_data, input_trigger_work); - - nrfx_gpiote_pin_t pin = data->dte_role ? data->ri_pin : data->dtr_pin; - int err; - - nrfx_gpiote_trigger_config_t trigger_config = { - .trigger = data->input_pin_trigger, - }; - - nrf_gpio_pin_pull_t pull_config; - - if (data->dtr_pin_toggle) { - /* Button must always have a pull-up resistor */ - pull_config = NRF_GPIO_PIN_PULLUP; - } else if (trigger_config.trigger == NRFX_GPIOTE_TRIGGER_LOW) { - pull_config = NRF_GPIO_PIN_PULLUP; - } else { - pull_config = NRF_GPIO_PIN_PULLDOWN; - } - - nrfx_gpiote_handler_config_t handler_config = { - .handler = input_pin_handler, - .p_context = data - }; - nrfx_gpiote_input_pin_config_t input_config = { - .p_pull_config = &pull_config, - .p_trigger_config = &trigger_config, - .p_handler_config = &handler_config - }; - - err = nrfx_gpiote_input_configure(get_gpiote(pin), pin, &input_config); - __ASSERT(err == NRFX_SUCCESS, "GPIO input configure (err:%d)", err); + struct dtr_uart_data *data = CONTAINER_OF(cb, struct dtr_uart_data, dtr_cb); - LOG_INF("input_trigger_work_fn, pin: %d, trigger: %d, pull: %d", pin, - trigger_config.trigger, pull_config); - - nrfx_gpiote_trigger_enable(get_gpiote(pin), pin, true); + k_work_reschedule(&data->dtr_work, K_MSEC(10)); } -static int input_pin_init(struct dtr_uart_data *data, nrfx_gpiote_pin_t pin, - nrfx_gpiote_trigger_t trigger) +static void dtr_work_handler(struct k_work *work) { - __ASSERT(trigger == NRFX_GPIOTE_TRIGGER_LOW || trigger == NRFX_GPIOTE_TRIGGER_HIGH, - "Only level triggers are supported."); - - nrfx_err_t err; - nrf_gpio_pin_pull_t pull_config = - trigger == NRFX_GPIOTE_TRIGGER_HIGH ? NRF_GPIO_PIN_PULLDOWN : NRF_GPIO_PIN_PULLUP; - - nrfx_gpiote_trigger_config_t trigger_config = { - .trigger = trigger, - }; - - nrfx_gpiote_handler_config_t handler_config = { - .handler = input_pin_handler, - .p_context = data - }; - nrfx_gpiote_input_pin_config_t input_config = { - .p_pull_config = &pull_config, - .p_trigger_config = &trigger_config, - .p_handler_config = &handler_config - }; - - LOG_INF("Input pin init, pin: %d, trigger: %d, pull: %d", pin, trigger, pull_config); + struct k_work_delayable *dwork = k_work_delayable_from_work(work); + struct dtr_uart_data *data = CONTAINER_OF(dwork, struct dtr_uart_data, dtr_work); + const struct dtr_uart_config *cfg = get_dev_config(data->dev); + bool asserted = gpio_pin_get_dt(&cfg->dtr_gpio); - nrfx_gpiote_trigger_disable(get_gpiote(pin), pin); - - err = nrfx_gpiote_input_configure(get_gpiote(pin), pin, &input_config); - if (err != NRFX_SUCCESS) { - LOG_INF("nrfx_gpiote_input_configure: %d", err); - return -EINVAL; + if (data->dtr_state == asserted) { + LOG_WRN("DTR is already %s, ignoring event", asserted ? "asserted" : "deasserted"); + return; } - LOG_INF("Input pin init, before enabling trigger"); - - // nrfx_gpiote_trigger_enable(get_gpiote(pin), pin, true); - - LOG_INF("Input pin init done, pin: %d, trigger: %d, pull: %d", pin, trigger, pull_config); + data->dtr_state = asserted; - return 0; -} - -static void init_input_pin_trigger(struct dtr_uart_data *data) -{ - static bool initialized; - nrfx_gpiote_pin_t pin; - - if (!initialized) { - pin = data->dte_role ? data->ri_pin : data->dtr_pin; - nrfx_gpiote_trigger_enable(get_gpiote(pin), pin, true); - initialized = true; - } -} + if (asserted) { + cancel_dtr_timeout_event(data); + gpio_pin_set_dt(&get_dev_config(get_dev(data))->ri_gpio, 0); -static int output_pin_init(struct dtr_uart_data *data, nrfx_gpiote_pin_t pin, bool set) -{ - nrfx_err_t err; - nrfx_gpiote_output_config_t output_config = NRFX_GPIOTE_DEFAULT_OUTPUT_CONFIG; + /* Power on UART module */ + const int err = power_on_uart(data); - err = nrfx_gpiote_output_configure(get_gpiote(pin), pin, &output_config, NULL); - if (err != NRFX_SUCCESS) { - LOG_ERR("err:%08x", err); - return -EINVAL; - } + if (err) { + LOG_ERR("Failed to resume UART device: %d", err); + } - if (set) { - nrf_gpio_pin_set(pin); + activate_rx(data); + activate_tx(data); } else { - nrf_gpio_pin_clear(pin); - } + deactivate_tx(data); + deactivate_rx(data); - return 0; + /* Wait for UART to be disabled, before powering it down. */ + } } /****************************** UART ********************************/ @@ -743,8 +415,7 @@ static void user_callback(const struct device *dev, struct uart_event *evt) } } -static void uart_callback(const struct device *uart, struct uart_event *evt, - void *user_data) +static void uart_callback(const struct device *uart, struct uart_event *evt, void *user_data) { struct device *dev = user_data; struct dtr_uart_data *data = get_dev_data(dev); @@ -761,13 +432,9 @@ static void uart_callback(const struct device *uart, struct uart_event *evt, user_callback(dev, evt); break; case UART_RX_RDY: - LOG_INF("RX: Ready buf:%p, offset: %d,len: %d", - (void *)evt->data.rx.buf, evt->data.rx.offset, evt->data.rx.len); + LOG_INF("RX: Ready buf:%p, offset: %d,len: %d", (void *)evt->data.rx.buf, + evt->data.rx.offset, evt->data.rx.len); user_callback(dev, evt); - if (data->dtr_state == DTR_UP_TO_DOWN) { - set_dtr_timeout_event(data, DTR_TIMEOUT_DTR_ACTIVATION, K_NO_WAIT); - } - notify_idle(data); break; case UART_RX_BUF_REQUEST: @@ -780,16 +447,23 @@ static void uart_callback(const struct device *uart, struct uart_event *evt, user_callback(dev, evt); break; - case UART_RX_DISABLED: - { + case UART_RX_DISABLED: { LOG_INF("UART_RX_DISABLED %d", data->dtr_state); - user_callback(dev, evt); + /* When RX disabled because of DTR down, we handle it ourselves. */ + if (data->dtr_state && data->app_rx_enable) { + data->app_rx_enable = false; + user_callback(dev, evt); + } + event_queue_delegate(data, DTR_UART_EVENT_RX_DISABLED); break; } case UART_RX_STOPPED: LOG_INF("Rx stopped"); - user_callback(dev, evt); + if (data->dtr_state && data->app_rx_enable) { + user_callback(dev, evt); + } + event_queue_delegate(data, DTR_UART_EVENT_RX_DISABLED); break; } } @@ -799,79 +473,88 @@ static int dtr_uart_init(const struct device *dev) struct dtr_uart_data *data = get_dev_data(dev); const struct dtr_uart_config *cfg = get_dev_config(dev); int err; + data->dev = dev; - data->uart = DEVICE_DT_GET(DT_INST_BUS(0)); - if (!device_is_ready(data->uart)) { + if (!device_is_ready(cfg->uart)) { + LOG_ERR("UART device not ready"); return -ENODEV; } - data->dev = (const struct device *)dev; - data->dte_role = cfg->dte_role; /* DTR */ - data->dtr_pin = cfg->dtr_pin; - data->dtr_pin_toggle = cfg->dtr_pin_toggle; - data->dtr_pin_debounce = cfg->dtr_pin_debounce; + if (!gpio_is_ready_dt(&cfg->dtr_gpio)) { + LOG_ERR("DTR GPIO not ready"); + return -ENODEV; + } + + /* Configure DTR GPIO as input */ + err = gpio_pin_configure_dt(&cfg->dtr_gpio, GPIO_INPUT); + if (err < 0) { + LOG_ERR("Failed to configure DTR GPIO: %d", err); + return err; + } /* RI */ - data->ri_pin = cfg->ri_pin; + if (!gpio_is_ready_dt(&cfg->ri_gpio)) { + LOG_ERR("RI GPIO not ready"); + return -ENODEV; + } + /* Configure RI GPIO as output, initially inactive */ + err = gpio_pin_configure_dt(&cfg->ri_gpio, GPIO_OUTPUT_INACTIVE); + if (err < 0) { + LOG_ERR("Failed to configure RI GPIO: %d", err); + return err; + } k_msgq_init(&data->event_queue, (char *)&data->event_queue_buf, sizeof(enum dtr_uart_event), sizeof(data->event_queue_buf) / sizeof(enum dtr_uart_event)); k_work_init(&data->event_queue_work, event_queue_work_fn); - + k_work_init_delayable(&data->dtr_work, dtr_work_handler); k_work_init_delayable(&data->dtr_timeout_work, dtr_timeout_work_fn); - k_work_init_delayable(&data->input_trigger_work, input_trigger_work_fn); - if (data->dte_role) { - /* DTE */ - /* At startup, set DTR high to disable DCE UART. */ - err = output_pin_init(data, data->dtr_pin, true); - if (err < 0) { - LOG_ERR("dtr pin init failed:%d", err); - return err; - } - /* Expect high RI pulse from DCE. */ - err = input_pin_init(data, data->ri_pin, NRFX_GPIOTE_TRIGGER_HIGH); - if (err < 0) { - LOG_ERR("ri pin init failed:%d", err); - return err; - } - } else { - /* DCE */ - if (data->dtr_pin_toggle) { - /* With DK, we start from (nearly) active. - * Application calls uart_rx_enable to finalize UART activation. - */ - data->dtr_state = DTR_UP_TO_DOWN; // MARKUS TODO: Unnecessary? - } - /* Expect DTR low from DTE to enable UART. */ - err = input_pin_init(data, data->dtr_pin, NRFX_GPIOTE_TRIGGER_LOW); - if (err < 0) { - LOG_ERR("dtr pin init failed:%d", err); - return err; - } + /* Read initial DTR state */ + int initial_dtr_state = gpio_pin_get_dt(&cfg->dtr_gpio); - /* Set sense to wake the DCE from possible shutdown */ - nrf_gpio_cfg_sense_set(data->dtr_pin, NRF_GPIO_PIN_SENSE_LOW); + if (initial_dtr_state < 0) { + LOG_ERR("Failed to read initial DTR state: %d", initial_dtr_state); + return initial_dtr_state; + } + /* Map GPIO input level directly to DTR state: + * Input level 0 → DTR deasserted (dtr_state = 0, UART inactive) + * Input level 1 → DTR asserted (dtr_state = 1, UART active) + */ + data->dtr_state = initial_dtr_state; - err = output_pin_init(data, data->ri_pin, false); - if (err < 0) { - LOG_ERR("ri pin init failed:%d", err); - return err; - } + /* TODO: How to do this from Zephyr API: Set sense to wake the DCE from possible shutdown */ + /* nrf_gpio_cfg_sense_set(data->dtr_pin, NRF_GPIO_PIN_SENSE_LOW); */ + + /* Set up GPIO interrupt for DTR changes */ + gpio_init_callback(&data->dtr_cb, uart_dtr_input_gpio_callback, BIT(cfg->dtr_gpio.pin)); + + err = gpio_add_callback(cfg->dtr_gpio.port, &data->dtr_cb); + if (err < 0) { + LOG_ERR("Failed to add DTR GPIO callback: %d", err); + return err; } - err = uart_callback_set(data->uart, uart_callback, (void *)dev); + + err = gpio_pin_interrupt_configure_dt(&cfg->dtr_gpio, GPIO_INT_EDGE_BOTH); + if (err < 0) { + LOG_ERR("Failed to configure DTR GPIO interrupt: %d", err); + return err; + } + + err = uart_callback_set(cfg->uart, uart_callback, (void *)dev); if (err < 0) { return -EINVAL; } - return err; + LOG_DBG("DTR UART initialized, initial DTR state: %d", data->dtr_state); + + return 0; } /**************************** API ********************************/ -static int api_callback_set(const struct device *dev, uart_callback_t callback, - void *user_data) +static int api_callback_set(const struct device *dev, uart_callback_t callback, void *user_data) { struct dtr_uart_data *data = get_dev_data(dev); @@ -881,10 +564,11 @@ static int api_callback_set(const struct device *dev, uart_callback_t callback, return 0; } -static int api_tx(const struct device *dev, const uint8_t *buf, - size_t len, int32_t timeout) +static int api_tx(const struct device *dev, const uint8_t *buf, size_t len, int32_t timeout) { - if(buf == NULL || len == 0) { + const struct dtr_uart_config *config = get_dev_config(dev); + + if (buf == NULL || len == 0) { return 0; } LOG_INF("api_tx: %.*s", len, buf); @@ -896,21 +580,14 @@ static int api_tx(const struct device *dev, const uint8_t *buf, return -EBUSY; } - notify_idle(data); - - if (data->dtr_state == DTR_DOWN) { - return uart_tx(data->uart, buf, len, timeout); + if (data->dtr_state == 1) { + return uart_tx(config->uart, buf, len, timeout); } data->tx_buf = buf; data->tx_len = len; - if (data->dte_role) { - /* DTE - Activate DCE and own UART. */ - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - } else { - /* DCE - Start RI pulse. */ - ri_start(data); - } + /* DCE - Start RI pulse. */ + ri_start(data); /* Buffer the data until DTR is down. */ return 0; @@ -922,123 +599,120 @@ static int api_tx_abort(const struct device *dev) } /* Application calls this when it is ready to receive data. */ -static int api_rx_enable(const struct device *dev, uint8_t *buf, - size_t len, int32_t timeout) +static int api_rx_enable(const struct device *dev, uint8_t *buf, size_t len, int32_t timeout) { LOG_INF("api_rx_enable: %p, %zu", (void *)buf, len); + const struct dtr_uart_config *config = get_dev_config(dev); struct dtr_uart_data *data = get_dev_data(dev); - // MARKUS TODO: Multiple calls to here must not overwrite existing buffers. - // MARKUS TODO: We must release all the buffers. - data->rx_buf = buf; - data->rx_len = len; - - init_input_pin_trigger(data); - - if (!data->app_rx_enable) { - data->app_rx_enable = true; - if (!data->dte_role) { - /* DCE */ - if (data->dtr_pin_toggle) { - /* With toggle, we start from DOWN state. */ - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - } else if (nrf_gpio_pin_read(data->dtr_pin)) { - /* Suspend UART in startup, if DTR signal is UP. */ - LOG_INF("Suspend UART in startup."); - power_off_uart(data); - data->dtr_state = DTR_UP; - } - } + if (data->app_rx_enable) { + LOG_ERR("RX already enabled"); + return -EBUSY; } - if (data->dte_role) { - /* DTE */ - if (IS_ENABLED(CONFIG_NRF_SW_DTR_UART_DTE_IDLE)) { - /* If we use idle, UART is activated with TX operation or with RI pulse. */ - LOG_INF("Suspend UART in startup."); - power_off_uart(data); - data->dtr_state = DTR_UP; - } else { - /* Activate DTR. */ - event_queue_delegate(data, DTR_UART_EVENT_DOWN); - } + data->app_rx_enable = true; + if (!data->dtr_state) { + struct uart_event evt = { + .type = UART_RX_BUF_RELEASED, + .data.rx_buf.buf = buf, + }; + user_callback(dev, &evt); + return 0; } - - return 0; + data->rx_active = true; + return uart_rx_enable(config->uart, buf, len, 2000); } static int api_rx_buf_rsp(const struct device *dev, uint8_t *buf, size_t len) { + const struct dtr_uart_config *config = get_dev_config(dev); struct dtr_uart_data *data = get_dev_data(dev); - if (data->rx_buf_requested) { - LOG_INF("Buf requested for RX start."); - data->rx_buf = buf; - data->rx_len = len; - return 0; + LOG_DBG("api_rx_buf_rsp: %p, %zu", (void *)buf, len); + + if (!data->dtr_state) { + goto release; } - LOG_INF("api_rx_buf_rsp: %p, %zu", (void *)buf, len); + if (!data->app_rx_enable) { + goto release; + } - return uart_rx_buf_rsp(data->uart, buf, len); + if (!data->rx_active) { + data->rx_active = true; + uart_rx_enable(config->uart, buf, len, 2000); + return 0; + } + + return uart_rx_buf_rsp(config->uart, buf, len); +release: + struct uart_event evt = { + .type = UART_RX_BUF_RELEASED, + .data.rx_buf.buf = buf, + }; + user_callback(dev, &evt); + return 0; } static int api_rx_disable(const struct device *dev) { - struct dtr_uart_data *data = get_dev_data(dev); - LOG_INF("api_rx_disable"); + struct dtr_uart_data *data = get_dev_data(dev); - if (data->dte_role) { - if (!IS_ENABLED(CONFIG_NRF_SW_DTR_UART_DTE_IDLE)) { - event_queue_delegate(data, DTR_UART_EVENT_UP); - } - return 0; + if (!data->app_rx_enable) { + LOG_ERR("RX not enabled"); + return -EINVAL; } - /* DCE cannot disable RX. */ // MARKUS TODO: Unsure what this should do in the application. - return -ENOTSUP; + data->app_rx_enable = false; + deactivate_rx(data); + return 0; } static int api_err_check(const struct device *dev) { - struct dtr_uart_data *data = get_dev_data(dev); + const struct dtr_uart_config *config = get_dev_config(dev); - return uart_err_check(data->uart); + return uart_err_check(config->uart); } #ifdef CONFIG_UART_USE_RUNTIME_CONFIGURE static int api_configure(const struct device *dev, const struct uart_config *cfg) { - const struct dtr_uart_data *data = get_dev_data(dev); - - // if (cfg->flow_ctrl != UART_CFG_FLOW_CTRL_NONE) { - // return -ENOTSUP; - // } + const struct dtr_uart_config *config = get_dev_config(dev); - return uart_configure(data->uart, cfg); + return uart_configure(config->uart, cfg); } static int api_config_get(const struct device *dev, struct uart_config *cfg) { - const struct dtr_uart_data *data = get_dev_data(dev); + const struct dtr_uart_config *config = get_dev_config(dev); - return uart_config_get(data->uart, cfg); + return uart_config_get(config->uart, cfg); } #endif /* CONFIG_UART_USE_RUNTIME_CONFIGURE */ -static const struct dtr_uart_config dtr_uart_config = { - .dte_role = DT_INST_PROP(0, dte_role), - - .dtr_pin = DT_INST_PROP(0, dtr_pin), // MARKUS TODO: Configure in the device tree. - .dtr_pin_toggle = DT_INST_PROP(0, dtr_pin_toggle), - .dtr_pin_debounce = DT_INST_PROP(0, dtr_pin_debounce), - - .ri_pin = DT_INST_PROP(0, ri_pin), -}; +/* Power management */ +#ifdef CONFIG_PM_DEVICE +static int dtr_uart_pm_action(const struct device *dev, enum pm_device_action action) +{ + switch (action) { + case PM_DEVICE_ACTION_SUSPEND: + LOG_DBG("PM SUSPEND - Disabling UART"); + /* TODO */ + break; + case PM_DEVICE_ACTION_RESUME: + LOG_DBG("PM RESUME - Enabling UART"); + /* TODO */ + break; + default: + return -ENOTSUP; + } -static struct dtr_uart_data dtr_uart_data; + return 0; +} +#endif static const struct uart_driver_api dtr_uart_api = { .callback_set = api_callback_set, @@ -1054,20 +728,16 @@ static const struct uart_driver_api dtr_uart_api = { #endif }; -#define GPIO_HAS_PIN(gpio_node, pin_prop) \ - (DT_PROP(gpio_node, port) == (DT_INST_PROP(0, pin_prop) >> 5)) - -/* There may be GPIO ports which cannot be used with GPIOTE. Check if pins are - * not from those ports. - */ -#define CHECK_GPIOTE_AVAILABLE(gpio_node) \ - BUILD_ASSERT((!GPIO_HAS_PIN(gpio_node, dtr_pin) && \ - !GPIO_HAS_PIN(gpio_node, ri_pin)) || \ - DT_NODE_HAS_PROP(gpio_node, gpiote_instance)); - -DT_FOREACH_STATUS_OKAY(nordic_nrf_gpio, CHECK_GPIOTE_AVAILABLE) - -DEVICE_DT_DEFINE(DT_NODELABEL(dtr_uart), dtr_uart_init, NULL, - &dtr_uart_data, &dtr_uart_config, - POST_KERNEL, CONFIG_NRF_SW_DTR_UART_INIT_PRIORITY, - &dtr_uart_api); +/* Device macro */ +#define DTR_UART_INIT(n) \ + static const struct dtr_uart_config dtr_uart_config_##n = { \ + .dtr_gpio = GPIO_DT_SPEC_INST_GET(n, dtr_gpios), \ + .ri_gpio = GPIO_DT_SPEC_INST_GET(n, ri_gpios), \ + .uart = DEVICE_DT_GET(DT_PARENT(DT_DRV_INST(n))), \ + }; \ + static struct dtr_uart_data dtr_uart_data_##n; \ + PM_DEVICE_DT_INST_DEFINE(n, dtr_uart_pm_action); \ + DEVICE_DT_INST_DEFINE(n, dtr_uart_init, PM_DEVICE_DT_INST_GET(n), &dtr_uart_data_##n, \ + &dtr_uart_config_##n, POST_KERNEL, 51, &dtr_uart_api); + +DT_INST_FOREACH_STATUS_OKAY(DTR_UART_INIT) diff --git a/dts/bindings/serial/nordic,nrf-sw-dtr-uart.yaml b/dts/bindings/serial/nordic,nrf-sw-dtr-uart.yaml index 42d45c07f85a..9528e2637a01 100644 --- a/dts/bindings/serial/nordic,nrf-sw-dtr-uart.yaml +++ b/dts/bindings/serial/nordic,nrf-sw-dtr-uart.yaml @@ -9,25 +9,12 @@ compatible: "nordic,nrf-sw-dtr-uart" include: uart-device.yaml properties: - dte-role: - type: boolean - description: Data Terminal Equipment role - - dtr-pin: - type: int + dtr-gpios: + type: phandle-array description: Data Terminal Ready pin required: true - dtr-pin-toggle: - type: boolean - description: DTR pin for DCE uses toggle on high-to-low, instead of level trigger - - dtr-pin-debounce: - type: int - description: DTR pin debounce time in milliseconds - default: 0 - - ri-pin: - type: int + ri-gpios: + type: phandle-array description: Ring Indicator pin required: true