From 76295c7a13a9aa53538b0915e81999862d871ccd Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Thu, 2 Mar 2017 18:46:59 +0800 Subject: [PATCH] Fix timing adjustment needed for higher speeds of SPI master bus. --- components/driver/include/driver/spi_master.h | 3 + components/driver/spi_master.c | 43 +++++++++++--- components/driver/test/test_spi_master.c | 57 +++++++++++-------- 3 files changed, 71 insertions(+), 32 deletions(-) diff --git a/components/driver/include/driver/spi_master.h b/components/driver/include/driver/spi_master.h index 83110d9d05..d78f5ea0b1 100644 --- a/components/driver/include/driver/spi_master.h +++ b/components/driver/include/driver/spi_master.h @@ -153,6 +153,9 @@ esp_err_t spi_bus_free(spi_host_device_t host); * peripheral and routes it to the indicated GPIO. All SPI master devices have three CS pins and can thus control * up to three devices. * + * @note While in general, speeds up to 80MHz on the dedicated SPI pins and 40MHz on GPIO-matrix-routed pins are + * supported, full-duplex transfers routed over the GPIO matrix only support speeds up to 26MHz. + * * @param host SPI peripheral to allocate device on * @param dev_config SPI interface protocol config for the device * @param handle Pointer to variable to hold the device handle diff --git a/components/driver/spi_master.c b/components/driver/spi_master.c index 17a68c3235..723fbdb035 100644 --- a/components/driver/spi_master.c +++ b/components/driver/spi_master.c @@ -268,7 +268,9 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, spi_bus_config_t *bus_confi spihost[host]->hw->dma_out_link.start=0; spihost[host]->hw->dma_in_link.start=0; spihost[host]->hw->dma_conf.val&=~(SPI_OUT_RST|SPI_AHBM_RST|SPI_AHBM_FIFO_RST); - + //Reset timing + spihost[host]->hw->ctrl2.val=0; + //Disable unneeded ints spihost[host]->hw->slave.rd_buf_done=0; spihost[host]->hw->slave.wr_buf_done=0; @@ -315,6 +317,7 @@ esp_err_t spi_bus_free(spi_host_device_t host) esp_err_t spi_bus_add_device(spi_host_device_t host, spi_device_interface_config_t *dev_config, spi_device_handle_t *handle) { int freecs; + int apbclk=APB_CLK_FREQ; 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); @@ -327,6 +330,9 @@ esp_err_t spi_bus_add_device(spi_host_device_t host, spi_device_interface_config //The hardware looks like it would support this, but actually setting cs_ena_pretrans when transferring in full //duplex mode does absolutely nothing on the ESP32. SPI_CHECK(dev_config->cs_ena_pretrans==0 || (dev_config->flags & SPI_DEVICE_HALFDUPLEX), "cs pretrans delay incompatible with full-duplex", ESP_ERR_INVALID_ARG); + //Speeds >=40MHz over GPIO matrix needs a dummy cycle, but these don't work for full-duplex connections. + SPI_CHECK(!( ((dev_config->flags & SPI_DEVICE_HALFDUPLEX)==0) && (dev_config->clock_speed_hz > ((apbclk*2)/5)) && (!spihost[host]->no_gpio_matrix)), + "No speeds >26MHz supported for full-duplex, GPIO-matrix SPI transfers", ESP_ERR_INVALID_ARG); //Allocate memory for device spi_device_t *dev=malloc(sizeof(spi_device_t)); @@ -394,8 +400,12 @@ 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; +/* + * Set the SPI clock to a certain frequency. Returns the effective frequency set, which may be slightly + * different from the requested frequency. + */ +static int spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { + int pre, n, h, l, eff_clk; //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)) { @@ -405,6 +415,7 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { hw->clock.clkcnt_n=0; hw->clock.clkdiv_pre=0; hw->clock.clk_equ_sysclk=1; + eff_clk=fapb; } else { //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. @@ -440,7 +451,9 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) { hw->clock.clkdiv_pre=pre-1; hw->clock.clkcnt_h=h-1; hw->clock.clkcnt_l=l-1; + eff_clk=spi_freq_for_pre_n(fapb, pre, n); } + return eff_clk; } @@ -516,14 +529,28 @@ static void IRAM_ATTR spi_intr(void *arg) //Assumes a hardcoded 80MHz Fapb for now. ToDo: figure out something better once we have //clock scaling working. int apbclk=APB_CLK_FREQ; - spi_set_clock(host->hw, apbclk, dev->cfg.clock_speed_hz, dev->cfg.duty_cycle_pos); + int effclk=spi_set_clock(host->hw, apbclk, dev->cfg.clock_speed_hz, dev->cfg.duty_cycle_pos); //Configure bit order host->hw->ctrl.rd_bit_order=(dev->cfg.flags & SPI_DEVICE_RXBIT_LSBFIRST)?1:0; host->hw->ctrl.wr_bit_order=(dev->cfg.flags & SPI_DEVICE_TXBIT_LSBFIRST)?1:0; //Configure polarity - //SPI iface needs to be configured for a delay unless it is not routed through GPIO and clock is >=apb/2 - int nodelay=(host->no_gpio_matrix && dev->cfg.clock_speed_hz >= (apbclk/2)); + //SPI iface needs to be configured for a delay in some cases. + int nodelay=0; + int extra_dummy=0; + if (host->no_gpio_matrix) { + if (effclk >= apbclk/2) { + nodelay=1; + } + } else { + if (effclk >= apbclk/2) { + nodelay=1; + extra_dummy=1; //Note: This only works on half-duplex connections. spi_bus_add_device checks for this. + } else if (effclk >= apbclk/4) { + nodelay=1; + } + } + if (dev->cfg.mode==0) { host->hw->pin.ck_idle_edge=0; host->hw->user.ck_out_edge=0; @@ -543,11 +570,11 @@ static void IRAM_ATTR spi_intr(void *arg) } //Configure bit sizes, load addr and command - host->hw->user.usr_dummy=(dev->cfg.dummy_bits)?1:0; + host->hw->user.usr_dummy=(dev->cfg.dummy_bits+extra_dummy)?1:0; host->hw->user.usr_addr=(dev->cfg.address_bits)?1:0; host->hw->user.usr_command=(dev->cfg.command_bits)?1:0; host->hw->user1.usr_addr_bitlen=dev->cfg.address_bits-1; - host->hw->user1.usr_dummy_cyclelen=dev->cfg.dummy_bits-1; + host->hw->user1.usr_dummy_cyclelen=dev->cfg.dummy_bits+extra_dummy-1; host->hw->user2.usr_command_bitlen=dev->cfg.command_bits-1; //Configure misc stuff host->hw->user.doutdin=(dev->cfg.flags & SPI_DEVICE_HALFDUPLEX)?0:1; diff --git a/components/driver/test/test_spi_master.c b/components/driver/test/test_spi_master.c index cb04af8c97..f88427a81b 100644 --- a/components/driver/test/test_spi_master.c +++ b/components/driver/test/test_spi_master.c @@ -80,41 +80,29 @@ TEST_CASE("SPI Master clockdiv calculation routines", "[spi]") } -TEST_CASE("SPI Master test", "[spi][ignore]") -{ - 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 - }; +static void test_spi_bus_speed(int hz) { + esp_err_t ret; + spi_device_handle_t handle; spi_device_interface_config_t devcfg={ .command_bits=8, .address_bits=64, .dummy_bits=0, - .clock_speed_hz=8000, + .clock_speed_hz=hz, .duty_cycle_pos=128, .mode=0, .spics_io_num=21, - .queue_size=3 + .queue_size=3, }; - esp_err_t ret; - spi_device_handle_t handle; - printf("THIS TEST NEEDS A JUMPER BETWEEN IO4 AND IO16\n"); - - ret=spi_bus_initialize(HSPI_HOST, &buscfg, 1); - TEST_ASSERT(ret==ESP_OK); ret=spi_bus_add_device(HSPI_HOST, &devcfg, &handle); TEST_ASSERT(ret==ESP_OK); printf("Bus/dev inited.\n"); spi_transaction_t t; - char sendbuf[16]="Hello World!"; - char recvbuf[16]="UUUUUUUUUUUUUUU"; + char sendbuf[64]="Hello World!"; + char recvbuf[64]="UUUUUUUUUUUUUUU"; memset(&t, 0, sizeof(t)); - t.length=16*8; + t.length=64*8; t.tx_buffer=sendbuf; t.rx_buffer=recvbuf; t.address=0xA00000000000000FL; @@ -130,11 +118,32 @@ TEST_CASE("SPI Master test", "[spi][ignore]") ret=spi_bus_remove_device(handle); TEST_ASSERT(ret==ESP_OK); - ret=spi_bus_free(HSPI_HOST); - TEST_ASSERT(ret==ESP_OK); - - TEST_ASSERT_EQUAL_INT8_ARRAY(sendbuf, recvbuf, 16); + TEST_ASSERT_EQUAL_INT8_ARRAY(sendbuf, recvbuf, 64); } +TEST_CASE("SPI Master test", "[spi][ignore]") +{ + 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; + printf("THIS TEST NEEDS A JUMPER BETWEEN IO4 AND IO16\n"); + + ret=spi_bus_initialize(HSPI_HOST, &buscfg, 1); + TEST_ASSERT(ret==ESP_OK); + + int freqs[]={8000, 1000000, 5000000, 10000000, 20000000, 26666666, 0}; + for (int x=0; freqs[x]!=0; x++) { + printf("Testing clock speed of %dHz...\n", freqs[x]); + test_spi_bus_speed(freqs[x]); + } + + ret=spi_bus_free(HSPI_HOST); + TEST_ASSERT(ret==ESP_OK); +} -- 2.40.0