From 94ea9e398edf811011be715495eb33bebdabff6f Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Thu, 28 Mar 2024 15:03:58 +0800 Subject: [PATCH] fix(i2s): rollback the breaking change of callback event data `i2s_event_data_t::data` is deprecated due to the cumbersome usage of secondary pointer Please use the newly added first-level pointer `i2s_event_data_t::dma_buf` instead --- components/esp_driver_i2s/i2s_common.c | 22 ++++++++++++++----- .../include/driver/i2s_common.h | 4 ++-- .../esp_driver_i2s/include/driver/i2s_types.h | 5 ++++- .../test_apps/i2s/main/test_i2s.c | 10 ++++----- .../test_apps/i2s/main/test_i2s_iram.c | 2 +- .../release-5.x/5.3/peripherals.rst | 5 +++++ .../release-5.x/5.3/peripherals.rst | 5 +++++ 7 files changed, 39 insertions(+), 14 deletions(-) diff --git a/components/esp_driver_i2s/i2s_common.c b/components/esp_driver_i2s/i2s_common.c index 82bc4002ed..a211abf51a 100644 --- a/components/esp_driver_i2s/i2s_common.c +++ b/components/esp_driver_i2s/i2s_common.c @@ -498,6 +498,10 @@ uint32_t i2s_get_source_clk_freq(i2s_clock_src_t clk_src, uint32_t mclk_freq_hz) return clk_freq; } +/* Temporary ignore the deprecated warning of i2s_event_data_t::data */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + #if SOC_GDMA_SUPPORTED static bool IRAM_ATTR i2s_dma_rx_callback(gdma_channel_handle_t dma_chan, gdma_event_data_t *event_data, void *user_data) { @@ -513,7 +517,8 @@ static bool IRAM_ATTR i2s_dma_rx_callback(gdma_channel_handle_t dma_chan, gdma_e esp_cache_msync((void *)finish_desc->buf, handle->dma.buf_size, ESP_CACHE_MSYNC_FLAG_INVALIDATE); #endif i2s_event_data_t evt = { - .data = (void *)finish_desc->buf, + .data = &(finish_desc->buf), + .dma_buf = (void *)finish_desc->buf, .size = handle->dma.buf_size, }; if (handle->callbacks.on_recv) { @@ -543,7 +548,8 @@ static bool IRAM_ATTR i2s_dma_tx_callback(gdma_channel_handle_t dma_chan, gdma_e finish_desc = (lldesc_t *)event_data->tx_eof_desc_addr; void *curr_buf = (void *)finish_desc->buf; i2s_event_data_t evt = { - .data = curr_buf, + .data = &(finish_desc->buf), + .dma_buf = curr_buf, .size = handle->dma.buf_size, }; if (handle->dma.auto_clear_before_cb) { @@ -553,7 +559,7 @@ static bool IRAM_ATTR i2s_dma_tx_callback(gdma_channel_handle_t dma_chan, gdma_e user_need_yield |= handle->callbacks.on_sent(handle, &evt, handle->user_data); } #if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE - /* Sync buffer after the callback incase users update the buffer in the callback */ + /* Sync buffer after the callback in case users update the buffer in the callback */ if (handle->dma.auto_clear_before_cb || handle->callbacks.on_sent) { esp_cache_msync(curr_buf, handle->dma.buf_size, ESP_CACHE_MSYNC_FLAG_DIR_C2M); } @@ -562,6 +568,7 @@ static bool IRAM_ATTR i2s_dma_tx_callback(gdma_channel_handle_t dma_chan, gdma_e xQueueReceiveFromISR(handle->msg_queue, &dummy, &need_yield1); if (handle->callbacks.on_send_q_ovf) { evt.data = NULL; + evt.dma_buf = NULL; user_need_yield |= handle->callbacks.on_send_q_ovf(handle, &evt, handle->user_data); } } @@ -596,7 +603,8 @@ static void IRAM_ATTR i2s_dma_rx_callback(void *arg) if (handle && (status & I2S_LL_EVENT_RX_EOF)) { i2s_hal_get_in_eof_des_addr(&(handle->controller->hal), (uint32_t *)&finish_desc); - evt.data = (void *)finish_desc->buf; + evt.data = &(finish_desc->buf); + evt.dma_buf = (void *)finish_desc->buf; evt.size = handle->dma.buf_size; if (handle->callbacks.on_recv) { user_need_yield |= handle->callbacks.on_recv(handle, &evt, handle->user_data); @@ -605,6 +613,7 @@ static void IRAM_ATTR i2s_dma_rx_callback(void *arg) xQueueReceiveFromISR(handle->msg_queue, &dummy, &need_yield1); if (handle->callbacks.on_recv_q_ovf) { evt.data = NULL; + evt.dma_buf = NULL; user_need_yield |= handle->callbacks.on_recv_q_ovf(handle, &evt, handle->user_data); } } @@ -635,7 +644,8 @@ static void IRAM_ATTR i2s_dma_tx_callback(void *arg) if (handle && (status & I2S_LL_EVENT_TX_EOF)) { i2s_hal_get_out_eof_des_addr(&(handle->controller->hal), (uint32_t *)&finish_desc); void *curr_buf = (void *)finish_desc->buf; - evt.data = curr_buf; + evt.data = &(finish_desc->buf); + evt.dma_buf = curr_buf; evt.size = handle->dma.buf_size; // Auto clear the dma buffer before data sent if (handle->dma.auto_clear_before_cb) { @@ -664,6 +674,8 @@ static void IRAM_ATTR i2s_dma_tx_callback(void *arg) } #endif +#pragma GCC diagnostic pop + /** * @brief I2S DMA interrupt initialization * @note I2S will use GDMA if chip supports, and the interrupt is triggered by GDMA. diff --git a/components/esp_driver_i2s/include/driver/i2s_common.h b/components/esp_driver_i2s/include/driver/i2s_common.h index abf63851a3..eaeb397a0d 100644 --- a/components/esp_driver_i2s/include/driver/i2s_common.h +++ b/components/esp_driver_i2s/include/driver/i2s_common.h @@ -65,9 +65,9 @@ typedef struct { * it should be the multiple of `3` when the data bit width is 24. */ union { - bool auto_clear; /*!< Alias of `auto_clear_after_cb` to be compatible with previous version */ + bool auto_clear; /*!< Alias of `auto_clear_after_cb` */ bool auto_clear_after_cb; /*!< Set to auto clear DMA TX buffer after `on_sent` callback, I2S will always send zero automatically if no data to send. - * So that user can assign the data to the DMA buffers directly in the callback, and the data won't be cleared after quitted the callback. + * So that user can assign the data to the DMA buffers directly in the callback, and the data won't be cleared after quit the callback. */ }; bool auto_clear_before_cb; /*!< Set to auto clear DMA TX buffer before `on_sent` callback, I2S will always send zero automatically if no data to send diff --git a/components/esp_driver_i2s/include/driver/i2s_types.h b/components/esp_driver_i2s/include/driver/i2s_types.h index 0e5a5f8f67..2135124389 100644 --- a/components/esp_driver_i2s/include/driver/i2s_types.h +++ b/components/esp_driver_i2s/include/driver/i2s_types.h @@ -63,7 +63,10 @@ typedef enum { * @brief Event structure used in I2S event queue */ typedef struct { - void *data; /**< The pointer of DMA buffer that just finished sending or receiving for `on_recv` and `on_sent` callback + void *data __attribute__((deprecated)); /**< (Deprecated) The secondary pointer of DMA buffer that just finished sending or receiving for `on_recv` and `on_sent` callback + * NULL for `on_recv_q_ovf` and `on_send_q_ovf` callback + */ + void *dma_buf;/**< The first level pointer of DMA buffer that just finished sending or receiving for `on_recv` and `on_sent` callback * NULL for `on_recv_q_ovf` and `on_send_q_ovf` callback */ size_t size; /**< The buffer size of DMA buffer when success to send or receive, diff --git a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c index 0be836584f..5cb9489b5d 100644 --- a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c +++ b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c @@ -912,10 +912,10 @@ finish: static IRAM_ATTR bool i2s_tx_on_sent_callback(i2s_chan_handle_t handle, i2s_event_data_t *event, void *user_ctx) { - uint32_t *data = (uint32_t *)(event->data); + uint32_t *dma_buf = (uint32_t *)(event->dma_buf); size_t len = event->size / sizeof(uint32_t); for (int i = 0; i < len; i++) { - data[i] = i + TEST_I2S_BUF_DATA_OFFSET; + dma_buf[i] = i + TEST_I2S_BUF_DATA_OFFSET; } return false; } @@ -923,11 +923,11 @@ static IRAM_ATTR bool i2s_tx_on_sent_callback(i2s_chan_handle_t handle, i2s_even static IRAM_ATTR bool i2s_rx_on_recv_callback(i2s_chan_handle_t handle, i2s_event_data_t *event, void *user_ctx) { bool *received = (bool *)user_ctx; - uint32_t *data = (uint32_t *)(event->data); + uint32_t *dma_buf = (uint32_t *)(event->dma_buf); size_t len = event->size / sizeof(uint32_t); for (int i = 0; i < len; i++) { - if (data[i] == TEST_I2S_BUF_DATA_OFFSET) { - for (int j = 0; i < len && data[i] == (j + TEST_I2S_BUF_DATA_OFFSET); i++, j++); + if (dma_buf[i] == TEST_I2S_BUF_DATA_OFFSET) { + for (int j = 0; i < len && dma_buf[i] == (j + TEST_I2S_BUF_DATA_OFFSET); i++, j++); if (i == len) { *received = true; break; diff --git a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s_iram.c b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s_iram.c index 021f4f30eb..88e95c934f 100644 --- a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s_iram.c +++ b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s_iram.c @@ -27,7 +27,7 @@ static bool IRAM_ATTR test_i2s_tx_done_callback(i2s_chan_handle_t handle, i2s_event_data_t *event, void *user_ctx) { int *is_triggered = (int *)user_ctx; - if (*(uint8_t *)(event->data) != 0) { + if (event->dma_buf != 0) { *is_triggered = 1; } return false; diff --git a/docs/en/migration-guides/release-5.x/5.3/peripherals.rst b/docs/en/migration-guides/release-5.x/5.3/peripherals.rst index 75256f5736..d22a878182 100644 --- a/docs/en/migration-guides/release-5.x/5.3/peripherals.rst +++ b/docs/en/migration-guides/release-5.x/5.3/peripherals.rst @@ -55,3 +55,8 @@ Secure Element The ATECC608A secure element interfacing example has been moved to `ESP Cryptoauthlib Repository `_ on GitHub. This example is also part of the `esp-cryptoauthlib `_ in the component manager registry. + +I2S +------- + +Due to the cumbersome usage of the secondary pointer of DMA buffer, the ``data`` field in the callback event :cpp:type:`i2s_event_data_t` is deprecated, please use the newly added first-level pointer ``dma_buf`` instead. diff --git a/docs/zh_CN/migration-guides/release-5.x/5.3/peripherals.rst b/docs/zh_CN/migration-guides/release-5.x/5.3/peripherals.rst index b3d012f4f6..77eb0728c4 100644 --- a/docs/zh_CN/migration-guides/release-5.x/5.3/peripherals.rst +++ b/docs/zh_CN/migration-guides/release-5.x/5.3/peripherals.rst @@ -55,3 +55,8 @@ ATECC608A 安全元素接口示例现已移至 GitHub 上的 `esp-cryptoauthlib 仓库 `_ 中。 该示例也是组件管理器注册表中 `esp-cryptoauthlib `_ 的一部分。 + +I2S +------- + +回调事件 :cpp:type:`i2s_event_data_t` 中指向 DMA 数组的二级指针 ``data`` 因使用过于繁琐已被弃用，请使用新增的指向 DMA 数组的一级指针 ``dma_buf`` 字段代替。