From 8e31a365629534eb54d91d2605dbd4e7011b44f5 Mon Sep 17 00:00:00 2001 From: michael Date: Sat, 30 Sep 2017 19:59:05 +0800 Subject: [PATCH 1/3] feat(spi_slave): append trans_len field in trans_desc to show length actually transferred. --- components/driver/include/driver/spi_slave.h | 1 + components/driver/spi_slave.c | 12 ++++++++++-- docs/api-reference/peripherals/spi_slave.rst | 10 ++++++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/components/driver/include/driver/spi_slave.h b/components/driver/include/driver/spi_slave.h index 047ab41392..ed12cb62df 100644 --- a/components/driver/include/driver/spi_slave.h +++ b/components/driver/include/driver/spi_slave.h @@ -53,6 +53,7 @@ typedef struct { */ struct spi_slave_transaction_t { size_t length; ///< Total data length, in bits + size_t trans_len; ///< Transaction data length, in bits const void *tx_buffer; ///< Pointer to transmit buffer, or NULL for no MOSI phase void *rx_buffer; ///< Pointer to receive buffer, or NULL for no MISO phase void *user; ///< User-defined variable. Can be used to store eg transaction ID. diff --git a/components/driver/spi_slave.c b/components/driver/spi_slave.c index 7600b4e45c..305bea6ab0 100644 --- a/components/driver/spi_slave.c +++ b/components/driver/spi_slave.c @@ -289,12 +289,20 @@ static void IRAM_ATTR spi_intr(void *arg) if (!host->hw->slave.trans_done) return; if (host->cur_trans) { + //when data of cur_trans->length are all sent, the slv_rdata_bit + //will be the length sent-1 (i.e. cur_trans->length-1 ), otherwise + //the length sent. + host->cur_trans->trans_len = host->hw->slv_rd_bit.slv_rdata_bit; + if ( host->cur_trans->trans_len == host->cur_trans->length - 1 ) { + host->cur_trans->trans_len++; + } + if (host->dma_chan == 0 && host->cur_trans->rx_buffer) { //Copy result out uint32_t *data = host->cur_trans->rx_buffer; - for (int x = 0; x < host->cur_trans->length; x += 32) { + for (int x = 0; x < host->cur_trans->trans_len; x += 32) { uint32_t word; - int len = host->cur_trans->length - x; + int len = host->cur_trans->trans_len - x; if (len > 32) len = 32; word = host->hw->data_buf[(x / 32)]; memcpy(&data[x / 32], &word, (len + 7) / 8); diff --git a/docs/api-reference/peripherals/spi_slave.rst b/docs/api-reference/peripherals/spi_slave.rst index 76f44c4942..727338ffa1 100644 --- a/docs/api-reference/peripherals/spi_slave.rst +++ b/docs/api-reference/peripherals/spi_slave.rst @@ -74,10 +74,12 @@ may decide to use DMA for transfers, so these buffers should be allocated in DMA The amount of data written to the buffers is limited by the ``length`` member of the transaction structure: the driver will never read/write more data than indicated there. The ``length`` cannot define the actual -length of the SPI transaction; this is determined by the master as it drives the clock and CS lines. In -case the length of the transmission is larger than the buffer length, only the start of the transmission -will be sent and received. In case the transmission length is shorter than the buffer length, only data up -to the length of the buffer will be exchanged. +length of the SPI transaction; this is determined by the master as it drives the clock and CS lines. The actual length +transferred can be read from the ``trans_len`` member of the ``spi_slave_transaction_t`` structure after transaction. +In case the length of the transmission is larger than the buffer length, only the start of the transmission +will be sent and received, and the ``trans_len`` is set to ``length`` instead of the actual length. It's recommended to +set ``length`` longer than the maximum length expected if the ``trans_len`` is required. In case the transmission +length is shorter than the buffer length, only data up to the length of the buffer will be exchanged. Warning: Due to a design peculiarity in the ESP32, if the amount of bytes sent by the master or the length of the transmission queues in the slave driver, in bytes, is not both larger than eight and dividable by From 88f602a0e0412421709e01f92b9f0f8de9c01965 Mon Sep 17 00:00:00 2001 From: michael Date: Tue, 17 Oct 2017 11:52:50 +0800 Subject: [PATCH 2/3] fix(periph_ctrl): fix reset function in `periph_ctrl.c` --- components/driver/periph_ctrl.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/components/driver/periph_ctrl.c b/components/driver/periph_ctrl.c index 61793f290b..c61e2a4cc9 100644 --- a/components/driver/periph_ctrl.c +++ b/components/driver/periph_ctrl.c @@ -46,10 +46,8 @@ void periph_module_disable(periph_module_t periph) void periph_module_reset(periph_module_t periph) { portENTER_CRITICAL(&periph_spinlock); - uint32_t rst_en = get_rst_en_mask(periph); - uint32_t mask = get_clk_en_mask(periph); - DPORT_SET_PERI_REG_MASK(rst_en, mask); - DPORT_CLEAR_PERI_REG_MASK(rst_en, mask); + DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph)); + DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph)); portEXIT_CRITICAL(&periph_spinlock); } From 8128bb95ef830718795c598ddc402190382784a0 Mon Sep 17 00:00:00 2001 From: Gabriel Carstoiu Date: Fri, 22 Sep 2017 17:47:06 +0800 Subject: [PATCH 3/3] fix(spi_slave): enable DMA clock when initialization. merging in the code updates to the SPI master code done in commit b834fcf78aa4cc16d4c3f5349286df3580d24edb. TW#15670, Closes #1027 --- components/driver/spi_master.c | 1 + components/driver/spi_slave.c | 19 +++- components/driver/test/test_spi_slave.c | 141 ++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 components/driver/test/test_spi_slave.c diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 465f068994..089a65d2b2 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -183,6 +183,7 @@ nomem: } free(spihost[host]); spicommon_periph_free(host); + spicommon_dma_chan_free(dma_chan); return ESP_ERR_NO_MEM; } diff --git a/components/driver/spi_slave.c b/components/driver/spi_slave.c index 305bea6ab0..204dc6b992 100644 --- a/components/driver/spi_slave.c +++ b/components/driver/spi_slave.c @@ -68,12 +68,21 @@ static void IRAM_ATTR spi_intr(void *arg); esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *bus_config, const spi_slave_interface_config_t *slave_config, int dma_chan) { - bool native, claimed; + bool native, spi_chan_claimed, dma_chan_claimed; //We only support HSPI/VSPI, period. SPI_CHECK(VALID_HOST(host), "invalid host", ESP_ERR_INVALID_ARG); + SPI_CHECK( dma_chan >= 0 && dma_chan <= 2, "invalid dma channel", ESP_ERR_INVALID_ARG ); - claimed = spicommon_periph_claim(host); - SPI_CHECK(claimed, "host already in use", ESP_ERR_INVALID_STATE); + spi_chan_claimed=spicommon_periph_claim(host); + SPI_CHECK(spi_chan_claimed, "host already in use", ESP_ERR_INVALID_STATE); + + if ( dma_chan != 0 ) { + dma_chan_claimed=spicommon_dma_chan_claim(dma_chan); + if ( !dma_chan_claimed ) { + spicommon_periph_free( host ); + SPI_CHECK(dma_chan_claimed, "dma channel already in use", ESP_ERR_INVALID_STATE); + } + } spihost[host] = malloc(sizeof(spi_slave_t)); if (spihost[host] == NULL) goto nomem; @@ -179,6 +188,7 @@ nomem: free(spihost[host]); spihost[host] = NULL; spicommon_periph_free(host); + spicommon_dma_chan_free(dma_chan); return ESP_ERR_NO_MEM; } @@ -188,6 +198,9 @@ esp_err_t spi_slave_free(spi_host_device_t host) SPI_CHECK(spihost[host], "host not slave", ESP_ERR_INVALID_ARG); if (spihost[host]->trans_queue) vQueueDelete(spihost[host]->trans_queue); if (spihost[host]->ret_queue) vQueueDelete(spihost[host]->ret_queue); + if ( spihost[host]->dma_chan > 0 ) { + spicommon_dma_chan_free ( spihost[host]->dma_chan ); + } free(spihost[host]->dmadesc_tx); free(spihost[host]->dmadesc_rx); free(spihost[host]); diff --git a/components/driver/test/test_spi_slave.c b/components/driver/test/test_spi_slave.c new file mode 100644 index 0000000000..f56ef7e50e --- /dev/null +++ b/components/driver/test/test_spi_slave.c @@ -0,0 +1,141 @@ +/* + Tests for the spi_slave device driver +*/ + +#include +#include "unity.h" +#include "driver/spi_master.h" +#include "driver/spi_slave.h" +#include "esp_log.h" + +#define PIN_NUM_MISO 25 +#define PIN_NUM_MOSI 23 +#define PIN_NUM_CLK 19 +#define PIN_NUM_CS 22 + +static const char MASTER_TAG[] = "test_master"; +static const char SLAVE_TAG[] = "test_slave"; + +#define MASTER_SEND {0x93, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0, 0xaa, 0xcc, 0xff, 0xee, 0x55, 0x77, 0x88, 0x43} +#define SLAVE_SEND { 0xaa, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0x13, 0x57, 0x9b, 0xdf, 0x24, 0x68, 0xac, 0xe0 } + +static inline void int_connect( uint32_t gpio, uint32_t sigo, uint32_t sigi ) +{ + gpio_matrix_out( gpio, sigo, false, false ); + gpio_matrix_in( gpio, sigi, false ); +} + +static void master_init_nodma( spi_device_handle_t* spi) +{ + esp_err_t ret; + spi_bus_config_t buscfg={ + .miso_io_num=PIN_NUM_MISO, + .mosi_io_num=PIN_NUM_MOSI, + .sclk_io_num=PIN_NUM_CLK, + .quadwp_io_num=-1, + .quadhd_io_num=-1 + }; + spi_device_interface_config_t devcfg={ + .clock_speed_hz=4*1000*1000, //currently only up to 4MHz for internel connect + .mode=0, //SPI mode 0 + .spics_io_num=PIN_NUM_CS, //CS pin + .queue_size=7, //We want to be able to queue 7 transactions at a time + .pre_cb=NULL, + .cs_ena_posttrans=1, + }; + //Initialize the SPI bus + ret=spi_bus_initialize(HSPI_HOST, &buscfg, 0); + TEST_ASSERT(ret==ESP_OK); + //Attach the LCD to the SPI bus + ret=spi_bus_add_device(HSPI_HOST, &devcfg, spi); + TEST_ASSERT(ret==ESP_OK); +} + +static void slave_init() +{ + //Configuration for the SPI bus + spi_bus_config_t buscfg={ + .mosi_io_num=PIN_NUM_MOSI, + .miso_io_num=PIN_NUM_MISO, + .sclk_io_num=PIN_NUM_CLK + }; + //Configuration for the SPI slave interface + spi_slave_interface_config_t slvcfg={ + .mode=0, + .spics_io_num=PIN_NUM_CS, + .queue_size=3, + .flags=0, + }; + //Enable pull-ups on SPI lines so we don't detect rogue pulses when no master is connected. + gpio_set_pull_mode(PIN_NUM_MOSI, GPIO_PULLUP_ONLY); + gpio_set_pull_mode(PIN_NUM_CLK, GPIO_PULLUP_ONLY); + gpio_set_pull_mode(PIN_NUM_CS, GPIO_PULLUP_ONLY); + //Initialize SPI slave interface + TEST_ESP_OK( spi_slave_initialize(VSPI_HOST, &buscfg, &slvcfg, 2) ); +} + +TEST_CASE("test slave startup","[spi]") +{ + uint8_t master_txbuf[320]=MASTER_SEND; + uint8_t master_rxbuf[320]; + uint8_t slave_txbuf[320]=SLAVE_SEND; + uint8_t slave_rxbuf[320]; + + spi_device_handle_t spi; + //initial master + master_init_nodma( &spi ); + //initial slave + slave_init(); + + //do internal connection + int_connect( PIN_NUM_MOSI, HSPID_OUT_IDX, VSPIQ_IN_IDX ); + int_connect( PIN_NUM_MISO, VSPIQ_OUT_IDX, HSPID_IN_IDX ); + int_connect( PIN_NUM_CS, HSPICS0_OUT_IDX, VSPICS0_IN_IDX ); + int_connect( PIN_NUM_CLK, HSPICLK_OUT_IDX, VSPICLK_IN_IDX ); + + for ( int i = 0; i < 3; i ++ ) { + //slave send + spi_slave_transaction_t slave_t; + spi_slave_transaction_t* out; + memset(&slave_t, 0, sizeof(spi_slave_transaction_t)); + slave_t.length=8*32; + slave_t.tx_buffer=slave_txbuf+2*i; + slave_t.rx_buffer=slave_rxbuf; + TEST_ESP_OK( spi_slave_queue_trans( VSPI_HOST, &slave_t, portMAX_DELAY ) ); + + //send + spi_transaction_t t = {}; + t.length = 32*(i+1); + if ( t.length != 0 ) { + t.tx_buffer = master_txbuf+i; + t.rx_buffer = master_rxbuf+i; + } + spi_device_transmit( spi, (spi_transaction_t*)&t ); + + //wait for end + TEST_ESP_OK( spi_slave_get_trans_result( VSPI_HOST, &out, portMAX_DELAY ) ); + + //show result + ESP_LOGI(SLAVE_TAG, "trans_len: %d", slave_t.trans_len); + ESP_LOG_BUFFER_HEX( "master tx", t.tx_buffer, t.length/8 ); + ESP_LOG_BUFFER_HEX( "master rx", t.rx_buffer, t.length/8 ); + ESP_LOG_BUFFER_HEX( "slave tx", slave_t.tx_buffer, (slave_t.trans_len+7)/8); + ESP_LOG_BUFFER_HEX( "slave rx", slave_t.rx_buffer, (slave_t.trans_len+7)/8); + + TEST_ASSERT_EQUAL_HEX8_ARRAY( t.tx_buffer, slave_t.rx_buffer, t.length/8 ); + TEST_ASSERT_EQUAL_HEX8_ARRAY( slave_t.tx_buffer, t.rx_buffer, t.length/8 ); + + TEST_ASSERT_EQUAL( t.length, slave_t.trans_len ); + + //clean + memset( master_rxbuf, 0x66, sizeof(master_rxbuf)); + memset( slave_rxbuf, 0x66, sizeof(slave_rxbuf)); + } + + TEST_ASSERT(spi_slave_free(VSPI_HOST) == ESP_OK); + + TEST_ASSERT(spi_bus_remove_device(spi) == ESP_OK); + TEST_ASSERT(spi_bus_free(HSPI_HOST) == ESP_OK); + + ESP_LOGI(MASTER_TAG, "test passed."); +}