ESP32S3: UART_ERR_WR_MASK not set in Zephyr 3.7

[larsbetronic10]

Describe the bug

We are trying to get Modbus RTU RS485 commutation working on the ESP32S3. We have custom hardware + software with Zephyr 3.6 working correctly. However, when migrating to Zephyr 3.7 Modbus errors are introduced. When a request is received through the tranceiver, the message is parsed and correctly answered. But when the answer is outputted through the TX-line of UART1 (UART0 is used for logging) then a new interrupt is generated on the RX line of UART1. This is because the RX line is pulled low when transmitting, this is also something we see in Zephyr 3.6 however did not pose any problems. When researching the issue a difference was found in the UART_ERR_WR_MASK bit in the configuration of the UART1. It seems that this bit is set in Zephyr 3.6, discarding any wrong received data. But this bit is not set in Zephyr 3.7, generating an interrupt on the RX line when it is pulled low during the transmit of the Modbus response. As a result of this the interrupt on the UART line is triggered, and Modbus generates an error due to the 'Frame length error' which is expected if one byte is received.

To Reproduce

We are able to reproduce it with a FTDI USB-RS485-WE cable connected to the TI SN65176BD modbus tranciever. Expected behavior

The expected behaviour would be the UART_ERR_WR_MASK bit being set in Zephyr 3.7, discarding the empty byte during transmission. It is found that the function uart_ll_discard_error_data was added in hal_espressif but never used. Impact

It is not possible to get Modbus working in Zephyr 3.7 for now. This is a blocking issue for us since features in Zephyr 3.7 are required.

Logs and console output

This is what the UART RX and TX lines look like when receiving and sending the data. In yellow is the receive line and green the transmit line. As you can see the receive line is pulled down when transmitting data, triggering an empty interrup.

Environment (please complete the following information):

Zephyr 3.7.0 hal_espressif 87e7902d7184a8280b4d13bce79801a723f4ddd8

Additional context

The pin settings in Zephyr:

&uart1 {
    status = "okay";
    current-speed = <115200>;
    pinctrl-0 = <&uart1_default>;
    pinctrl-names = "default";
    hw-rs485-hd-mode;

    modbus0 {
        compatible = "zephyr,modbus-serial";
        status = "okay";
        de-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
    };
};

uart1_default: uart1_default {
        group1 {
            pinmux = <UART1_TX_GPIO17>;
            output-high;
        };
        group2 {
            pinmux = <UART1_RX_GPIO18>;
            bias-disable;
        };
    };

[github-actions[bot]]

Hi @larsbetronic10! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[mmahadevan108]

@larsbetronic10 , have you checked if this issue exists on the latest Zephyr main branch?

[mmahadevan108]

cc @jfischer-no

[larsbetronic10]

@larsbetronic10 , have you checked if this issue exists on the latest Zephyr main branch?

Yes. Just checked this. I put a workaround in place for the following error when building using hal_espressif version (ef9d884533208005468a118c7ada56e92ae9ba8b): In file included from /workdir/Sources/Libraries/modules/hal/espressif/components/esp_timer/src/esp_timer.c:13: /workdir/Sources/Libraries/modules/hal/espressif/components/esp_timer/src/esp_timer.c: In function 'init_timer_task': /workdir/Sources/Libraries/modules/hal/espressif/zephyr/esp32s3/../../components/esp_system/include/esp_task.h:25:28: error: 'CONFIG_ESP_WIFI_MAX_THREAD_PRIORITY' undeclared (first use in this function); did you mean 'CONFIG_MAIN_THREAD_PRIORITY'? 25 | #define ESP_TASK_PRIO_MAX (CONFIG_ESP_WIFI_MAX_THREAD_PRIORITY)

This unfortunately resulted in the same behavior as before.

[sylvioalves]

@larsbetronic10 I am checking this.. First you mentioned about modbus and uart.. and last comment is regarding esp_timer.c. Can you bring full log so I can fully check that?

[sylvioalves]

I checked v3.6.0 regarding the below:

When researching the issue a difference was found in the UART_ERR_WR_MASK bit
in the configuration of the UART1. It seems that this bit is set in Zephyr 3.6

Can you point out where you see this being used in v.3.6.0?

[larsbetronic10]

I checked v3.6.0 regarding the below:

When researching the issue a difference was found in the UART_ERR_WR_MASK bit
in the configuration of the UART1. It seems that this bit is set in Zephyr 3.6

Can you point out where you see this being used in v.3.6.0?

We noticed this value was different for the case of v3.6.0 and v.3.7.0. When adding a breakpoint to cb_handler_rx(struct modbus_context *ctx) in file modbus_serial.c. The variable cfg->dev consists of the low level settings of the UART peripheral located at address 0x60010000. This was compared for both Zephyr versions and only v3.6.0 has the value for err_wr_mask set to 1. It is not the case that we see the value being used, but we do see the interrupt not triggering when sending the data for the case of v3.6.0. This flag being 0 looks like the reason for the interrupt triggered for the case of v3.7.0.

The issue of the esp_timer.c is unrelated to this issue. I hope this is clear. If you have any more questions, please let me know.

[larsbetronic10]

An update from our side: We just updated the functions which enables the interrupts on the UART RX line: static void uart_esp32_irq_rx_enable. This function was modified to include a function call to uart_ll_discard_error_data, which set the previously mentioned err_wr_mask bit active. This resulted in the expected behaviour of not generating an interrupt if the data is incorrect. The function now looks like this: static void uart_esp32_irq_rx_enable(const struct device *dev) { struct uart_esp32_data *data = dev->data; uart_ll_discard_error_data(data->hal.dev, true); uart_hal_clr_intsts_mask(&data->hal, UART_INTR_RXFIFO_FULL); uart_hal_clr_intsts_mask(&data->hal, UART_INTR_RXFIFO_TOUT); uart_hal_ena_intr_mask(&data->hal, UART_INTR_RXFIFO_FULL); uart_hal_ena_intr_mask(&data->hal, UART_INTR_RXFIFO_TOUT); }

This validates the direction we were looking into as the solution. I know this is not a pretty solution, but a functional one. I hope this variable can be set in the correct way and no modifications on our side are needed.