Need to use a different SPI_MODE on ESP32 (SPI_MODE1)

[rfestag]

Description

I am unable to read/write from the Teensy Prop Shield's flash storage on the ESP32. This appears to be because I need to use a different SPI_MODE. I initially reported the issue to arduino-esp32 , and @stickbreaker suggested I change the SPI mode. I found that SPI_MODE1 worked properly, while SerialFlash's SPICONFIG uses SPI_MODE0. This can be seen here.

I'm happy to create a PR for this, but I'm not sure how you would want to see this resolved:

1. Use an ifdef for ESP32 and change the SPI_MODE
2. Somehow make the SPICONFIG configurable so that users can replace the settings in SPICONFIG (or maybe there's already a way to do that I'm not aware of?)

Steps To Reproduce Problem

I am using a Teensy Prop Shield attached to an Adafruit ESP32 Feather. A minimal sketch is provided below

Hardware & Software

Board: Adafruit ESP32 Feather
Shields / modules used: Teensy Prop Shield
Arduino IDE version: 1.8.8
Version info & package name (from Tools > Boards > Board Manager): arduino-esp32 (latest from git)
Operating system & version: Manjaro

Arduino Sketch

#include <SerialFlash.h>
#include <SPI.h>

#define MEM_CS 12

//These are copied from SerialFlash for illustration. They 
#define DEFAULT_MAXFILES      600
#define DEFAULT_STRINGS_SIZE  25560

static uint32_t check_signature(void)
{
        uint32_t sig[2];
        uint32_t sig2[2];

        SerialFlash.read(0, sig, 8);
        Serial.printf("sig: %08X %08X\n", sig[0], sig[1]);
        if (sig[0] == 0xFA96554C) return sig[1];
        if (sig[0] == 0xFFFFFFFF) {
                sig[0] = 0xFA96554C;
                sig[1] = ((uint32_t)(DEFAULT_STRINGS_SIZE/4) << 16) | DEFAULT_MAXFILES;
                Serial.printf("Writing sig: %08X %08X\n", sig[0], sig[1]);
                SerialFlash.write(0, sig, 8);
                while (!SerialFlash.ready()) ; // TODO: timeout
                SerialFlash.read(0, sig2, 8);
                Serial.printf("Reading sig2: %08X %08X\n", sig2[0], sig2[1]);
                if (sig2[0] == 0xFA96554C) return sig2[1];
        }
        return 0;
}
void setup() {
  Serial.begin(115200);
  Serial.println();

  //Setup Serial Flash
  if (!SerialFlash.begin(MEM_CS)) {
    Serial.println("Unable to access SPI Flash chip");
  }
  
  //These writes actually work. We do it to ensure the first part of
  //the flash storage is 0xFFFFFFFF, forcing it to try to write the magic number
  SerialFlash.eraseAll(); 
  unsigned long dotMillis = millis();
  unsigned char dotcount = 0;
  while (SerialFlash.ready() == false) {
    if (millis() - dotMillis > 1000) {
      dotMillis = dotMillis + 1000;
      Serial.print(".");
      dotcount = dotcount + 1;
      if (dotcount >= 60) {
        Serial.println();
        dotcount = 0;
      }
    }
  }
  //We call the function SerialFlash uses internally to verify
  //that the state of the storage is valid (i.e., it has the magic number)
  check_signature();
}

void loop() {}

Errors or Incorrect Output

Below is the output of the sketch provided above. Note that I copied the check_signature method from SerialFlash, and added some debugginng to it to print out what it is writing and the result of immediately reading that write upon finding an erased flash chip.

...................sig: FFFFFFFF FFFFFFFF
Writing sig: FA96554C 18F60258
Reading sig2: 7DCB2A26 0C7B012C

[PaulStoffregen]

Is this a bug in ESP32's SPI library? If so, it should be fixed there.

[rfestag]

Good question. Because I was told to try using a different mode, I assumed different architectures could define them differently. If there are standard definitions and this flash chip inherently should be mode 0, then you're probably right. This seems like something somebody would have noticed before though on their end. Would you expect this to be an arduino-esp32 issue, or an esp32 core issue?

[PaulStoffregen]

These modes absolutely are standard definitions (or at least de-facto standards - not necessarily something published by a standards organization like IEEE or JEDEC). SPI_MODE0 is supposed to do the same thing on all microcontrollers. If ESP32 is doing something different from everyone else, that's definitely a bug with ESP32.

However, it's also possible there could be some other subtle problem. Very hard to know without a deep investigation by looking at the actual waveforms.

[rfestag]

That definitely makes sense. I submitted a ticket to them. Unfortunately I don't have a way to look at the waveforms (and honestly wouldn't really know what I'm looking at anyways). Hopefully somebody on there can verify whether the handling of the SPI modes is correct.

[fa1ke5]

Hi all, i found the problem, and it no in SPI mode.

...................sig: FFFFFFFF FFFFFFFF
Writing sig: FA96554C 18F60258
Reading sig2: 7DCB2A26 0C7B012C

Take first two bytes and an analyse them
Writing sig: FA 96
Reading sig2: 7D CB

...now in bin
FA 11111010; 96 10010110
7D 01111101; CB 11001011
as you can see in fast SPI speed transaction you hardware miss one first bit of data.
CS signal is not accurate.....
I make some test and find that critical speed of SPI mode 0 is up to 26 MHz, SPI mode 1 is 39MHz.
I think in this topic analogue problem
SmingHub/Sming#618
I think you need use software CS signal, but this library dont have optimisation for fast software gpios on esp32, that in SerialFlash_directWrite

SerialFlash/util/SerialFlash_directwrite.h

Line 86 in 71cfa7a

#elif defined(ARDUINO_ARCH_ESP8266)

and now i see only esp8266 in it.
Sorry for my bad english....
Nice day....

[rfestag]

@fa1ke5 - Thanks for the input. I looked at your investigation in #58, and tried changing the maximum speed parameter instead of the SPI mode:

#define SPICONFIG   SPISettings(26000000, MSBFIRST, SPI_MODE0)

This appears to be working correctly now. While you may be right that some special definition for ARDUINO_ARCH_ESP32 would be the correct solution, I'm personally not familiar enough with how to make the appropriate changes. I did come across some code that looked relevant in the OneWire library (https://github.com/PaulStoffregen/OneWire/blob/master/util/OneWire_direct_gpio.h#L119), but pulling those defines into SerialFlash (and keeping the old SPICONFIG definition), didn't work.

I think a few questions back to @PaulStoffregen

1. Would you have expected the definitions from OneWire to also work here? Or are they not related at all? Admittedly, I'm just assuming that because they have the same names they may be related, but that may be incorrect.
2. Would it be appropriate to make SPICONFIG configurable somehow so that the max speed can be defined by users? Or is this still indicative of some other issue that should be solved in a different way?

[fa1ke5]

Did you copy all code of defines with low level functions, like this
IO_REG_TYPE directRead(IO_REG_TYPE pin)
{
if ( pin < 32 )
return (GPIO.in >>

[fa1ke5]

and tried changing the maximum speed parameter instead of the SPI mode:

#define SPICONFIG SPISettings(26000000, MSBFIRST, SPI_MODE0)

you are right, 26MHz is critical for MODE0.
GPIO matrix on esp32 work on 26MHz.....but if you switch to MODE1 you take up to 39MHz SPI speed, this is wrong setting but it work, black china magic :)

[PaulStoffregen]

Do you suppose it works reliably across all chips? How about when the temperature changes?

[fa1ke5]

this is not temperature problem, its only esp32 hardware/SDK problem

[rfestag]

I think I have found documentation on what @fa1ke5 is referring to as the "critical" speed:

https://docs.espressif.com/projects/esp-idf/en/latest/api-reference/peripherals/spi_master.html#speed-and-timing-considerations

If you scroll down a bit from there, you'll see a chart outlining the following frequency limits:

GPIO matrix	IOMUX pins	Dummy Bits Used By Driver
26.6	80	No
40	–	Yes

I haven't had a chance to go through the pinouts to trace it, but I'm going to assume the MEM_CS pin on Prop Shield isn't mapped to the ESP32 Pin 15 (CS0) through the Adafruit feather adapter (I'll verify this later, but it seems unlikely that we were lucky enough to have it map to the best pin, and the behavior is consistent). That implies we are going through the GPIO matrix, and have a limit of 26.6Mhz when not using the "dummy bit" workaround, and 40Mhz when using it.

I also found this interesting header file: https://github.com/espressif/arduino-esp32/blob/master/tools/sdk/include/driver/driver/spi_master.h

At the top, it describes the SPI frequencies, and mentions that it will round down to the closest frequency. Since SerialFlash hard codes 50Mhz, it is rounded down to 40Mhz and fails. I tried specifying 30Mhz (which would round it down to 26Mhz), and it worked fine.

Because we have a R/W workload with SerialFlash (not write-only), we probably shouldn't use the dummy-bit workaround. As such, I think the best course of action is to make the SPICONFIG configurable. I say this for 2 reasons:

1. If we add an architecture specific define for the max value, we ignore the case where somebody actually does have the flash mapped to the IOMUX CS0 pin on ESP32 and can operate at higher frequencies. For instance, I think there is a way for us to redefine the IOMUX pins, but that shoudn't be a prerequisite for using this library.
2. It is clearly possible that some users will have speed limits based on their architecture, so making it configurable allows SerialFlash to be used by them.

I presume the best place to do this would be to add an overloaded begin() method that allows passing in an SPISettings from the user, and setting SPICONFIG there?

I'd be happy to create a pull request for this if it makes sense.