From 71d348c5e538173c503beffd5e6dd1c3013c45ea Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Thu, 10 Oct 2019 21:54:39 +0800 Subject: [PATCH 01/15] precache() - preload code into the flash cache. By preloading code into the flash cache we can take control over when SPI Flash reads will occur when code is executing. This can be useful where the timing of a section of code is extremely critical and we don't want random pauses to pull code in from the SPI flash chip. It can also be useful for code that accesses/uses SPI0 which is connected to the flash chip. Non interrupt handler code that is infrequently called but might otherwise require being in valuable IRAM - such as bit-banging I/O code or some code run at bootup can avoid being permanently in IRAM. Macros are provided to make precaching one or more blocks of code in any function easy. --- cores/esp8266/core_esp8266_features.cpp | 39 +++++++++++++++++++++++++ cores/esp8266/core_esp8266_features.h | 13 +++++++++ 2 files changed, 52 insertions(+) create mode 100644 cores/esp8266/core_esp8266_features.cpp diff --git a/cores/esp8266/core_esp8266_features.cpp b/cores/esp8266/core_esp8266_features.cpp new file mode 100644 index 0000000000..bb5acd9e68 --- /dev/null +++ b/cores/esp8266/core_esp8266_features.cpp @@ -0,0 +1,39 @@ + +/* + core_esp8266_features.cpp + + Copyright (c) 2019 Mike Nix. All rights reserved. + This file is part of the esp8266 core for Arduino environment. + + 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 +*/ + +/* precache() + * pre-loads flash data into the flash cache + * if f==0, preloads instructions starting at the address we were called from. + * otherwise preloads flash at the given address. + * All preloads are word aligned. + */ +void precache(void *f, uint32_t bytes) { + // Size of a cache page in words. We only need to read one word per + // page (ie 1 word in 8) for this to work. + #define CACHE_PAGE_SIZE (32/4) + + register uint32_t a0 asm("a0"); + volatile uint32_t *p = (uint32_t*)((f ? (uint32_t)f : a0) & ~0x03); + uint32_t x; + for (uint32_t i=0; i<=(bytes/4); i+=CACHE_PAGE_SIZE, p+=CACHE_PAGE_SIZE) x=*p; + (void)x; +} diff --git a/cores/esp8266/core_esp8266_features.h b/cores/esp8266/core_esp8266_features.h index 6b8e22c9b0..0a67048134 100644 --- a/cores/esp8266/core_esp8266_features.h +++ b/cores/esp8266/core_esp8266_features.h @@ -93,4 +93,17 @@ inline uint32_t esp_get_cycle_count() { } #endif // not CORE_MOCK + +// Tools for preloading code into the flash cache +#define PRECACHE_ATTR __attribute__((optimize("no-reorder-blocks"))) + +#define PRECACHE_START(tag) \ + precache(NULL,(uint8_t *)&&_precache_end_##tag - (uint8_t*)&&_precache_start_##tag); \ + _precache_start_##tag: + +#define PRECACHE_END(tag) \ + _precache_end_##tag: + +void precache(void *f, uint32_t bytes); + #endif // CORE_ESP8266_FEATURES_H From acde951ec38fa8dd6e3ecde01d2edcae77edc915 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Thu, 10 Oct 2019 22:25:20 +0800 Subject: [PATCH 02/15] Fix missing include --- cores/esp8266/core_esp8266_features.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cores/esp8266/core_esp8266_features.cpp b/cores/esp8266/core_esp8266_features.cpp index bb5acd9e68..9463e181a9 100644 --- a/cores/esp8266/core_esp8266_features.cpp +++ b/cores/esp8266/core_esp8266_features.cpp @@ -20,6 +20,8 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ +#include + /* precache() * pre-loads flash data into the flash cache * if f==0, preloads instructions starting at the address we were called from. From e74736c2d4d08661ce7b9fd0ccdbabed3aa830fd Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Fri, 11 Oct 2019 14:09:07 +0800 Subject: [PATCH 03/15] Make precache extern "C" --- cores/esp8266/core_esp8266_features.cpp | 2 +- cores/esp8266/core_esp8266_features.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/core_esp8266_features.cpp b/cores/esp8266/core_esp8266_features.cpp index 9463e181a9..cdb189f947 100644 --- a/cores/esp8266/core_esp8266_features.cpp +++ b/cores/esp8266/core_esp8266_features.cpp @@ -28,7 +28,7 @@ * otherwise preloads flash at the given address. * All preloads are word aligned. */ -void precache(void *f, uint32_t bytes) { +extern "C" void precache(void *f, uint32_t bytes) { // Size of a cache page in words. We only need to read one word per // page (ie 1 word in 8) for this to work. #define CACHE_PAGE_SIZE (32/4) diff --git a/cores/esp8266/core_esp8266_features.h b/cores/esp8266/core_esp8266_features.h index 0a67048134..88b4e5fed8 100644 --- a/cores/esp8266/core_esp8266_features.h +++ b/cores/esp8266/core_esp8266_features.h @@ -104,6 +104,6 @@ inline uint32_t esp_get_cycle_count() { #define PRECACHE_END(tag) \ _precache_end_##tag: -void precache(void *f, uint32_t bytes); +extern "C" void precache(void *f, uint32_t bytes); #endif // CORE_ESP8266_FEATURES_H From 2cbdb413ebf55289238f2f395184ee3910106c59 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Fri, 11 Oct 2019 15:15:13 +0800 Subject: [PATCH 04/15] Attempt 2 at making precache extern "C" --- cores/esp8266/core_esp8266_features.cpp | 10 +++++++++- cores/esp8266/core_esp8266_features.h | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/core_esp8266_features.cpp b/cores/esp8266/core_esp8266_features.cpp index cdb189f947..4b058b27df 100644 --- a/cores/esp8266/core_esp8266_features.cpp +++ b/cores/esp8266/core_esp8266_features.cpp @@ -28,7 +28,11 @@ * otherwise preloads flash at the given address. * All preloads are word aligned. */ -extern "C" void precache(void *f, uint32_t bytes) { +#ifdef __cplusplus +extern "C" { +#endif + +void precache(void *f, uint32_t bytes) { // Size of a cache page in words. We only need to read one word per // page (ie 1 word in 8) for this to work. #define CACHE_PAGE_SIZE (32/4) @@ -39,3 +43,7 @@ extern "C" void precache(void *f, uint32_t bytes) { for (uint32_t i=0; i<=(bytes/4); i+=CACHE_PAGE_SIZE, p+=CACHE_PAGE_SIZE) x=*p; (void)x; } + +#ifdef __cplusplus +} +#endif diff --git a/cores/esp8266/core_esp8266_features.h b/cores/esp8266/core_esp8266_features.h index 88b4e5fed8..31b0dfa6e1 100644 --- a/cores/esp8266/core_esp8266_features.h +++ b/cores/esp8266/core_esp8266_features.h @@ -104,6 +104,14 @@ inline uint32_t esp_get_cycle_count() { #define PRECACHE_END(tag) \ _precache_end_##tag: -extern "C" void precache(void *f, uint32_t bytes); +#ifdef __cplusplus +extern "C" { +#endif + +void precache(void *f, uint32_t bytes); + +#ifdef __cplusplus +} +#endif #endif // CORE_ESP8266_FEATURES_H From 0a7862a258f685b13a49bce3a83f17fd28cc05ab Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Fri, 11 Oct 2019 17:39:09 +0800 Subject: [PATCH 05/15] Fix calculation of number of cache lines to preload With certain alignments/lengths of code it was possible to not read enough into the flash cache. This commit makes the length calculation clearer and adds an extra cache line to ensure we precache enough code. --- cores/esp8266/core_esp8266_features.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/core_esp8266_features.cpp b/cores/esp8266/core_esp8266_features.cpp index 4b058b27df..03396e5008 100644 --- a/cores/esp8266/core_esp8266_features.cpp +++ b/cores/esp8266/core_esp8266_features.cpp @@ -33,14 +33,15 @@ extern "C" { #endif void precache(void *f, uint32_t bytes) { - // Size of a cache page in words. We only need to read one word per + // Size of a cache page in bytes. We only need to read one word per // page (ie 1 word in 8) for this to work. - #define CACHE_PAGE_SIZE (32/4) + #define CACHE_PAGE_SIZE 32 register uint32_t a0 asm("a0"); + register uint32_t lines = (bytes/CACHE_PAGE_SIZE)+2; volatile uint32_t *p = (uint32_t*)((f ? (uint32_t)f : a0) & ~0x03); uint32_t x; - for (uint32_t i=0; i<=(bytes/4); i+=CACHE_PAGE_SIZE, p+=CACHE_PAGE_SIZE) x=*p; + for (uint32_t i=0; i Date: Fri, 11 Oct 2019 17:53:58 +0800 Subject: [PATCH 06/15] SPI0Command - A utility function for generic SPI commands on SPI0 The rom code does not support some flash functions, or have a generic way of sending custom commands to the flash chip. In particular XMC flash chips have a third status register, and the ROM only supports two. There are also certain requirements for using SPI0 such as waiting for the flash to be idle and not allowing your code to trigger a flash cache miss while using SPI0. --- cores/esp8266/core_esp8266_spi_utils.cpp | 127 +++++++++++++++++++++++ cores/esp8266/spi_utils.h | 44 ++++++++ 2 files changed, 171 insertions(+) create mode 100644 cores/esp8266/core_esp8266_spi_utils.cpp create mode 100644 cores/esp8266/spi_utils.h diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp new file mode 100644 index 0000000000..cf7e568881 --- /dev/null +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -0,0 +1,127 @@ +/* + core_esp8266_spi_utils.cpp + + Copyright (c) 2019 Mike Nix. All rights reserved. + This file is part of the esp8266 core for Arduino environment. + + 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 +*/ + +#include +#include + +// register names +#include "esp8266_peri.h" + +// for flashchip +#include "spi_flash.h" + +// for PRECACHE_* +#include "core_esp8266_features.h" + +#include "spi_utils.h" + +extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc); + +/* + * critical part of SPICommand. + * Kept in a separate function to aid with precaching + * and because this is the part that's really specific to SPI0. + * PRECACHE_* saves having to make the function IRAM_ATTR. + * + * Note: if porting to ESP32 mosi/miso bits are set in 2 registers, not 1. + */ +static SpiOpResult PRECACHE_ATTR +_SPI0Command(uint32_t spi0c,uint32_t spi0u,uint32_t spi0u1,uint32_t spi0u2, + uint32_t *data,uint32_t mosi_bytes,uint32_t miso_bytes) +{ + PRECACHE_START(); + //precache(NULL,200); + + Wait_SPI_Idle(flashchip); + uint32_t old_spi_usr = SPI0U; + uint32_t old_spi_usr2= SPI0U2; + uint32_t old_spi_c = SPI0C; + + //SPI0S &= ~(SPISE|SPISBE|SPISSE|SPISCD); + SPI0C = spi0c; + SPI0U = spi0u; + SPI0U1= spi0u1; + SPI0U2= spi0u2; + + if (mosi_bytes>0) { + // copy the outgoing data to the SPI hardware + memcpy((void*)&(SPI0W0),data,mosi_bytes); + } + + // Start the transfer + SPI0CMD = SPICMDUSR; + + // wait for the command to complete + uint32_t timeout = 1000; + while ((SPI0CMD & SPICMDUSR) && timeout--) {} + + if ((miso_bytes>0) && (timeout>0)) { + // copy the response back to the buffer + memcpy(data,(void *)&(SPI0W0),miso_bytes); + } + + SPI0U = old_spi_usr; + SPI0U2= old_spi_usr2; + SPI0C = old_spi_c; + + PRECACHE_END(); + return (timeout>0 ? SPI_RESULT_OK : SPI_RESULT_TIMEOUT); +} + + +/* SPI0Command: send a custom SPI command. + * This part calculates register values and passes them to _SPI0Command(). + */ +SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits) { + if (mosi_bits>(64*8)) return SPI_RESULT_ERR; + if (miso_bits>(64*8)) return SPI_RESULT_ERR; + + uint32_t mosi_words=mosi_bits/32; + uint32_t miso_words=miso_bits/32; + if (mosi_bits % 32 != 0) mosi_words++; + if (miso_bits % 32 != 0) miso_words++; + + uint32_t spiu=SPIUCOMMAND; //SPI_USR_COMMAND + uint32_t spiu2 = ((7 & SPIMCOMMAND)<0) { + spiu1 |= ((mosi_bits-1) & SPIMMOSI) << SPILMOSI; + spiu |= SPIUMOSI; // SPI_USR_MOSI + } + if (miso_bits>0) { + spiu1 |= ((miso_bits-1) & SPIMMISO) << SPILMISO; + spiu |= SPIUMISO; // SPI_USR_MISO + } + + uint32_t spic = SPI0C; + spic &= ~(SPICQIO | SPICDIO | SPICQOUT | SPICDOUT | SPICAHB | SPICFASTRD); + spic |= (SPICRESANDRES | SPICSHARE | SPICWPR | SPIC2BSE); + + SpiOpResult rc =_SPI0Command(spic,spiu,spiu1,spiu2,data,mosi_words*4,miso_words*4); + + if (rc==SPI_RESULT_OK) { + // clear any bits we did not read in the last word. + if (miso_bits % 32) { + data[miso_bits/32] &= ~(0xFFFFFFFF << (miso_bits % 32)); + } + } + return rc; +} diff --git a/cores/esp8266/spi_utils.h b/cores/esp8266/spi_utils.h new file mode 100644 index 0000000000..4fb56a79f8 --- /dev/null +++ b/cores/esp8266/spi_utils.h @@ -0,0 +1,44 @@ +/* + spi_utils.h - SPI utility function + Copyright (c) 2015 Ivan Grokhotkov. 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 +*/ + + +#ifndef SPI_UTILS_H +#define SPI_UTILS_H + + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef enum { + SPI_RESULT_OK, + SPI_RESULT_ERR, + SPI_RESULT_TIMEOUT +} SpiOpResult; + +SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits); + +#ifdef __cplusplus +} +#endif + + +#endif //SPI_UTILS_H From e2c6505568ef44d353a3ae57f00a7a485b33bec5 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Fri, 11 Oct 2019 19:13:17 +0800 Subject: [PATCH 07/15] Clean some trailing spaces --- cores/esp8266/core_esp8266_spi_utils.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index cf7e568881..b855fa94d0 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -72,7 +72,7 @@ _SPI0Command(uint32_t spi0c,uint32_t spi0u,uint32_t spi0u1,uint32_t spi0u2, // wait for the command to complete uint32_t timeout = 1000; while ((SPI0CMD & SPICMDUSR) && timeout--) {} - + if ((miso_bytes>0) && (timeout>0)) { // copy the response back to the buffer memcpy(data,(void *)&(SPI0W0),miso_bytes); @@ -81,7 +81,7 @@ _SPI0Command(uint32_t spi0c,uint32_t spi0u,uint32_t spi0u1,uint32_t spi0u2, SPI0U = old_spi_usr; SPI0U2= old_spi_usr2; SPI0C = old_spi_c; - + PRECACHE_END(); return (timeout>0 ? SPI_RESULT_OK : SPI_RESULT_TIMEOUT); } @@ -116,7 +116,7 @@ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_ spic |= (SPICRESANDRES | SPICSHARE | SPICWPR | SPIC2BSE); SpiOpResult rc =_SPI0Command(spic,spiu,spiu1,spiu2,data,mosi_words*4,miso_words*4); - + if (rc==SPI_RESULT_OK) { // clear any bits we did not read in the last word. if (miso_bits % 32) { From d9d07cb1fbbaa263c06d4e4bff635e1bf29aecfb Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Mon, 14 Oct 2019 23:40:01 +0800 Subject: [PATCH 08/15] Upgrade _SPI0Command to _SPICommand We needed to assess the SPI registers as base+offset to avoid referring to the registers using constant addresses as these addresses were loaded from flash and had the potential to trigger a flash cache miss. For similar reasons functions need to be called via function pointers stored in RAM. Also avoid constants in FLASH, use a copy stored in RAM. As a side effect we can now select which controller to access as a parameter. --- cores/esp8266/core_esp8266_spi_utils.cpp | 68 +++++++++++++++--------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index b855fa94d0..a104e52800 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -44,44 +44,62 @@ extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc); * Note: if porting to ESP32 mosi/miso bits are set in 2 registers, not 1. */ static SpiOpResult PRECACHE_ATTR -_SPI0Command(uint32_t spi0c,uint32_t spi0u,uint32_t spi0u1,uint32_t spi0u2, - uint32_t *data,uint32_t mosi_bytes,uint32_t miso_bytes) -{ - PRECACHE_START(); - //precache(NULL,200); - - Wait_SPI_Idle(flashchip); - uint32_t old_spi_usr = SPI0U; - uint32_t old_spi_usr2= SPI0U2; - uint32_t old_spi_c = SPI0C; +_SPICommand(volatile uint32_t spiIfNum, + uint32_t spic,uint32_t spiu,uint32_t spiu1,uint32_t spiu2, + uint32_t *data,uint32_t write_words,uint32_t read_words) +{ + if (spiIfNum>1) return SPI_RESULT_ERR; + + // force SPI register access via base+offest. + // Prevents loading individual address constants from flash. + uint32_t *spibase = (uint32_t*)(spiIfNum ? &(SPI1CMD) : &(SPI0CMD)); + #define SPIREG(reg) *((volatile uint32_t *)(spibase+(&(reg) - &(SPI0CMD)))) + + // preload any constants and functions we need into variables + // Everything must be volatile or the optimizer can treat them as + // constants, resulting in the flash reads we're trying to avoid + void *(* volatile memcpyp)(void *,const void *, size_t) = memcpy; + uint32_t (* volatile Wait_SPI_Idlep)(SpiFlashChip *) = Wait_SPI_Idle; + volatile SpiFlashChip *fchip=flashchip; + volatile uint32_t spicmdusr=SPICMDUSR; + + if (!spiIfNum) { + // Only need to precache when using SPI0 + PRECACHE_START(); + Wait_SPI_Idlep((SpiFlashChip *)fchip); + } + + uint32_t old_spi_usr = SPIREG(SPI0U); + uint32_t old_spi_usr2= SPIREG(SPI0U2); + uint32_t old_spi_c = SPIREG(SPI0C); //SPI0S &= ~(SPISE|SPISBE|SPISSE|SPISCD); - SPI0C = spi0c; - SPI0U = spi0u; - SPI0U1= spi0u1; - SPI0U2= spi0u2; + SPIREG(SPI0C) = spic; + SPIREG(SPI0U) = spiu; + SPIREG(SPI0U1)= spiu1; + SPIREG(SPI0U2)= spiu2; - if (mosi_bytes>0) { + if (write_words>0) { // copy the outgoing data to the SPI hardware - memcpy((void*)&(SPI0W0),data,mosi_bytes); + memcpyp((void*)&(SPIREG(SPI0W0)),data,write_words*4); } // Start the transfer - SPI0CMD = SPICMDUSR; + SPIREG(SPI0CMD) = spicmdusr; // wait for the command to complete uint32_t timeout = 1000; - while ((SPI0CMD & SPICMDUSR) && timeout--) {} + while ((SPIREG(SPI0CMD) & spicmdusr) && timeout--) {} - if ((miso_bytes>0) && (timeout>0)) { + if ((read_words>0) && (timeout>0)) { // copy the response back to the buffer - memcpy(data,(void *)&(SPI0W0),miso_bytes); + memcpyp(data,(void *)&(SPIREG(SPI0W0)),read_words*4); } - SPI0U = old_spi_usr; - SPI0U2= old_spi_usr2; - SPI0C = old_spi_c; - + SPIREG(SPI0U) = old_spi_usr; + SPIREG(SPI0U2)= old_spi_usr2; + SPIREG(SPI0C) = old_spi_c; + PRECACHE_END(); return (timeout>0 ? SPI_RESULT_OK : SPI_RESULT_TIMEOUT); } @@ -115,7 +133,7 @@ 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 =_SPI0Command(spic,spiu,spiu1,spiu2,data,mosi_words*4,miso_words*4); + SpiOpResult rc =_SPICommand(0,spic,spiu,spiu1,spiu2,data,mosi_words,miso_words); if (rc==SPI_RESULT_OK) { // clear any bits we did not read in the last word. From 34084e90db241a640ccb908a901354957e5dbba9 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Tue, 15 Oct 2019 10:52:08 +0800 Subject: [PATCH 09/15] Tidy up a comment thats no longer applicable --- cores/esp8266/core_esp8266_spi_utils.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index a104e52800..8f33862a91 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -38,15 +38,14 @@ extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc); /* * critical part of SPICommand. * Kept in a separate function to aid with precaching - * and because this is the part that's really specific to SPI0. * PRECACHE_* saves having to make the function IRAM_ATTR. * * Note: if porting to ESP32 mosi/miso bits are set in 2 registers, not 1. */ 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 write_words,uint32_t read_words) + uint32_t spic,uint32_t spiu,uint32_t spiu1,uint32_t spiu2, + uint32_t *data,uint32_t write_words,uint32_t read_words) { if (spiIfNum>1) return SPI_RESULT_ERR; From 63cbbf72ed1f3964232e74aac802d9edd1cca4a6 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Mon, 28 Oct 2019 23:29:41 +0800 Subject: [PATCH 10/15] Comments, formatting and variable renames Added a number of comments to better explain the code and improved the formatting. Also renamed some variables for consistency. --- cores/esp8266/core_esp8266_spi_utils.cpp | 76 +++++++++++++++++------- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index 8f33862a91..4614c1e36d 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -40,23 +40,29 @@ extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc); * Kept in a separate function to aid with precaching * PRECACHE_* saves having to make the function IRAM_ATTR. * + * spiIfNum needs to be volatile to keep the optimiser from + * deciding it can be treated as a constant (due to this being a + * static function only called with spiIfNum set to 0) + * * Note: if porting to ESP32 mosi/miso bits are set in 2 registers, not 1. */ 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 write_words,uint32_t read_words) + uint32_t *data,uint32_t writeWords,uint32_t readWords) { - if (spiIfNum>1) return SPI_RESULT_ERR; + if (spiIfNum>1) + return SPI_RESULT_ERR; // force SPI register access via base+offest. // Prevents loading individual address constants from flash. uint32_t *spibase = (uint32_t*)(spiIfNum ? &(SPI1CMD) : &(SPI0CMD)); - #define SPIREG(reg) *((volatile uint32_t *)(spibase+(&(reg) - &(SPI0CMD)))) + #define SPIREG(reg) (*((volatile uint32_t *)(spibase+(&(reg) - &(SPI0CMD))))) // preload any constants and functions we need into variables - // Everything must be volatile or the optimizer can treat them as - // constants, resulting in the flash reads we're trying to avoid + // Everything defined here must be volatile or the optimizer can + // treat them as constants, resulting in the flash reads we're + // trying to avoid void *(* volatile memcpyp)(void *,const void *, size_t) = memcpy; uint32_t (* volatile Wait_SPI_Idlep)(SpiFlashChip *) = Wait_SPI_Idle; volatile SpiFlashChip *fchip=flashchip; @@ -68,9 +74,11 @@ _SPICommand(volatile uint32_t spiIfNum, Wait_SPI_Idlep((SpiFlashChip *)fchip); } - uint32_t old_spi_usr = SPIREG(SPI0U); - uint32_t old_spi_usr2= SPIREG(SPI0U2); - uint32_t old_spi_c = SPIREG(SPI0C); + // preserve essential controller state such as incoming/outgoing + // data lengths and IO mode. + uint32_t oldSPI0U = SPIREG(SPI0U); + uint32_t oldSPI0U2= SPIREG(SPI0U2); + uint32_t oldSPI0C = SPIREG(SPI0C); //SPI0S &= ~(SPISE|SPISBE|SPISSE|SPISCD); SPIREG(SPI0C) = spic; @@ -78,26 +86,27 @@ _SPICommand(volatile uint32_t spiIfNum, SPIREG(SPI0U1)= spiu1; SPIREG(SPI0U2)= spiu2; - if (write_words>0) { + if (writeWords>0) { // copy the outgoing data to the SPI hardware - memcpyp((void*)&(SPIREG(SPI0W0)),data,write_words*4); + memcpyp((void*)&(SPIREG(SPI0W0)),data,writeWords*4); } // Start the transfer SPIREG(SPI0CMD) = spicmdusr; - // wait for the command to complete + // wait for the command to complete (typically only 1-3 iterations) uint32_t timeout = 1000; - while ((SPIREG(SPI0CMD) & spicmdusr) && timeout--) {} + while ((SPIREG(SPI0CMD) & spicmdusr) && timeout--); - if ((read_words>0) && (timeout>0)) { + if ((readWords>0) && (timeout>0)) { // copy the response back to the buffer - memcpyp(data,(void *)&(SPIREG(SPI0W0)),read_words*4); + memcpyp(data,(void *)&(SPIREG(SPI0W0)),readWords*4); } - SPIREG(SPI0U) = old_spi_usr; - SPIREG(SPI0U2)= old_spi_usr2; - SPIREG(SPI0C) = old_spi_c; + // Restore saved registers + SPIREG(SPI0U) = oldSPI0U; + SPIREG(SPI0U2)= oldSPI0U2; + SPIREG(SPI0C) = oldSPI0C; PRECACHE_END(); return (timeout>0 ? SPI_RESULT_OK : SPI_RESULT_TIMEOUT); @@ -106,29 +115,54 @@ _SPICommand(volatile uint32_t spiIfNum, /* SPI0Command: send a custom SPI command. * This part calculates register values and passes them to _SPI0Command(). + * Parameters: + * cmd The command byte (first 8 bits) to send to the SPI device + * *data The buffer containing the outgoing data for the SPI bus. + * The data is expected to be mosi_bits long, and the buffer + * is overwritten by the incoming bus data, which will be + * miso_bits long. + * mosi_bits + * Number of bits to be sent after the command byte. + * miso_bits + * Number of bits to read from the SPI bus after the outgoing + * data has been sent. */ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits) { - if (mosi_bits>(64*8)) return SPI_RESULT_ERR; - if (miso_bits>(64*8)) return SPI_RESULT_ERR; + if (mosi_bits>(64*8)) + return SPI_RESULT_ERR; + if (miso_bits>(64*8)) + return SPI_RESULT_ERR; + // Calculate the number of data words (aka registers) that need to be copied + // to/from the SPI controller. uint32_t mosi_words=mosi_bits/32; uint32_t miso_words=miso_bits/32; - if (mosi_bits % 32 != 0) mosi_words++; - if (miso_bits % 32 != 0) miso_words++; + if (mosi_bits % 32 != 0) + mosi_words++; + if (miso_bits % 32 != 0) + miso_words++; + // Select user defined command mode in the controller uint32_t spiu=SPIUCOMMAND; //SPI_USR_COMMAND + + // Set the command byte to send uint32_t spiu2 = ((7 & SPIMCOMMAND)<0) { + // set the number of outgoing data bits to send spiu1 |= ((mosi_bits-1) & SPIMMOSI) << SPILMOSI; spiu |= SPIUMOSI; // SPI_USR_MOSI } if (miso_bits>0) { + // set the number of incoming bits to read spiu1 |= ((miso_bits-1) & SPIMMISO) << SPILMISO; spiu |= SPIUMISO; // SPI_USR_MISO } uint32_t spic = SPI0C; + // Select the most basic IO mode for maximum compatibility + // Some flash commands are only available in this mode. spic &= ~(SPICQIO | SPICDIO | SPICQOUT | SPICDOUT | SPICAHB | SPICFASTRD); spic |= (SPICRESANDRES | SPICSHARE | SPICWPR | SPIC2BSE); From abcb70987ad3053eb39b4efb84603127d13e38a8 Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Mon, 28 Oct 2019 23:49:52 +0800 Subject: [PATCH 11/15] put SPI0Command in namespace experimental --- cores/esp8266/spi_utils.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cores/esp8266/spi_utils.h b/cores/esp8266/spi_utils.h index 4fb56a79f8..6ea8bf4e59 100644 --- a/cores/esp8266/spi_utils.h +++ b/cores/esp8266/spi_utils.h @@ -34,7 +34,9 @@ typedef enum { SPI_RESULT_TIMEOUT } SpiOpResult; +namespace experimental { SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits); +} #ifdef __cplusplus } From 7b2710cc9a0269af478708f067607665290cd95f Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Tue, 29 Oct 2019 07:01:08 +0800 Subject: [PATCH 12/15] Add a comment noting that the code has only been tested on bus 0 --- cores/esp8266/core_esp8266_spi_utils.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index 4614c1e36d..a0ee9b4ebb 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -35,6 +35,8 @@ extern "C" uint32_t Wait_SPI_Idle(SpiFlashChip *fc); +namespace experimental { + /* * critical part of SPICommand. * Kept in a separate function to aid with precaching @@ -126,6 +128,10 @@ _SPICommand(volatile uint32_t spiIfNum, * miso_bits * Number of bits to read from the SPI bus after the outgoing * data has been sent. + * + * Note: This code has only been tested with SPI bus 0, but should work + * equally well with other busses. The ESP8266 has bus 0 and 1, + * newer chips may have more one day. */ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits) { if (mosi_bits>(64*8)) @@ -176,3 +182,5 @@ SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_ } return rc; } + +} // namespace experimental From 97ce07cc03c30ec80795194f320ccf3ba7c09dbe Mon Sep 17 00:00:00 2001 From: Mike Nix Date: Fri, 1 Nov 2019 15:25:16 +0800 Subject: [PATCH 13/15] Replace use of memcpy with for loops in _SPICommand() memcpy is not guaranteed to be safe (IRAM_ATTR or ROM) like I thought. As a bonus the for loop is guaranteed to do 32-bit wide transfers, unlike memcpy. --- cores/esp8266/core_esp8266_spi_utils.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index a0ee9b4ebb..697afd3098 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -65,7 +65,6 @@ _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 - void *(* volatile memcpyp)(void *,const void *, size_t) = memcpy; uint32_t (* volatile Wait_SPI_Idlep)(SpiFlashChip *) = Wait_SPI_Idle; volatile SpiFlashChip *fchip=flashchip; volatile uint32_t spicmdusr=SPICMDUSR; @@ -90,7 +89,10 @@ _SPICommand(volatile uint32_t spiIfNum, if (writeWords>0) { // copy the outgoing data to the SPI hardware - memcpyp((void*)&(SPIREG(SPI0W0)),data,writeWords*4); + uint32_t *src=data; + volatile uint32_t *dst=&SPIREG(SPI0W0); + for (uint32_t i=0; i0) && (timeout>0)) { // copy the response back to the buffer - memcpyp(data,(void *)&(SPIREG(SPI0W0)),readWords*4); + uint32_t *dst=data; + volatile uint32_t *src=&SPIREG(SPI0W0); + for (uint32_t i=0; i Date: Fri, 1 Nov 2019 15:43:44 +0800 Subject: [PATCH 14/15] Typo fix what happens when you forget to edit after copy/paste --- cores/esp8266/core_esp8266_spi_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/core_esp8266_spi_utils.cpp b/cores/esp8266/core_esp8266_spi_utils.cpp index 697afd3098..110b139688 100644 --- a/cores/esp8266/core_esp8266_spi_utils.cpp +++ b/cores/esp8266/core_esp8266_spi_utils.cpp @@ -106,7 +106,7 @@ _SPICommand(volatile uint32_t spiIfNum, // copy the response back to the buffer uint32_t *dst=data; volatile uint32_t *src=&SPIREG(SPI0W0); - for (uint32_t i=0; i Date: Tue, 5 Nov 2019 08:15:56 +0800 Subject: [PATCH 15/15] Move the SpiOpResult enum into experimental namespace --- cores/esp8266/spi_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/spi_utils.h b/cores/esp8266/spi_utils.h index 6ea8bf4e59..bf0928f288 100644 --- a/cores/esp8266/spi_utils.h +++ b/cores/esp8266/spi_utils.h @@ -28,13 +28,13 @@ extern "C" { #include +namespace experimental { typedef enum { SPI_RESULT_OK, SPI_RESULT_ERR, SPI_RESULT_TIMEOUT } SpiOpResult; -namespace experimental { SpiOpResult SPI0Command(uint8_t cmd, uint32_t *data, uint32_t mosi_bits, uint32_t miso_bits); }