Fixes exceptions resulting from using SPI0Command (#9140)

* Resolves exceptions occuring when using SPI0Command for flash write operations
such as: Write Status Register-1, Sector Eraser, etc.

Moved PRECACHE_END to ensure `Wait_SPI_Idlep` and `xt_wsr_ps` are included in the iCache.

Added SPIUCSSETUP to give more settling time for #CS.

* There was a risk of flash reads inserted between an "enable opcode"
and the "target opcode". They are now tightly coupled.

Update flash quirks.

* When sending instruction Write Enable 0x06, use BootROM API
SPI_write_enable for the special handling of the WEL bit.

Corrected zero mask for fractional byte returns where the partial
byte bits are positioned at the most significant bit position in the byte.

cores/esp8266/core_esp8266_flash_quirks.cpp

@@ -66,12 +66,13 @@ void initFlashQuirks() {
             newSR3=SR3;
             if (get_flash_mhz()>26) { // >26Mhz?
                // Set the output drive to 100%
+               // These definitions are for the XM25QH32B part. On a XM25QH32C
+               // part, the XM25QH32B's 100% is C's 25% driver strength.
                newSR3 &= ~(SPI_FLASH_SR3_XMC_DRV_MASK << SPI_FLASH_SR3_XMC_DRV_S);
                newSR3 |= (SPI_FLASH_SR3_XMC_DRV_100 << SPI_FLASH_SR3_XMC_DRV_S);
             }
             if (newSR3 != SR3) { // only write if changed
-               if (SPI0Command(SPI_FLASH_CMD_WEVSR,NULL,0,0)==SPI_RESULT_OK)  // write enable volatile SR
+               SPI0Command(SPI_FLASH_CMD_WSR3,&newSR3,8,0,SPI_FLASH_CMD_WEVSR);  // write to SR3, use write enable volatile prefix
-                  SPI0Command(SPI_FLASH_CMD_WSR3,&newSR3,8,0);  // write to SR3
                SPI0Command(SPI_FLASH_CMD_WRDI,NULL,0,0);        // write disable - probably not needed
             }
          }

cores/esp8266/core_esp8266_spi_utils.cpp

@@ -32,6 +32,7 @@
 #include "core_esp8266_features.h"
 
 #include "spi_utils.h"
+#include "spi_flash_defs.h"
 
 extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc);
 
@@ -51,7 +52,7 @@ namespace experimental {
 static SpiOpResult PRECACHE_ATTR
 _SPICommand(volatile uint32_t spiIfNum,
             uint32_t spic,uint32_t spiu,uint32_t spiu1,uint32_t spiu2,
-            uint32_t *data,uint32_t writeWords,uint32_t readWords)
+            uint32_t *data,uint32_t writeWords,uint32_t readWords, uint32_t pre_cmd)
 {
   if (spiIfNum>1)
      return SPI_RESULT_ERR;
@@ -65,6 +66,7 @@ _SPICommand(volatile uint32_t spiIfNum,
   // Everything defined here must be volatile or the optimizer can
   // treat them as constants, resulting in the flash reads we're
   // trying to avoid
+  SpiFlashOpResult (* volatile SPI_write_enablep)(SpiFlashChip *) = SPI_write_enable;
   uint32_t (* volatile Wait_SPI_Idlep)(SpiFlashChip *) = Wait_SPI_Idle;
   volatile SpiFlashChip *fchip=flashchip;
   volatile uint32_t spicmdusr=SPICMDUSR;
@@ -84,8 +86,23 @@ _SPICommand(volatile uint32_t spiIfNum,
   uint32_t oldSPI0U2= SPIREG(SPI0U2);
   uint32_t oldSPI0C = SPIREG(SPI0C);
 
-  //SPI0S &= ~(SPISE|SPISBE|SPISSE|SPISCD);
   SPIREG(SPI0C) = spic;
+
+  if (SPI_FLASH_CMD_WREN == pre_cmd) {
+     // See SPI_write_enable comments in esp8266_undocumented.h
+     SPI_write_enablep((SpiFlashChip *)fchip);
+  } else
+  if (pre_cmd) {
+     // Send prefix cmd w/o data - sends 8 bits. eg. Volatile SR Write Enable, 0x50
+     SPIREG(SPI0U)  = (spiu & ~(SPIUMOSI|SPIUMISO));
+     SPIREG(SPI0U1) = 0;
+     SPIREG(SPI0U2) = (spiu2 & ~0xFFFFu) | pre_cmd;
+
+     SPIREG(SPI0CMD) = spicmdusr;   //Send cmd
+     while ((SPIREG(SPI0CMD) & spicmdusr));
+  }
+
+  //SPI0S &= ~(SPISE|SPISBE|SPISSE|SPISCD);
   SPIREG(SPI0U) = spiu;
   SPIREG(SPI0U1)= spiu1;
   SPIREG(SPI0U2)= spiu2;
@@ -118,10 +135,21 @@ _SPICommand(volatile uint32_t spiIfNum,
   SPIREG(SPI0U2)= oldSPI0U2;
   SPIREG(SPI0C) = oldSPI0C;
 
-  PRECACHE_END();
   if (!spiIfNum) {
+      // w/o a call to Wait_SPI_Idlep, 'Exception 0' or other exceptions (saw
+      // 28) may occur later after returning to iCache code. This issue was
+      // observed with non-volatile status register writes.
+      //
+      // My guess is: Returning too soon to uncached iCache executable space. An
+      // iCache read may not complete properly because the Flash or SPI
+      // interface is still busy with the last write operation. In such a case,
+      // I expect new reads from iROM to result in zeros. This would explain
+      // the Exception 0 for code, and Exception 20, 28, and 29 where a literal
+      // was misread as 0 and then used as a pointer.
+      Wait_SPI_Idlep((SpiFlashChip *)fchip);
       xt_wsr_ps(saved_ps);
   }
+  PRECACHE_END();
   return (timeout>0 ? SPI_RESULT_OK : SPI_RESULT_TIMEOUT);
 }
 
@@ -139,12 +167,37 @@ _SPICommand(volatile uint32_t spiIfNum,
  *	miso_bits
  *		Number of bits to read from the SPI bus after the outgoing
  *		data has been sent.
+ *  pre_cmd
+ *    A few SPI Flash commands require enable commands to immediately preceed
+ *    them. Since two calls to SPI0Command from ICACHE memory most likely would
+ *    be separated by SPI Flash read request for iCache, use this option to
+ *    supply a prefix command, 8-bits w/o read or write data.
+ *
+ *    Case in point from the GD25Q32E datasheet: "The Write Enable for Volatile
+ *    Status Register command must be issued prior to a Write Status Register
+ *    command and any other commands can’t be inserted between them."
  *
  *  Note: This code has only been tested with SPI bus 0, but should work
  *        equally well with other buses. The ESP8266 has bus 0 and 1,
  *        newer chips may have more one day.
+ *
+ *  Supplemental Notes:
+ *
+ *  SPI Bus wire view: Think of *data as an array of bytes, byte[0] goes out
+ *  first with the most significant bit shifted out first and so on. When
+ *  thinking of the data as an array of 32bit-words, the least significant byte
+ *  of the first 32bit-word goes out first on the SPI bus with the most
+ *  significant bit of that byte shifted out first onto the wire.
+ *
+ *  When presenting a 3 or 4-byte address, the byte order will need to be
+ *  reversed. Don't overthink it. For a 3-byte address, view *data as a byte
+ *  array and set the first 3-bytes to the address. eg. byteData[0] MSB,
+ *  byteData[1] middle, and byteData[2] LSB.
+ *
+ *  When sending a fractional byte, fill in the most significant bit positions
+ *  of the byte first.
  */
-SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits) {
+SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits, uint32_t pre_cmd) {
   if (mosi_bits>(64*8))
      return SPI_RESULT_ERR;
   if (miso_bits>(64*8))
@@ -159,8 +212,16 @@ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_
   if (miso_bits % 32 != 0)
      miso_words++;
 
+  // Use SPI_CS_SETUP to add time for #CS to settle (ringing) before SPI CLK
+  // begins. The BootROM does not do this; however, RTOS SDK and NONOS SDK do
+  // as part of flash init/configuration.
+  //
+  // One SPI bus clock cycle time inserted between #CS active and the 1st SPI
+  // bus clock cycle. The number of clock cycles is in SPI_CNTRL2
+  // SPI_SETUP_TIME, which defaults to 1.
+  //
   // Select user defined command mode in the controller
-  uint32_t spiu=SPIUCOMMAND; //SPI_USR_COMMAND
+  uint32_t spiu=SPIUCOMMAND | SPIUCSSETUP; //SPI_USR_COMMAND | SPI_CS_SETUP
 
   // Set the command byte to send
   uint32_t spiu2 = ((7 & SPIMCOMMAND)<<SPILCOMMAND) | cmd;
@@ -183,12 +244,19 @@ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_
   spic &= ~(SPICQIO | SPICDIO | SPICQOUT | SPICDOUT | SPICAHB | SPICFASTRD);
   spic |= (SPICRESANDRES | SPICSHARE | SPICWPR | SPIC2BSE);
 
-  SpiOpResult rc =_SPICommand(0,spic,spiu,spiu1,spiu2,data,mosi_words,miso_words);
+  SpiOpResult rc =_SPICommand(0,spic,spiu,spiu1,spiu2,data,mosi_words,miso_words,pre_cmd);
 
   if (rc==SPI_RESULT_OK) {
-     // clear any bits we did not read in the last word.
+     // Clear any bits we did not read in the last word. Bits in a fractional
-     if (miso_bits % 32) {
+     // bytes will be stored in the most significant part of the byte first.
-        data[miso_bits/32] &= ~(0xFFFFFFFF << (miso_bits % 32));
+     if (miso_bits % 32u) {
+        uint32_t whole_byte_bits = (miso_bits % 32u) & ~7u;
+        uint32_t mask = ~(0xFFFFFFFFu << whole_byte_bits);
+        if (miso_bits % 8u) {
+           // Select fractional byte bits.
+           mask |= (~(0xFFu >> (miso_bits % 8u)) & 0xFFu) << whole_byte_bits;
+        }
+        data[miso_bits/32u] &= mask;
      }
   }
   return rc;

cores/esp8266/esp8266_undocumented.h

@@ -241,6 +241,17 @@ extern fn_c_exception_handler_t _xtos_c_handler_table[XCHAL_EXCCAUSE_NUM];
 extern fn_c_exception_handler_t _xtos_set_exception_handler(int cause, fn_c_exception_handler_t fn);
 #endif
 
+/*
+  BootROM function that sends the SPI Flash "Write Enable" command, 0x06.
+  The function internally calls Wait_SPI_Idle before enabling.
+  Polls status register forever waiting for WEL bit to set.
+  This function always returns 0; however, most examples test for 0.
+
+  Every function I find that needs WEL set, call this function. I suspect the
+  waiting for the WEL bit to set is a Flash chip anomaly workaround.
+*/
+extern SpiFlashOpResult SPI_write_enable(SpiFlashChip *fc);
+
 extern uint32_t Wait_SPI_Idle(SpiFlashChip *fc);
 extern void Cache_Read_Disable();
 extern int32_t system_func1(uint32_t);

cores/esp8266/spi_utils.h

@@ -35,7 +35,7 @@ typedef enum {
     SPI_RESULT_TIMEOUT
 } SpiOpResult;
 
-SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits);
+SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits, uint32_t pre_cmd=0);
 }
 
 #ifdef __cplusplus