Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPI bit order is misinterpreted #478

Open
andreer opened this issue Jan 15, 2023 · 3 comments
Open

SPI bit order is misinterpreted #478

andreer opened this issue Jan 15, 2023 · 3 comments

Comments

@andreer
Copy link

andreer commented Jan 15, 2023

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
Copy link
Contributor

paulvha commented Jan 19, 2023

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
Copy link
Author

andreer commented Jan 22, 2023

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
Copy link
Contributor

paulvha commented Jan 22, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants