stm32 F4 SPI.h, SPI.c dma transfer using nullptr as transmit buf sends data from location 0

[ag88]

following up on this topic https://www.stm32duino.com/viewtopic.php?f=7&t=911 there are some use cases where nullptr is used as the transmitBuf the call to https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/master/STM32F4/libraries/SPI/src/SPI.cpp#L736 SPIClass::dmaTransfer(const void *transmitBuf, void *receiveBuf, uint16 length, uint16 flags) the receiveBuf is valid though the guessed intent is that the caller intended only to receive data in receiveBuf but not send anything. calling this with nullptr in transmitBuf possibly caused the block of data from location zero to be transmitted across mosi

maybe the condition check could be if (transmitBuf == nullptr && receiveBuf != nullptr) for the 'receive only' intent

[stevstrong]

This commit should fix this issue. Please check it.

[ag88]

thanks, i took a look at the commit https://github.com/rogerclarkmelbourne/Arduino_STM32/commit/792414923514d9f89e99c830f192ff049832c96b

uint8_t SPIClass::dmaTransfer(const void *transmitBuf, void *receiveBuf, uint16 length, uint16 flags)
{
    if ( transmitBuf==NULL ) {
        if ( receiveBuf!=NULL ) {
            return dmaTransfer(ff, receiveBuf, length, flags);            
        } else //both null
            return 10;
    } else {
        if ( receiveBuf==NULL ) { //transmitBuf != NULL
            //dmaSend(receiveBuf, length, flags);
            // do you mean this?
            return dmaSend(transmitBuf, length, flags);
        } else  { // both are not null
           PRINTF("<dTb-");
           dmaWaitCompletion();
           _currentSetting->dmaTxBuffer = transmitBuf;
           _currentSetting->dmaTrxLength = length;
           _currentSetting->dmaTrxAsync = (flags&DMA_ASYNC);
           dmaTransferSet(receiveBuf, (flags&(DMA_CIRC_MODE|DMA_TRNS_HALF)) | DMA_MINC_MODE);
           dmaTransferRepeat();
           PRINTF("-dTb>\n");
           return 0; //non zero indicates error, but we'd just patch zero (success)?
      }
  }

one more thing, i think we'd make the dmaTransfer() and dmaSend() functions return uint8_t just like in the F1 core. this is so that apps using them don't need if-defs to connect to spi.h, spi.cpp i.e. return 0 - success, non-zero - error there is no hurry really as i'm using offline edits as well with a forked Greiman SdFat i can make a PR if you prefer, pulling to bed for now

[stevstrong]

Thanks for the reply, you are right, I have corrected.

[stevstrong]

@ag88 have you managed to test the fix?

[ag88]

thanks steve, i did some tests, the fixes works well enough.

the only minor caveat is that greiman's sdfat has codes that expects a uint8_t return code 0 for no errors i.e.

uint8_t SdSpiArduinoDriver::receive(uint8_t* buf, size_t count) {
#if USE_STM32_DMA
    return m_spi->dmaTransfer(nullptr, buf, count);
#else
...

this is simply resolved with a minor change in the app's code

uint8_t SdSpiArduinoDriver::receive(uint8_t* buf, size_t count) {
#if USE_STM32_DMA
   m_spi->dmaTransfer(nullptr, buf, count);
   return 0;
#else

However, as greiman won't commit to maintaining sdfat for libmaple. I think we'd make do with the current updates in spi.c, spi.h without any further changes. users who wanted these changes can make the patches in the app's codes themselves

[stevstrong]

I did adapt that long time ago in my fork of SdFat, see here: https://github.com/stevstrong/SdFat/blob/master/src/SpiDriver/SdSpiSTM32.cpp#L54-L64 As the return value from the DMA functions is anyway a non-real value (it would be just a copy of the input parameter), I think it does not make sense to change that, so rather adapt the SPI driver of SdFat, as I did.

[ag88]

hi no worries, i've thought about the context of return values for the dmaSend and dmaTransfer functions. e.g. 0 on success and non zero on errors. those are useful but currently if configuration are appropriately done it is unlikely to result in mid transmission failures. Hence, i'd guess we'd make do for now and we can close this issue