Fix timing adjustment needed for higher speeds of SPI master bus.

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

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
+            //SPI iface needs to be configured for a delay in some cases.
-            int nodelay=(host->no_gpio_matrix && dev->cfg.clock_speed_hz >= (apbclk/2));
+            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;

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]")
+static void test_spi_bus_speed(int hz) {
-{
+    esp_err_t ret;
-    spi_bus_config_t buscfg={
+    spi_device_handle_t handle;
-        .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,
         .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 sendbuf[64]="Hello World!";
-    char recvbuf[16]="UUUUUUUUUUUUUUU";
+    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);
+}