Bug (with fix) in ASF4 SERCOM SPI DMA prevents simultaneous Tx and Rx

Go To Last Post
7 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

I'm persevering with ASF4 for now since the chip I'm using isn't supported in ASF3.

 

The SERCOM SPI DMA driver prevents simultaneous transmit of a Tx buffer while receiving an Rx buffer due to a silly bug in the function _spi_m_dma_transfer in hpl/sercom/hpl_sercom.c

 

This function is called with pointers for txbuf and rxbuf. If either of these is NULL, then only the other direction buffer is used, as you would expect.

 

In theory if you provide buffers for txbuf and rxbuf, it should use both. However in the function it sets up the Rx DMA transaction and then, if there is a Tx buffer specified, immediately disables the Rx transaction it has just set up.

 

Broken code:

	if (rxbuf) {
		/* Enable spi rx */
		_spi_m_dma_rx_enable(dev);
		_dma_set_source_address(rx_ch, (void *)_spi_m_get_source_for_dma(dev->prvt));
		_dma_set_destination_address(rx_ch, rxbuf);
		_dma_set_data_amount(rx_ch, length);
		_dma_enable_transaction(rx_ch, false);
	}

	if (txbuf) {
		/* Enable spi tx */
		_spi_m_dma_rx_disable(dev);
		_dma_set_source_address(tx_ch, txbuf);
		_dma_set_destination_address(tx_ch, (void *)_spi_m_get_destination_for_dma(dev->prvt));
		_dma_srcinc_enable(tx_ch, true);
		_dma_set_data_amount(tx_ch, length);
	} else {
		_dma_set_source_address(tx_ch, &regs->dummy_byte);
		_dma_set_destination_address(tx_ch, (void *)_spi_m_get_destination_for_dma(dev->prvt));
		_dma_srcinc_enable(tx_ch, false);
		_dma_set_data_amount(tx_ch, length);
	}

 

At the very least it is present in these driver versions:

 

Device: ATSAME51J20A
Device Pack: SAME51_DFP
Version: 1.0.65

 

Device: ATSAMC21E18A
Device Pack: SAMC21_DFP
Version: 1.1.105

 

 

It's easy to fix, see one possibility below.

 

Fixed code:

	if (rxbuf) {
		/* Enable spi rx */
		_spi_m_dma_rx_enable(dev);
		_dma_set_source_address(rx_ch, (void *)_spi_m_get_source_for_dma(dev->prvt));
		_dma_set_destination_address(rx_ch, rxbuf);
		_dma_set_data_amount(rx_ch, length);
		_dma_enable_transaction(rx_ch, false);
	}

	if (txbuf) {
		/* Enable spi tx */
		if (!rxbuf) {
			/* Rx not used, so disable */
			_spi_m_dma_rx_disable(dev);
		}
		_dma_set_source_address(tx_ch, txbuf);
		_dma_set_destination_address(tx_ch, (void *)_spi_m_get_destination_for_dma(dev->prvt));
		_dma_srcinc_enable(tx_ch, true);
		_dma_set_data_amount(tx_ch, length);
	} else {
		_dma_set_source_address(tx_ch, &regs->dummy_byte);
		_dma_set_destination_address(tx_ch, (void *)_spi_m_get_destination_for_dma(dev->prvt));
		_dma_srcinc_enable(tx_ch, false);
		_dma_set_data_amount(tx_ch, length);
	}

 

Is there somewhere to report bugs that actually works? I see http://asf.atmel.no/bugzilla/ is broken. I see mention of myAtmel Support Case system but that seems to have gone away too. There doesn't seem to be a ticketing system in myMicrochip.

Last Edited: Mon. Aug 2, 2021 - 08:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

justyn wrote:
There doesn't seem to be a ticketing system in myMicrochip.
by the following?

Microchip

Technical Support

http://www.microchip.com/support/hottopics.aspx

(My Support)

IIRC, there's a sticky thread in a sub-forum here on how for support at Microchip.

 

"Dare to be naïve." - Buckminster Fuller

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Great, thanks, I will submit a ticket.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

gchapman wrote:

 

IIRC, there's a sticky thread in a sub-forum here on how for support at Microchip.

 

I don't see it on:

 

https://www.avrfreaks.net/forums...

 

...where there is a sticky titled [HOWTO] Bug reporting that suggests the bugzilla.

 

perhaps a moderator could replace that with updated instructions?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

"Dare to be naïve." - Buckminster Fuller

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A small update on this, from my support ticket.

 

Apparently Microchip are aware of the issue, and have suggested a fix:

 

if (rxbuf) {
/* Enable spi rx */
_spi_m_dma_rx_enable(dev);
_dma_set_source_address(rx_ch, (void *)_spi_m_get_source_for_dma(dev->prvt));
_dma_set_destination_address(rx_ch, rxbuf);
_dma_set_data_amount(rx_ch, length);
_dma_enable_transaction(rx_ch, false);
}else{
//Stop RX DMA from being disable when tx is enabled
_spi_m_dma_rx_disable(dev);
} 

 

What's disappointing is that despite being aware of this internally, there is no external clue that the bug exists, causing people like me to waste time trying to work out why it wasn't working.

 

Apparently they discovered this issue too recently for it to be listed in the "known issues" of the most recent release.

 

It was also disappointing to learn from this ticket that there is apparently no mechanism for being notified of ASF4 driver updates, other than to poll the Changelog page yourself.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

It's a shame this bug came to life in the first place. They already fixed the exact same problem for txbuf a few lines below.

 

Attached my own fix (slightly cleaner and takes less flash memory).

Attachment(s):