From 6aa90862066cc579c8cd1880621fcba982e55d9e Mon Sep 17 00:00:00 2001 From: wanlei Date: Tue, 22 Aug 2023 11:15:10 +0800 Subject: [PATCH] fix(spi_master): polling_transmit forgot release bus lock when alloc DMA buffer failed Close https://github.com/espressif/esp-idf/issues/11845 --- components/driver/spi/gpspi/spi_master.c | 19 +++++++++++++------ .../driver/spi/include/driver/spi_master.h | 3 +++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/components/driver/spi/gpspi/spi_master.c b/components/driver/spi/gpspi/spi_master.c index 434056ddfe..15fdf96025 100644 --- a/components/driver/spi/gpspi/spi_master.c +++ b/components/driver/spi/gpspi/spi_master.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -1042,10 +1042,14 @@ esp_err_t SPI_MASTER_ISR_ATTR spi_device_polling_start(spi_device_handle_t handl if (ret!=ESP_OK) return ret; SPI_CHECK(!spi_bus_device_is_polling(handle), "Cannot send polling transaction while the previous polling transaction is not terminated.", ESP_ERR_INVALID_STATE ); + spi_host_t *host = handle->host; + spi_trans_priv_t priv_polling_trans; + ret = setup_priv_desc(trans_desc, &priv_polling_trans, (host->bus_attr->dma_enabled)); + if (ret!=ESP_OK) return ret; + /* If device_acquiring_lock is set to handle, it means that the user has already * acquired the bus thanks to the function `spi_device_acquire_bus()`. * In that case, we don't need to take the lock again. */ - spi_host_t *host = handle->host; if (host->device_acquiring_lock != handle) { /* The user cannot ask for the CS to keep active has the bus is not locked/acquired. */ if ((trans_desc->flags & SPI_TRANS_CS_KEEP_ACTIVE) != 0) { @@ -1056,13 +1060,16 @@ esp_err_t SPI_MASTER_ISR_ATTR spi_device_polling_start(spi_device_handle_t handl } else { ret = spi_bus_lock_wait_bg_done(handle->dev_lock, ticks_to_wait); } - if (ret != ESP_OK) return ret; - - ret = setup_priv_desc(trans_desc, &host->cur_trans_buf, (host->bus_attr->dma_enabled)); - if (ret!=ESP_OK) return ret; + if (ret != ESP_OK) { + uninstall_priv_desc(&priv_polling_trans); + ESP_LOGE(SPI_TAG, "polling can't get buslock"); + return ret; + } + //After holding the buslock, common resource can be accessed !! //Polling, no interrupt is used. host->polling = true; + host->cur_trans_buf = priv_polling_trans; ESP_LOGV(SPI_TAG, "polling trans"); spi_new_trans(handle, &host->cur_trans_buf); diff --git a/components/driver/spi/include/driver/spi_master.h b/components/driver/spi/include/driver/spi_master.h index 5cc1f469c4..e14344e72d 100644 --- a/components/driver/spi/include/driver/spi_master.h +++ b/components/driver/spi/include/driver/spi_master.h @@ -313,6 +313,9 @@ esp_err_t spi_device_polling_end(spi_device_handle_t handle, TickType_t ticks_to * @param trans_desc Description of transaction to execute * @return * - ESP_ERR_INVALID_ARG if parameter is invalid + * - ESP_ERR_TIMEOUT if the device cannot get control of the bus + * - ESP_ERR_NO_MEM if allocating DMA-capable temporary buffer failed + * - ESP_ERR_INVALID_STATE if previous transactions of same device are not finished * - ESP_OK on success */ esp_err_t spi_device_polling_transmit(spi_device_handle_t handle, spi_transaction_t *trans_desc);