From 5eb8eb385599ee459655516e10ca168309e5a5e0 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Tue, 10 Jan 2017 14:41:12 +0800 Subject: [PATCH 1/6] SPI master: rename transaction flags from SPI_* to SPI_TRANS_*, like the documentation says. Also add some explanation about the SPI signals in the documentation --- components/driver/include/driver/spi_master.h | 10 +++---- components/driver/spi_master.c | 28 +++++++++---------- docs/api/spi_master.rst | 21 ++++++++++---- examples/26_spi_master/main/spi_master.c | 4 +-- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/components/driver/include/driver/spi_master.h b/components/driver/include/driver/spi_master.h index 4dd8738ad7..ea901b9924 100644 --- a/components/driver/include/driver/spi_master.h +++ b/components/driver/include/driver/spi_master.h @@ -86,11 +86,11 @@ typedef struct { } spi_device_interface_config_t; -#define SPI_MODE_DIO (1<<0) ///< Transmit/receive data in 2-bit mode -#define SPI_MODE_QIO (1<<1) ///< Transmit/receive data in 4-bit mode -#define SPI_MODE_DIOQIO_ADDR (1<<2) ///< Also transmit address in mode selected by SPI_MODE_DIO/SPI_MODE_QIO -#define SPI_USE_RXDATA (1<<2) ///< Receive into rx_data member of spi_transaction_t instead into memory at rx_buffer. -#define SPI_USE_TXDATA (1<<3) ///< Transmit tx_data member of spi_transaction_t instead of data at tx_buffer. Do not set tx_buffer when using this. +#define SPI_TRANS_MODE_DIO (1<<0) ///< Transmit/receive data in 2-bit mode +#define SPI_TRANS_MODE_QIO (1<<1) ///< Transmit/receive data in 4-bit mode +#define SPI_TRANS_MODE_DIOQIO_ADDR (1<<2) ///< Also transmit address in mode selected by SPI_MODE_DIO/SPI_MODE_QIO +#define SPI_TRANS_USE_RXDATA (1<<2) ///< Receive into rx_data member of spi_transaction_t instead into memory at rx_buffer. +#define SPI_TRANS_USE_TXDATA (1<<3) ///< Transmit tx_data member of spi_transaction_t instead of data at tx_buffer. Do not set tx_buffer when using this. /** * This structure describes one SPI transaction diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index fd4c5b2e29..1f1ea1ef08 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -452,10 +452,10 @@ static void IRAM_ATTR spi_intr(void *arg) if (host->cur_trans) { //Okay, transaction is done. - if ((host->cur_trans->rx_buffer || (host->cur_trans->flags & SPI_USE_RXDATA)) && host->cur_trans->rxlength<=THRESH_DMA_TRANS) { + if ((host->cur_trans->rx_buffer || (host->cur_trans->flags & SPI_TRANS_USE_RXDATA)) && host->cur_trans->rxlength<=THRESH_DMA_TRANS) { //Need to copy from SPI regs to result buffer. uint32_t *data; - if (host->cur_trans->flags & SPI_USE_RXDATA) { + if (host->cur_trans->flags & SPI_TRANS_USE_RXDATA) { data=(uint32_t*)&host->cur_trans->rx_data[0]; } else { data=(uint32_t*)host->cur_trans->rx_buffer; @@ -557,8 +557,8 @@ static void IRAM_ATTR spi_intr(void *arg) //QIO/DIO host->hw->ctrl.val &= ~(SPI_FREAD_DUAL|SPI_FREAD_QUAD|SPI_FREAD_DIO|SPI_FREAD_QIO); host->hw->user.val &= ~(SPI_FWRITE_DUAL|SPI_FWRITE_QUAD|SPI_FWRITE_DIO|SPI_FWRITE_QIO); - if (trans->flags & SPI_MODE_DIO) { - if (trans->flags & SPI_MODE_DIOQIO_ADDR) { + if (trans->flags & SPI_TRANS_MODE_DIO) { + if (trans->flags & SPI_TRANS_MODE_DIOQIO_ADDR) { host->hw->ctrl.fread_dio=1; host->hw->user.fwrite_dio=1; } else { @@ -566,8 +566,8 @@ static void IRAM_ATTR spi_intr(void *arg) host->hw->user.fwrite_dual=1; } host->hw->ctrl.fastrd_mode=1; - } else if (trans->flags & SPI_MODE_QIO) { - if (trans->flags & SPI_MODE_DIOQIO_ADDR) { + } else if (trans->flags & SPI_TRANS_MODE_QIO) { + if (trans->flags & SPI_TRANS_MODE_DIOQIO_ADDR) { host->hw->ctrl.fread_qio=1; host->hw->user.fwrite_qio=1; } else { @@ -579,9 +579,9 @@ static void IRAM_ATTR spi_intr(void *arg) //Fill DMA descriptors - if (trans->rx_buffer || (trans->flags & SPI_USE_RXDATA)) { + if (trans->rx_buffer || (trans->flags & SPI_TRANS_USE_RXDATA)) { uint32_t *data; - if (trans->flags & SPI_USE_RXDATA) { + if (trans->flags & SPI_TRANS_USE_RXDATA) { data=(uint32_t *)&trans->rx_data[0]; } else { data=trans->rx_buffer; @@ -604,9 +604,9 @@ static void IRAM_ATTR spi_intr(void *arg) host->hw->user.usr_miso=0; } - if (trans->tx_buffer || (trans->flags & SPI_USE_TXDATA)) { + if (trans->tx_buffer || (trans->flags & SPI_TRANS_USE_TXDATA)) { uint32_t *data; - if (trans->flags & SPI_USE_TXDATA) { + if (trans->flags & SPI_TRANS_USE_TXDATA) { data=(uint32_t *)&trans->tx_data[0]; } else { data=(uint32_t *)trans->tx_buffer; @@ -657,10 +657,10 @@ esp_err_t spi_device_queue_trans(spi_device_handle_t handle, spi_transaction_t * { BaseType_t r; SPI_CHECK(handle!=NULL, "invalid dev handle", ESP_ERR_INVALID_ARG); - SPI_CHECK((trans_desc->flags & SPI_USE_RXDATA)==0 ||trans_desc->length <= 32, "rxdata transfer > 32bytes", ESP_ERR_INVALID_ARG); - SPI_CHECK((trans_desc->flags & SPI_USE_TXDATA)==0 ||trans_desc->length <= 32, "txdata transfer > 32bytes", ESP_ERR_INVALID_ARG); - SPI_CHECK(!((trans_desc->flags & (SPI_MODE_DIO|SPI_MODE_QIO)) && (handle->cfg.flags & SPI_DEVICE_3WIRE)), "incompatible iface params", ESP_ERR_INVALID_ARG); - SPI_CHECK(!((trans_desc->flags & (SPI_MODE_DIO|SPI_MODE_QIO)) && (!(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX))), "incompatible iface params", ESP_ERR_INVALID_ARG); + SPI_CHECK((trans_desc->flags & SPI_TRANS_USE_RXDATA)==0 ||trans_desc->length <= 32, "rxdata transfer > 32bytes", ESP_ERR_INVALID_ARG); + SPI_CHECK((trans_desc->flags & SPI_TRANS_USE_TXDATA)==0 ||trans_desc->length <= 32, "txdata transfer > 32bytes", ESP_ERR_INVALID_ARG); + SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (handle->cfg.flags & SPI_DEVICE_3WIRE)), "incompatible iface params", ESP_ERR_INVALID_ARG); + SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (!(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX))), "incompatible iface params", ESP_ERR_INVALID_ARG); r=xQueueSend(handle->trans_queue, (void*)&trans_desc, ticks_to_wait); if (!r) return ESP_ERR_TIMEOUT; esp_intr_enable(handle->host->intr); diff --git a/docs/api/spi_master.rst b/docs/api/spi_master.rst index 99f2f6a7ca..4d7693c07c 100644 --- a/docs/api/spi_master.rst +++ b/docs/api/spi_master.rst @@ -29,6 +29,17 @@ The spi_master driver uses the following terms: * Bus: The SPI bus, common to all SPI devices connected to one host. In general the bus consists of the spid, spiq, spiclk and optionally spiwp and spihd signals. The SPI slaves are connected to these signals in parallel. + + - spiq - Also known as MISO, this is the input of the serial stream into the ESP32 + + - spid - Also known as MOSI, this is the output of the serial stream from the ESP32 + + - spiclk - Clock signal. Each data bit is clocked out or in on the positive or negative edge of this signal + + - spiwp - Write Protect signal. Only used for 4-bit (qio/qout) transactions. + + - spihd - Hold signal. Only used for 4-bit (qio/qout) transactions. + * Device: A SPI slave. Each SPI slave has its own chip select (CS) line, which is made active when a transmission to/from the SPI slave occurs. * Transaction: One instance of CS going active, data transfer from and/or to a device happening, and @@ -113,11 +124,11 @@ Macros .. doxygendefine:: SPI_DEVICE_HALFDUPLEX .. doxygendefine:: SPI_DEVICE_CLK_AS_CS -.. doxygendefine:: SPI_MODE_DIO -.. doxygendefine:: SPI_MODE_QIO -.. doxygendefine:: SPI_MODE_DIOQIO_ADDR -.. doxygendefine:: SPI_USE_RXDATA -.. doxygendefine:: SPI_USE_TXDATA +.. doxygendefine:: SPI_TRANS_MODE_DIO +.. doxygendefine:: SPI_TRANS_MODE_QIO +.. doxygendefine:: SPI_TRANS_MODE_DIOQIO_ADDR +.. doxygendefine:: SPI_TRANS_USE_RXDATA +.. doxygendefine:: SPI_TRANS_USE_TXDATA Type Definitions ^^^^^^^^^^^^^^^^ diff --git a/examples/26_spi_master/main/spi_master.c b/examples/26_spi_master/main/spi_master.c index 1cdbf72ad5..2013dfd9c9 100644 --- a/examples/26_spi_master/main/spi_master.c +++ b/examples/26_spi_master/main/spi_master.c @@ -168,7 +168,7 @@ void send_line(spi_device_handle_t spi, int ypos, uint16_t *line) trans[x].length=8*4; trans[x].user=(void*)1; } - trans[x].flags=SPI_USE_TXDATA; + trans[x].flags=SPI_TRANS_USE_TXDATA; } trans[0].tx_data[0]=0x2A; //Column Address Set trans[1].tx_data[0]=0; //Start Col High @@ -183,7 +183,7 @@ void send_line(spi_device_handle_t spi, int ypos, uint16_t *line) trans[4].tx_data[0]=0x2C; //memory write trans[5].tx_buffer=line; //finally send the line data trans[5].length=320*2*8; //Data length, in bits - trans[5].flags=0; //undo SPI_USE_TXDATA flag + trans[5].flags=0; //undo SPI_TRANS_USE_TXDATA flag //Queue all transactions. for (x=0; x<6; x++) { From ee59fa75f4edaf5124369a7751382ece9c6e32ef Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Wed, 11 Jan 2017 11:25:56 +0800 Subject: [PATCH 2/6] Rename SPI Master IO pins to more common terminology, add better explanation to queue_length initialization value --- components/driver/include/driver/spi_master.h | 12 +-- components/driver/spi_master.c | 78 +++++++++---------- components/driver/test/test_spi_master.c | 12 ++- examples/26_spi_master/main/spi_master.c | 10 +-- 4 files changed, 55 insertions(+), 57 deletions(-) diff --git a/components/driver/include/driver/spi_master.h b/components/driver/include/driver/spi_master.h index ea901b9924..83110d9d05 100644 --- a/components/driver/include/driver/spi_master.h +++ b/components/driver/include/driver/spi_master.h @@ -46,11 +46,11 @@ typedef enum { * the IO_MUX or are -1. In that case, the IO_MUX is used, allowing for >40MHz speeds. */ typedef struct { - int spid_io_num; ///< GPIO pin for spi_d (=MOSI)signal, or -1 if not used. - int spiq_io_num; ///< GPIO pin for spi_q (=MISO) signal, or -1 if not used. - int spiclk_io_num; ///< GPIO pin for spi_clk signal, or -1 if not used. - int spiwp_io_num; ///< GPIO pin for spi_wp signal, or -1 if not used. - int spihd_io_num; ///< GPIO pin for spi_hd signal, or -1 if not used. + int mosi_io_num; ///< GPIO pin for Master Out Slave In (=spi_d) signal, or -1 if not used. + int miso_io_num; ///< GPIO pin for Master In Slave Out (=spi_q) signal, or -1 if not used. + int sclk_io_num; ///< GPIO pin for Spi CLocK signal, or -1 if not used. + int quadwp_io_num; ///< GPIO pin for WP (Write Protect) signal which is used as D2 in 4-bit communication modes, or -1 if not used. + int quadhd_io_num; ///< GPIO pin for HD (HolD) signal which is used as D3 in 4-bit communication modes, or -1 if not used. } spi_bus_config_t; @@ -80,7 +80,7 @@ typedef struct { int clock_speed_hz; ///< Clock speed, in Hz int spics_io_num; ///< CS GPIO pin for this device, or -1 if not used uint32_t flags; ///< Bitwise OR of SPI_DEVICE_* flags - int queue_size; ///< Transaction queue size + int queue_size; ///< Transaction queue size. This sets how many transactions can be 'in the air' (queued using spi_device_queue_trans but not yet finished using spi_device_get_trans_result) at the same time transaction_cb_t pre_cb; ///< Callback to be called before a transmission is started. This callback is called within interrupt context. transaction_cb_t post_cb; ///< Callback to be called after a transmission has completed. This callback is called within interrupt context. } spi_device_interface_config_t; diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 1f1ea1ef08..e9b8fb3cb0 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -200,11 +200,11 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, spi_bus_config_t *bus_confi SPI_CHECK(host>=SPI_HOST && host<=VSPI_HOST, "invalid host", ESP_ERR_INVALID_ARG); SPI_CHECK(spihost[host]==NULL, "host already in use", ESP_ERR_INVALID_STATE); - SPI_CHECK(bus_config->spid_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->spid_io_num), "spid pin invalid", ESP_ERR_INVALID_ARG); - SPI_CHECK(bus_config->spiclk_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->spiclk_io_num), "spiclk pin invalid", ESP_ERR_INVALID_ARG); - SPI_CHECK(bus_config->spiq_io_num<0 || GPIO_IS_VALID_GPIO(bus_config->spiq_io_num), "spiq pin invalid", ESP_ERR_INVALID_ARG); - SPI_CHECK(bus_config->spiwp_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->spiwp_io_num), "spiwp pin invalid", ESP_ERR_INVALID_ARG); - SPI_CHECK(bus_config->spihd_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->spihd_io_num), "spihd pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(bus_config->mosi_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->mosi_io_num), "spid pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(bus_config->sclk_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->sclk_io_num), "spiclk pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(bus_config->miso_io_num<0 || GPIO_IS_VALID_GPIO(bus_config->miso_io_num), "spiq pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(bus_config->quadwp_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->quadwp_io_num), "spiwp pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(bus_config->quadhd_io_num<0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->quadhd_io_num), "spihd pin invalid", ESP_ERR_INVALID_ARG); //The host struct contains two dma descriptors, so we need DMA'able memory for this. spihost[host]=pvPortMallocCaps(sizeof(spi_host_t), MALLOC_CAP_DMA); @@ -212,51 +212,51 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, spi_bus_config_t *bus_confi memset(spihost[host], 0, sizeof(spi_host_t)); //Check if the selected pins correspond to the native pins of the peripheral - if (bus_config->spid_io_num >= 0 && bus_config->spid_io_num!=io_signal[host].spid_native) native=false; - if (bus_config->spiq_io_num >= 0 && bus_config->spiq_io_num!=io_signal[host].spiq_native) native=false; - if (bus_config->spiclk_io_num >= 0 && bus_config->spiclk_io_num!=io_signal[host].spiclk_native) native=false; - if (bus_config->spiwp_io_num >= 0 && bus_config->spiwp_io_num!=io_signal[host].spiwp_native) native=false; - if (bus_config->spihd_io_num >= 0 && bus_config->spihd_io_num!=io_signal[host].spihd_native) native=false; + if (bus_config->mosi_io_num >= 0 && bus_config->mosi_io_num!=io_signal[host].spid_native) native=false; + if (bus_config->miso_io_num >= 0 && bus_config->miso_io_num!=io_signal[host].spiq_native) native=false; + if (bus_config->sclk_io_num >= 0 && bus_config->sclk_io_num!=io_signal[host].spiclk_native) native=false; + if (bus_config->quadwp_io_num >= 0 && bus_config->quadwp_io_num!=io_signal[host].spiwp_native) native=false; + if (bus_config->quadhd_io_num >= 0 && bus_config->quadhd_io_num!=io_signal[host].spihd_native) native=false; spihost[host]->no_gpio_matrix=native; if (native) { //All SPI native pin selections resolve to 1, so we put that here instead of trying to figure //out which FUNC_GPIOx_xSPIxx to grab; they all are defined to 1 anyway. - if (bus_config->spid_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spid_io_num], 1); - if (bus_config->spiq_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiq_io_num], 1); - if (bus_config->spiwp_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiwp_io_num], 1); - if (bus_config->spihd_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spihd_io_num], 1); - if (bus_config->spiclk_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiclk_io_num], 1); + if (bus_config->mosi_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->mosi_io_num], 1); + if (bus_config->miso_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->miso_io_num], 1); + if (bus_config->quadwp_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->quadwp_io_num], 1); + if (bus_config->quadhd_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->quadhd_io_num], 1); + if (bus_config->sclk_io_num > 0) PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->sclk_io_num], 1); } else { //Use GPIO - if (bus_config->spid_io_num>0) { - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spid_io_num], PIN_FUNC_GPIO); - gpio_set_direction(bus_config->spid_io_num, GPIO_MODE_OUTPUT); - gpio_matrix_out(bus_config->spid_io_num, io_signal[host].spid_out, false, false); - gpio_matrix_in(bus_config->spid_io_num, io_signal[host].spid_in, false); + if (bus_config->mosi_io_num>0) { + PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->mosi_io_num], PIN_FUNC_GPIO); + gpio_set_direction(bus_config->mosi_io_num, GPIO_MODE_OUTPUT); + gpio_matrix_out(bus_config->mosi_io_num, io_signal[host].spid_out, false, false); + gpio_matrix_in(bus_config->mosi_io_num, io_signal[host].spid_in, false); } - if (bus_config->spiq_io_num>0) { - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiq_io_num], PIN_FUNC_GPIO); - gpio_set_direction(bus_config->spiq_io_num, GPIO_MODE_INPUT); - gpio_matrix_out(bus_config->spiq_io_num, io_signal[host].spiq_out, false, false); - gpio_matrix_in(bus_config->spiq_io_num, io_signal[host].spiq_in, false); + if (bus_config->miso_io_num>0) { + PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->miso_io_num], PIN_FUNC_GPIO); + gpio_set_direction(bus_config->miso_io_num, GPIO_MODE_INPUT); + gpio_matrix_out(bus_config->miso_io_num, io_signal[host].spiq_out, false, false); + gpio_matrix_in(bus_config->miso_io_num, io_signal[host].spiq_in, false); } - if (bus_config->spiwp_io_num>0) { - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiwp_io_num], PIN_FUNC_GPIO); - gpio_set_direction(bus_config->spiwp_io_num, GPIO_MODE_OUTPUT); - gpio_matrix_out(bus_config->spiwp_io_num, io_signal[host].spiwp_out, false, false); - gpio_matrix_in(bus_config->spiwp_io_num, io_signal[host].spiwp_in, false); + if (bus_config->quadwp_io_num>0) { + PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->quadwp_io_num], PIN_FUNC_GPIO); + gpio_set_direction(bus_config->quadwp_io_num, GPIO_MODE_OUTPUT); + gpio_matrix_out(bus_config->quadwp_io_num, io_signal[host].spiwp_out, false, false); + gpio_matrix_in(bus_config->quadwp_io_num, io_signal[host].spiwp_in, false); } - if (bus_config->spihd_io_num>0) { - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spihd_io_num], PIN_FUNC_GPIO); - gpio_set_direction(bus_config->spihd_io_num, GPIO_MODE_OUTPUT); - gpio_matrix_out(bus_config->spihd_io_num, io_signal[host].spihd_out, false, false); - gpio_matrix_in(bus_config->spihd_io_num, io_signal[host].spihd_in, false); + if (bus_config->quadhd_io_num>0) { + PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->quadhd_io_num], PIN_FUNC_GPIO); + gpio_set_direction(bus_config->quadhd_io_num, GPIO_MODE_OUTPUT); + gpio_matrix_out(bus_config->quadhd_io_num, io_signal[host].spihd_out, false, false); + gpio_matrix_in(bus_config->quadhd_io_num, io_signal[host].spihd_in, false); } - if (bus_config->spiclk_io_num>0) { - PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->spiclk_io_num], PIN_FUNC_GPIO); - gpio_set_direction(bus_config->spiclk_io_num, GPIO_MODE_OUTPUT); - gpio_matrix_out(bus_config->spiclk_io_num, io_signal[host].spiclk_out, false, false); + if (bus_config->sclk_io_num>0) { + PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->sclk_io_num], PIN_FUNC_GPIO); + gpio_set_direction(bus_config->sclk_io_num, GPIO_MODE_OUTPUT); + gpio_matrix_out(bus_config->sclk_io_num, io_signal[host].spiclk_out, false, false); } } periph_module_enable(io_signal[host].module); diff --git a/components/driver/test/test_spi_master.c b/components/driver/test/test_spi_master.c index 0fd47cbdba..119c94bc57 100644 --- a/components/driver/test/test_spi_master.c +++ b/components/driver/test/test_spi_master.c @@ -21,11 +21,11 @@ TEST_CASE("SPI Master test", "[spi]") { spi_bus_config_t buscfg={ - .spid_io_num=4, - .spiq_io_num=16, - .spiclk_io_num=25, - .spiwp_io_num=-1, - .spihd_io_num=-1 + .mosi_io_num=4, + .miso_io_num=16, + .sclk_io_num=25, + .quadwp_io_num=-1, + .quadhd_io_num=-1 }; spi_device_interface_config_t devcfg={ .command_bits=8, @@ -33,8 +33,6 @@ TEST_CASE("SPI Master test", "[spi]") .dummy_bits=0, .clock_speed_hz=8000, .duty_cycle_pos=128, - .cs_ena_pretrans=7, - .cs_ena_posttrans=7, .mode=0, .spics_io_num=21, .queue_size=3 diff --git a/examples/26_spi_master/main/spi_master.c b/examples/26_spi_master/main/spi_master.c index 2013dfd9c9..577cada4c2 100644 --- a/examples/26_spi_master/main/spi_master.c +++ b/examples/26_spi_master/main/spi_master.c @@ -249,11 +249,11 @@ void app_main() esp_err_t ret; spi_device_handle_t spi; spi_bus_config_t buscfg={ - .spiq_io_num=PIN_NUM_MISO, - .spid_io_num=PIN_NUM_MOSI, - .spiclk_io_num=PIN_NUM_CLK, - .spiwp_io_num=-1, - .spihd_io_num=-1 + .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=10000000, //Clock out at 10 MHz From 69a7cfa9766e6b712fd7aa2f2a9e356423b19fe2 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Wed, 11 Jan 2017 11:55:23 +0800 Subject: [PATCH 3/6] Also update documentation to new conventions --- docs/api/spi_master.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/api/spi_master.rst b/docs/api/spi_master.rst index 4d7693c07c..5b57d85f42 100644 --- a/docs/api/spi_master.rst +++ b/docs/api/spi_master.rst @@ -27,18 +27,18 @@ The spi_master driver uses the following terms: now, only HSPI or VSPI are actually supported in the driver; it will support all 3 peripherals somewhere in the future.) * Bus: The SPI bus, common to all SPI devices connected to one host. In general the bus consists of the - spid, spiq, spiclk and optionally spiwp and spihd signals. The SPI slaves are connected to these + miso, mosi, sclk and optionally quadwp and quadhd signals. The SPI slaves are connected to these signals in parallel. - - spiq - Also known as MISO, this is the input of the serial stream into the ESP32 + - miso - Also known as q, this is the input of the serial stream into the ESP32 - - spid - Also known as MOSI, this is the output of the serial stream from the ESP32 + - mosi - Also known as d, this is the output of the serial stream from the ESP32 - - spiclk - Clock signal. Each data bit is clocked out or in on the positive or negative edge of this signal + - sclk - Clock signal. Each data bit is clocked out or in on the positive or negative edge of this signal - - spiwp - Write Protect signal. Only used for 4-bit (qio/qout) transactions. + - quadwp - Write Protect signal. Only used for 4-bit (qio/qout) transactions. - - spihd - Hold signal. Only used for 4-bit (qio/qout) transactions. + - quadhd - Hold signal. Only used for 4-bit (qio/qout) transactions. * Device: A SPI slave. Each SPI slave has its own chip select (CS) line, which is made active when a transmission to/from the SPI slave occurs. From a98d07d65027ea8e8e3bfc81203c6e8893aab869 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Wed, 11 Jan 2017 13:01:48 +0800 Subject: [PATCH 4/6] Fix clock divider calculation --- components/driver/spi_master.c | 60 ++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index e9b8fb3cb0..204e577eda 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -389,39 +389,51 @@ esp_err_t spi_bus_remove_device(spi_device_handle_t handle) return ESP_OK; } +static int spi_freq_for_pre_n(int fapb, int pre, int n) { + return (fapb / (pre * n)); +} + static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { int pre, n, h, l; - //In hw, n, h and l are 1-32, pre is 0-8K. Value written to register is one lower than used value. - if (hz>(fapb/2)) { - //Can only solve this using fapb directly. + + //In hw, n, h and l are 1-32, pre is 1-8K. Value written to register is one lower than used value. + if (hz>((fapb/4)*3)) { + //Using Fapb directly will give us the best result here. hw->clock.clkcnt_l=0; hw->clock.clkcnt_h=0; hw->clock.clkcnt_n=0; hw->clock.clkdiv_pre=0; hw->clock.clk_equ_sysclk=1; } else { - //For best duty cycle resolution, we want n to be as close to 32 as possible. - //ToDo: - //This algo could use some tweaking; at the moment it either fixes n to 32 and - //uses the prescaler to get a suitable division factor, or sets the prescaler to 0 - //and uses n to set a value. In practice, sometimes a better result can be - //obtained by setting both n and pre to well-chosen valued... ToDo: fix up some algo to - //do this automatically (worst-case: bruteforce n/pre combo's) - JD - //Also ToDo: - //The ESP32 has a SPI_CK_OUT_HIGH_MODE and SPI_CK_OUT_LOW_MODE register; it looks like we can - //use those to specify the duty cycle in a more precise way. Figure out how to use these. - JD - n=(fapb/(hz*32)); - if (n>32) { - //Need to use prescaler - n=32; + //For best duty cycle resolution, we want n to be as close to 32 as possible, but + //we also need a pre/n combo that gets us as close as possible to the intended freq. + //To do this, we bruteforce n and calculate the best pre to go along with that. + //If there's a choice between pre/n combos that give the same result, use the one + //with the higher n. + int bestn=-1; + int bestpre=-1; + int besterr=hz; + int errval; + for (n=1; n<33; n++) { + //Effectively, this does pre=round((fapb/n)/hz). + pre=((fapb/n)+(hz/2))/hz; + if (pre<0) pre=0; + if (pre>8192) pre=8192; + errval=abs(spi_freq_for_pre_n(fapb, pre, n)-hz); + if (errval<=besterr) { + besterr=errval; + bestn=n; + bestpre=pre; + } } - if (n<32) { - //No need for prescaler. - n=(fapb/hz); - } - pre=(fapb/n)/hz; - h=n; - l=(((256-duty_cycle)*n+127)/256); + + n=bestn; + pre=bestpre; + l=n; + //This effectively does round((duty_cycle*n)/256) + h=(duty_cycle*n+127)/256; + if (h<=0) h=1; + hw->clock.clk_equ_sysclk=0; hw->clock.clkcnt_n=n-1; hw->clock.clkdiv_pre=pre-1; From daa2b7cbc968feaa0f259e6b52020e954dde6ffb Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Wed, 11 Jan 2017 14:13:37 +0800 Subject: [PATCH 5/6] n, h and l actually are 6-bit; they go from 1 to 64. --- components/driver/spi_master.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 204e577eda..53be8b29de 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -396,7 +396,7 @@ static int spi_freq_for_pre_n(int fapb, int pre, int n) { static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { int pre, n, h, l; - //In hw, n, h and l are 1-32, pre is 1-8K. Value written to register is one lower than used value. + //In hw, n, h and l are 1-64, pre is 1-8K. Value written to register is one lower than used value. if (hz>((fapb/4)*3)) { //Using Fapb directly will give us the best result here. hw->clock.clkcnt_l=0; @@ -414,7 +414,7 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { int bestpre=-1; int besterr=hz; int errval; - for (n=1; n<33; n++) { + for (n=1; n<=64; n++) { //Effectively, this does pre=round((fapb/n)/hz). pre=((fapb/n)+(hz/2))/hz; if (pre<0) pre=0; From 356e01545c08888463442fa3c3c7adbec1658a97 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Wed, 11 Jan 2017 16:13:33 +0800 Subject: [PATCH 6/6] Add test for spi clock, fix corner cases) --- components/driver/spi_master.c | 7 +-- components/driver/test/test_spi_master.c | 62 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 53be8b29de..6162edba36 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -318,6 +318,7 @@ esp_err_t spi_bus_add_device(spi_host_device_t host, spi_device_interface_config SPI_CHECK(host>=SPI_HOST && host<=VSPI_HOST, "invalid host", ESP_ERR_INVALID_ARG); SPI_CHECK(spihost[host]!=NULL, "host not initialized", ESP_ERR_INVALID_STATE); SPI_CHECK(dev_config->spics_io_num < 0 || GPIO_IS_VALID_OUTPUT_GPIO(dev_config->spics_io_num), "spics pin invalid", ESP_ERR_INVALID_ARG); + SPI_CHECK(dev_config->clock_speed_hz > 0, "invalid sclk speed", ESP_ERR_INVALID_ARG); for (freecs=0; freecsdevice[freecs], NULL, (spi_device_t *)1)) break; @@ -412,15 +413,15 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { //with the higher n. int bestn=-1; int bestpre=-1; - int besterr=hz; + int besterr=0; int errval; for (n=1; n<=64; n++) { //Effectively, this does pre=round((fapb/n)/hz). pre=((fapb/n)+(hz/2))/hz; - if (pre<0) pre=0; + if (pre<=0) pre=1; if (pre>8192) pre=8192; errval=abs(spi_freq_for_pre_n(fapb, pre, n)-hz); - if (errval<=besterr) { + if (bestn==-1 || errval<=besterr) { besterr=errval; bestn=n; bestpre=pre; diff --git a/components/driver/test/test_spi_master.c b/components/driver/test/test_spi_master.c index 119c94bc57..d54cc5a633 100644 --- a/components/driver/test/test_spi_master.c +++ b/components/driver/test/test_spi_master.c @@ -15,8 +15,70 @@ #include "freertos/xtensa_api.h" #include "unity.h" #include "driver/spi_master.h" +#include "soc/dport_reg.h" +#include "soc/spi_reg.h" +#include "soc/spi_struct.h" +static void check_spi_pre_n_for(int clk, int pre, int n) +{ + esp_err_t ret; + spi_device_handle_t handle; + + spi_device_interface_config_t devcfg={ + .command_bits=0, + .address_bits=0, + .dummy_bits=0, + .clock_speed_hz=clk, + .duty_cycle_pos=128, + .mode=0, + .spics_io_num=21, + .queue_size=3 + }; + char sendbuf[16]=""; + spi_transaction_t t; + memset(&t, 0, sizeof(t)); + + ret=spi_bus_add_device(HSPI_HOST, &devcfg, &handle); + TEST_ASSERT(ret==ESP_OK); + + t.length=16*8; + t.tx_buffer=sendbuf; + ret=spi_device_transmit(handle, &t); + + printf("Checking clk rate %dHz. expect pre %d n %d, got pre %d n %d\n", clk, pre, n, SPI2.clock.clkdiv_pre+1, SPI2.clock.clkcnt_n+1); + + TEST_ASSERT(SPI2.clock.clkcnt_n+1==n); + TEST_ASSERT(SPI2.clock.clkdiv_pre+1==pre); + + ret=spi_bus_remove_device(handle); + TEST_ASSERT(ret==ESP_OK); +} + + +TEST_CASE("SPI Master clockdiv calculation routines", "[spi]") +{ + spi_bus_config_t buscfg={ + .mosi_io_num=4, + .miso_io_num=16, + .sclk_io_num=25, + .quadwp_io_num=-1, + .quadhd_io_num=-1 + }; + esp_err_t ret; + ret=spi_bus_initialize(HSPI_HOST, &buscfg, 1); + TEST_ASSERT(ret==ESP_OK); + + check_spi_pre_n_for(8000000, 1, 10); + check_spi_pre_n_for(800000, 2, 50); + check_spi_pre_n_for(100000, 16, 50); + check_spi_pre_n_for(333333, 4, 60); + check_spi_pre_n_for(1, 8192, 64); //Actually should generate the minimum clock speed, 152Hz + + ret=spi_bus_free(HSPI_HOST); + TEST_ASSERT(ret==ESP_OK); +} + TEST_CASE("SPI Master test", "[spi]") {