sparkfun / Arduino_Apollo3

Arduino core to support the Apollo3 microcontroller from Ambiq Micro
83 stars 38 forks source link

SPI bit order is misinterpreted #478

Open andreer opened 1 year ago

andreer commented 1 year ago

Hi,

Currently LSBFIRST and MSBFIRST is interpreted as byte order when handling SPI transfers, and only used for transfer16 (swapping byte A, B -> B, A).

Unfortunately I believe this is wrong, it should be interpreted as bit order, affecting the transfer order of the bits for each byte in all transfers - e.g. whether to transfer the 8 bits of 0xA0 as 1010 0000 or 0000 0101.

Some more detail (code examples, logic analyzer capture) can be found in https://forum.sparkfun.com/viewtopic.php?f=169&t=58885&p=238325

Thank you, Andreas

paulvha commented 1 year ago

Just in case someone else has the problem, depending on whether you have V1.2.3 or V 2.2.3 you can apply the following change.

V1.2.3

In SparkFun/hardware/apollo3/1.2.3/libraries/SPI/src/SPI.cpp, the function config() add at the end after the line ' initialize(); // Initialize the IOM'

  // fix for LSBFIRST / MSBFIRST
  if (_order == LSBFIRST)
    IOMn(_instance)->MSPICFG |= _VAL2FLD(IOM0_MSPICFG_SPILSB,  IOM0_MSPICFG_SPILSB_LSB);
  else
    IOMn(_instance)->MSPICFG |= _VAL2FLD(IOM0_MSPICFG_SPILSB,  IOM0_MSPICFG_SPILSB_MSB);

V2.2.1

In SparkFun/hardware/apollo3/2.2.1/libraries/SPI/src/SPI.cpp 2 changes.

add a new function:

/**
 * {paulvha -Jan 2023}
 * Set the SPI bit order (MSBFIRST or LSBFIRST)
 * Select the right IOM based on _mosi and _mosi pins
 */
int arduino::MbedSPI::SPI_Bitorder(byte order) {
  int ret = -1;
  if     (_mosi == 7  && _miso == 6 ) ret = 0;
  else if(_mosi == 10  && _miso == 9) ret = 1;
  else if(_mosi == 28 && _miso == 25) ret = 2;
  else if(_mosi == 44 && _miso == 40) ret = 4;
  else if(_mosi == 47 && _miso == 49) ret = 5;
#if defined (AM_PACKAGE_BGA)
  else if(_mosi == 38 && _miso == 43) ret = 3;
#endif

  if (ret != -1){
    IOMn(ret)->MSPICFG |= _VAL2FLD(IOM0_MSPICFG_SPILSB,! order);
  }

  return ret;
}

change in the existing function beginTransaction(SPISettings settings) so it shows as :


        dev->format(8, settings.getDataMode());
        dev->frequency(settings.getClockFreq());

        // {paulvha jan - 2023}
        if (SPI_Bitorder(settings.getBitOrder()) == -1){
          Serial.println("Error during setting SPI Bitorder");
          return;
        }

        this->settings = settings;

In SparkFun/hardware/apollo3/2.2.1/libraries/SPI/src/SPI.h 1 change.

add in the private section, just below the line 'SPISettings settings...'

virtual int SPI_Bitorder(byte order); // {paulvha jan - 2023}

regards, Paul

andreer commented 1 year ago

Thank you so much again Paul, I can confirm that this fixed the issue for me and also noticeably increases performance compared to my bit-reversing workaround - very happy with that!

I previously had the MISO pin set to "NC" in my program - after applying your changes, that produces the "Error during setting SPI Bitorder" message you added here. So it will then be required to specify both MISO and MOSI pins, which might be worth noting.

paulvha commented 1 year ago

Thanks for the feedback. Good to hear it also worked for you. Good point on the MISO, although I would expect that most people will just use the standard SPI instance definitions.

regards, Paul