]> granicus.if.org Git - esp-idf/commitdiff
SPI: Small fixes according to MR comments
authorJeroen Domburg <jeroen@espressif.com>
Thu, 13 Apr 2017 03:14:35 +0000 (11:14 +0800)
committerJeroen Domburg <jeroen@espressif.com>
Thu, 27 Apr 2017 03:49:04 +0000 (11:49 +0800)
components/driver/include/driver/spi_common.h
components/driver/include/driver/spi_slave.h
components/driver/spi_common.c
components/driver/spi_master.c
components/driver/spi_slave.c
components/driver/test/test_spi_master.c
docs/api/peripherals/spi_slave.rst
examples/peripherals/spi_slave/README.md
examples/peripherals/spi_slave/receiver/Makefile
examples/peripherals/spi_slave/sender/Makefile
tools/unit-test-app/sdkconfig

index e410c75b8fa2787a94d44883f7bb441bf0ceafb1..c3aaa8da450fce0e3dc060cf0470664508d020d9 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright 2010-2016 Espressif Systems (Shanghai) PTE LTD
+// Copyright 2010-2017 Espressif Systems (Shanghai) PTE LTD
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -78,9 +78,9 @@ bool spicommon_periph_claim(spi_host_device_t host);
 bool spicommon_periph_free(spi_host_device_t host);
 
 
-#define SPICOMMON_BUSFLAG_SLAVE 0    ///< Initialize I/O in slave mode
-#define SPICOMMON_BUSFLAG_MASTER 1   ///< Initialize I/O in master mode
-#define SPICOMMON_BUSFLAG_QUAD 2     ///< Also initialize WP/HD pins, if specified
+#define SPICOMMON_BUSFLAG_SLAVE  0          ///< Initialize I/O in slave mode
+#define SPICOMMON_BUSFLAG_MASTER (1<<0)     ///< Initialize I/O in master mode
+#define SPICOMMON_BUSFLAG_QUAD   (1<<1)     ///< Also initialize WP/HD pins, if specified
 
 /**
  * @brief Connect a SPI peripheral to GPIO pins
@@ -93,13 +93,13 @@ bool spicommon_periph_free(spi_host_device_t host);
  * @param bus_config Pointer to a spi_bus_config struct detailing the GPIO pins
  * @param dma_chan DMA-channel (1 or 2) to use, or 0 for no DMA.
  * @param flags Combination of SPICOMMON_BUSFLAG_* flags
- * @param is_native A value of 'true' will be written to this address if the GPIOs can be
+ * @param[out] is_native A value of 'true' will be written to this address if the GPIOs can be
  *                  routed using the IO_mux, 'false' if the GPIO matrix is used.
  * @return 
  *         - ESP_ERR_INVALID_ARG   if parameter is invalid
  *         - ESP_OK                on success
  */
-esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, spi_bus_config_t *bus_config, int dma_chan, int flags, bool *is_native);
+esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_config_t *bus_config, int dma_chan, int flags, bool *is_native);
 
 /**
  * @brief Free the IO used by a SPI peripheral
@@ -170,7 +170,7 @@ int spicommon_irqsource_for_host(spi_host_device_t host);
 
 
 /**
- * @note V0 and V1 of the ESP32 silicon has a bug where in some (well-known) cases a SPI DMA channel will get confused. This can be remedied
+ * @note In some (well-defined) cases in the ESP32 (at least rev v.0 and v.1), a SPI DMA channel will get confused. This can be remedied
  * by resetting the SPI DMA hardware in case this happens. Unfortunately, the reset knob used for thsi will reset _both_ DMA channels, and
  * as such can only done safely when both DMA channels are idle. These functions coordinate this.
  * 
@@ -190,7 +190,7 @@ typedef void(*dmaworkaround_cb_t)(void *arg);
 /**
  * @brief Request a reset for a certain DMA channel
  *
- * @param host The SPI host
+ * @param dmachan DMA channel associated with the SPI host that needs a reset
  * @param cb Callback to call in case DMA channel cannot be reset immediately
  * @param arg Argument to the callback
  *
index c27809bc8b0d44f339e896b05ebfa94098271a18..9045c2928853135038f33ca7aefe56de26d7909e 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright 2010-2016 Espressif Systems (Shanghai) PTE LTD
+// Copyright 2010-2017 Espressif Systems (Shanghai) PTE LTD
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -31,7 +31,6 @@ extern "C"
 #define SPI_SLAVE_TXBIT_LSBFIRST          (1<<0)  ///< Transmit command/address/data LSB first instead of the default MSB first
 #define SPI_SLAVE_RXBIT_LSBFIRST          (1<<1)  ///< Receive data LSB first instead of the default MSB first
 #define SPI_SLAVE_BIT_LSBFIRST            (SPI_TXBIT_LSBFIRST|SPI_RXBIT_LSBFIRST); ///< Transmit and receive LSB first
-#define SPI_SLAVE_POSITIVE_CS             (1<<3)  ///< Make CS positive during a transaction instead of negative
 
 
 typedef struct spi_slave_transaction_t spi_slave_transaction_t;
@@ -41,9 +40,9 @@ typedef void(*slave_transaction_cb_t)(spi_slave_transaction_t *trans);
  * @brief This is a configuration for a SPI host acting as a slave device.
  */
 typedef struct {
-    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. 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
+    int spics_io_num;               ///< CS GPIO pin for this device
+    uint32_t flags;                 ///< Bitwise OR of SPI_SLAVE_* flags
+    int queue_size;                 ///< Transaction queue size. This sets how many transactions can be 'in the air' (queued using spi_slave_queue_trans but not yet finished using spi_slave_get_trans_result) at the same time
     uint8_t mode;                   ///< SPI mode (0-3)
     slave_transaction_cb_t post_setup_cb; ///< Callback called after the SPI registers are loaded with new data
     slave_transaction_cb_t post_trans_cb; ///< Callback called after a transaction is done
@@ -56,6 +55,7 @@ struct spi_slave_transaction_t {
     size_t length;                  ///< Total 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.
 };
 
 /**
@@ -75,7 +75,7 @@ struct spi_slave_transaction_t {
  *         - ESP_ERR_NO_MEM        if out of memory
  *         - ESP_OK                on success
  */
-esp_err_t spi_slave_initialize(spi_host_device_t host, spi_bus_config_t *bus_config, spi_slave_interface_config_t *slave_config, int dma_chan);
+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);
 
 /**
  * @brief Free a SPI bus claimed as a SPI slave interface
@@ -97,26 +97,27 @@ esp_err_t spi_slave_free(spi_host_device_t host);
  * unhandled transactions before it and the master initiates a SPI transaction by pulling down CS and sending out
  * clock signals.
  *
- * @param handle Device handle obtained using spi_host_add_dev
- * @param trans_desc Description of transaction to execute
+ * @param host SPI peripheral that is acting as a slave
+ * @param trans_desc Description of transaction to execute. Not const because we may want to write status back
+ *                   into the transaction description.
  * @param ticks_to_wait Ticks to wait until there's room in the queue; use portMAX_DELAY to
  *                      never time out.
  * @return 
  *         - ESP_ERR_INVALID_ARG   if parameter is invalid
  *         - ESP_OK                on success
  */
-esp_err_t spi_slave_queue_trans(spi_host_device_t host, spi_slave_transaction_t *trans_desc, TickType_t ticks_to_wait);
+esp_err_t spi_slave_queue_trans(spi_host_device_t host, const spi_slave_transaction_t *trans_desc, TickType_t ticks_to_wait);
 
 
 /**
  * @brief Get the result of a SPI transaction queued earlier
  *
  * This routine will wait until a transaction to the given device (queued earlier with 
- * spi_device_queue_trans) has succesfully completed. It will then return the description of the
+ * spi_slave_queue_trans) has succesfully completed. It will then return the description of the
  * completed transaction so software can inspect the result and e.g. free the memory or 
  * re-use the buffers.
  *
- * @param handle Device handle obtained using spi_host_add_dev
+ * @param host SPI peripheral to that is acting as a slave
  * @param trans_desc Pointer to variable able to contain a pointer to the description of the 
  *                   transaction that is executed
  * @param ticks_to_wait Ticks to wait until there's a returned item; use portMAX_DELAY to never time
@@ -131,13 +132,14 @@ esp_err_t spi_slave_get_trans_result(spi_host_device_t host, spi_slave_transacti
 /**
  * @brief Do a SPI transaction
  *
- * Essentially does the same as spi_device_queue_trans followed by spi_device_get_trans_result. Do
+ * Essentially does the same as spi_slave_queue_trans followed by spi_slave_get_trans_result. Do
  * not use this when there is still a transaction queued that hasn't been finalized 
- * using spi_device_get_trans_result.
+ * using spi_slave_get_trans_result.
  *
- * @param handle Device handle obtained using spi_host_add_dev
+ * @param host SPI peripheral to that is acting as a slave
  * @param trans_desc Pointer to variable able to contain a pointer to the description of the 
- *                   transaction that is executed
+ *                   transaction that is executed. Not const because we may want to write status back
+ *                   into the transaction description.
  * @param ticks_to_wait Ticks to wait until there's a returned item; use portMAX_DELAY to never time
  *                      out.
  * @return 
index 4b098b8309d19fa9d00ebc7bdf9e49296dc959d3..0287e66a5058be3413bf417db33df052111b29b4 100644 (file)
@@ -19,7 +19,6 @@
 #include "soc/spi_reg.h"
 #include "soc/dport_reg.h"
 #include "soc/spi_struct.h"
-#include "soc/rtc_cntl_reg.h"
 #include "rom/ets_sys.h"
 #include "esp_types.h"
 #include "esp_attr.h"
 #include "esp_intr_alloc.h"
 #include "esp_log.h"
 #include "esp_err.h"
-#include "freertos/FreeRTOS.h"
-#include "freertos/semphr.h"
-#include "freertos/xtensa_api.h"
-#include "freertos/task.h"
-#include "freertos/ringbuf.h"
 #include "soc/soc.h"
 #include "soc/dport_reg.h"
-#include "soc/uart_struct.h"
 #include "rom/lldesc.h"
-#include "driver/uart.h"
 #include "driver/gpio.h"
 #include "driver/periph_ctrl.h"
 #include "esp_heap_alloc_caps.h"
@@ -189,7 +181,7 @@ Do the common stuff to hook up a SPI host to a bus defined by a bunch of GPIO pi
 bus config struct and it'll set up the GPIO matrix and enable the device. It will set is_native to 1 if the bus 
 config can be done using the IOMUX instead of using the GPIO matrix.
 */
-esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, spi_bus_config_t *bus_config, int dma_chan, int flags, bool *is_native)
+esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_config_t *bus_config, int dma_chan, int flags, bool *is_native)
 {
     bool native=true;
     bool is_master=(flags&SPICOMMON_BUSFLAG_MASTER)?true:false;
index 06d29081e1db09e783623d211f79851d21bed846..28e964787e767015585757e5919e2914e199b837 100644 (file)
@@ -40,7 +40,6 @@ queue and re-enabling the interrupt will trigger the interrupt again, which can
 #include "soc/spi_reg.h"
 #include "soc/dport_reg.h"
 #include "soc/spi_struct.h"
-#include "soc/rtc_cntl_reg.h"
 #include "rom/ets_sys.h"
 #include "esp_types.h"
 #include "esp_attr.h"
@@ -55,9 +54,7 @@ queue and re-enabling the interrupt will trigger the interrupt again, which can
 #include "freertos/ringbuf.h"
 #include "soc/soc.h"
 #include "soc/dport_reg.h"
-#include "soc/uart_struct.h"
 #include "rom/lldesc.h"
-#include "driver/uart.h"
 #include "driver/gpio.h"
 #include "driver/periph_ctrl.h"
 #include "esp_heap_alloc_caps.h"
index f321799908f38d79b97323e44d7d2b434d0dff3c..d5c9fed7c075d0d4bbd9655b2e9c4e3f6505c85e 100644 (file)
@@ -19,7 +19,6 @@
 #include "soc/spi_reg.h"
 #include "soc/dport_reg.h"
 #include "soc/spi_struct.h"
-#include "soc/rtc_cntl_reg.h"
 #include "rom/ets_sys.h"
 #include "esp_types.h"
 #include "esp_attr.h"
@@ -34,9 +33,7 @@
 #include "freertos/ringbuf.h"
 #include "soc/soc.h"
 #include "soc/dport_reg.h"
-#include "soc/uart_struct.h"
 #include "rom/lldesc.h"
-#include "driver/uart.h"
 #include "driver/gpio.h"
 #include "driver/periph_ctrl.h"
 #include "esp_heap_alloc_caps.h"
@@ -48,7 +45,7 @@ static const char *SPI_TAG = "spi_slave";
         return (ret_val); \
     }
 
-#define VALID_HOST(x) (host>SPI_HOST && host<=VSPI_HOST)
+#define VALID_HOST(x) (x>SPI_HOST && x<=VSPI_HOST)
 
 typedef struct {
     spi_slave_interface_config_t cfg;
@@ -68,7 +65,7 @@ static spi_slave_t *spihost[3];
 
 static void IRAM_ATTR spi_intr(void *arg);
 
-esp_err_t spi_slave_initialize(spi_host_device_t host, spi_bus_config_t *bus_config, spi_slave_interface_config_t *slave_config, int dma_chan)
+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;
     //We only support HSPI/VSPI, period.
@@ -200,7 +197,7 @@ esp_err_t spi_slave_free(spi_host_device_t host)
 }
 
 
-esp_err_t spi_slave_queue_trans(spi_host_device_t host, spi_slave_transaction_t *trans_desc, TickType_t ticks_to_wait)
+esp_err_t spi_slave_queue_trans(spi_host_device_t host, const spi_slave_transaction_t *trans_desc, TickType_t ticks_to_wait)
 {
     BaseType_t r;
     SPI_CHECK(VALID_HOST(host), "invalid host", ESP_ERR_INVALID_ARG);
@@ -368,7 +365,7 @@ static void IRAM_ATTR spi_intr(void *arg)
             host->hw->user.usr_miso_highpart=0;
             host->hw->user.usr_mosi_highpart=0;
             if (trans->tx_buffer) {
-                uint32_t *data=host->cur_trans->tx_buffer;
+                const uint32_t *data=host->cur_trans->tx_buffer;
                 for (int x=0; x<trans->length; x+=32) {
                     uint32_t word;
                     memcpy(&word, &data[x/32], 4);
index 9a55f699903c98849a94068d933ea4c978f85ece..665f432481ba30aaf80b91d1de7e62f671f6d835 100644 (file)
@@ -150,14 +150,14 @@ static void spi_test(spi_device_handle_t handle, int num_bytes) {
         int from=x-16;
         if (from<0) from=0;
         printf("Error at %d! Sent vs recved: (starting from %d)\n" , x, from);
-               for (int i=0; i<32; i++) {
-                       if (i+from<num_bytes) printf("%02X ", sendbuf[from+i]);
-               }
-               printf("\n");
-               for (int i=0; i<32; i++) {
-                       if (i+from<num_bytes) printf("%02X ", recvbuf[from+i]);
-               }
-               printf("\n");
+        for (int i=0; i<32; i++) {
+            if (i+from<num_bytes) printf("%02X ", sendbuf[from+i]);
+        }
+        printf("\n");
+        for (int i=0; i<32; i++) {
+            if (i+from<num_bytes) printf("%02X ", recvbuf[from+i]);
+        }
+        printf("\n");
 //        TEST_ASSERT(0);
     }
 
@@ -231,26 +231,26 @@ TEST_CASE("SPI Master test, interaction of multiple devs", "[spi][ignore]") {
         .queue_size=3,
     };
     spi_device_handle_t handle1=setup_spi_bus(80000, true);
-       spi_device_handle_t handle2;
-       spi_bus_add_device(HSPI_HOST, &devcfg, &handle2);
+    spi_device_handle_t handle2;
+    spi_bus_add_device(HSPI_HOST, &devcfg, &handle2);
 
-       printf("Sending to dev 1\n");
+    printf("Sending to dev 1\n");
     spi_test(handle1, 7);
-       printf("Sending to dev 1\n");
+    printf("Sending to dev 1\n");
     spi_test(handle1, 15);
-       printf("Sending to dev 2\n");
+    printf("Sending to dev 2\n");
     spi_test(handle2, 15);
-       printf("Sending to dev 1\n");
+    printf("Sending to dev 1\n");
     spi_test(handle1, 32);
-       printf("Sending to dev 2\n");
+    printf("Sending to dev 2\n");
     spi_test(handle2, 32);
-       printf("Sending to dev 1\n");
+    printf("Sending to dev 1\n");
     spi_test(handle1, 63);
-       printf("Sending to dev 2\n");
+    printf("Sending to dev 2\n");
     spi_test(handle2, 63);
-       printf("Sending to dev 1\n");
+    printf("Sending to dev 1\n");
     spi_test(handle1, 5000);
-       printf("Sending to dev 2\n");
+    printf("Sending to dev 2\n");
     spi_test(handle2, 5000);
 
 
index 87c181f0d206f864b09d4850fae717f3983a8a27..fc0e212cdab2f765dae6dd005a9d2f681c3ffea9 100644 (file)
@@ -47,7 +47,7 @@ starts sending out clock pulses on the CLK line: every clock pulse causes a data
 the master to the slave on the MOSI line and vice versa on the MISO line. At the end of the transaction,
 the master makes CS high again.
 
-Using the spi_master driver
+Using the spi_slave driver
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Initialize a SPI peripheral as a slave by calling ``spi_slave_initialize``. Make sure to set the 
@@ -79,9 +79,9 @@ case the length of the transmission is larger than the buffer length, only the s
 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.
 
-Warning: Due to a design peculiarity in the V0 and V1 silicon of the ESP32, if the amount of bytes sent 
-by the master or the length of the transmission sent to the slave driver is not both larger than eight and 
-dividable by four, the SPI hardware can fail to write the last one to seven bytes to the receive buffer.
+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 
+four, the SPI hardware can fail to write the last one to seven bytes to the receive buffer.
 
 
 Application Example
index 6e83f96ad98df099ce6f4520041ca9bf26f46bcb..3b2222e20a7b3edc55f63740649a53611de585b1 100644 (file)
@@ -3,4 +3,20 @@
 These two projects illustrate the SPI Slave driver. They're supposed to be flashed into two separate ESP32s connected to
 eachother using the SPI pins defined in app_main.c. Once connected and flashed, they will use the spi master and spi slave
 driver to communicate with eachother. The example also includes a handshaking line to allow the master to only poll the 
-slave when it is actually ready to parse a transaction.
\ No newline at end of file
+slave when it is actually ready to parse a transaction.
+
+Please run wires between the following GPIOs between the slave and master to make the example function:
+
+========= ====== =======
+Signal    Slave  Master
+========= ====== =======
+Handshake GPIO2  GPIO2
+MOSI      GPIO12 GPIO12
+MISO      GPIO13 GPIO13
+SCLK      GPIO15 GPIO15
+CS        GPIO14 GPIO14
+========= ====== =======
+
+Be aware that the example by default uses lines normally reserved for JTAG. If this is an issue, either because of hardwired
+JTAG hardware or because of the need to do JTAG debugging, feel free to change the GPIO settings by editing defines in the top
+of main.c in the master/slave source code.
\ No newline at end of file
index 9f2c32783bff6336da6ebe35d26cef24d914f5f7..fc52431a30b6aa30b5a02bf6da31ecf9fceb9410 100644 (file)
@@ -3,7 +3,7 @@
 # project subdirectory.
 #
 
-PROJECT_NAME := app-template
+PROJECT_NAME := spi-slave-receiver
 
 include $(IDF_PATH)/make/project.mk
 
index 9f2c32783bff6336da6ebe35d26cef24d914f5f7..e4af518bccc369a75031ab06e0ba3e11b65b9b73 100644 (file)
@@ -3,7 +3,7 @@
 # project subdirectory.
 #
 
-PROJECT_NAME := app-template
+PROJECT_NAME := spi-slave-sender
 
 include $(IDF_PATH)/make/project.mk
 
index ca80784d2a6fdf14916bdf329367e24e79dd3e18..154cdd5d43253ac56162195c199fa867a2203343 100644 (file)
@@ -196,7 +196,7 @@ CONFIG_FATFS_MAX_LFN=255
 # CONFIG_FREERTOS_UNICORE is not set
 CONFIG_FREERTOS_CORETIMER_0=y
 # CONFIG_FREERTOS_CORETIMER_1 is not set
-CONFIG_FREERTOS_HZ=100
+CONFIG_FREERTOS_HZ=1000
 CONFIG_FREERTOS_ASSERT_ON_UNTESTED_FUNCTION=y
 # CONFIG_FREERTOS_CHECK_STACKOVERFLOW_NONE is not set
 # CONFIG_FREERTOS_CHECK_STACKOVERFLOW_PTRVAL is not set