Skip to content

Commit

Permalink
Merge branch 'bugfix/add_len_check_per_spi_master_transaction' into '…
Browse files Browse the repository at this point in the history
…master'

spi master: added transaction length check to refuse longer than hardware supported length

Closes IDF-7236

See merge request espressif/esp-idf!23199
  • Loading branch information
Icarus113 committed May 5, 2023
2 parents 5c4ffe4 + 0e72940 commit 313f8cd
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 11 deletions.
28 changes: 27 additions & 1 deletion components/driver/spi/gpspi/spi_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ We have two bits to control the interrupt:
*/

#include <string.h>
#include <sys/param.h>
#include "esp_private/spi_common_internal.h"
#include "driver/spi_master.h"
#include "esp_clk_tree.h"
Expand All @@ -123,9 +124,9 @@ We have two bits to control the interrupt:
#include "soc/soc_memory_layout.h"
#include "driver/gpio.h"
#include "hal/spi_hal.h"
#include "hal/spi_ll.h"
#include "esp_heap_caps.h"


typedef struct spi_device_t spi_device_t;

/// struct to hold private transaction data (like tx and rx buffer for DMA).
Expand Down Expand Up @@ -803,6 +804,14 @@ static SPI_MASTER_ISR_ATTR esp_err_t check_trans_valid(spi_device_handle_t handl
//Dummy phase is not available when both data out and in are enabled, regardless of FD or HD mode.
SPI_CHECK(!tx_enabled || !rx_enabled || !dummy_enabled || !extra_dummy_enabled, "Dummy phase is not available when both data out and in are enabled", ESP_ERR_INVALID_ARG);

if (bus_attr->dma_enabled) {
SPI_CHECK(trans_desc->length <= SPI_LL_DMA_MAX_BIT_LEN, "txdata transfer > hardware max supported len", ESP_ERR_INVALID_ARG);
SPI_CHECK(trans_desc->rxlength <= SPI_LL_DMA_MAX_BIT_LEN, "rxdata transfer > hardware max supported len", ESP_ERR_INVALID_ARG);
} else {
SPI_CHECK(trans_desc->length <= SPI_LL_CPU_MAX_BIT_LEN, "txdata transfer > hardware max supported len", ESP_ERR_INVALID_ARG);
SPI_CHECK(trans_desc->rxlength <= SPI_LL_CPU_MAX_BIT_LEN, "rxdata transfer > hardware max supported len", ESP_ERR_INVALID_ARG);
}

return ESP_OK;
}

Expand Down Expand Up @@ -1102,3 +1111,20 @@ esp_err_t SPI_MASTER_ISR_ATTR spi_device_polling_transmit(spi_device_handle_t ha

return spi_device_polling_end(handle, portMAX_DELAY);
}

esp_err_t spi_bus_get_max_transaction_len(spi_host_device_t host_id, size_t *max_bytes)
{
SPI_CHECK(is_valid_host(host_id), "invalid host", ESP_ERR_INVALID_ARG);
if (bus_driver_ctx[host_id] == NULL || max_bytes == NULL) {
return ESP_ERR_INVALID_ARG;
}

spi_host_t *host = bus_driver_ctx[host_id];
if (host->bus_attr->dma_enabled) {
*max_bytes = MIN(host->bus_attr->max_transfer_sz, (SPI_LL_DMA_MAX_BIT_LEN / 8));
} else {
*max_bytes = MIN(host->bus_attr->max_transfer_sz, (SPI_LL_CPU_MAX_BIT_LEN / 8));
}

return ESP_OK;
}
12 changes: 12 additions & 0 deletions components/driver/spi/include/driver/spi_master.h
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,18 @@ void spi_get_timing(bool gpio_is_used, int input_delay_ns, int eff_clk, int *dum
*/
int spi_get_freq_limit(bool gpio_is_used, int input_delay_ns);

/**
* @brief Get max length (in bytes) of one transaction
*
* @param host_id SPI peripheral
* @param[out] max_bytes Max length of one transaction, in bytes
*
* @return
* - ESP_OK: On success
* - ESP_ERR_INVALID_ARG: Invalid argument
*/
esp_err_t spi_bus_get_max_transaction_len(spi_host_device_t host_id, size_t *max_bytes);

#ifdef __cplusplus
}
#endif
9 changes: 6 additions & 3 deletions components/esp_lcd/src/esp_lcd_panel_io_spi.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "esp_log.h"
#include "esp_check.h"
#include "esp_lcd_common.h"
#include "esp_private/spi_common_internal.h"

static const char *TAG = "lcd_panel.io.spi";

Expand Down Expand Up @@ -107,9 +106,13 @@ esp_err_t esp_lcd_new_panel_io_spi(esp_lcd_spi_bus_handle_t bus, const esp_lcd_p
spi_panel_io->base.tx_color = panel_io_spi_tx_color;
spi_panel_io->base.del = panel_io_spi_del;
spi_panel_io->base.register_event_callbacks = panel_io_spi_register_event_callbacks;
spi_panel_io->spi_trans_max_bytes = spi_bus_get_attr((spi_host_device_t)bus)->max_transfer_sz;

size_t max_trans_bytes = 0;
ESP_GOTO_ON_ERROR(spi_bus_get_max_transaction_len((spi_host_device_t)bus, &max_trans_bytes), err, TAG, "get spi max transaction len failed");
spi_panel_io->spi_trans_max_bytes = max_trans_bytes;

*ret_io = &(spi_panel_io->base);
ESP_LOGD(TAG, "new spi lcd panel io @%p", spi_panel_io);
ESP_LOGD(TAG, "new spi lcd panel io @%p, max_trans_bytes: %d", spi_panel_io, (int)max_trans_bytes);

return ESP_OK;

Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? &SPI1:((ID)==1? &SPI2 : &SPI3))

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 24)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 24) //reg len: 24 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32c2/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):&GPSPI2)

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 18)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 18) //reg len: 18 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32c3/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):&GPSPI2)

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 18)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 18) //reg len: 18 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32c6/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):&GPSPI2)

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 18)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 18) //reg len: 18 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32h2/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ extern "C" {
#define SPI_LL_PERIPH_CLK_FREQ (80 * 1000000)
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):&GPSPI2)

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 18)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 18) //reg len: 18 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32s2/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):((ID)==1? &GPSPI2 : &GPSPI3))

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 23)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 23) //reg len: 23 bits
#define SPI_LL_CPU_MAX_BIT_LEN (18 * 32) //Fifo len: 18 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down
3 changes: 2 additions & 1 deletion components/hal/esp32s3/include/hal/spi_ll.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ extern "C" {
#define HAL_SPI_SWAP_DATA_TX(data, len) HAL_SWAP32((uint32_t)(data) << (32 - len))
#define SPI_LL_GET_HW(ID) ((ID)==0? ({abort();NULL;}):((ID)==1? &GPSPI2 : &GPSPI3))

#define SPI_LL_DATA_MAX_BIT_LEN (1 << 18)
#define SPI_LL_DMA_MAX_BIT_LEN (1 << 18) //reg len: 18 bits
#define SPI_LL_CPU_MAX_BIT_LEN (16 * 32) //Fifo len: 16 words

/**
* The data structure holding calculated clock configuration. Since the
Expand Down

0 comments on commit 313f8cd

Please sign in to comment.