Nuvoton M480: Some SPI fixes using test shield (#467)

* FIx M480 word size issues, implement extra functionality

* Fix spi_init() not freeing DMA channels, causing DMA to get stuck

* Fix spi_abort_async leaving flags set that will cause the next transfer to fail

* Fix incorrect spi active logic, add assert to catch timing issue

* Fix comment

* Style

Author: multiplemonomials

drivers/include/drivers/SPI.h

@@ -179,9 +179,12 @@ const use_gpio_ssel_t use_gpio_ssel;
  * applies to receiving data, so be sure to check examples in the datasheet to determine what frame
  * size to use and whether byte swapping is needed when working with an external chip.</p>
  *
- * <p>Note: Some Mbed targets support frame sizes that are not standard integer sizes, e.g. 4 bits, 7 bits, or
- * 24 bits.  However, the behavior of these frame sizes is not well defined and may differ across targets
- * or across API calls (e.g. single-byte vs transaction vs async).  More work is needed to make these consistent.</p>
+ * \note Some Mbed targets support frame sizes that are not standard integer sizes, e.g. 4 bits, 7 bits, or
+ *     24 bits. However, the behavior of these frame sizes is currently not tested, and you may need to test what works or
+ *     inspect your target's SPI driver code. More work is needed to make these consistent. When available, using these frame sizes
+ *     requires writing each word to be transmitted into the next-largest C integer type. For example, to send 24-bit frames, you
+ *     would create an array of uint32_ts and write each 24-bit integer into its own uint32_t.
+ *     Then you would pass in the byte length of the array (number of frames * 4, NOT number of frames * 3) to the SPI driver.
  *
  * <h1>Asynchronous API</h1>
  * <p>On many processors, Mbed OS also supports asynchronous %SPI.  This feature allows you to run %SPI

drivers/source/SPI.cpp

@@ -427,7 +427,7 @@ void SPI::abort_all_transfers()
 
 int SPI::set_dma_usage(DMAUsage usage)
 {
-    if (spi_active(&_peripheral->spi)) {
+    if (_peripheral->initialized && spi_active(&_peripheral->spi)) {
         return -1;
     }
     _usage = usage;

hal/include/hal/dma_api.h

@@ -30,7 +30,7 @@
  * @brief Enumeration of possible DMA usage hints
  */
 typedef enum {
-    DMA_USAGE_NEVER, ///< Never use DMA
+    DMA_USAGE_NEVER = 0, ///< Never use DMA
     DMA_USAGE_OPPORTUNISTIC, ///< Use DMA if possible but deallocate DMA resources when not being used.
     DMA_USAGE_ALWAYS, ///< Always use DMA when possible
     DMA_USAGE_TEMPORARY_ALLOCATED, // Seems to be used as an internal state indicator for "we need to deallocate these channels."

hal/include/hal/spi_api.h

@@ -192,6 +192,10 @@ SPIName spi_get_peripheral_name(PinName mosi, PinName miso, PinName mclk);
 
 /**
  * Fills the given spi_capabilities_t structure with the capabilities of the given peripheral.
+ *
+ * @param ssel The CS pin being used, for checking the \c hw_cs_handle flag
+ * @param slave True to get capabilities for slave mode, false to get capabilities for master mode
+ * @param[out] cap Capabilities are returned here
  */
 void spi_get_capabilities(PinName ssel, bool slave, spi_capabilities_t *cap);
 
@@ -419,7 +423,7 @@ const PinMap *spi_slave_cs_pinmap(void);
  * @param[in] rx_length The number of bytes to receive
  * @param[in] bit_width The bit width of buffer words
  * @param[in] event     The logical OR of events to be registered
- * @param[in] handler   SPI interrupt handler
+ * @param[in] handler   SPI interrupt handler. This will point, through a bit of indirection, to \c SPI::irq_handler_asynch() for the correct SPI instance
  * @param[in] hint      A suggestion for how to use DMA with this transfer
  *
  * @return True if DMA was actually used for the transfer, false otherwise (if interrupts or another CPU-based
@@ -429,6 +433,10 @@ const PinMap *spi_slave_cs_pinmap(void);
  * after the transfer is complete.  If this function returns true, the driver layer will cache invalidate the Rx buffer under
  * the assumption that the data needs to be re-read from main memory.  Be careful, because if the read was not actually
  * done by DMA, and the rx data is in the CPU cache, this invalidation will corrupt it.
+ *
+ * @note The application layer will always acquire the SPI peripheral first before calling this, including setting the frequency and the bit width. So,
+ *     the \c bit_width argument will never be different from the SPI's currently set bit width, and can actually be ignored.
+ *     TODO remove this argument entirely.
  */
 bool spi_master_transfer(spi_t *obj, const void *tx, size_t tx_length, void *rx, size_t rx_length, uint8_t bit_width, uint32_t handler, uint32_t event, DMAUsage hint);
 

targets/TARGET_NUVOTON/TARGET_M480/PeripheralNames.h

@@ -127,6 +127,8 @@ typedef enum {
     SPI_5 = (int) NU_MODNAME(QSPI1_BASE, 5, 0),
 } SPIName;
 
+#define DEVICE_SPI_COUNT 6
+
 typedef enum {
     I2C_0 = (int) NU_MODNAME(I2C0_BASE, 0, 0),
     I2C_1 = (int) NU_MODNAME(I2C1_BASE, 1, 0),

targets/TARGET_NUVOTON/TARGET_M480/dma_api.c

@@ -84,6 +84,19 @@ int dma_channel_allocate(uint32_t capabilities)
 
 int dma_channel_free(int channelid)
 {
+    PDMA_T * pdma = dma_modbase();
+
+    // Make sure channel is disabled
+    pdma->CHCTL &= ~(1 << channelid);
+
+    // Also clear the request source for a channel in case still enabled.
+    // This allows this request source to be assigned to a different channel later.
+    PDMA_SetTransferMode(pdma,
+                        channelid,
+                        PDMA_MEM, // No peripheral
+                        0,  // Scatter-gather disabled
+                        0); // Scatter-gather descriptor address
+
     if (channelid != DMA_ERROR_OUT_OF_CHANNELS) {
         dma_chn_mask &= ~(1 << channelid);
     }

targets/TARGET_NUVOTON/TARGET_M480/objects.h

@@ -87,11 +87,14 @@ struct spi_s {
     PinName pin_sclk;
     PinName pin_ssel;
 
+    // Current word size of the SPI
+    uint8_t word_size_bits;
+
     // Async transfer related fields
     DMAUsage    dma_usage;
     int         dma_chn_id_tx;
     int         dma_chn_id_rx;
-    uint32_t    event;
+    uint32_t    event_mask;
     uint32_t    txrx_rmn;       // Track tx/rx frames remaining in interrupt way
 };