SPI1 crashes when transfering from flashmem

Official Hardware Mega GIGA R1 WiFi
1

I have found a possible bug in the mbed-os driver code that causes the GIGA to crash when transferring data from flashmem to a SPI device. In this case the RA8876.
In this code:

//**************************************************************//
// Send data from the microcontroller to the RA8876
// Does a Raster OPeration to combine with an image already in memory
// For a simple overwrite operation, use ROP 12
//**************************************************************//
void RA8876_GIGA::bteMpuWriteWithROPData8(ru32 s1_addr, ru16 s1_image_width, ru16 s1_x, ru16 s1_y, ru32 des_addr, ru16 des_image_width,
                                        ru16 des_x, ru16 des_y, ru16 width, ru16 height, ru8 rop_code, const unsigned char *data) {
    bteMpuWriteWithROP(s1_addr, s1_image_width, s1_x, s1_y, des_addr, des_image_width, des_x, des_y, width, height, rop_code);

    startSend();
    _pspi->transfer(RA8876_SPI_DATAWRITE);

    // If you try _pspi->transfer(data, length) then this tries to write received data into the data buffer
    // but if we were given a PROGMEM (unwriteable) data pointer then _pspi->transfer will lock up totally.
    // So we explicitly tell it we don't care about any return data.
    // _pspi->transfer((char *)data, NULL, width * height * 2);

    _pspi->transfer((char *)data, width * height * 2); // GIGA version. Destroys "data" buffer and will
													   // will crash if data buffer is located in flashmem.
    endSend(true);

}

The call to "transfer((char *)data, width * height * 2)" calls this:

void arduino::MbedSPI::transfer(void *buf, size_t count) {
    dev->obj->write((const char*)buf, count, (char*)buf, count);
}

("buf" is used for xmit and recv buffer. When the data buf is in flashmem and the rx data is written back to it the GIGA will crash or if buffer is in ram memory and rx data is written back to it, any previous data in the buffer is destroyed.)
Transfer() calls:

int SPI::write(const char *tx_buffer, int tx_length, char *rx_buffer, int rx_length)
{
    select();
    int ret = spi_master_block_write(&_peripheral->spi, tx_buffer, tx_length, rx_buffer, rx_length, _write_fill);
    deselect();
    return ret;
}

My workaroud is:

  unsigned char tmpBuf[sizeof(image_565)];

  memcpy(tmpBuf,image_565,sizeof(image_565));

  tft.putPicture(20, 5, IMG_WIDTH, IMG_HEIGHT, tmpBuf);  //basic send, using 8-bit data
//  tft.putPicture(20, 5, IMG_WIDTH, IMG_HEIGHT, image_565);  //basic send, using 8-bit data

This is not an ideal solution if you have many images of different sizes. A lot of extra overhead. Maybe the easiest solution is another transfer call like this:

void arduino::MbedSPI::transfer(void *txbuf, void *rxbuf, size_t count) {
    dev->obj->write((const char*)buf, count, (char*)buf, count);
}

Where rxbuf could be a nullptr?

int spi_master_block_write(spi_t *obj, const char *tx_buffer, int tx_length,
                           char *rx_buffer, int rx_length, char write_fill)
{
    struct spi_s *spiobj = SPI_S(obj);
    SPI_HandleTypeDef *handle = &(spiobj->handle);
    const int bitshift = datasize_to_transfer_bitshift(handle->Init.DataSize);
    /* check buffer sizes are multiple of spi word size */
    MBED_ASSERT(tx_length >> bitshift << bitshift == tx_length);
    MBED_ASSERT(rx_length >> bitshift << bitshift == rx_length);
    int total = (tx_length > rx_length) ? tx_length : rx_length;

    if (handle->Init.Direction == SPI_DIRECTION_2LINES) {
        int write_fill_frame = write_fill;
        /* extend fill symbols for 16/32 bit modes */
        for (int i = 0; i < bitshift; i++) {
            write_fill_frame = (write_fill_frame << 8) | write_fill;
        }

        const int word_size = 0x01 << bitshift;
        for (int i = 0; i < total; i += word_size) {
            int out = (i < tx_length) ? spi_get_word_from_buffer(tx_buffer + i, bitshift) : write_fill_frame;
            int in = spi_master_write(obj, out);
            if (i < rx_length) { <------------- Here is where the return value is written back.
                spi_put_word_to_buffer(rx_buffer + i, bitshift, in);
            }
        }
    } else {
        /* 1 wire case */
        int result = spi_master_one_wire_transfer(obj, tx_buffer, tx_length, rx_buffer, rx_length);
        if (result != tx_length + rx_length) {
            /*  report an error */
            total = 0;
        }
    }

    return total;
}

Any advice is welcome...

2

Have a little bit better solution.

I changed three files:
In "~/.arduino15/packages/arduino/hardware/mbed_giga/4.2.1/libraries/SPI" folder changes were made to SPI.cpp:

  SPI.cpp - wrapper over mbed SPI class
  Part of Arduino - http://www.arduino.cc/

  Copyright (c) 2018-2019 Arduino SA

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General
  Public License along with this library; if not, write to the
  Free Software Foundation, Inc., 59 Temple Place, Suite 330,
  Boston, MA  02111-1307  USA
*/

#include "SPI.h"
#include "pinDefinitions.h"
#if !defined(ARDUINO_AS_MBED_LIBRARY)
#include "drivers/SPIMaster.h"
#else
#include "drivers/SPI.h"
#endif

struct _mbed_spi {
  mbed::SPI* obj;
};


arduino::MbedSPI::MbedSPI(int miso, int mosi, int sck) :
  _miso(digitalPinToPinName(miso)), _mosi(digitalPinToPinName(mosi)), _sck(digitalPinToPinName(sck)) {

}

arduino::MbedSPI::MbedSPI(PinName miso, PinName mosi, PinName sck) : _miso(miso), _mosi(mosi), _sck(sck) {

}

uint8_t arduino::MbedSPI::transfer(uint8_t data) {
    uint8_t ret;
    dev->obj->write((const char*)&data, 1, (char*)&ret, 1);
    return ret;
}

uint16_t arduino::MbedSPI::transfer16(uint16_t data) {

    union { uint16_t val; struct { uint8_t lsb; uint8_t msb; }; } t;
    t.val = data;

    if (settings.getBitOrder() == LSBFIRST) {
        t.lsb = transfer(t.lsb);
        t.msb = transfer(t.msb);
    } else {
        t.msb = transfer(t.msb);
        t.lsb = transfer(t.lsb);
    }
    return t.val;
}

void arduino::MbedSPI::transfer(void *buf, size_t count) {
    dev->obj->write((const char*)buf, count, (char*)buf, count);
}

//----------------------------------------------------------------------
// Added: 12-07-24 wwatson
//----------------------------------------------------------------------
void arduino::MbedSPI::transfer(void *txbuf, void *rxbuf, size_t count) {
    if(rxbuf == nullptr) {
		dev->obj->write((const char*)txbuf, count, (char*)rxbuf, -1);
	} else {
		dev->obj->write((const char*)txbuf, count, (char*)rxbuf, count);
	}
}

void arduino::MbedSPI::transfer(void *txbuf, size_t tx_count, void *rxbuf, size_t rx_count) {
    dev->obj->write((const char*)txbuf, tx_count, (char*)rxbuf, rx_count);
}
//----------------------------------------------------------------------

void arduino::MbedSPI::usingInterrupt(int interruptNumber) {

}

void arduino::MbedSPI::notUsingInterrupt(int interruptNumber) {

}

void arduino::MbedSPI::beginTransaction(SPISettings settings) {
    if (settings != this->settings) {
        dev->obj->format(8, settings.getDataMode());
        dev->obj->frequency(settings.getClockFreq());
        this->settings = settings;
    }
}

void arduino::MbedSPI::endTransaction(void) {
    // spinlock until transmission is over (if using ASYNC transfer)
}

void arduino::MbedSPI::attachInterrupt() {

}

void arduino::MbedSPI::detachInterrupt() {

}

void arduino::MbedSPI::begin() {
    if (dev == NULL) {
      dev = new mbed_spi;
      dev->obj = NULL;
    }
    if (dev->obj == NULL) {
      dev->obj = new mbed::SPI(_mosi, _miso, _sck);
    }
}

void arduino::MbedSPI::end() {
    if (dev != NULL && dev->obj != NULL) {
        delete dev->obj;
        delete dev;
        dev = NULL;
    }
}

#if !defined(ARDUINO_AS_MBED_LIBRARY)

#if SPI_HOWMANY > 0
arduino::MbedSPI SPI(SPI_MISO, SPI_MOSI, SPI_SCK);
#endif
#if SPI_HOWMANY > 1
arduino::MbedSPI SPI1(SPI_MISO1, SPI_MOSI1, SPI_SCK1);
#endif

#endif

And in SPI.h:

/*
  Copyright (c) 2016 Arduino LLC.  All right reserved.

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
  See the GNU Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General Public
  License along with this library; if not, write to the Free Software
  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
*/

#pragma once

#include "Arduino.h"
#include "api/HardwareSPI.h"

typedef struct _mbed_spi mbed_spi;

namespace arduino {

class MbedSPI : public SPIClass
{
public:
    MbedSPI(int miso, int mosi, int sck);
    MbedSPI(PinName miso, PinName mosi, PinName sck);
    virtual uint8_t transfer(uint8_t data);
    virtual uint16_t transfer16(uint16_t data);
    virtual void transfer(void *buf, size_t count);

//----------------------------------------------------------------------
// Added: 12-07-24 wwatson
//----------------------------------------------------------------------
    virtual void transfer(void *txbuf, void *rxbuf, size_t count);
    virtual void transfer(void *txbuf, size_t tx_count, void *rxbuf, size_t rx_count);
//----------------------------------------------------------------------

    // Transaction Functions
    virtual void usingInterrupt(int interruptNumber);
    virtual void notUsingInterrupt(int interruptNumber);
    virtual void beginTransaction(SPISettings settings);
    virtual void endTransaction(void);

    // SPI Configuration methods
    virtual void attachInterrupt();
    virtual void detachInterrupt();

    virtual void begin();
    virtual void end();

private:
    SPISettings settings = SPISettings(0, MSBFIRST, SPI_MODE0);
    _mbed_spi* dev = NULL;
    PinName _miso;
    PinName _mosi;
    PinName _sck;
};

}

#if !defined(ARDUINO_AS_MBED_LIBRARY)

#if SPI_HOWMANY > 0
extern arduino::MbedSPI SPI;
#endif
#if SPI_HOWMANY > 1
#ifdef SPI1
#undef SPI1
#endif
extern arduino::MbedSPI SPI1;
#endif

#endif

And finally in: "~/.arduino15/packages/arduino/hardware/mbed_giga/4.2.1/cores/arduino/api"
Changes were needed in HardwareSPI.h:

/*
  HardwareSPI.h - Hardware SPI interface for Arduino
  Copyright (c) 2018 Arduino LLC.  All right reserved.

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General Public
  License along with this library; if not, write to the Free Software
  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
*/

#pragma once

#include "Common.h"
#include <inttypes.h>
#include "Stream.h"

#define SPI_HAS_TRANSACTION

namespace arduino {

typedef enum {
  SPI_MODE0 = 0,
  SPI_MODE1 = 1,
  SPI_MODE2 = 2,
  SPI_MODE3 = 3,
} SPIMode;


class SPISettings {
  public:
  SPISettings(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) {
    if (__builtin_constant_p(clock)) {
      init_AlwaysInline(clock, bitOrder, dataMode);
    } else {
      init_MightInline(clock, bitOrder, dataMode);
    }
  }

  SPISettings(uint32_t clock, BitOrder bitOrder, int dataMode) {
    if (__builtin_constant_p(clock)) {
      init_AlwaysInline(clock, bitOrder, (SPIMode)dataMode);
    } else {
      init_MightInline(clock, bitOrder, (SPIMode)dataMode);
    }
  }

  // Default speed set to 4MHz, SPI mode set to MODE 0 and Bit order set to MSB first.
  SPISettings() { init_AlwaysInline(4000000, MSBFIRST, SPI_MODE0); }

  bool operator==(const SPISettings& rhs) const
  {
    if ((this->clockFreq == rhs.clockFreq) &&
        (this->bitOrder == rhs.bitOrder) &&
        (this->dataMode == rhs.dataMode)) {
      return true;
    }
    return false;
  }

  bool operator!=(const SPISettings& rhs) const
  {
    return !(*this == rhs);
  }

  uint32_t getClockFreq() const {
    return clockFreq;
  }
  SPIMode getDataMode() const {
    return dataMode;
  }
  BitOrder getBitOrder() const {
    return (bitOrder);
  }

  private:
  void init_MightInline(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) {
    init_AlwaysInline(clock, bitOrder, dataMode);
  }

  // Core developer MUST use an helper function in beginTransaction() to use this data
  void init_AlwaysInline(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) __attribute__((__always_inline__)) {
    this->clockFreq = clock;
    this->dataMode = dataMode;
    this->bitOrder = bitOrder;
  }

  uint32_t clockFreq;
  SPIMode dataMode;
  BitOrder bitOrder;

  friend class HardwareSPI;
};

const SPISettings DEFAULT_SPI_SETTINGS = SPISettings();

class HardwareSPI
{
  public:
    virtual ~HardwareSPI() { }

    virtual uint8_t transfer(uint8_t data) = 0;
    virtual uint16_t transfer16(uint16_t data) = 0;
    virtual void transfer(void *buf, size_t count) = 0;

//----------------------------------------------------------------------
// Added: 12-07-24 wwatson
//----------------------------------------------------------------------
    virtual void transfer(void *txbuf, void *rxbuf, size_t count) = 0;
    virtual void transfer(void *txbuf, size_t tx_count, void *rxbuf, size_t rx_count) = 0;
//----------------------------------------------------------------------

    // Transaction Functions
    virtual void usingInterrupt(int interruptNumber) = 0;
    virtual void notUsingInterrupt(int interruptNumber) = 0;
    virtual void beginTransaction(SPISettings settings) = 0;
    virtual void endTransaction(void) = 0;

    // SPI Configuration methods
    virtual void attachInterrupt() = 0;
    virtual void detachInterrupt() = 0;

    virtual void begin() = 0;
    virtual void end() = 0;
};

// Alias SPIClass to HardwareSPI since it's already the defacto standard for SPI class name
typedef HardwareSPI SPIClass;

}

In all three files I marked where I made the additions. Two methods were added:

void arduino::MbedSPI::transfer(void *txbuf, void *rxbuf, size_t count) {
    if(rxbuf == nullptr) {
		dev->obj->write((const char*)txbuf, count, (char*)rxbuf, -1);
	} else {
		dev->obj->write((const char*)txbuf, count, (char*)rxbuf, count);
	}
}

void arduino::MbedSPI::transfer(void *txbuf, size_t tx_count, void *rxbuf, size_t rx_count) {
    dev->obj->write((const char*)txbuf, tx_count, (char*)rxbuf, rx_count);
}

In the first one a tx and rx buffer can be used. The buffer length count is the same for both buffers.
If you just want to use the tx buffer the set the rx buffer as "NULL". This sets the rx count to -1 which
disables the receive portion of the low level code.

The second method is almost the same except two different size buffers can be used for tx and rx. Again
setting rx count to -1 disables rx buffer reception.

This is pretty much a hack to get away from crashing the GIGA by accidentally using a constant type buffer
on the rx side. As such, every time you update Mbed-OS you would need to reinstall the three files again...

EDIT:
Or maybe this:

//----------------------------------------------------------------------
// Added: 12-07-24 wwatson
//----------------------------------------------------------------------
void arduino::MbedSPI::transfer(void *txbuf, void *rxbuf, size_t count) {
	int txonly = count;
	if(rxbuf == nullptr)  txonly = -1;
	dev->obj->write((const char*)txbuf, count, (char*)rxbuf, txonly);
}

instead...

EDIT: Or an example of how to properly use "SPI_DIRECTION_2LINES_TXONLY"?