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

A build flag to disable CRC check for ESP32 esp_efuse_mac_get_default() (IDFGH-8989) #10401

Closed
mmakaay opened this issue Dec 19, 2022 · 13 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@mmakaay
Copy link

mmakaay commented Dec 19, 2022

Is your feature request related to a problem?

The problem that I run into has to do with the esp_efuse_mac_get_default() function:
https://github.com/espressif/esp-idf/blob/release/v4.4/components/esp_hw_support/mac_addr.c#L106

For ESP32, (CONFIG_IDF_TARGET_ESP32 is defined), the code will always perform a CRC check on the MAC address that was read from efuse. When this fails, abort() is called, and the device will reboot.

While this is a good policy, I do run into problems with devices for which the manufacturer managed to burn an invalid CRC alongside the MAC address. As a result, the device ends up in a boot loop.

One device for which an invalid CRC seems to be the rule rather than the exception, is the ESP32-based Xiaomi Bedside Lamp 2. For this project, I maintain an open source ESPHome-based firmware replacement

Describe the solution you'd like.

I kindly request for a new sdkconfig option, that would make it possible to optionally skip the CRC check in esp_efuse_mac_get_default(), for example in the section ESP32-specific:

CONFIG_ESP32_CHECK_EFUSE_MAC_CRC=y

What I want to happen:

  • I use CONFIG_ESP32_CHECK_EFUSE_MAC_CRC=n for my build
  • During compilation, lines 121-138 from esp_efuse_mac_get_default() would not be included.

Describe alternatives you've considered.

One alternative was to take the esp-idf project, remove the conflicting code from it manually, compile binary platform packages and distribute those myself for people that want to build the firmware. I had this working, but it was a painful path that I don't want to follow anymore.

Another alternative is the solution that I currently have in place. I managed to work around the reboot loop issue by learning ESPHome to read the MAC address itself and using that for the WiFi setup, along with disabling CONFIG_ESP_PHY_CALIBRATION_AND_DATA_STORAGE to circumvent a piece of code that would otherwise call esp_efuse_mac_get_default() at all times.
This does not feel like a very stable and maintainable solution for the issue though. Additionally, it blocks the route to using CONFIG_ESP_PHY_CALIBRATION_AND_DATA_STORAGE.

Additional context.

No response

@mmakaay mmakaay added the Type: Feature Request Feature request for IDF label Dec 19, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 19, 2022
@github-actions github-actions bot changed the title A build flag to disable CRC check for ESP32 esp_efuse_mac_get_default() A build flag to disable CRC check for ESP32 esp_efuse_mac_get_default() (IDFGH-8989) Dec 19, 2022
@igrr
Copy link
Member

igrr commented Dec 19, 2022

Could you please attach espfuse.py summary output for the device which has this issue? Another question, when the device in question runs the official firmware, which MAC address does it present on the network?

@mmakaay
Copy link
Author

mmakaay commented Dec 19, 2022

The official firmware presents the same MAC address as the one that I can read from efuse.
Here is a little section of the serial logging for the original firmware:

[20:27:01]MIIO WIFI MAC: 6490c17b7651
[20:27:01]MIIO MODEL: yeelink.light.bslamp2
[20:27:01]ARCH TYPE: esp32,0x0000a601

And here's a section of the logging from the ESPHome firmware:

[15:43:44][C][wifi:491]: WiFi:
[15:43:44][C][wifi:353]:   Local MAC: 64:90:C1:7B:76:51

The MAC address in both logs is 64:90:c1:7b:76:51

I created the summary using my development device (hence a different MAC):
espefuse.summary.txt

The interesting section from it (formatted for readability):

Identity fuses:
MAC (BLOCK0): Factory MAC Address = 54:48:e6:7d:52:c0 (CRC 0xbf invalid - calculated 0xb6) R/W
MAC_CRC (BLOCK0): CRC8 for factory MAC address   = 191 R/W (0xbf)

@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development Status: In Progress Work is in progress and removed Status: Opened Issue is new Status: Selected for Development Issue is selected for development labels Dec 21, 2022
@Jason2866
Copy link

+1 for this Feature. For project Tasmota we have to use a fork with patched IDF and compile the needed Arduino libs ourself too to support this MCUs from Xiaomi.

@kriegste
Copy link

Is this called somewhere in the bootloader, too?

What is the rationale to call abort() in the first place? This problem won't go away by rebooting.

@mmakaay
Copy link
Author

mmakaay commented Dec 28, 2022

Is this called somewhere in the bootloader, too?

Not that I am aware of. I only have seen this issue at the firmware level.

What is the rationale to call abort() in the first place?

Likely, the rationale is based on the assumption that the checksum will always be correct and that an invalid checksum means that there could be some read error or memory corruption that possibly could be corrected by rebooting the device.

For project Tasmota we have to use a fork with patched IDF

That sounds painfully familiar. I have maintained such a patched ESP-IDF myself. I stopped maintaining that one, because I do have these MCUs working without patching ESP-IDF now, but as I described above, that solution is sub-optimal.

@Jason2866
Copy link

@mmakaay Yep, first it was a pain, since we had to do, now we customized (stripped down)
and added stuff which is not in standard build. Having the build and release process fully done with Github Actions it has more advantages than disadvantages. Going this way Platformio to fork was the next logical step since integrating the single core framework nicely was possible only this way. AFAIK there is no other actual espressif esp32 solo1 Arduino framework than ours 🙂
But it is better to have as less as possible changes in source code to maintain.
All other changes we have in IDF are just different settings in sdkconfig.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Jan 6, 2023
@kriegste
Copy link

Backport to 4.x?

@KonstantinKondrashov
Copy link
Collaborator

Hi @kriegste!
Yes, it will be backported to v5.0 and 4.4.

@mmakaay
Copy link
Author

mmakaay commented Jan 15, 2023

Wonderful! Thanks for the implementation @KonstantinKondrashov
This will make the life of us MAC CRC victims a lot better 😄👍

espressif-bot pushed a commit that referenced this issue Feb 17, 2023
…fuse_mac_get_default returns INVALID_CRC instead of abort

Closes #10401
@GeorgeIoak
Copy link

I just ran across this same error for the first time on one of my custom ESP32 board. I'm using the Arduino framework and PlaformIO so I tried adding:

build_flags = 
	-D ESP_MAC_IGNORE_MAC_CRC_ERROR=1

to my plaformio.ini file and I still am seeing the MAC CRC error:

Rebooting...
ets Jul 29 2019 12:21:46

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:1184
load:0x40078000,len:13132
load:0x40080400,len:3036
entry 0x400805e4
E (567) system_api: Base MAC address from BLK0 of EFUSE CRC error, efuse_crc = 0xeb; calc_crc = 0xb5

abort() was called at PC 0x4013a6c4 on core 0


Backtrace:0x40083e01:0x3ffe3a800x4008f8e5:0x3ffe3aa0 0x40095165:0x3ffe3ac0 0x4013a6c4:0x3ffe3b40 0x401083ca:0x3ffe3b80 0x400e018b:0x3ffe3bb0 0x400e02ce:0x3ffe3bf0 0x4011a69f:0x3ffe3c10 0x40083502:0x3ffe3c40 0x400792ba:0x3ffe3c90  |<-CORRUPTED




ELF file SHA256: 0000000000000000

Using espefuse tool I see this:

Identity fuses:
MAC (BLOCK0):                                      Factory MAC Address
   = 40:91:51:2e:66:21 (CRC 0xeb invalid - calculated 0xb5) R/W
MAC_CRC (BLOCK0):                                  CRC8 for factory MAC address                       = 235 R/W (0xeb)

From the PIO build I see this:

CONFIGURATION: https://docs.platformio.org/page/boards/espressif32/esp32dev.html
PLATFORM: Espressif 32 (5.2.0) > Espressif ESP32 Dev Module
HARDWARE: ESP32 240MHz, 320KB RAM, 4MB Flash
DEBUG: Current (cmsis-dap) External (cmsis-dap, esp-bridge, esp-prog, iot-bus-jtag, jlink, minimodule, olimex-arm-usb-ocd, olimex-arm-usb-ocd-h, olimex-arm-usb-tiny-h, olimex-jtag-tiny, tumpa)
PACKAGES:
 - framework-arduinoespressif32 @ 3.20005.220925 (2.0.5)
 - tool-esptoolpy @ 1.40201.0 (4.2.1)
 - tool-mkfatfs @ 2.0.1
 - tool-mklittlefs @ 1.203.210628 (2.3)
 - tool-mkspiffs @ 2.230.0 (2.30)
 - toolchain-xtensa-esp32 @ 8.4.0+2021r2-patch3

So maybe the flag hasn't made it to the Arduino framework yet?

I apologize in advance if this is the wrong place to post this question.

@igrr
Copy link
Member

igrr commented Feb 28, 2023

There are two reasons defining this flag didn't change the behavior:

  1. In Arduino, ESP-IDF libraries are pre-compiled, so the preprocessor macros you pass to the Arduino compilation process won't affect the ESP-IDF libraries.
  2. Arduino-esp32 project hasn't been updated to use the latest IDF release/v4.4 branch, yet.

@GeorgeIoak
Copy link

There are two reasons defining this flag didn't change the behavior:

  1. In Arduino, ESP-IDF libraries are pre-compiled, so the preprocessor macros you pass to the Arduino compilation process won't affect the ESP-IDF libraries.
    OK, that makes sense, thanks
  2. Arduino-esp32 project hasn't been updated to use the latest IDF release/v4.4 branch, yet.
    By this you mean that Espressif hasn't updated or I need to do something different?

@igrr
Copy link
Member

igrr commented Mar 1, 2023

Looking at this arduino-esp32 issue, I suppose there is a possibility of a v2.0.8 release which could include this change, but it's better to ask about this in the arduino-esp32 project discussions.

espressif-bot pushed a commit that referenced this issue Mar 4, 2023
…fuse_mac_get_default returns INVALID_CRC instead of abort

Closes #10401
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

7 participants