espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.6k stars 7.4k forks source link

esp32-2.0.15+esp32-2.0.16 Arduino boards breaks code from running on esp32-s3 (lilygo-t-dongle) #9618

Closed mrdude2478 closed 5 months ago

mrdude2478 commented 5 months ago

Board

lilygo-t-dongle-s3

Device Description

ESP32-s3 dev module

Hardware Configuration

Dongle has a winbond - W25Q32 chip + small st3775 lcd screen built in: https://github.com/Xinyuan-LilyGO/T-Dongle-S3

Version

latest master (checkout manually)

IDE Name

Arduino IDE 1.8.19

Operating System

Windows 10

Flash frequency

QUI 80 mhz

PSRAM enabled

no

Upload speed

921600

Description

Sketch works fine, compiles and runs fine using esp32 board definitions version 2.0.14.

When the board is updated via Arduino board manager to a greater version 2.0.15+2.0.16 the sketch compiles fine and uploads but the dongle doesn't run the code anymore. I traced the fault to this file:

C:\Users\someuser\AppData\Local\Arduino15\packages\esp32\hardware\esp32\2.0.16\tools\sdk\esp32s3\include\soc\esp32s3\include\soc\soc.h

A new entry was added to line 122:

define REG_SPI_BASE(i) (((i)>=2) ? (DR_REG_SPI2_BASE + (i-2) * 0x1000) : (0)) // GPSPI2 and GPSPI3

This line when commented out lets my code run fine on my dongle again using the 2.0.15+2.0.16, somehow though that line of code breaks the lilygo-t-dongle from being able to run the compiled and uploaded code.

Sketch

No issue with the sketch using the modded 2.0.16\tools\sdk\esp32s3\include\soc\esp32s3\include\soc\soc.h file with the line commented out as above.

Debug Message

No debug message is displayed.

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

Jason2866 commented 5 months ago

The change done in IDF to #define REG_SPI_BASE(i) (((i)>=2) ? (DR_REG_SPI2_BASE + (i-2) * 0x1000) : (0)) // GPSPI2 and GPSPI3 is wrong. Correct is #define REG_SPI_BASE(i) (((i)==2) ? (DR_REG_SPI2_BASE) : (DR_REG_SPI0_BASE - ((i) * 0x1000))) // GPSPI2 and GPSPI3 Changed this in our IDF fork for Tasmota, which is the base for compiling the Tasmota Arduino libs. Edit: The changes done for the other SOCs are imho wrong too. Only ESP32 looks correct.

me-no-dev commented 5 months ago

is this also the case with 3.0.0-rc2?

Jason2866 commented 5 months ago

Yes, since it is wrong in IDF branch release/v5.1 https://github.com/espressif/esp-idf/blob/442a7980835731c0a720bcfd8d3ff2f7acd01835/components/soc/esp32s3/include/soc/soc.h#L37 Edit: Did a PR for the C3. Later the IDF crew implemented different (=wrong). So I gave up and fixed this in our fork the way I think it is correct and does work for us.

mrdude2478 commented 5 months ago

@Jason2866

Thanks for your reply, I tried your modded code and unfortunately that still breaks the dongle from being able to run after being compiled and uploaded. Commenting out that line and the dongle works again.

Jason2866 commented 5 months ago

@mrdude2478 Looks like a specific issue with the code used. Without this define it shouldnt work at all. Probably something special is done

mrdude2478 commented 5 months ago

@Jason2866

Well the code works fine in 2.0.14 and previous boards code, it works fine in 2.0.16 with that line commented out?

I just tried the new updated RC that's posted in the releases and now my dongle shows up as this: BN: ESP32H2 Dev Module VID: 303a PID: 1001 SN: F4:12:FA:44:A5:04

Instead of being a ESP32S3 Dev Module. (not that it's related to my fault). In the meantime I will just go back to using 2.0.14 as I don't get any issues with that.

Jason2866 commented 5 months ago

Works fine with older core does not help anything. The used code may does some stuff which is not compatible. Newer cores do have changes and bug fixes ;-)

mrdude2478 commented 5 months ago

@Jason2866

I appreciate you are trying to help and I am thankful you have read this error in the code. I am not very technical and don't understand about chip registers etc, all I can say about the issue I am having has been said above. For Lilygo-t-dongle (esp32-s3) the last boards code which worked without errors was in 2.0.14, ever since this line of code was added to soc.h

define REG_SPI_BASE(i) (((i)>=2) ? (DR_REG_SPI2_BASE + (i-2) * 0x1000) : (0)) // GPSPI2 and GPSPI3

This breaks the dongle code from being able to run, commenting that line out - the code runs just fine so I guess that there's some sort of issue with the addresses being used in it that overwrites something in the code when trying to run, I am just guessing that though, all I can really do is report the fault and hope that someone with knowledge about such things can fix it.

me-no-dev commented 5 months ago

been using S3 all day today and have no issues at least with 3.0.0-RC2. The addresses are not really used in Arduino (unless to print out some SPI info) but I run in debug mode that prints everything and do not have the problem. I am also interested why would it be an issue to cause board not to boot.

mrdude2478 commented 5 months ago

@me-no-dev

That dongle I am using has a winbond w25q32 flash attached it via esp32-s3 spi pins, but you can't print out any debug code if the dongle won't boot.

Jason2866 commented 5 months ago

@mrdude2478 Do you have a link to the code which fails?

mrdude2478 commented 5 months ago

@Jason2866

Yep, here you go. Firmware.zip

If you need the libraries I am using, let me know and I'll post those as well.

Jason2866 commented 5 months ago

@mrdude2478 Sorry cant compile since there are dependencies to libs not mentioned where to find

Jason2866 commented 5 months ago

@me-no-dev This lines in Arduino https://github.com/espressif/arduino-esp32/blob/50ef6f4369fb85139f000f7bbc5a9f9d5bc02b9f/cores/esp32/Esp.cpp#L73-L77 does set the REG_SPI_BASE(i) base correctly. After release of Arduino core 2.0.14 the missing REG_SPI_BASE(i) settings are done in IDF 4.4 and 5.1 (sadly wrong) so the correct ones in Arduino are not used anymore.

me-no-dev commented 5 months ago

@Jason2866 how can we test this?

Jason2866 commented 5 months ago

The function getFlashChipMode should fail, since it needs the correct setting from REG_SPI_BASE(i) esptool.py is a good source for getting the real values too. https://github.com/espressif/esptool/blob/master/esptool/targets/esp32s3.py looking in the data sheet of the S3 for the addresses of all SPI controller shows that the way it is defined in IDF is not completely correct.

image

@me-no-dev

me-no-dev commented 5 months ago

which chips should we verify? S3, C3 and?

mrdude2478 commented 5 months ago

@Jason2866 The function getFlashChipMode seems to work fine for me as shown, also here are the libraries you probably need to compile it. libraries.zip

This works fine for me, as you can see from the webserver screenshot that's running on the dongle with the line commented out as mention in previous posts. Info

Jason2866 commented 5 months ago

@me-no-dev

which chips should we verify? S3, C3 and?

all except the esp32. afair that's the only one with correct setting.

Jason2866 commented 5 months ago

Some digging in the example code. There are two bugs. One i have mentioned in IDF AND one in the used TFT_eSPI library in file TFT_eSPI_ESP32_S3.h !!

The done changes in the file are not compatible/correct for general use.

// Processor ID reported by getSetup()
#define PROCESSOR_ID 0x32

// Include processor specific header
#include "soc/spi_reg.h"
#include "driver/spi_master.h"
#include "hal/gpio_ll.h"

#if !defined(CONFIG_IDF_TARGET_ESP32S3) && !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32)
  #define CONFIG_IDF_TARGET_ESP32
#endif

#ifndef VSPI
  #define VSPI FSPI
#endif

// Fix IDF problems with ESP32S3
// Note illogical enumerations: FSPI_HOST=SPI2_HOST=1   HSPI_HOST=SPI3_HOST=2
#if CONFIG_IDF_TARGET_ESP32S3
  // Fix ESP32C3 IDF bug for missing definition (FSPI only tested at the moment)
  #ifndef REG_SPI_BASE //                      HSPI                 FSPI/VSPI
    #define REG_SPI_BASE(i) (((i)>1) ? (DR_REG_SPI3_BASE) : (DR_REG_SPI2_BASE))
  #endif

  // Fix ESP32S3 IDF bug for name change
  #ifndef SPI_MOSI_DLEN_REG
    #define SPI_MOSI_DLEN_REG(x) SPI_MS_DLEN_REG(x)
  #endif

#endif

The TFT_eSPI library "fixes" the missing define for there use case. But it is not a full correct fix

mrdude2478 commented 5 months ago

@Jason2866

Thanks for looking into this, i restored the original 2.0.16 soc.h file, and then commented out this line in TFT_eSPI\Processors\TFT_eSPI_ESP32_S3.h: //#define REG_SPI_BASE(i) (((i)>1) ? (DR_REG_SPI3_BASE) : (DR_REG_SPI2_BASE))

After compiling/uploading the code to the board it's still broken by using the orignal soc.h. The only thing that is working for me is to use older 2.0.14 boards files, or to comment out the line of code in 2.0.16 and to leave TFT_eSPI\Processors\TFT_eSPI_ESP32_S3.h as it is. It's probably a problem with the way the TFT_eSPI library works which is maintained by Bodmer.

Anyway thanks for looking, I guess I have a workaround for now and hopefully others that are having issues can come across this post and see what the issue is.