Merge pull request #10668 from dhalbert/spi-dma-fix

Fix SPI DMA on SAMD

Author: tannewt

ports/atmel-samd/audio_dma.c

@@ -39,14 +39,14 @@ uint8_t find_sync_event_channel_raise(void) {
 }
 
 void audio_dma_disable_channel(uint8_t channel) {
-    if (channel >= AUDIO_DMA_CHANNEL_COUNT) {
+    if (channel == NO_DMA_CHANNEL) {
         return;
     }
     dma_disable_channel(channel);
 }
 
 void audio_dma_enable_channel(uint8_t channel) {
-    if (channel >= AUDIO_DMA_CHANNEL_COUNT) {
+    if (channel == NO_DMA_CHANNEL) {
         return;
     }
     dma_enable_channel(channel);
@@ -171,8 +171,8 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     bool output_signed,
     uint32_t output_register_address,
     uint8_t dma_trigger_source) {
-    uint8_t dma_channel = dma_allocate_channel(true);
-    if (dma_channel >= AUDIO_DMA_CHANNEL_COUNT) {
+    uint8_t dma_channel = dma_allocate_audio_channel();
+    if (dma_channel == NO_DMA_CHANNEL) {
         return AUDIO_DMA_DMA_BUSY;
     }
 
@@ -298,14 +298,14 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
 
 void audio_dma_stop(audio_dma_t *dma) {
     uint8_t channel = dma->dma_channel;
-    if (channel < AUDIO_DMA_CHANNEL_COUNT) {
+    if (channel != NO_DMA_CHANNEL) {
         audio_dma_disable_channel(channel);
         disable_event_channel(dma->event_channel);
         MP_STATE_PORT(playing_audio)[channel] = NULL;
         audio_dma_state[channel] = NULL;
         dma_free_channel(dma->dma_channel);
     }
-    dma->dma_channel = AUDIO_DMA_CHANNEL_COUNT;
+    dma->dma_channel = NO_DMA_CHANNEL;
     dma->playing_in_progress = false;
 }
 
@@ -318,7 +318,7 @@ void audio_dma_resume(audio_dma_t *dma) {
 }
 
 bool audio_dma_get_paused(audio_dma_t *dma) {
-    if (dma->dma_channel >= AUDIO_DMA_CHANNEL_COUNT) {
+    if (dma->dma_channel == NO_DMA_CHANNEL) {
         return false;
     }
     uint32_t status = dma_transfer_status(dma->dma_channel);
@@ -327,7 +327,7 @@ bool audio_dma_get_paused(audio_dma_t *dma) {
 }
 
 void audio_dma_init(audio_dma_t *dma) {
-    dma->dma_channel = AUDIO_DMA_CHANNEL_COUNT;
+    dma->dma_channel = NO_DMA_CHANNEL;
 }
 
 void audio_dma_reset(void) {
@@ -341,7 +341,7 @@ void audio_dma_reset(void) {
 }
 
 bool audio_dma_get_playing(audio_dma_t *dma) {
-    if (dma->dma_channel >= AUDIO_DMA_CHANNEL_COUNT) {
+    if (dma->dma_channel == NO_DMA_CHANNEL) {
         return false;
     }
     return dma->playing_in_progress;

ports/atmel-samd/common-hal/audiobusio/PDMIn.c

@@ -364,7 +364,7 @@ static uint16_t filter_sample(uint32_t pdm_samples[4]) {
 // output_buffer_length is the number of slots, not the number of bytes.
 uint32_t common_hal_audiobusio_pdmin_record_to_buffer(audiobusio_pdmin_obj_t *self,
     uint16_t *output_buffer, uint32_t output_buffer_length) {
-    uint8_t dma_channel = dma_allocate_channel(true);
+    uint8_t dma_channel = dma_allocate_audio_channel();
     pdmin_event_channel = find_sync_event_channel_raise();
     pdmin_dma_block_done = false;
 

ports/atmel-samd/common-hal/busio/SPI.c

@@ -21,7 +21,17 @@
 #include "samd/dma.h"
 #include "samd/sercom.h"
 
-void setup_pin(const mcu_pin_obj_t *pin, uint32_t pinmux);
+
+static void setup_pin(const mcu_pin_obj_t *pin, uint32_t pinmux, const enum gpio_direction direction) {
+    gpio_set_pin_direction(pin->number, direction);
+    gpio_set_pin_pull_mode(pin->number, GPIO_PULL_OFF);
+    gpio_set_pin_function(pin->number, pinmux);
+    if (direction == GPIO_DIRECTION_OUT) {
+        // Use strong drive strength for SPI outputs.
+        hri_port_set_PINCFG_DRVSTR_bit(PORT, (enum gpio_port)GPIO_PORT(pin->number), GPIO_PIN(pin->number));
+    }
+    claim_pin(pin);
+}
 
 void common_hal_busio_spi_construct(busio_spi_obj_t *self,
     const mcu_pin_obj_t *clock, const mcu_pin_obj_t *mosi,
@@ -128,6 +138,7 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
     // Pads must be set after spi_m_sync_init(), which uses default values from
     // the prototypical SERCOM.
 
+    // Set to SPI host mode and choose pads.
     hri_sercomspi_write_CTRLA_MODE_bf(sercom, 3);
     hri_sercomspi_write_CTRLA_DOPO_bf(sercom, dopo);
     hri_sercomspi_write_CTRLA_DIPO_bf(sercom, miso_pad);
@@ -141,20 +152,20 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self,
         mp_raise_OSError(MP_EIO);
     }
 
-    setup_pin(clock, clock_pinmux);
+    setup_pin(clock, clock_pinmux, GPIO_DIRECTION_OUT);
     self->clock_pin = clock->number;
 
     if (mosi_none) {
         self->MOSI_pin = NO_PIN;
     } else {
-        setup_pin(mosi, mosi_pinmux);
+        setup_pin(mosi, mosi_pinmux, GPIO_DIRECTION_OUT);
         self->MOSI_pin = mosi->number;
     }
 
     if (miso_none) {
         self->MISO_pin = NO_PIN;
     } else {
-        setup_pin(miso, miso_pinmux);
+        setup_pin(miso, miso_pinmux, GPIO_DIRECTION_IN);
         self->MISO_pin = miso->number;
     }
 
@@ -317,11 +328,3 @@ uint8_t common_hal_busio_spi_get_polarity(busio_spi_obj_t *self) {
     void *hw = self->spi_desc.dev.prvt;
     return hri_sercomspi_get_CTRLA_CPOL_bit(hw);
 }
-
-void setup_pin(const mcu_pin_obj_t *pin, uint32_t pinmux) {
-    gpio_set_pin_direction(pin->number, GPIO_DIRECTION_OUT);
-    gpio_set_pin_pull_mode(pin->number, GPIO_PULL_OFF);
-    gpio_set_pin_function(pin->number, pinmux);
-    claim_pin(pin);
-    hri_port_set_PINCFG_DRVSTR_bit(PORT, (enum gpio_port)GPIO_PORT(pin->number), GPIO_PIN(pin->number));
-}

ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c

@@ -135,7 +135,8 @@ void common_hal_imagecapture_parallelimagecapture_singleshot_capture(imagecaptur
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_RW);
 
-    uint8_t dma_channel = dma_allocate_channel(true);
+    // Allocate a permanent channel (not really audio).
+    uint8_t dma_channel = dma_allocate_audio_channel();
 
     uint32_t *dest = bufinfo.buf;
     size_t count = bufinfo.len / 4; // PCC receives 4 bytes (2 pixels) at a time

ports/atmel-samd/common-hal/spitarget/SPITarget.c

@@ -192,15 +192,15 @@ void common_hal_spitarget_spi_target_transfer_start(spitarget_spi_target_obj_t *
     self->miso_packet = miso_packet;
 
     Sercom *sercom = self->spi_desc.dev.prvt;
-    self->running_dma = shared_dma_transfer_start(sercom, miso_packet, &sercom->SPI.DATA.reg, &sercom->SPI.DATA.reg, mosi_packet, len, 0);
+    shared_dma_transfer_start(&self->running_dma, sercom, miso_packet, &sercom->SPI.DATA.reg, &sercom->SPI.DATA.reg, mosi_packet, len, 0);
 
     // There is an issue where if an unexpected SPI transfer is received before the user calls "end" for the in-progress, expected
     // transfer, the SERCOM has an error and gets confused. This can be detected from INTFLAG.ERROR. I think the code in
     // ports/atmel-samd/peripherals/samd/dma.c at line 277 (as of this commit; it's the part that reads s->SPI.INTFLAG.bit.RXC and
     // s->SPI.DATA.reg) is supposed to fix this, but experimentation seems to show that it does not in fact fix anything. Anyways, if
     // the ERROR bit is set, let's just reset the peripheral and then setup the transfer again -- that seems to work.
     if (hri_sercomspi_get_INTFLAG_ERROR_bit(sercom)) {
-        shared_dma_transfer_close(self->running_dma);
+        shared_dma_transfer_close(&self->running_dma);
 
         // disable the sercom
         spi_m_sync_disable(&self->spi_desc);
@@ -223,19 +223,19 @@ void common_hal_spitarget_spi_target_transfer_start(spitarget_spi_target_obj_t *
         spi_m_sync_enable(&self->spi_desc);
         hri_sercomspi_wait_for_sync(sercom, SERCOM_SPI_SYNCBUSY_MASK);
 
-        self->running_dma = shared_dma_transfer_start(sercom, miso_packet, &sercom->SPI.DATA.reg, &sercom->SPI.DATA.reg, mosi_packet, len, 0);
+        shared_dma_transfer_start(&self->running_dma, sercom, miso_packet, &sercom->SPI.DATA.reg, &sercom->SPI.DATA.reg, mosi_packet, len, 0);
     }
 }
 
 bool common_hal_spitarget_spi_target_transfer_is_finished(spitarget_spi_target_obj_t *self) {
-    return self->running_dma.failure == 1 || shared_dma_transfer_finished(self->running_dma);
+    return self->running_dma.failure == 1 || shared_dma_transfer_finished(&self->running_dma);
 }
 
 int common_hal_spitarget_spi_target_transfer_close(spitarget_spi_target_obj_t *self) {
     if (self->running_dma.failure == 1) {
         return 0;
     }
-    int res = shared_dma_transfer_close(self->running_dma);
+    int res = shared_dma_transfer_close(&self->running_dma);
     self->running_dma.failure = 1;
 
     self->mosi_packet = NULL;

ports/atmel-samd/common-hal/spitarget/SPITarget.h

@@ -18,7 +18,7 @@ typedef struct {
     uint8_t *mosi_packet;
     const uint8_t *miso_packet;
 
-    dma_descr_t running_dma;
+    dma_transfer_t running_dma;
 } spitarget_spi_target_obj_t;
 
 #endif // MICROPY_INCLUDED_ATMEL_SAMD_COMMON_HAL_BUSIO_SPI_TARGET_H